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.

(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.)

Last edited 2 years ago by gh-kliem (previous) (diff)

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 https://trac.sagemath.org/ticket/29935.

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

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

comment:22 Changed 19 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.