Sampling in unit tests

[roed]

In writing unit tests (e.g. _test_associativity) it can be useful to pass in lots of elements to test. But some tests scale linearly while others scale cubically, so it's not practical to have the same list of elements for all tests. This patch adds a max_runs option for TestSuite.run that forces sampling of the element list when a large list is specified.

---

Apply

1. 14284.patch​
2. trac_14284_review.patch​
3. trac_14284_review_review.patch​
4. trac_14284_fix.patch​

[saraedum]

There is a problem with short lists.

[roed]

Fixed the problem with nth root and with unrank not working correctly.

[roed]

See #14293 for a bug revealed by this ticket.

[nthiery]

Hi!

Sorry to jump in a bit late in the discussion; I had not noticed this ticket before.

I definitely see and approve the point of the ticket. On the other hand, I find the current idiom to be used in _test_associativity and friends a bit heavy. What about an idiom like:

    S = tester.some_elements()
    for x,y,z in tester.some_elements(CartesianProduct(S,S,S)):
        ...

It's short, and encapsulate TestSuite?'s inner logic for testing strategies (on how many elements to run the tests, whether to do tests at random or not, ...).

This of course requires implementing:

tester.some_elements(XXX)

The default implementation could be to run XXX.some_elements(). Or iterate through XXX, stopping if there are more than n_max elements. Or take a sample if XXX implements sample. Or ...

What do you think? If you agree, then I would suggest doing the changes in this ticket, in order to minimize changes and counter changes (they probably will require some rebasing of my upcoming category patches; the less rebasing the better :-)).

By the way: for reproducibility of test failures, I am not super comfortable with random testing as a default; but that might be just me.

Cheers,

Nicolas

[roed]

Replying to nthiery:

Hi!

Sorry to jump in a bit late in the discussion; I had not noticed this ticket before.

Well, we did just open it a couple days ago (Julian's visiting me in Calgary so we're working in person).

I definitely see and approve the point of the ticket. On the other hand, I find the current idiom to be used in _test_associativity and friends a bit heavy. What about an idiom like:

    S = tester.some_elements()
    for x,y,z in tester.some_elements(CartesianProduct(S,S,S)):
        ...

It's short, and encapsulate TestSuite?'s inner logic for testing strategies (on how many elements to run the tests, whether to do tests at random or not, ...).

This of course requires implementing:

tester.some_elements(XXX)

The default implementation could be to run XXX.some_elements(). Or iterate through XXX, stopping if there are more than n_max elements. Or take a sample if XXX implements sample. Or ...

What do you think? If you agree, then I would suggest doing the changes in this ticket, in order to minimize changes and counter changes (they probably will require some rebasing of my upcoming category patches; the less rebasing the better :-)).

Sounds okay to me: I'll make a new version. There will be a couple tests that can't use this idiom (_test_eq_symmetric for example), but they can certainly use the current approach.

By the way: for reproducibility of test failures, I am not super comfortable with random testing as a default; but that might be just me.

Since Sage sets the random seed at the beginning of each run, tests that use randomness should be reproducible....

[nthiery]

Replying to roed:

Sounds okay to me: I'll make a new version. There will be a couple tests that can't use this idiom (_test_eq_symmetric for example), but they can certainly use the current approach.

Thanks!

By the way: there are quite a few spots where _test_associativity is disabled, precisely because they are too expensive (either due to too many elements, but also in some cases because of high degree calculations). See:

grep -r _test_associativity . | grep skip

You might want to play around with those and maybe fix a couple if things work better now.

Since Sage sets the random seed at the beginning of each run, tests that use randomness should be reproducible....

Unless some edit elsewhere in the file changes the order in which things are run. Or if one wants to reproduce an error in the terminal (hopefully not so critical with the upcoming doctest framework if one can finally explore a failing test with the debugger).

But I see the advantages too.

[saraedum]

[I uploaded the patches, but they were actually written by David Roe]