Bug 578

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