Bugzilla – Bug 578
[contribution] ZipfVariable
Last modified: 2009-06-24 01:48:31 EDT
Created attachment 445 [details] The patch The attached patch adds a new ZipfVariable random variable. The implementation is likely suboptimal, but it may still be useful.
Created attachment 446 [details] Patch, new version This new version adds a default constructor. Additionally, it compiles correctly also in optimized mode.
Created attachment 447 [details] Patch (for real) The patch done against che correct version of ns-3
This looks fine, though I didn't check the math -- I assume it's correct. Just to check, can you generate a PDF or CDF using this code and compare it against the theoretical PDF/CDF? I do have a question about the Copy constructor: RandomVariableBase* ZipfVariableImpl::Copy () const { return new ZipfVariableImpl (m_n, m_alpha); } All of the other random variables have Copy constructors like this: RandomVariableBase* NormalVariableImpl::Copy() const { return new NormalVariableImpl(*this); } Why is this one different?
Created attachment 448 [details] Patch with correct copy constructor This version has a correct copy constructor
(In reply to comment #4) > Created an attachment (id=448) [details] > Patch with correct copy constructor > > This version has a correct copy constructor > This version still has incorrect coding style: can you fix it before merging ? i.e., for () //do stuff should be: for () { // do stuff } yes, even for one-line for loops. see http://www.nsnam.org/codingstyle.html
(In reply to comment #3) > This looks fine, though I didn't check the math -- I assume it's correct. > Just to check, can you generate a PDF or CDF using this code and compare it > against the theoretical PDF/CDF? Craig just did some chi-squared testing of our other distributions, so I suspect he could easily check this one too.
Created attachment 449 [details] Patch with (hopefully) correct coding style
Created attachment 450 [details] correct coding style
Once we get a validation of the implementation, I'm fine with accepting this.
(In reply to comment #9) > Once we get a validation of the implementation, I'm fine with accepting this. > I can easily generate data for this implementation. Can you generate reference data I can check this against ?
Created attachment 458 [details] include header diff
(In reply to comment #9) > Once we get a validation of the implementation, I'm fine with accepting this. > FYI, I asked Craig to run this through his chi-squared test framework and gsl, and report back here.
GSL does not provide a Zipf/zeta distribution to use as a known-good PDF. I can probably get the needed info out of Mathematica tomorrow; but this will be a hand-crafted very much special case validation test. Anyone know of an (authoritative) open-source Zipf/zeta PDF and CDF function I can get my hands on?
I found something at NIST. http://www.itl.nist.gov/div898/software/dataplot/refman2/auxillar/zippdf.htm Looks like they have a Zipf distirbution in their Dataplot application: http://www.itl.nist.gov/div898/software/dataplot/homepage.htm Hope this helps. -Michele
Thanks Michele, ZIPCDF from DataPlot is exactly what I needed. Heh, I love this. Fortran-77 code with the original bleeding-edge version released by James J. Filliben in 1978. Still useful (and runnable!) after all these years.
Created attachment 470 [details] Validation plot for Zipf RNG with n=100. shape=1 It took a little work, but I finally put together a simple validation test for this RNG. This is by no means a comprehensive test, but the Zipf RNG appears to work nicely as advertised for N=100, alpha=1. I don't plan on doing any more testing. See attached PNG for a pretty picture.
OK, I'm fine with this to be committed. -Michele
pushed, changeset: c86681050541