Opened 9 months ago
Closed 9 months ago
#33751 closed defect (fixed)
random doctest failure in src/sage/matrix/matrix_integer_dense_hnf.py
Reported by:  Lorenz Panny  Owned by:  

Priority:  blocker  Milestone:  sage9.6 
Component:  linear algebra  Keywords:  
Cc:  Merged in:  
Authors:  Jonathan Kliem  Reviewers:  Kevin Lui 
Report Upstream:  N/A  Work issues:  
Branch:  7ac22ad (Commits, GitHub, GitLab)  Commit:  7ac22ad241f631db06e2f725f23c67130f8f19b6 
Dependencies:  Stopgaps: 
Description
Part of #32544:
sage t randomseed=47467259736041671371069989299524409608 src/sage/matrix/matrix_integer_dense_hnf.py ********************************************************************** File "src/sage/matrix/matrix_integer_dense_hnf.py", line 1240, in sage.matrix.matrix_integer_dense_hnf.? Failed example: matrix_integer_dense_hnf.sanity_checks(times=5, check_using_magma=False) Exception raised: Traceback (most recent call last): File "~/sage/src/sage/doctest/forker.py", line 695, in _run self.compile_and_execute(example, compiler, test.globs) File "~/sage/src/sage/doctest/forker.py", line 1093, in compile_and_execute exec(compiled, globs) File "<doctest sage.matrix.matrix_integer_dense_hnf.?[1]>", line 1, in <module> matrix_integer_dense_hnf.sanity_checks(times=Integer(5), check_using_magma=False) File "~/sage/src/sage/matrix/matrix_integer_dense_hnf.py", line 1285, in sanity_checks __do_check([random_matrix(ZZ, n, m, x=2**32, y=2**32) File "~/sage/src/sage/matrix/matrix_integer_dense_hnf.py", line 1276, in __do_check if hnf(a)[0] != a.echelon_form(algorithm='pari'): File "~/sage/src/sage/matrix/matrix_integer_dense_hnf.py", line 1106, in hnf H, pivots = probable_hnf(A, include_zero_rows=include_zero_rows, File "~/sage/src/sage/matrix/matrix_integer_dense_hnf.py", line 926, in probable_hnf H = hnf_square(C, proof=proof) File "~/sage/src/sage/matrix/matrix_integer_dense_hnf.py", line 568, in hnf_square raise ValueError("A must be square.") ValueError: A must be square. **********************************************************************
(From a patchbot run in #33619.)
Change History (7)
comment:1 Changed 9 months ago by
comment:2 Changed 9 months ago by
But running something in an infinite loop until it works is also an awful idea, I guess.
comment:3 Changed 9 months ago by
Authors:  → Jonathan Kliem 

Branch:  → public/33751 
Commit:  → 7ac22ad241f631db06e2f725f23c67130f8f19b6 
Status:  new → needs_review 
I guess in the case of proof=True
we just add another test to the loop.
I still do not like the idea of running something in infinite loop (especially as I don't even know what it is doing).
But probable_hnf
explicitly states that this can happen and that in this case it should be called again, so hnf
with proof=True
should account for this.
New commits:
7ac22ad  catch value error of probable_hnf

comment:4 Changed 9 months ago by
Reviewers:  → Kevin Lui 

Status:  needs_review → positive_review 
comment:6 Changed 9 months ago by
Priority:  minor → blocker 

comment:7 Changed 9 months ago by
Branch:  public/33751 → 7ac22ad241f631db06e2f725f23c67130f8f19b6 

Resolution:  → fixed 
Status:  positive_review → closed 
Note: See
TracTickets for help on using
tickets.
This is awful.
hnf
callsprobable_hnf
unconditionally.probable_hnf
clearly states that it might raise an exception and must be called again in this case. The documentation ofhnf
makes no such statements and I guess just assumes thatprobable_hnf
is always correct.