Opened 7 years ago

Closed 6 years ago

#9053 closed defect (fixed)

Sage's new generic HNF doesn't quite work right wrt the free modules code

Reported by: was Owned by: jason, was
Priority: major Milestone: sage-4.7
Component: linear algebra Keywords:
Cc: minz Merged in: sage-4.7.alpha4
Authors: Moritz Minzlaff Reviewers: Keshav Kini
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by kini)

The last output below should obviously be True, but it is False.

sage: R.<x> = GF(7)[]
sage: A = R^3
sage: L = A.span([x*A.0 + (x^3 + 1)*A.1, x*A.2]); L
Free module of degree 3 and rank 2 over Univariate Polynomial Ring in x over Finite Field of size 7
Echelon basis matrix:
[      x x^3 + 1       0]
[      0       0       x]
sage: M = A.span([x*L.0]); M
Free module of degree 3 and rank 1 over Univariate Polynomial Ring in x over Finite Field of size 7
Echelon basis matrix:
[    x^2 x^4 + x       0]
sage: M.0 in L

Apply trac_9053_fixes_pivots.v2.patch

Attachments (2)

trac_9053_fixes_pivots.patch (1.5 KB) - added by minz 7 years ago.
trac_9053_fixes_pivots.v2.patch (4.0 KB) - added by kini 7 years ago.
line wrapping

Download all attachments as: .zip

Change History (8)

comment:1 Changed 7 years ago by AlexGhitza

  • Component changed from algebra to linear algebra
  • Owner changed from AlexGhitza to jason, was

Changed 7 years ago by minz

comment:2 Changed 7 years ago by minz

  • Authors set to Moritz Minzlaff
  • Cc minz added
  • Status changed from new to needs_review

The bug was a single line in _echelon_form_PID which returned the wrong pivot element for matrices of one row. The attached patch should fix that.

While doctesting all of Sage I received two errors (that seem unrelated?):

The following tests failed:

        sage -t  -long -force_lib devel/sage/sage/schemes/elliptic_curves/ # Time out
        sage -t  -long -force_lib devel/sage/sage/interfaces/ # 2 doctests failed

The first apparently also came up during discussions on #9390. The doctest failure in "randomly" appeared or not when I reran the test mutiple times. I'm not quite sure what to make of this...

comment:3 Changed 7 years ago by minz

I just reran the above two doctests on a different machine and receieved no doctest failures. *shrug*

Changed 7 years ago by kini

line wrapping

comment:4 Changed 7 years ago by kini

  • Reviewers set to Keshav Kini
  • Status changed from needs_review to positive_review

I can't replicate your doctest failures. Everything passes on sage.math, except the ever-troublesome devel/sage/sage/tests/ , which I tried again individually with no problems. The fix itself looks good. Reference builds, though how that could be affected I don't know. IIRC all code should be within 79 columns, so I split some lines in this function for you while you're at it. Feel free to rewrite it if it looks ugly, haha.

comment:5 Changed 7 years ago by kini

  • Description modified (diff)

(for patchbot...)

comment:6 Changed 6 years ago by jdemeyer

  • Merged in set to sage-4.7.alpha4
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.