Make combinat doctests ready for random seeds
Closed this issue · 30 comments
Part of #29935.
This ticket makes
sage -t --long --random-seed=n src/sage/combinat/
pass for n more general than just 0.
See also:
- #30186 containment check for standard bracketed Lyndon words does not work.
Along the way we fix a tiny but annoying bug, where random derangements could fail.
That was because randint(0, n) has range range(0, n + 1),
which had caused confusion.
Depends on #29962
Component: doctest framework
Author: Jonathan Kliem
Branch: 928863f
Reviewer: Sébastien Labbé
Issue created by migration from https://trac.sagemath.org/ticket/29974
Branch: public/29974
Merge conflict.
I also need to go through it again and check I respected the design decision in #29935.
Note that elements of g = GelfandTsetlinPatterns(4, 5) have the wrong parent.
Description changed:
---
+++
@@ -4,3 +4,5 @@
sage -t --long --random-seed=n src/sage/combinat/
```
pass for different values `n` than just `0`.
+
+See also: #30186 containment check for standard bracketed lyndon words does not work Changed branch from public/29974 to public/29974-reb
Commit: a914b8d
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
eedbe5e | document random_seed |
998b1b9 | default random seed 0 for now |
1d7b00e | dash instead of underscore for command line options |
b31e2d5 | Merge branch 'public/29962' of git://trac.sagemath.org/sage into public/29962-reb |
2f30dd9 | small fixes |
b62f781 | doctests do not start from a random seed by default yet |
1d99129 | fix merge conflict |
d40d826 | make combinat fuzz ready |
b9f88cf | another file for combinat |
a914b8d | modify doctests according to decisions in 29935 |
I get one failure in combinat:
----------------------------------------------------------------------
sage -t --long --random-seed=2 src/sage/combinat/derangements.py # 1 doctest failed
----------------------------------------------------------------------
Can you reproduce it?
sage -t --long --random-seed=2 src/sage/combinat/derangements.py
**********************************************************************
File "src/sage/combinat/derangements.py", line 129, in sage.combinat.derangements.Derangements
Failed example:
D2.random_element() # random
Exception raised:
Traceback (most recent call last):
File "/home/slabbe/GitBox/sage/local/lib/python3.7/site-packages/sage/doctest/forker.py", line 715, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/home/slabbe/GitBox/sage/local/lib/python3.7/site-packages/sage/doctest/forker.py", line 1139, in compile_and_execute
exec(compiled, globs)
File "<doctest sage.combinat.derangements.Derangements[7]>", line 1, in <module>
D2.random_element() # random
File "/home/slabbe/GitBox/sage/local/lib/python3.7/site-packages/sage/combinat/derangements.py", line 504, in random_element
return L[i]
IndexError: list index out of range
**********************************************************************
1 item had failures:
1 of 9 in sage.combinat.derangements.Derangements
[85 tests, 1 failure, 0.10 s]
Yes, I can confirm the problem.
Reviewer: Sébastien Labbé
But it works for --random-seed=0, --random-seed=1, --random-seed=3, --random-seed=4.
Changed branch from public/29974-reb to public/29974-reb2
Description changed:
---
+++
@@ -5,4 +5,6 @@
```
pass for different values `n` than just `0`.
-See also: #30186 containment check for standard bracketed lyndon words does not work
+See also: #30186 containment check for standard bracketed lyndon words does not work.
+
+Along the way we fix a tiny but annoying bug, where random dearangments could fail, because `randint(0,n)` has range `range(0,n+1)`. There is
from random import randrange
for that. Do you mind to change to the lesser known randrange?
Branch pushed to git repo; I updated commit sha1. New commits:
a391a3e | use randrange instead of randint |
I like randrange much better. It is much more intuitive if you are used to range. (But reading the very short docstring that was exactly the point of introducing it.)
In fact, I think we want to import it from prandom
from sage.misc.prandom import randrange
in order for it to depend on the --random-seed.
I was wondering about what the point was of duplicating that thing.
Branch pushed to git repo; I updated commit sha1. New commits:
928863f | import randrange from the correct place |
Thank you.
Changed branch from public/29974-reb2 to 928863f
Description changed:
---
+++
@@ -1,10 +1,16 @@
+Part of #29935.
+
This ticket makes
```
sage -t --long --random-seed=n src/sage/combinat/
```
-pass for different values `n` than just `0`.
+pass for `n` more general than just `0`.
-See also: #30186 containment check for standard bracketed lyndon words does not work.
+See also:
+- #30186 containment check for standard bracketed Lyndon words does not work.
-Along the way we fix a tiny but annoying bug, where random dearangments could fail, because `randint(0,n)` has range `range(0,n+1)`.
+Along the way we fix a tiny but annoying bug, where random derangements could fail.
+
+That was because `randint(0, n)` has range `range(0, n + 1)`,
+which had caused confusion.