sagemath/sage

Make modules doctests ready for random seeds

Closed this issue · 23 comments

kliem commented

This ticket makes

sage -t --long --random-seed=n src/sage/modules/

pass for different values n than just 0.

Depends on #29962

Component: doctest framework

Keywords: random_seed, fuzz

Author: Jonathan Kliem

Branch/Commit: 6799dc0

Reviewer: Markus Wageringel

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

kliem commented
comment:1

I have a partial fix. After that at least the following need fixes

sage -t --long --random-seed=151058820726654196682836430928254760259 src/sage/modules/fg_pid/fgp_module.py  # 3 doctests failed
sage -t --long --random-seed=151058820726654196682836430928254760259 src/sage/modules/filtered_vector_space.py  # 1 doctest failed
sage -t --long --random-seed=151058820726654196682836430928254760259 src/sage/modules/free_module_element.pyx  # 11 doctests failed
sage -t --long --random-seed=151058820726654196682836430928254760259 src/sage/modules/free_module.py  # 9 doctests failed
sage -t --long --random-seed=151058820726654196682836430928254760259 src/sage/modules/multi_filtered_vector_space.py  # 1 doctest failed
sage -t --long --random-seed=151058820726654196682836430928254760259 src/sage/modules/vector_integer_dense.pyx  # 1 doctest failed
sage -t --long --random-seed=151058820726654196682836430928254760259 src/sage/modules/vector_mod2_dense.pyx  # 17 doctests failed
kliem commented

Dependencies: #29962

kliem commented

New commits:

da1c6bestart from a "random" random seed for doctesting
b7b836dmake random seed reproducible
eedbe5edocument random_seed
998b1b9default random seed 0 for now
1d7b00edash instead of underscore for command line options
38d74d1partially make modules fuzz ready
kliem commented

Branch: public/29978

kliem commented

Commit: 38d74d1

comment:5

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

Changed commit from 38d74d1 to 3d85786

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

3d85786make modules fuzz ready
kliem commented

Author: Jonathan Kliem

kliem commented

Changed keywords from none to random_seed, fuzz

Reviewer: Markus Wageringel

comment:8

The following test is not quite robust, but otherwise the changes look good to me.

sage -t --long --warn-long 83.2 --random-seed=3032 src/sage/modules/free_module_integer.py
**********************************************************************
File "src/sage/modules/free_module_integer.py", line 314, in sage.modules.free_module_integer.FreeModule_submodule_with_basis_integer.reduced_basis
Failed example:
    LLL == L.reduced_basis
Expected:
    True
Got:
    False
sage -t --long --warn-long 83.2 --random-seed=3034 src/sage/modules/free_module_integer.py
**********************************************************************
File "src/sage/modules/free_module_integer.py", line 316, in sage.modules.free_module_integer.FreeModule_submodule_with_basis_integer.reduced_basis
Failed example:
    bool(min(a.norm() for a in LLL) == LLL[0].norm())
Expected:
    True
Got:
    False
**********************************************************************

Changed commit from 3d85786 to 7fbdf5d

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

7fbdf5dreplace unstable doctest by doctest according to code and documentation
comment:11

Thanks.

kliem commented
comment:12

Thank you.

comment:13

Merge conflict

kliem commented
comment:14

This is the merge conflict:

diff --cc src/sage/modules/fg_pid/fgp_module.py
index 935d0ab5c0,9349f2fd42..0000000000
--- a/src/sage/modules/fg_pid/fgp_module.py
+++ b/src/sage/modules/fg_pid/fgp_module.py
@@@ -1698,8 -1698,13 +1698,18 @@@ class FGP_Module_class(Module)
  
              sage: V = span([[1/2,1,1],[3/2,2,1],[0,0,1]],ZZ); W = V.span([2*V.0+4*V.1, 9*V.0+12*V.1, 4*V.2])
              sage: Q = V/W
++<<<<<<< HEAD
 +            sage: Q.random_element()
 +            (1, 5)
++=======
+             sage: Q.random_element().parent() is Q
+             True
+             sage: Q.cardinality()
+             48
+             sage: S = set()
+             sage: while len(S) < 48:
+             ....:     S.add(Q.random_element())
++>>>>>>> 7fbdf5d2d9b177bfa812427dcc83931e83c62be4
          """
          return self(self._V.random_element(*args, **kwds))
kliem commented

Changed commit from 7fbdf5d to 6799dc0

kliem commented
comment:15

This should do it.

Can't test it at the moment, as the new release breaks my sympy (see sage-release post).


New commits:

6799dc0merge public/29978
kliem commented

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

kliem commented
comment:16

Ok, I pip installed mpmath. Which is probably not clean, but it works to test that I didn't mess up anthing fixing the merge conflict.

Changed branch from public/29978-reb to 6799dc0