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: convenience functions always use the same seed (1). at least use time.Now() #16486

Closed
ProhtMeyhet opened this issue Jul 24, 2016 · 10 comments

Comments

@ProhtMeyhet
Copy link

@ProhtMeyhet ProhtMeyhet commented Jul 24, 2016

Please answer these questions before submitting your issue. Thanks!

  1. What version of Go are you using (go version)?
    go version go1.6.2 linux/amd64
  2. What operating system and processor architecture are you using (go env)?
    linux/amd64
  3. What did you do?
    globalRand in math/rand is always initialised with the constant seed 1. this is very unexpected. it should at least use time.Now().UnixNano() instead, else the random sequence is always the same.

even most of the go programmers didn't notice this. there are about 60 tests in the standard library that use "math/rand", but only 2 call rand.Seed() with time.Now(). as such they do not really test random, but the same sequence again and again.

  1. What did you expect to see?
    pseudo-random numbers, different with every call.
  2. What did you see instead?
    the same pseudo-random numbers on every call.
@minux
Copy link
Member

@minux minux commented Jul 24, 2016

@ProhtMeyhet
Copy link
Author

@ProhtMeyhet ProhtMeyhet commented Jul 24, 2016

If you think this is helpful in tests, then you should seed. Not the other way round.

@adg
Copy link
Contributor

@adg adg commented Jul 25, 2016

Can you give specific examples of tests you think are negatively affected by this?

@ProhtMeyhet
Copy link
Author

@ProhtMeyhet ProhtMeyhet commented Jul 25, 2016

@adg every one of them, as someone who writes and/or reads and runs the test - without manually checking the random data - most probably will think the test runs with different data each time, when in fact it's the same data each time. yes, the data otherwise would be random, and as such only at random problems will be highlighted, but like this in tests that’s never the case.

this is especially true with out of bounds testing and shifting etc. especially with fuzzing.

@adg
Copy link
Contributor

@adg adg commented Jul 25, 2016

@ProhtMeyhet if this is your contention then it should be easy to cite one of those examples and explain how the test is bad.

@ProhtMeyhet
Copy link
Author

@ProhtMeyhet ProhtMeyhet commented Jul 25, 2016

@adg i'm saying the behavior is totally unexpected and COULD be harmful for tests with the strong implication that go programmers didn't notice this either. nothing more, nothing less.

i repeat myself: something that calls itself "random" should give back different data on each call - even beyond program begin/end. otherwise it's unexpected. this is true for every language i know of.

@fdietze probably got it wrong too just 3 months ago: #15142

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jul 25, 2016

Sorry, this is documented behavior and we can't change it anyway for go1compat reasons. We actually have gone out of our way in the past to keep the random behavior fixed and generating the same numbers.

@bradfitz bradfitz closed this Jul 25, 2016
@adg
Copy link
Contributor

@adg adg commented Jul 25, 2016

In your report you say:

globalRand in math/rand is always initialised with the constant seed 1. this is very unexpected.

But the docs say:

Use the Seed function to initialize the default Source if different behavior is required for each run

This also matches my expectation with standard library pseudo-random number generators in various languages. I don't think I've worked in an environment that seeds the PRNG for me.

We can't change this, anyway, because people may be depending on behavior. https://golang.org/doc/go1compat

@ProhtMeyhet there's nothing wrong about @minux's comment in #15142. Can you please elaborate?

@ProhtMeyhet
Copy link
Author

@ProhtMeyhet ProhtMeyhet commented Jul 25, 2016

well, then i should be waiting for go2. hopefully it'll get a major cleanup of fuckups. :-)

@adg sorry, meant the reporter fdietze, but copy+paste got me.

@adg
Copy link
Contributor

@adg adg commented Jul 25, 2016

FWIW I think the decision to see math/rand's default source with 1 was the correct choice.

@golang golang locked and limited conversation to collaborators Jul 25, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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.