our random testing is compromised

9 views
Skip to first unread message

smichr

unread,
Jun 3, 2012, 9:11:15 PM6/3/12
to sy...@googlegroups.com
The runtest script uses random ordering of tests and assigns a random seed to the random number generator at the start. However, there are several places (most in mpmath) where the seed is reset. This essential guarantees that the tests run *after* that reset are always run in the same order, regardless of the seed that the testing process started with.

>>> def foo():
...  import random
...  random.seed(0)
...
>>> import random
>>> random.seed(0)
>>> random.random()
0.84442185152504812
>>> foo() # this resets the seed so we get the same random number again
>>> random.random()
0.84442185152504812


I've removed this misuse of seed in sympy's code (and made a patch to compatibility to allow the same seed to generate the same randint, etc..., under different versions of python at https://github.com/sympy/sympy/pull/1310) but am not sure what to do about mpmath. Perhaps the test runner could be patched to getstate before running mpmath and setstate afterwards?

/c

Joachim Durchholz

unread,
Jun 4, 2012, 1:29:12 AM6/4/12
to sy...@googlegroups.com
Am 04.06.2012 03:11, schrieb smichr:
> Perhaps the test runner could be patched to getstate before running
> mpmath and setstate afterwards?

I don't understand the details of the issue clearly enough to see all
ramifications, but I'd generally lean towards such a solution on the
general principle of "do not assume that called code is always correct".

In other words, if something breaks as a consequence of such a change,
I'd assume that the bug is in the callee, not in the caller that reset
the PRNG.

Chris Smith

unread,
Jun 4, 2012, 4:43:54 AM6/4/12
to sy...@googlegroups.com
On Mon, Jun 4, 2012 at 11:14 AM, Joachim Durchholz <j...@durchholz.org> wrote:
> Am 04.06.2012 03:11, schrieb smichr:
>>
>> Perhaps the test runner could be patched to getstate before running
>> mpmath and setstate afterwards?
>
>
> I don't understand the details of the issue clearly enough to see all
> ramifications, but I'd generally lean towards such a solution on the general
> principle of "do not assume that called code is always correct".

Nor do I, but the general idea is that a current test can be
influenced by results cached previously. By testing things in random
order every time tests are run, we can help flush out such problems.
But if the order is not different every time we run tests then this
has defeated the whole point of random testing. And as it is now,
mpmath is resetting the seed and thus all tests after it are always
being run in the same order (unless some new tests have been added in
the suite after mpmath).

Also, for testing purposes: random(), for a given seed, will always
return the same decimal number under current and future versions of
python -- this is stated in the documentation. What is not constant,
however, is the output of functions like randint, randrange, etc... so
although random() might produce 0.123456 under 2.6 and 3.2,
randint(1,100) might produce 23 under 2.6 and 42 under 3.2. This means
that we have to put in ellipses in tests that use output that was
generated randomly or skip the test -- which defeats the point of
having the test. So I would like to see the changes that I've proposed
for random in compatibility.py incorporated.

/c

Tom Bachmann

unread,
Jun 4, 2012, 8:48:20 AM6/4/12
to sy...@googlegroups.com
The seed is re-set upon running each test file. So as far as I can tell
setting the seed inside a test file should not compromise anything.
> --
> You received this message because you are subscribed to the Google
> Groups "sympy" group.
> To view this discussion on the web visit
> https://groups.google.com/d/msg/sympy/-/mQbxWO4FecAJ.
> To post to this group, send email to sy...@googlegroups.com.
> To unsubscribe from this group, send email to
> sympy+un...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/sympy?hl=en.

Chris Smith

unread,
Jun 4, 2012, 10:47:10 AM6/4/12
to sy...@googlegroups.com
On Mon, Jun 4, 2012 at 6:33 PM, Tom Bachmann <e_m...@web.de> wrote:
> The seed is re-set upon running each test file. So as far as I can tell
> setting the seed inside a test file should not compromise anything.

OK, so the shuffling within suites is not affected. That's good new.
Thanks for looking at that more closely.

Do you have any feelings about the changes I proposed to compatibility
to get inter-version reproducible results for a given seed? If not,
I'll just let the issue drop.

Tom Bachmann

unread,
Jun 4, 2012, 11:47:40 AM6/4/12
to sy...@googlegroups.com
> Do you have any feelings about the changes I proposed to compatibility
> to get inter-version reproducible results for a given seed? If not,
> I'll just let the issue drop.
>

What are you referring to?

Chris Smith

unread,
Jun 4, 2012, 8:34:30 PM6/4/12
to sy...@googlegroups.com

Tom Bachmann

unread,
Jun 5, 2012, 12:05:29 PM6/5/12
to sy...@googlegroups.com
While I think this pull request is skillfully executed (as usual), I am
not sure the merits outweigh the maintenance requirements.

On 05.06.2012 01:34, Chris Smith wrote:
> https://github.com/sympy/sympy/pull/1310
>

Chris Smith

unread,
Jun 5, 2012, 1:58:09 PM6/5/12
to sy...@googlegroups.com
On Tue, Jun 5, 2012 at 9:50 PM, Tom Bachmann <e_m...@web.de> wrote:
> While I think this pull request is skillfully executed (as usual), I am not
> sure the merits outweigh the maintenance requirements.

OK, then residual bug fixes are all that remain in the pull request.
If you would be willing to take (literally) a minute to look at those,
then we can at least fix what is broken/misleading.

Thanks,
Chris
Reply all
Reply to author
Forward
0 new messages