[
https://issues.apache.org/jira/browse/RNG-37?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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)
