Opened 7 years ago

Closed 7 years ago

#13034 closed defect (fixed)

Fix index in saturation of zero matrices

Reported by: novoselt Owned by: jason, was
Priority: major Milestone: sage-5.1
Component: linear algebra Keywords: sd40.5
Cc: rbeezer, was Merged in: sage-5.1.beta6
Authors: Andrey Novoseltsev Reviewers: William Stein
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

Now:

sage: m = matrix(ZZ, 3)
sage: m.index_in_saturation()
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)

/home/novoselt/sage-5.1.beta0/devel/sage-main/<ipython console> in <module>()

/home/novoselt/sage-5.1.beta0/local/lib/python2.7/site-packages/sage/matrix/matrix_integer_dense.so in sage.matrix.matrix_integer_dense.Matrix_integer_dense.index_in_saturation (sage/matrix/matrix_integer_dense.c:17074)()

/home/novoselt/sage-5.1.beta0/local/lib/python2.7/site-packages/sage/matrix/matrix_integer_dense_saturation.pyc in index_in_saturation(A, proof)
    308     A = A.transpose()
    309     A = A.hermite_form(proof=proof,include_zero_rows=False)
--> 310     return abs(A.determinant(proof=proof))
    311     
    312 

/home/novoselt/sage-5.1.beta0/local/lib/python2.7/site-packages/sage/matrix/matrix_integer_dense.so in sage.matrix.matrix_integer_dense.Matrix_integer_dense.determinant (sage/matrix/matrix_integer_dense.c:25359)()

ValueError: self must be a square matrix

while it should be 1.

Attachments (1)

trac_13034_fix_index_in_saturation.patch (1.2 KB) - added by novoselt 7 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 7 years ago by novoselt

  • Authors set to Andrey Novoseltsev
  • Cc rbeezer was added
  • Status changed from new to needs_review

comment:2 Changed 7 years ago by was

  • Status changed from needs_review to needs_work
  1. Can you change return 1 to something like return Integer(1)?
  1. Can you rebase this against sage-5.0? For me, I get
    adding trac_13034_fix_index_in_saturation.patch to series file
    applying trac_13034_fix_index_in_saturation.patch
    patching file sage/matrix/matrix_integer_dense_saturation.py
    Hunk #1 FAILED at 290
    Hunk #2 FAILED at 298
    2 out of 2 hunks FAILED -- saving rejects to file sage/matrix/matrix_integer_dense_saturation.py.rej
    patch failed, unable to continue (try -v)
    patch failed, rejects left in working dir
    errors during apply, please fix and refresh trac_13034_fix_index_in_saturation.patch
    

Changed 7 years ago by novoselt

comment:3 Changed 7 years ago by novoselt

  • Status changed from needs_work to needs_review

I've changed 1 to ZZ(1), but didn't rebase - it applies smoothly to sage-5.1.beta0 with Python-2.7.3 from #13013 (I can't pop Python upgrade patch)

comment:4 Changed 7 years ago by was

  • Status changed from needs_review to positive_review

I didn't actually apply the patch (can't), but it looks good to me.

comment:5 Changed 7 years ago by jdemeyer

Please fill in your real name in the Author / Reviewer fields.

comment:6 Changed 7 years ago by novoselt

  • Reviewers set to William Stein

comment:7 Changed 7 years ago by jdemeyer

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