Opened 19 months ago

Closed 19 months ago

Last modified 8 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: slelievre 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 slelievre)

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 19 months ago by gh-kliem

  • Branch set to public/29962
  • Commit set to 998b1b94ce1289ea92451a86e5f6191c37eaeb5a
  • Status changed from new to needs_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 19 months ago by git

  • Commit changed from 998b1b94ce1289ea92451a86e5f6191c37eaeb5a to b6a5dc7c96125b6140e56feafb82dd13599998e7

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 19 months ago by git

  • Commit changed from b6a5dc7c96125b6140e56feafb82dd13599998e7 to 998b1b94ce1289ea92451a86e5f6191c37eaeb5a

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

comment:4 Changed 19 months ago by git

  • Commit changed from 998b1b94ce1289ea92451a86e5f6191c37eaeb5a to 1d7b00e3fc2ebc1dc9982a2df91d15e3f12e9432

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

1d7b00edash instead of underscore for command line options

comment:5 Changed 19 months ago by gh-kliem

  • Description modified (diff)

comment:6 Changed 19 months ago by gh-mwageringel

  • Reviewers set to 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 19 months ago by mkoeppe

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 19 months ago by gh-kliem

  • Branch changed from public/29962 to public/29962-reb
  • Commit changed from 1d7b00e3fc2ebc1dc9982a2df91d15e3f12e9432 to b62f781647851a3f27ecc95ea4b98b53838da112

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 19 months ago by mkoeppe

  • Reviewers changed from Markus Wageringel to Markus Wageringel, Matthias Koeppe

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

comment:10 Changed 19 months ago by gh-mwageringel

  • Status changed from needs_review to positive_review

Thanks. This successfully passes ptestlong.

comment:11 Changed 19 months ago by gh-kliem

Thank you.

comment:12 Changed 19 months ago by vbraun

  • Status changed from positive_review to needs_work

Merge conflict

comment:13 Changed 19 months ago by gh-kliem

  • Branch changed from public/29962-reb to public/29962-reb2
  • Commit changed from b62f781647851a3f27ecc95ea4b98b53838da112 to 1d99129f26f4a065f9f9e5e13c3d5120a029e89f
  • Status changed from needs_work to needs_review

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


New commits:

1d99129fix merge conflict

comment:14 Changed 19 months ago by mkoeppe

  • Status changed from needs_review to positive_review

comment:15 Changed 19 months ago by gh-kliem

Thank you.

comment:16 Changed 19 months ago by vbraun

  • Branch changed from public/29962-reb2 to 1d99129f26f4a065f9f9e5e13c3d5120a029e89f
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:17 Changed 17 months ago by slelievre

  • Commit 1d99129f26f4a065f9f9e5e13c3d5120a029e89f deleted

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

comment:18 Changed 17 months 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 17 months ago by gh-kliem (next)

comment:19 Changed 17 months ago by slelievre

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 17 months ago by slelievre

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

comment:21 Changed 17 months 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 9 months ago by slelievre (previous) (diff)

comment:22 Changed 9 months ago by slelievre

  • Description modified (diff)

comment:23 Changed 8 months ago by slelievre

  • Cc slelievre added
  • Description modified (diff)
  • Summary changed from Allow fuzzing of doctests to Introduce random-seed option to allow fuzzing of doctests
Note: See TracTickets for help on using tickets.