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

Make modules doctests ready for random seeds #29978

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

Make modules doctests ready for random seeds #29978

kliem opened this issue Jun 24, 2020 · 23 comments

Comments

@kliem
Copy link
Contributor

kliem commented Jun 24, 2020

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 kliem added this to the sage-9.2 milestone Jun 24, 2020
@kliem
Copy link
Contributor Author

kliem commented Jun 24, 2020

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
Copy link
Contributor Author

kliem commented Jun 24, 2020

Dependencies: #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
1d7b00edash instead of underscore for command line options
38d74d1partially make modules fuzz ready

@kliem
Copy link
Contributor Author

kliem commented Jun 24, 2020

Branch: public/29978

@kliem
Copy link
Contributor Author

kliem commented Jun 24, 2020

Commit: 38d74d1

@mkoeppe mkoeppe modified the milestones: sage-9.2, sage-9.3 Sep 5, 2020
@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 15, 2021

comment:5

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

@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.4 Mar 15, 2021
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 9, 2021

Changed commit from 38d74d1 to 3d85786

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 9, 2021

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

3d85786make modules fuzz ready

@kliem
Copy link
Contributor Author

kliem commented Jun 9, 2021

Author: Jonathan Kliem

@kliem
Copy link
Contributor Author

kliem commented Jun 9, 2021

Changed keywords from none to random_seed, fuzz

@mwageringel
Copy link

Reviewer: Markus Wageringel

@mwageringel
Copy link

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 11, 2021

Changed commit from 3d85786 to 7fbdf5d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 11, 2021

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

7fbdf5dreplace unstable doctest by doctest according to code and documentation

@mwageringel
Copy link

comment:11

Thanks.

@kliem
Copy link
Contributor Author

kliem commented Jun 11, 2021

comment:12

Thank you.

@vbraun
Copy link
Member

vbraun commented Jun 19, 2021

comment:13

Merge conflict

@kliem
Copy link
Contributor Author

kliem commented Jun 22, 2021

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
Copy link
Contributor Author

kliem commented Jun 22, 2021

Changed commit from 7fbdf5d to 6799dc0

@kliem
Copy link
Contributor Author

kliem commented Jun 22, 2021

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
Copy link
Contributor Author

kliem commented Jun 22, 2021

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

@kliem
Copy link
Contributor Author

kliem commented Jun 22, 2021

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.

@vbraun
Copy link
Member

vbraun commented Jun 29, 2021

Changed branch from public/29978-reb to 6799dc0

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

4 participants