Opened 2 years ago

Closed 2 years ago

Last modified 18 months ago

#29962 closed enhancement (fixed)

Introduce random-seed option to allow fuzzing of doctests

Reported by: gh-kliem Owned by:
Priority: major Milestone: sage-9.2
Component: doctest framework Keywords: doctests, random
Cc: Samuel Lelièvre Merged in:
Authors: Jonathan Kliem Reviewers: Markus Wageringel, Matthias Koeppe
Report Upstream: N/A Work issues:
Branch: 1d99129 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Samuel Lelièvre)

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.

Change History (23)

comment:1 Changed 2 years ago by gh-kliem

Branch: public/29962
Commit: 998b1b94ce1289ea92451a86e5f6191c37eaeb5a
Status: newneeds_review

New commits:

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

comment:2 Changed 2 years ago by git

Commit: 998b1b94ce1289ea92451a86e5f6191c37eaeb5ab6a5dc7c96125b6140e56feafb82dd13599998e7

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

comment:3 Changed 2 years ago by git

Commit: b6a5dc7c96125b6140e56feafb82dd13599998e7998b1b94ce1289ea92451a86e5f6191c37eaeb5a

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

comment:4 Changed 2 years ago by git

Commit: 998b1b94ce1289ea92451a86e5f6191c37eaeb5a1d7b00e3fc2ebc1dc9982a2df91d15e3f12e9432

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

1d7b00edash instead of underscore for command line options

comment:5 Changed 2 years ago by gh-kliem

Description: modified (diff)

comment:6 Changed 2 years ago by Markus Wageringel

Reviewers: Markus Wageringel

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

comment:7 Changed 2 years ago by Matthias Köppe

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
+    **********************************************************************

comment:8 Changed 2 years ago by gh-kliem

Branch: public/29962public/29962-reb
Commit: 1d7b00e3fc2ebc1dc9982a2df91d15e3f12e9432b62f781647851a3f27ecc95ea4b98b53838da112

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

comment:9 Changed 2 years ago by Matthias Köppe

Reviewers: Markus WageringelMarkus Wageringel, Matthias Koeppe

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

comment:10 Changed 2 years ago by Markus Wageringel

Status: needs_reviewpositive_review

Thanks. This successfully passes ptestlong.

comment:11 Changed 2 years ago by gh-kliem

Thank you.

comment:12 Changed 2 years ago by Volker Braun

Status: positive_reviewneeds_work

Merge conflict

comment:13 Changed 2 years ago by gh-kliem

Branch: public/29962-rebpublic/29962-reb2
Commit: b62f781647851a3f27ecc95ea4b98b53838da1121d99129f26f4a065f9f9e5e13c3d5120a029e89f
Status: needs_workneeds_review

sage -t --long --all passes for me.


New commits:

1d99129fix merge conflict

comment:14 Changed 2 years ago by Matthias Köppe

Status: needs_reviewpositive_review

comment:15 Changed 2 years ago by gh-kliem

Thank you.

comment:16 Changed 2 years ago by Volker Braun

Branch: public/29962-reb21d99129f26f4a065f9f9e5e13c3d5120a029e89f
Resolution: fixed
Status: positive_reviewclosed

comment:17 Changed 2 years ago by Samuel Lelièvre

Commit: 1d99129f26f4a065f9f9e5e13c3d5120a029e89f

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

comment:18 Changed 2 years ago by gh-kliem

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.

Version 0, edited 2 years ago by gh-kliem (next)

comment:19 Changed 2 years ago by Samuel Lelièvre

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?

comment:20 Changed 2 years ago by Samuel Lelièvre

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

comment:21 Changed 2 years ago by gh-kliem

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.

Last edited 20 months ago by Samuel Lelièvre (previous) (diff)

comment:22 Changed 20 months ago by Samuel Lelièvre

Description: modified (diff)

comment:23 Changed 18 months ago by Samuel Lelièvre

Cc: Samuel Lelièvre added
Description: modified (diff)
Summary: Allow fuzzing of doctestsIntroduce random-seed option to allow fuzzing of doctests
Note: See TracTickets for help on using tickets.