Bugzilla – Bug 797
Enhancements to src/core/random-variable.cc/h
Last modified: 2010-02-04 08:17:00 UTC
Zipf random variable needed documentation. Truncated Exponential random variable was simply wrong (old one had a formula such as the PDF integral was not even bounded, let alone equal to one). Code indentation was slightly non-conformant in the whole file too. Plus, since I was at it, I added the Zeta distribution (it's just a Zipf distribution with N=infinity, but the generation method is rather different). No changes to the interfaces or the behavior of the actual random number generators. The changeset is in the codereview repository.
Michele said that she'd review this.
This looks good and thanks for fixing the style issues! Can you generate a couple representative PDFs along with a reference graph to validate the new code? Here are some typos that should be fixed. random-variable.h: Section @@ -282,17 +282,18 @@ internal -> interval b/(e^(\alpha \, b)-1) -> b/(e^{\alpha \, b}-1) Section @@ -301,33 +302,33 @@ m - b/(e^(b/m)-1) -> b/e^{b/m}-1) Section @@ -683,50 +686,87 @@ \param N the number of possible items, Must be a positive integer. , -> . Zeta distribution have one parameters, alpha, -> has one parameter There are the same changes needed in random-variable.cc as regards the latex functions in Exponential doxygen comments (replace some '(' with '{').
(In reply to comment #2) > This looks good and thanks for fixing the style issues! > > Can you generate a couple representative PDFs along with a reference graph to > validate the new code? Thanks, and sure, it shouldn't be a problem. > Here are some typos that should be fixed. > [...] ... and I double checked those formulas as well! I must be blind to leave those typos. Everything will be fixed in the next submission, thank you very much ! Cheers, Tommaso
Created attachment 745 [details] CDF for alpha = 1.57
Created attachment 746 [details] CDF for alpha = 3.14
I updated the code as for your comments, plus I did some testing. 10.000 numbers generated, alpha = 3.14 or 1.57. The CDFs are attached. Alpha 1.57 doesn't have an expected value, while alpha = 3.14 have an expected value of 1.30111. Generated data gave 1.2978. Quite good precision.
This looks good, please feel free to push the changes yourself.
Thanks, the patch has been pushed.