Opened 8 years ago

Closed 8 years ago

#14210 closed defect (fixed)

clean up Matrix_mpolynomial_dense

Reported by: malb Owned by: jason, was
Priority: major Milestone: sage-5.12
Component: linear algebra Keywords:
Cc: Bouillaguet, SimonKing Merged in: sage-5.12.beta1
Authors: Martin Albrecht, Volker Braun Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

Matrix_mpolynomial_dense while short is a mess. It's docstrings are not doctested due to formatting errors and it used hardcoded calls to libSingular where the more modern libSingular function factory should be used.

Attachments (2)

trac_14210_matrix_mpolynomial_dense.patch​ (28.8 KB) - added by malb 8 years ago.
trac_14210_reviewer.patch​ (14.6 KB) - added by vbraun 8 years ago.
Updated patch

Download all attachments as: .zip

Change History (9)

Changed 8 years ago by malb

comment:1 Changed 8 years ago by malb

  • Authors set to Martin Albrecht
  • Cc Bouillaguet SimonKing added
  • Status changed from new to needs_review

The attached patch

  • removes hardcoded calls from matrix_mpolynomial_dense.pyx and replaces them by calls through the libSingular function interface.
  • speeds up calls to det() for small matrices (n<= 3)
  • fixes formatting errors which meant that the file was not doctested
  • makes conversions from SingularElement to BooleanPolynomials faster (used to test code in matrix_mpolynomial_dense.pyx)
Last edited 8 years ago by malb (previous) (diff)

comment:2 Changed 8 years ago by vbraun

Looks good. You touch det but don't fix the following bug: 

sage: R.<x,y> = QQ[]
sage: m = matrix([[y,y,y,y]] * 4)
sage: m.charpoly()
x^4 - 4*y*x^3
sage: m.det()
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-13-398222b460ca> in <module>()
----> 1 m.det()

/home/vbraun/opt/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/matrix/matrix2.so in sage.matrix.matrix2.Matrix.det (sage/matrix/matrix2.c:9875)()

/home/vbraun/opt/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/matrix/matrix_mpolynomial_dense.so in sage.matrix.matrix_mpolynomial_dense.Matrix_mpolynomial_dense.determinant (sage/matrix/matrix_mpolynomial_dense.cpp:5736)()

/home/vbraun/opt/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/structure/element.so in sage.structure.element.Element.__getattr__ (sage/structure/element.c:3871)()

/home/vbraun/opt/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/structure/misc.so in sage.structure.misc.getattr_from_other_class (sage/structure/misc.c:1696)()

AttributeError: 'sage.rings.polynomial.polynomial_element.Polynomial_generic_dense' object has no attribute 'keys'

Changed 8 years ago by vbraun

Updated patch

comment:3 Changed 8 years ago by vbraun

  • Authors changed from Martin Albrecht to Martin Albrecht, Volker Braun
  • Reviewers set to Volker Braun

I've added a reviewer patch that cleans up the docstrings some more, achieves 100% coverage, and actually adds it to the reference manual. And fixes the det() bug.

Positive review to the original patch...

comment:4 Changed 8 years ago by vbraun

  • Type changed from PLEASE CHANGE to defect

comment:5 Changed 8 years ago by malb

  • Status changed from needs_review to positive_review

Reviewer patch looks okay to me.

comment:6 Changed 8 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:7 Changed 8 years ago by jdemeyer

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