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: documentation on rng.go is lacking important context and information #36133

Open
Plazmaz opened this issue Dec 13, 2019 · 4 comments
Open

Comments

@Plazmaz
Copy link

@Plazmaz Plazmaz commented Dec 13, 2019

Does this issue reproduce with the latest release?

Yes

What did you expect to see?

Details about the algorithm used, links to source material, context about why this algorithm was used and how it differs from other methods.

What did you see instead?

go/src/math/rand/rng.go

Lines 7 to 12 in c2edcf4

/*
* Uniform distribution
*
* algorithm by
* DP Mitchell and JA Reeds
*/

A single vague comment listing two names, without the title of sources used, the name of the algorithm used, or any details whatsoever. The names alone don't seem to be sufficient for finding the source material (at least based on a good amount of googling)
It seems like I'm not the only person having this issue:
https://www.seehuhn.de/blog/134.html

Could provide some more context on what algorithm is being used, why it was chosen, and how it differs from something like mersenne twister? I think it would be valuable knowledge, and a good addition to that documentation.

@robpike

This comment has been minimized.

Copy link
Contributor

@robpike robpike commented Dec 13, 2019

See #21835.
Perhaps this package should suggest using x/exp/rand.

@ALTree ALTree changed the title Documentation on rng.go is lacking important context and information math/rand: documentation on rng.go is lacking important context and information Dec 13, 2019
@zephyrtronium

This comment has been minimized.

Copy link

@zephyrtronium zephyrtronium commented Dec 14, 2019

For whoever might update the documentation: the default Source is a lagged Fibonacci generator with j=273, k=607.

To answer how it differs from Mersenne Twister, LFG is essentially a simple LFSR over elements in GF(2^64), which as #21835 mentions has a number of empirical and theoretical problems; MT is a linear recurrence over a polynomial in GF(2^19937) in its usual implementation with another linear transform applied to its outputs, giving it strong theoretical performance. (I can offer more details on the differences if desired later, but I have a final to take now.)

While I agree that math/rand probably ought to recommend a different generator for many applications, I find it odd for a standard package to recommend something in x/exp. How close is x/exp/rand to being promoted to x/math/rand or similar?

@robpike

This comment has been minimized.

Copy link
Contributor

@robpike robpike commented Dec 14, 2019

The proposal has not even been accepted yet. If it does get accepted, it will still be a while, as the process described in the proposal makes clear.

@dmitshur dmitshur added this to the Backlog milestone Dec 23, 2019
@dmitshur

This comment has been minimized.

Copy link
Member

@dmitshur dmitshur commented Dec 23, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.