[jira] [Commented] (RNG-37) Ziggurat algorithm

2017-12-28 Thread ASF GitHub Bot (JIRA)

[ 
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

2017-12-18 Thread Gilles (JIRA)

[ 
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

2017-12-17 Thread OlgaK (JIRA)

[ 
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

2017-12-15 Thread Gilles (JIRA)

[ 
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

2017-12-15 Thread Gilles (JIRA)

[ 
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

2017-12-14 Thread ASF GitHub Bot (JIRA)

[ 
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 Kirillova 
Date:   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

2017-12-13 Thread OlgaK (JIRA)

[ 
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

2017-12-11 Thread Gilles (JIRA)

[ 
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

2017-12-10 Thread OlgaK (JIRA)

[ 
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

2017-12-09 Thread Gilles (JIRA)

[ 
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

2017-12-08 Thread Gilles (JIRA)

[ 
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

2017-10-11 Thread Gilles (JIRA)

[ 
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

2017-10-10 Thread OlgaK (JIRA)

[ 
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

2017-10-10 Thread Gilles (JIRA)

[ 
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

2017-10-09 Thread Bruno P. Kinoshita (JIRA)

[ 
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

2017-10-09 Thread OlgaK (JIRA)

[ 
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

2017-10-09 Thread Gilles (JIRA)

[ 
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

2017-10-09 Thread Bruno P. Kinoshita (JIRA)

[ 
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

2017-10-08 Thread Bruno P. Kinoshita (JIRA)

[ 
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

2017-10-08 Thread OlgaK (JIRA)

[ 
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

2017-10-03 Thread OlgaK (JIRA)

[ 
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

2017-10-03 Thread Gilles (JIRA)

[ 
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

2017-10-02 Thread OlgaK (JIRA)

[ 
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)