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

zero argument srand() #8313

Closed
ivarne opened this issue Sep 11, 2014 · 9 comments
Closed

zero argument srand() #8313

ivarne opened this issue Sep 11, 2014 · 9 comments
Labels
help wanted Indicates that a maintainer wants help on an issue or pull request randomness Random number generation and the Random stdlib

Comments

@ivarne
Copy link
Member

ivarne commented Sep 11, 2014

Currently we initialize our RNG only when the Random module is loaded, but in some situations it might be useful to reseed the RNG with good entropy from the OS.

@rfourquet suggested in #8309 that the logic form Base.Random.__init__() in base/random.jl is moved to a zero argument srand(), so that it becomes easier to use.

I think that is a good idea.

@ivarne ivarne added feature help wanted Indicates that a maintainer wants help on an issue or pull request labels Sep 11, 2014
@rfourquet
Copy link
Member

And the test/random.jl file must be changed to use this function: srand(0) is called at the beginning of the tests, so if a tested condition is true only in 1/100 cases, it may still always pass tests if it is lucky. Each (group of) test wich uses srand(constant) must reseed ths RNG with srand() at the end.

@StefanKarpinski
Copy link
Member

No, automated tests should always be deterministic.

@StefanKarpinski
Copy link
Member

To elaborate a little less tersely now that I'm not on a phone with a monthly cap on internet data, there are two purposes for randomized testing:

  1. checking that an algorithm is correct
  2. guarding against regressions

For the former use case, you should absolutely use truly random values and many of them until you're convinced that your algorithm is in fact correct. The latter use case is the only thing that automated tests are for and you absolutely do not want random failures – you want to pick some seed that is known to pass and make sure that it continues to pass.

@ivarne ivarne closed this as completed in 7be11df Sep 12, 2014
ivarne added a commit that referenced this issue Sep 12, 2014
move Base.random.__init__ to zero-arg srand(); fixes #8313
@rfourquet
Copy link
Member

Oh I consciously decided in my last comment to use less often statements like "IIUC"/"I guess"/"maybe"/... ;-)
I understand that random failures which are hard/impossible to reproduce would be frustrating, but still preferable than never knowing a bug is there... couldn't the seed be printed when a test fails? I thought that "randomized testing 1" could be useful for bugs in tests themselves (e.g. @test rand(1:4) in 1:2), but more importantly for bugs which will never happen on my configuration when testing my algo (or too rarely).
Now that srand() exists, your comment could be inserted in "test/random.jl" if you want to enforce that only the second kind of tests exist there.

@rfourquet
Copy link
Member

In particular, what made me think srand() was needed in this file is:

seed = rand(Uint) #leave state nondeterministic as above
srand(seed)

I didn't understand well the comment (do we go back to nondeterminism?), but I really thought that srand(rand(Uint)) was a substitute for the not-yet-existing of srand().

@ivarne
Copy link
Member Author

ivarne commented Sep 12, 2014

@rfourquet The point of those tests seem to be to ensure that srand works. We call srand(seed) multiple times with the same seed and see that you get the same "random" numbers back.

@StefanKarpinski
Copy link
Member

If a test should always pass regardless of what random values are produced, then it's ok to call it on changing random values but you do want to record what they were for later inspection in case the tests fail. This is different than stochastic tests that are expected to fail some of the time – statistical tests are notoriously of this variety. Those should only be run automatically with some set of known good values to prevent regressions.

@rfourquet
Copy link
Member

OK thanks, I knew close to nothing about stochastic tests.
My point was that tests like rand() != rand() are weird if they are always ever run with the same global state: all that is tested is that after srand(0), the n-th call to rand() is different from the (n+1)-th call to rand().

But I think I understand now the source of my confusion:

  1. the first line of test/random.jl contains srand(0). So all the file is always executed deterministically.
  2. "leave state nondeterministic as above" can mean "leave the nondeterministic state (and enter determinism)" or "the state is nondeterministic after this point". After looking at commit cf26c7e which introduced a dozen of lines at the end of the file containing the historically first (26 Jan. 2014) call to srand, it is clear that the first meaning is intended.

So I see 2 bugs: code which doesn't need/want srand inserted after the "leave nondeterminism" (without calling srand() to reseed randomly). Second bug is the insertion of srand(0) (whose scope is meant to cover only 2 lines of tests) as the first line of the file to fix an issue: this is commit 17913d8 (19 Apr. 2014, ancestor of cf26c7e which is then probably rebased, without the author noticing that after rebasing the "above state" was no more non-determinist).

So I will issue a PR with test code that plays with srand(constant) at the end of the file, so that the first part is non-determinist.

@rfourquet
Copy link
Member

In fact the first "bug" I mentioned is not really a bug but again the comment "leave nondeterminism" is slightly misleading: there were repeated calls to srand(seed) with seed = rand(Uint), so it is rather a "repeated non-determinism" and code after that is still non-determinism.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Indicates that a maintainer wants help on an issue or pull request randomness Random number generation and the Random stdlib
Projects
None yet
Development

No branches or pull requests

3 participants