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

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

Closed
williamstein opened this issue May 26, 2010 · 9 comments
Closed

Comments

@williamstein
Copy link
Contributor

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
False

Apply trac_9053_fixes_pivots.v2.patch

CC: @mminzlaff

Component: linear algebra

Author: Moritz Minzlaff

Reviewer: Keshav Kini

Merged: sage-4.7.alpha4

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

@mminzlaff
Copy link
Mannequin

mminzlaff mannequin commented Mar 18, 2011

comment:2

Attachment: trac_9053_fixes_pivots.patch.gz

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/gal_reps.py # Time out
        sage -t  -long -force_lib devel/sage/sage/interfaces/sage0.py # 2 doctests failed

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

@mminzlaff
Copy link
Mannequin

mminzlaff mannequin commented Mar 18, 2011

Author: Moritz Minzlaff

@mminzlaff mminzlaff mannequin added the s: needs review label Mar 18, 2011
@mminzlaff
Copy link
Mannequin

mminzlaff mannequin commented Mar 21, 2011

comment:3

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

@kini
Copy link
Contributor

kini commented Mar 22, 2011

line wrapping

@kini
Copy link
Contributor

kini commented Mar 22, 2011

comment:4

Attachment: trac_9053_fixes_pivots.v2.patch.gz

I can't replicate your doctest failures. Everything passes on sage.math, except the ever-troublesome devel/sage/sage/tests/startup.py , 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.

@kini
Copy link
Contributor

kini commented Mar 22, 2011

Reviewer: Keshav Kini

@kini
Copy link
Contributor

kini commented Mar 22, 2011

comment:5

(for patchbot...)

@kini

This comment has been minimized.

@jdemeyer
Copy link

jdemeyer commented Apr 7, 2011

Merged: sage-4.7.alpha4

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

5 participants