[jira] [Commented] (RNG-37) Ziggurat algorithm
[ https://issues.apache.org/jira/browse/RNG-37?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16305956#comment-16305956 ] ASF GitHub Bot commented on RNG-37: --- Github user cur4so closed the pull request at: https://github.com/apache/commons-rng/pull/6 > Ziggurat algorithm > -- > > Key: RNG-37 > URL: https://issues.apache.org/jira/browse/RNG-37 > Project: Commons RNG > Issue Type: Wish >Reporter: Gilles >Priority: Minor > Fix For: 1.1 > > Attachments: RNG-37.patch, ziggurat.buggy.png, ziggurat.patch.png > > > Fast algorithm for sampling a Gaussian distribution: > https://en.wikipedia.org/wiki/Ziggurat_algorithm -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (RNG-37) Ziggurat algorithm
[ https://issues.apache.org/jira/browse/RNG-37?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16294801#comment-16294801 ] Gilles commented on RNG-37: --- bq. I can't help you with this, I've too many higher priority tasks on my plate. I'm signing out of this topic. There is no enforced deadline for this kind of project based on volunteering. Your contributions were very promising, but you didn't seem to grasp that what should happen here is team work; we all try to go forward: you submitted source code, others provided comments to improve it in order to help still other people down the time line. bq. I already regret of my 1st reply in this topic, at that time I didn't know who you are, well ... lesson learned ... I wonder which of my comments could elicit such a negative stance on your part. None of us know really "who we are". It's by sticking to the facts, and not attacking the persons, that we can work together. In this particular case, I've stumbled on hints that further work is necessary to get the expected functionality. The "process" was running smoothly, as the comments (at least those which you could not delete) show... > Ziggurat algorithm > -- > > Key: RNG-37 > URL: https://issues.apache.org/jira/browse/RNG-37 > Project: Commons RNG > Issue Type: Wish >Reporter: Gilles >Priority: Minor > Fix For: 1.1 > > Attachments: ziggurat.buggy.png > > > Fast algorithm for sampling a Gaussian distribution: > https://en.wikipedia.org/wiki/Ziggurat_algorithm -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (RNG-37) Ziggurat algorithm
[ https://issues.apache.org/jira/browse/RNG-37?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16294427#comment-16294427 ] OlgaK commented on RNG-37: -- [~erans] I'm sorry I can't help you with this, I've too many higher priority tasks on my plate. I'm signing out of this topic. > Ziggurat algorithm > -- > > Key: RNG-37 > URL: https://issues.apache.org/jira/browse/RNG-37 > Project: Commons RNG > Issue Type: Wish >Reporter: Gilles >Priority: Minor > Fix For: 1.1 > > Attachments: ziggurat.buggy.png > > > Fast algorithm for sampling a Gaussian distribution: > https://en.wikipedia.org/wiki/Ziggurat_algorithm -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (RNG-37) Ziggurat algorithm
[ https://issues.apache.org/jira/browse/RNG-37?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16292375#comment-16292375 ] Gilles commented on RNG-37: --- bq. If you think it doesn't make a check for the wholes, It doesn't. AFAICT, it would require generating _a lot_ of numbers. Thus, it would not be a nice *unit* test. Such a stress test was performed (using external tools) to ensure that the underlying sources generate a uniform distribution within the expected range. See [userguide|https://commons.apache.org/proper/commons-rng/userguide/rng.html#a5._Quality]. The test suite in the {{commons-rng-sampling}} module "only" tests that values fall within each of 10 bins with the expected frequency. bq. it definitely would be a great idea to add it (not just for Ziggurat test but for all distributions). Sure (but see above). bq. The person who had written the testing module, definitely could do it better and faster. For a "side person" imho it'd require substantial time and efforts. I won't be faster in writing the simple code which I outlined in a previous comment. This would be a "brute-force" visual inspection, just to get the feeling that the generated sequence has no obvious bias. bq. But it's not a big deal to switch to Long. Moreover, it has been my original thought to make the calcs in long and I've even added the missing long random in the main codebase but then I switched to int. I'm afraid that it's not only a matter of replacing {{int}} with {{long}}. Although there is quite possibly a precision issue, I suspect that the main one is the density, which your patch does not seem to address. E.g. there are 2^63 different {{long}} positive values but this line {code} final double m = 2147483648.0; // 2^31. {code} seems tailored to the {{int}} type. That's one reason why _all magic numbers must be replaced by documented constants_: e.g. being defined as {code} private static final double MAX_INT_PLUS_ONE = (double) Integer.MAX_VALUE + 1d; {code} a reviewer could be led to suspect that "m" might need to be adapted when switching to the {{long}} type. > Ziggurat algorithm > -- > > Key: RNG-37 > URL: https://issues.apache.org/jira/browse/RNG-37 > Project: Commons RNG > Issue Type: Wish >Reporter: Gilles >Priority: Minor > Fix For: 1.1 > > > Fast algorithm for sampling a Gaussian distribution: > https://en.wikipedia.org/wiki/Ziggurat_algorithm -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (RNG-37) Ziggurat algorithm
[ https://issues.apache.org/jira/browse/RNG-37?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16292288#comment-16292288 ] Gilles commented on RNG-37: --- bq. I didn't get your last message "Fix Version/s" ?? I guess that you mean the automated mail sent by the bug-tracking system. I've set the "Fix version/s" so that issues that could have arisen from porting C code to Java (as noted in the previous comment) are fixed before we consider the release of the next version. > Ziggurat algorithm > -- > > Key: RNG-37 > URL: https://issues.apache.org/jira/browse/RNG-37 > Project: Commons RNG > Issue Type: Wish >Reporter: Gilles >Priority: Minor > Fix For: 1.1 > > > Fast algorithm for sampling a Gaussian distribution: > https://en.wikipedia.org/wiki/Ziggurat_algorithm -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (RNG-37) Ziggurat algorithm
[ https://issues.apache.org/jira/browse/RNG-37?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16291809#comment-16291809 ] ASF GitHub Bot commented on RNG-37: --- GitHub user cur4so opened a pull request: https://github.com/apache/commons-rng/pull/6 RNG-37 make KN table long switch from into to long in auxiliary table KN calculation, plus calculation `uni` constant with higher precision You can merge this pull request into a Git repository by running: $ git pull https://github.com/cur4so/commons-rng feature-RNG-37 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-rng/pull/6.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #6 commit 6c3f060081e73d0d678c43396e7d82d567d25246 Author: Olga KirillovaDate: 2017-12-14T23:48:58Z RNG-37 make KN table long > Ziggurat algorithm > -- > > Key: RNG-37 > URL: https://issues.apache.org/jira/browse/RNG-37 > Project: Commons RNG > Issue Type: Wish >Reporter: Gilles >Priority: Minor > Fix For: 1.1 > > > Fast algorithm for sampling a Gaussian distribution: > https://en.wikipedia.org/wiki/Ziggurat_algorithm -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (RNG-37) Ziggurat algorithm
[ https://issues.apache.org/jira/browse/RNG-37?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16290342#comment-16290342 ] OlgaK commented on RNG-37: -- {quote}Then it looks like the whole procedure needs to be adapted, not only the tail. Indeed generating a single 32-bits value (nextInt(), at line 98) won't return any of the double values (64-bits) that would require a different setting of any one of the missing 32-bits.{quote} I think missing 32-bits contribute to the precision (having 7 decimal digits vs having 16). For Gaussian in high density area having 8th and after digits doesn't make a notable effect, while in the low density are it does indeed. I'm fairly sure it's just the tail. Besides when one tries to increase precision switching from float to double but still keeps the same number of fitting blocks... Overall the fit is so to say good enough to pass the test. But it's not a big deal to switch to Long. Moreover, it has been my original thought to make the calcs in long and I've even added the missing long random in the main codebase but then I switched to int. Actually as you may notice in the paper there is another application of the algorithm - to fit exponential distribution. I've tried to add this one as well but there was severe test failure and since it has not been requested, I decided to skip it. But there indeed all numbers need to be recalculated. {quote}Then, I wonder how to make sure that no holes remain. Is it possible to set up a unit test? If not, a last resort idea might be to make a plot of many generated values, superimposed with a plot of a uniform sampling, and look for anomalies (empty regions). {quote} The rng has a very well done testing module. It allows people, coming from street or bb, to test their code adding just a single line in the test suite. I was really impressed. I didn't get into details by what criteria it assesses the quality of generated distribution but it definitely more than just a single number pick (based on what I recall how the 2d distribution test failed). If you think it doesn't make a check for the wholes, it definitely would be a great idea to add it (not just for Ziggurat test but for all distributions). The person who had written the testing module, definitely could do it better and faster. For a "side person" imho it'd require substantial time and efforts. One can make it as a task for "Help wanted" bb but don't have over expectations :). I'll do switch int -> long. Try to do it to the nearest weekend. I didn't get your last message"Fix Version/s" ?? > Ziggurat algorithm > -- > > Key: RNG-37 > URL: https://issues.apache.org/jira/browse/RNG-37 > Project: Commons RNG > Issue Type: Wish >Reporter: Gilles >Priority: Minor > Fix For: 1.1 > > > Fast algorithm for sampling a Gaussian distribution: > https://en.wikipedia.org/wiki/Ziggurat_algorithm -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (RNG-37) Ziggurat algorithm
[ https://issues.apache.org/jira/browse/RNG-37?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16285686#comment-16285686 ] Gilles commented on RNG-37: --- bq. All parameters values has been taken from the paper and are for 32 bits. Then it looks like the whole procedure needs to be adapted, not only the tail. Indeed generating a single 32-bits value ({{nextInt()}}, at line 98) won't return any of the {{double}} values (64-bits) that would require a different setting of any one of the missing 32-bits. bq. how to make the adjustment As Bruno suggested previously, a good start would be to replace hard-coded numbers with named constants that make it clear what they mean ("m" at line 59, for example). It will fairly surely give us a hint about the needed adjustments. Table "KN" should probably contain {{long}} values, and calls to {{nextInt()}} be replaced with {{nextLong()}} (method to be added to {{SamplerBase}}). Then, I wonder how to make sure that no holes remain. Is it possible to set up a unit test? If not, a last resort idea might be to make a plot of many generated values, superimposed with a plot of a uniform sampling, and look for anomalies (empty regions). Such a program could be a usage example for the {{commons-rng-examples}} module of the project. > Ziggurat algorithm > -- > > Key: RNG-37 > URL: https://issues.apache.org/jira/browse/RNG-37 > Project: Commons RNG > Issue Type: Wish >Reporter: Gilles >Priority: Minor > > Fast algorithm for sampling a Gaussian distribution: > https://en.wikipedia.org/wiki/Ziggurat_algorithm -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (RNG-37) Ziggurat algorithm
[ https://issues.apache.org/jira/browse/RNG-37?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16285525#comment-16285525 ] OlgaK commented on RNG-37: -- Hi [~erans], yes, you're right for higher (64 bits) precision the tail (low probability values) needs to be adjusted. All parameters values has been taken from the paper and are for 32 bits. But from the top of my head I can't say how to make the adjustment. > Ziggurat algorithm > -- > > Key: RNG-37 > URL: https://issues.apache.org/jira/browse/RNG-37 > Project: Commons RNG > Issue Type: Wish >Reporter: Gilles >Priority: Minor > > Fast algorithm for sampling a Gaussian distribution: > https://en.wikipedia.org/wiki/Ziggurat_algorithm -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (RNG-37) Ziggurat algorithm
[ https://issues.apache.org/jira/browse/RNG-37?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16284697#comment-16284697 ] Gilles commented on RNG-37: --- bq. keeps only 7 bits out of the 32 Sorry, this is incorrect: the value returned by {{nextInt()}} serves as a multiplier, so all 32 are used. So, for a source that generate 64-bits at a time, "only" 32-bits are wasted; the question is then whether the additional (branching) logic for reusing those bits would not completely offset the gain of generating one value in place of two. I did some trial, and it seems that the gain is indeed insignificant. A different issue is that the code seems to copy statements that generate {{float}} values in the [reference implementation|http://www.jstatsoft.org/article/view/v005i08/ziggurat.pdf]. In the Java port, the hard-coded values that seems to have less precision than they should (since {{double}} values are generated here). E.g. is a statement such as {code} uni = 0.5 + nextInt() * 0.2328306e-9; {code} not going to miss {{double}} values close to 1? > Ziggurat algorithm > -- > > Key: RNG-37 > URL: https://issues.apache.org/jira/browse/RNG-37 > Project: Commons RNG > Issue Type: Wish >Reporter: Gilles >Priority: Minor > > Fast algorithm for sampling a Gaussian distribution: > https://en.wikipedia.org/wiki/Ziggurat_algorithm -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (RNG-37) Ziggurat algorithm
[ https://issues.apache.org/jira/browse/RNG-37?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16283664#comment-16283664 ] Gilles commented on RNG-37: --- Hi [~cur4so]. I ran the benchmark; table is available in the [userguide|https://git1-us-west.apache.org/repos/asf?p=commons-rng.git;a=blob_plain;f=src/site/apt/userguide/rng.apt;hb=5b88619b732e0d556e61a48b8fbb8d628bf16ea8] (commit 5b88619b732e0d556e61a48b8fbb8d628bf16ea8). Better performance was expected; however, although a {{double}} value is computed, it seems that 64-bits sources lost their edge. Indeed, for each sample, the implementation calls {{nextInt()}} and keeps only 7 bits out of the 32 (resp. 64) that were computed by the underlying source. I wonder whether performance could be improved if we kept bits for reuse at the next call to {{sample()}}... > Ziggurat algorithm > -- > > Key: RNG-37 > URL: https://issues.apache.org/jira/browse/RNG-37 > Project: Commons RNG > Issue Type: Wish >Reporter: Gilles >Priority: Minor > > Fast algorithm for sampling a Gaussian distribution: > https://en.wikipedia.org/wiki/Ziggurat_algorithm -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (RNG-37) Ziggurat algorithm
[ https://issues.apache.org/jira/browse/RNG-37?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16199972#comment-16199972 ] Gilles commented on RNG-37: --- Merged in "master" with minor changes. > Ziggurat algorithm > -- > > Key: RNG-37 > URL: https://issues.apache.org/jira/browse/RNG-37 > Project: Commons RNG > Issue Type: Wish >Reporter: Gilles >Priority: Minor > > Fast algorithm for sampling a Gaussian distribution: > https://en.wikipedia.org/wiki/Ziggurat_algorithm -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (RNG-37) Ziggurat algorithm
[ https://issues.apache.org/jira/browse/RNG-37?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16199556#comment-16199556 ] OlgaK commented on RNG-37: -- Gilles, are you trying to pull it from https://github.com/apache/commons-rng/pull/5 (this is the last one)? In my case it works smoothly, no conflicts ```git pull https://github.com/cur4so/commons-rng.git >From https://github.com/cur4so/commons-rng * branchHEAD -> FETCH_HEAD Already up-to-date.``` and on the github site it points that there are no conflicts. I've changed `fixes` on `Javadoc` > Ziggurat algorithm > -- > > Key: RNG-37 > URL: https://issues.apache.org/jira/browse/RNG-37 > Project: Commons RNG > Issue Type: Wish >Reporter: Gilles >Priority: Minor > > Fast algorithm for sampling a Gaussian distribution: > https://en.wikipedia.org/wiki/Ziggurat_algorithm -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (RNG-37) Ziggurat algorithm
[ https://issues.apache.org/jira/browse/RNG-37?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16199505#comment-16199505 ] Gilles commented on RNG-37: --- {{git}} reports a conflict when I try to pull the latest set of changes ("fixes 3"): {noformat} You have unmerged paths. (fix conflicts and run "git commit") (use "git merge --abort" to abort the merge) Unmerged paths: (use "git add ..." to mark resolution) both added: commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/ZigguratGaussianSampler.java {noformat} By the way, Olga, the log must be useful for later review; a message like "fixes" is useless. If the change has improved the Javadoc; the message can just be "Javadoc"; if it fixes problems signaled by a reporting tool, the message would be like "Avoid warning from " etc. If the code is modified in a non-obvious, it's always nice to summarize the changes for the log. > Ziggurat algorithm > -- > > Key: RNG-37 > URL: https://issues.apache.org/jira/browse/RNG-37 > Project: Commons RNG > Issue Type: Wish >Reporter: Gilles >Priority: Minor > > Fast algorithm for sampling a Gaussian distribution: > https://en.wikipedia.org/wiki/Ziggurat_algorithm -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (RNG-37) Ziggurat algorithm
[ https://issues.apache.org/jira/browse/RNG-37?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16198178#comment-16198178 ] Bruno P. Kinoshita commented on RNG-37: --- >I've checked, all samplers in >commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution > except one have this format. So, I assume it's how it's expected, I've left >it as is. You are correct, checked another class. Thanks for looking into that Olga. Commented your pull request :-) we are making good progress. Thank **you** for your contribution and patience. Cheers Bruno > Ziggurat algorithm > -- > > Key: RNG-37 > URL: https://issues.apache.org/jira/browse/RNG-37 > Project: Commons RNG > Issue Type: Wish >Reporter: Gilles >Priority: Minor > > Fast algorithm for sampling a Gaussian distribution: > https://en.wikipedia.org/wiki/Ziggurat_algorithm -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (RNG-37) Ziggurat algorithm
[ https://issues.apache.org/jira/browse/RNG-37?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16197495#comment-16197495 ] OlgaK commented on RNG-37: -- thanks [~kinow] for your reply and comments. I've made a new [pull request|https://github.com/apache/commons-rng/pull/5] from the fork of commons-rng and implemented your suggestions in the last commit. Regarding +public class ZigguratGaussianSampler +extends SamplerBase Not sure which checkstyle rule will be used, but normally these are kept in a single line, unless it breaks the other rule of the maximum number of chars per line" I've checked, all samplers in commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution except one have this format. So, I assume it's how it's expected, I've left it as is. > Ziggurat algorithm > -- > > Key: RNG-37 > URL: https://issues.apache.org/jira/browse/RNG-37 > Project: Commons RNG > Issue Type: Wish >Reporter: Gilles >Priority: Minor > > Fast algorithm for sampling a Gaussian distribution: > https://en.wikipedia.org/wiki/Ziggurat_algorithm -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (RNG-37) Ziggurat algorithm
[ https://issues.apache.org/jira/browse/RNG-37?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16196960#comment-16196960 ] Gilles commented on RNG-37: --- Sorry for the delay. Bruno, thanks a lot for the review and advice. :) Olga, alternatively to a pull request, you could also attach patches to this report, (or the whole source, since it is a new file). > Ziggurat algorithm > -- > > Key: RNG-37 > URL: https://issues.apache.org/jira/browse/RNG-37 > Project: Commons RNG > Issue Type: Wish >Reporter: Gilles >Priority: Minor > > Fast algorithm for sampling a Gaussian distribution: > https://en.wikipedia.org/wiki/Ziggurat_algorithm -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (RNG-37) Ziggurat algorithm
[ https://issues.apache.org/jira/browse/RNG-37?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16196788#comment-16196788 ] Bruno P. Kinoshita commented on RNG-37: --- Submitted some comments to the pull request. But I think it's not a fork of the Commons RNG component? But a repository under your user? You need to fork the Apache project (here https://github.com/apache/commons-rng, there should be a button at the top right corner "Fork"). That will create a new repository under your user account (https://github.com/cur4so/commons-rng by default). Then you can create a branch, and send your pull request, following instructions from some tutorial like this one https://biologyguy.github.io/git-novice/09-pull-request/ or this one https://help.github.com/articles/creating-a-pull-request-from-a-fork/ Hope it helps, Bruno > Ziggurat algorithm > -- > > Key: RNG-37 > URL: https://issues.apache.org/jira/browse/RNG-37 > Project: Commons RNG > Issue Type: Wish >Reporter: Gilles >Priority: Minor > > Fast algorithm for sampling a Gaussian distribution: > https://en.wikipedia.org/wiki/Ziggurat_algorithm -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (RNG-37) Ziggurat algorithm
[ https://issues.apache.org/jira/browse/RNG-37?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16196425#comment-16196425 ] Bruno P. Kinoshita commented on RNG-37: --- Hi [~cur4so] >but it's almost a week has past. Any updates? As the project is maintained by volunteers, it is normal that some issues have a long delay to have a response. This delay may be hours, days, weeks, or months. Depends mostly on the schedule of each volunteer. I was on vacation with limited Internet for the past three and half weeks, coming back today, and first reviewing things at $work, then seeing family/friends, re-organizing life, and only then going to have fun again as a volunteer with Open Source. So don't feel bad to bump issues if there's no activity, but don't expect a response as fast as in projects where we have a maintainer being paid to work and reply on issues :-) [~erans] has managed most issues in math and rng. I will try to review the pull request this week, but without spending much time on the actual implementation as I'm not familiar with the algorithm or its use cases. I will look at the points Giles pointed before, and the normal ASF standards (test regressions, code style, etc). Cheers Bruno > Ziggurat algorithm > -- > > Key: RNG-37 > URL: https://issues.apache.org/jira/browse/RNG-37 > Project: Commons RNG > Issue Type: Wish >Reporter: Gilles >Priority: Minor > > Fast algorithm for sampling a Gaussian distribution: > https://en.wikipedia.org/wiki/Ziggurat_algorithm -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (RNG-37) Ziggurat algorithm
[ https://issues.apache.org/jira/browse/RNG-37?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16196396#comment-16196396 ] OlgaK commented on RNG-37: -- Hi [~erans], I understand that you have many things on your agenda, but it's almost a week has past. Any updates? > Ziggurat algorithm > -- > > Key: RNG-37 > URL: https://issues.apache.org/jira/browse/RNG-37 > Project: Commons RNG > Issue Type: Wish >Reporter: Gilles >Priority: Minor > > Fast algorithm for sampling a Gaussian distribution: > https://en.wikipedia.org/wiki/Ziggurat_algorithm -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (RNG-37) Ziggurat algorithm
[ https://issues.apache.org/jira/browse/RNG-37?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16190708#comment-16190708 ] OlgaK commented on RNG-37: -- Hi [~erans], I've made the changes as you requested. Regarding the performance, I've added a test into your tests suite, just that, I'm not an inventor of the algorithm, so rely on words of the authors. > Ziggurat algorithm > -- > > Key: RNG-37 > URL: https://issues.apache.org/jira/browse/RNG-37 > Project: Commons RNG > Issue Type: Wish >Reporter: Gilles >Priority: Minor > > Fast algorithm for sampling a Gaussian distribution: > https://en.wikipedia.org/wiki/Ziggurat_algorithm -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (RNG-37) Ziggurat algorithm
[ https://issues.apache.org/jira/browse/RNG-37?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16189554#comment-16189554 ] Gilles commented on RNG-37: --- Hi Olga. I've just quickly read through the proposed code; here are a few remarks: * Please add a link (web page, paper, code) to the reference(s) which you used for the implementation. * Please follow the coding style (e.g. "space" around operators). Running "mvn site" should produce a clean "CheckStyle" report. * Magic numbers should be avoided; use a "static final" instance field instead. * Constants should be declared "final". * Reuse already computed values (cf. lines 56, 65, 71 and lines 102, 103, ...) * Javadoc must be provided for all method and fields ("private" also). Did you check the performance? Thanks a lot for your interest in, and contribution to, the "Commons RNG" project! > Ziggurat algorithm > -- > > Key: RNG-37 > URL: https://issues.apache.org/jira/browse/RNG-37 > Project: Commons RNG > Issue Type: Wish >Reporter: Gilles >Priority: Minor > > Fast algorithm for sampling a Gaussian distribution: > https://en.wikipedia.org/wiki/Ziggurat_algorithm -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (RNG-37) Ziggurat algorithm
[ https://issues.apache.org/jira/browse/RNG-37?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16189108#comment-16189108 ] OlgaK commented on RNG-37: -- [~webcomponents] and others, here is a [pull request|https://github.com/cur4so/apache/pull/1] with Ziggurat algorithm implementation. I can't commit to apache-commons, therefore I've made a mirror in my github account. Let me know if you have any questions. > Ziggurat algorithm > -- > > Key: RNG-37 > URL: https://issues.apache.org/jira/browse/RNG-37 > Project: Commons RNG > Issue Type: Wish >Reporter: Gilles >Priority: Minor > > Fast algorithm for sampling a Gaussian distribution: > https://en.wikipedia.org/wiki/Ziggurat_algorithm -- This message was sent by Atlassian JIRA (v6.4.14#64029)