Update the developers guide for implicitly fuzzing doctests
Closed this issue · 10 comments
From #29935 comment:139:
We updated the developers guide to point out that doctests are expected to pass for any random seed.
Component: doctest framework
Keywords: random_seed, fuzz
Author: Jonathan Kliem
Branch/Commit: fa5482e
Reviewer: Michael Orlitzky
Issue created by migration from https://trac.sagemath.org/ticket/32216
While we're in there... could we just give the example that it's alluding to? The suggestion can even be simplified a bit, e.g.:
...
sage: hash(c) # random
This doctest passes too, as the output is not checked
When possible, avoid the problem. For example, rather than printing the (unpredictable) value of the hash in a doctest, one could check that
hash()successfully returns an integer:
sage: c = CombinatorialObject([1,2,3])
sage: hash(c) in ZZ
True
This eliminates the randomness from the output, which is important because all doctests are expected to pass with any state of the pseudorandom number generators (PRNGs).
I am going to nitpick this to death, sorry:
sage: while ZZ.random_element() != 0:
....: pass
sage: total = ZZ.random_element(-2, 3)
sage: count = 1
sage: while abs(total/count) > 0.01:
....: total += ZZ.random_element(-2, 3)
....: count += 1
Why skip an initial sequence of zeros? If it's because we don't want the test to pass immediately... is it any more reassuring if our sequence is [-1,1]? I think this only tests the mean of the distribution, and if we really want to claim we've tested that, then we should insist on some minimum number of loops.
I'm not really suggesting that we go crazy on this example... could we test something simpler that's more obviously doing the right thing? Like checking that each element of [-2,-1,0,1,2] is generated eventually?
I wanted to illustrate two tests here and this apparently is not clear at all. (Edit: Typo)
One test is, you can test that a certain element is found. E.g. 0 appears as a random element.
The next test calculates the mean. I didn't care too much, if the tests passes immediately. This happens, but if the test is run on random seeds, it would still be meaningful.
I like the simpler test. If I state that distribution can be checked like this, people can figure this out themselves and I have also some examples like this in the ZZ.random_element documentation prepared in #29979.
Branch pushed to git repo; I updated commit sha1. New commits:
fa5482e | simpler doctest for documentation of testing probability |
Ok, LGTM now. I can make the change in comment:2 myself later if I care enough about it.
One last question though: I think this should depend on 29935, and not the other way around? Otherwise this
sage: while not all(found.values()):
....: found[ZZ.random_element(-2, 3)] = True
is going to loop forever as ZZ.random_element(-2, 3) generates the same number over and over again.
Thank you. I didn't read comment:2 until later, when I was only on the smartphone and couldn't do anything about it anymore.
Reviewer: Michael Orlitzky
Changed branch from u/gh-kliem/document_random_doctests_for_developers to fa5482e