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

Introduce random-seed option to allow fuzzing of doctests #29962

Closed
kliem opened this issue Jun 24, 2020 · 34 comments
Closed

Introduce random-seed option to allow fuzzing of doctests #29962

kliem opened this issue Jun 24, 2020 · 34 comments

Comments

@kliem
Copy link
Contributor

kliem commented Jun 24, 2020

This is the first step towards #29935.

We introduce an option for doctests: --random-seed.

This allows specifying which seed to use for tests
involving randomness.

The seed is displayed in the test log:

sage -t --long --random-seed=9876543210 src/sage/all.py 
...
Doctesting 1 file.
sage -t --long --random-seed=9876543210 src/sage/all.py
    [16 tests, 0.73 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 0.8 seconds
    cpu time: 0.7 seconds
    cumulative wall time: 0.7 seconds

which makes it easy to re-run tests with the same seed.

The seed defaults to 0 for now:

sage -t --long src/sage/all.py 
...
Doctesting 1 file.
sage -t --long --random-seed=0 src/sage/all.py
    [16 tests, 0.73 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 0.8 seconds
    cpu time: 0.7 seconds
    cumulative wall time: 0.7 seconds

but the plan in #29935 is to eventually have
the random seed itself picked at random by default.

CC: @slel

Component: doctest framework

Keywords: random

Author: Jonathan Kliem

Branch: 1d99129

Reviewer: Markus Wageringel, Matthias Koeppe

Issue created by migration from https://trac.sagemath.org/ticket/29962

@kliem kliem added this to the sage-9.2 milestone Jun 24, 2020
@kliem
Copy link
Contributor Author

kliem commented Jun 24, 2020

Branch: public/29962

@kliem
Copy link
Contributor Author

kliem commented Jun 24, 2020

New commits:

da1c6bestart from a "random" random seed for doctesting
b7b836dmake random seed reproducible
eedbe5edocument random_seed
998b1b9default random seed 0 for now

@kliem
Copy link
Contributor Author

kliem commented Jun 24, 2020

Commit: 998b1b9

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 24, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

23ed583fix doctest in hyperbolic_space/hyperbolic_point
5283dc4use abs tol flag
7b244c0modify doctests to the extend that they hold with fuzz
228f379Merge branch 'public/29936' of git://trac.sagemath.org/sage into public/29962
5c7e562fix double description of hypercube
e1bf211remove set_random_seed
0e7a998Merge branch 'public/29904' of git://trac.sagemath.org/sage into public/29962
b6a5dc7fix random test in geometry/linear_expression

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 24, 2020

Changed commit from 998b1b9 to b6a5dc7

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 24, 2020

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 24, 2020

Changed commit from b6a5dc7 to 998b1b9

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 24, 2020

Changed commit from 998b1b9 to 1d7b00e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 24, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

1d7b00edash instead of underscore for command line options

@kliem

This comment has been minimized.

@mwageringel
Copy link

comment:6

Overall, this looks good to me. The following changes seem to be needed:

-    sage: subprocess.call(["sage", "-t", "--warn-long", "0", "--random-seed=0", -random_seed.rst"], **kwds)  # long time
+    sage: subprocess.call(["sage", "-t", "--warn-long", "0", "--random-seed=0", "random_seed.rst"], **kwds)  # long time
-  - ``--random_seed[=seed]`` -- random seed for fuzzing doctests
+  - ``--random-seed[=seed]`` -- random seed for fuzzing doctests

@mwageringel
Copy link

Reviewer: Markus Wageringel

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 10, 2020

comment:7

The branch adds this to the documentation, which is not true for this ticket:

--- a/src/doc/en/developer/doctesting.rst
+++ b/src/doc/en/developer/doctesting.rst
...
+Doctests start from a random seed::
+
+    [kliem@sage sage-9.2]$ sage -t src/sage/doctest/tests/random_seed.rst
...
+    sage -t --warn-long 89.5 --random-seed=112986622569797306072457879734474628454 src/sage/doctest/tests/random_seed.rst
+    **********************************************************************

@kliem
Copy link
Contributor Author

kliem commented Jul 11, 2020

New commits:

b31e2d5Merge branch 'public/29962' of git://trac.sagemath.org/sage into public/29962-reb
2f30dd9small fixes
b62f781doctests do not start from a random seed by default yet

@kliem
Copy link
Contributor Author

kliem commented Jul 11, 2020

Changed commit from 1d7b00e to b62f781

@kliem
Copy link
Contributor Author

kliem commented Jul 11, 2020

Changed branch from public/29962 to public/29962-reb

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 11, 2020

Changed reviewer from Markus Wageringel to Markus Wageringel, Matthias Koeppe

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 11, 2020

comment:9

From my side this is a positive review - if patchbot is green

@mwageringel
Copy link

comment:10

Thanks. This successfully passes ptestlong.

@kliem
Copy link
Contributor Author

kliem commented Jul 11, 2020

comment:11

Thank you.

@vbraun
Copy link
Member

vbraun commented Jul 11, 2020

comment:12

Merge conflict

@vbraun
Copy link
Member

vbraun commented Jul 19, 2020

Changed branch from public/29962-reb2 to 1d99129

@slel
Copy link
Member

slel commented Aug 23, 2020

Changed commit from 1d99129 to none

@slel
Copy link
Member

slel commented Aug 23, 2020

comment:17

What random seeds are allowed? Any integer?
Of any sign? Arbitrarily large?

@kliem
Copy link
Contributor Author

kliem commented Aug 23, 2020

comment:18

This is how we will eventually get the random seeds.

sage: import sage.misc.randstate as randstate                                   
sage: randstate.set_random_seed()                                               
sage: randstate.initial_seed()                                                  
11032378495085541661748859066830408537

At least that's the plan for now. Any random seed that you can feed into set_random_seed should work.

(Edit: But I wouldn't be surprised if there are bugs somewhere with some strange random seed. There is no way, you can avoid all of them. But at least you can prepare for rather probable cases.)

@slel
Copy link
Member

slel commented Aug 23, 2020

comment:19

The random_seed docstring says seed "must be coercible to a Python long".

Is that from -2**127 to 2**127 - 1?
Or are 32-bit and 64-bit systems different there?

Also isn't "long" a Python 2 concept,
with only "int" existing in Python 3?

Could we document the admissible range in a follow-up ticket, for non-experts like me?

@slel
Copy link
Member

slel commented Aug 23, 2020

comment:20

And thanks for the work on varying random seeds when testing!

@kliem
Copy link
Contributor Author

kliem commented Aug 24, 2020

comment:21

I added this issue to #29935.

The idea was that once all parts of Sage are ready for it, the default would be to select a "random" random seed.

@slel

This comment has been minimized.

@slel

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants