Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

math/rand: remove statistical correlation in ziggurat algorithms #8731

Closed
zephyrtronium opened this issue Sep 15, 2014 · 5 comments
Closed
Labels
FrozenDueToAge v2 An incompatible library change
Milestone

Comments

@zephyrtronium
Copy link
Contributor

A statistical correlation is present in the algorithms used to generate normally and
exponentially distributed values due to the reuse of random bits for both the
x-coordinate and the segment index. This reuse could be avoided by generating a 63-bit
number, taking the low 32 bits for the x-coordinate, the next 7 or 8 bits for the
segment number, and on the normal distribution, one more bit to determine the sign of
values generated from the tail.

For example: http://play.golang.org/p/xeCjwEFUSs
@ianlancetaylor
Copy link
Contributor

Comment 1:

Thanks for the report.  Please send patches using the process described at
http://golang.org/doc/contribute.html.  Thanks.

Labels changed: added repo-main, release-go1.5.

@zephyrtronium
Copy link
Contributor Author

Comment 2:

Alas, a complication: after making these changes and running all.bash, I see that there
are tests to make sure that the outputs of Rand.NormFloat64() and Rand.ExpFloat64() stay
consistent: a contract which this improvement necessarily violates.

@josharian
Copy link
Contributor

Based on the discussion in #8013, I don't think we can't change this.

@ianlancetaylor what do you think?

@ianlancetaylor
Copy link
Contributor

We deliberately postponed the decision in #8013. There were various arguments, but no final decision.

If the numbers that we get from the generators are statistically correlated, that sounds to me like a bug. I personally would be OK with changing the generated sequence to fix a bug.

This probably should be discussed on golang-dev, if somebody has a patch.

@ianlancetaylor
Copy link
Contributor

For Go 2 I think we are likely to move in the direction of #21835, which seems to be a superior random number generator in any case. I think that is a better place to discuss the future of math/rand. Closing this issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge v2 An incompatible library change
Projects
None yet
Development

No branches or pull requests

7 participants