Opened 10 years ago
Closed 10 years ago
#11558 closed defect (fixed)
Echelonize with transformation=True oddness for sparse matrix
Reported by: | vbraun | Owned by: | jason, was |
---|---|---|---|
Priority: | major | Milestone: | sage-4.7.2 |
Component: | linear algebra | Keywords: | |
Cc: | Merged in: | sage-4.7.2.alpha1 | |
Authors: | Volker Braun | Reviewers: | Rob Beezer |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
sage: matrix(ZZ, 3, 4, [1..12], sparse=False).echelon_form(transformation=True) ( [ 1 2 3 4] [ 0 2 -1] [ 0 4 8 12] [ 0 9 -5] [ 0 0 0 0], [ 1 -2 1] ) sage: matrix(ZZ, 3, 4, [1..12], sparse=True).echelon_form(transformation=True) --------------------------------------------------------------------------- TypeError Traceback (most recent call last) /home/vbraun/opt/sage-4.7.1.alpha3/devel/sage-main/<ipython console> in <module>() /home/vbraun/Sage/sage/local/lib/python2.6/site-packages/sage/matrix/matrix2.so in sage.matrix.matrix2.Matrix.echelon_form (sage/matrix/matrix2.c:27633)() /home/vbraun/Sage/sage/local/lib/python2.6/site-packages/sage/matrix/matrix2.so in sage.matrix.matrix2.Matrix.echelonize (sage/matrix/matrix2.c:27292)() /home/vbraun/Sage/sage/local/lib/python2.6/site-packages/sage/matrix/matrix2.so in sage.matrix.matrix2.Matrix._echelonize_ring (sage/matrix/matrix2.c:26560)() TypeError: Cannot convert tuple to sage.matrix.matrix2.Matrix
Appy:
Attachments (1)
Change History (8)
comment:1 Changed 10 years ago by
- Reviewers set to Rob Beezer
- Status changed from new to needs_info
comment:2 Changed 10 years ago by
- I think copying elementwise is necessary since the lhs and rhs matrices have potentially different storage order. In particular for sparse matrices, the echelon form is computed with dense matrices and then copied back to a sparse matrix.
It seems like hermite_form()
is supposed to be an alias for echelon_form()
. But sparse matrices don't have a hermite_form()
method? I'll update the patch to fix that and also your points 1 & 2.
comment:3 Changed 10 years ago by
- Status changed from needs_info to needs_review
I've added more documentation. Some matrix backends don't support echelon_form(transformation=True)
, so eventually we should compute the transformation matrix from the pivots if necessary. I'll leave that for later :-)
comment:4 Changed 10 years ago by
Applies, builds, passes obvious tests on 4.7.1.alpha3. And looks good.
I'll run full tests overnight and then flip this to positive review.
Rob
comment:5 Changed 10 years ago by
- Description modified (diff)
- Status changed from needs_review to positive_review
Passes all long tests on 4.7.1.alpha3, so positive review. Thanks for cleaning this up.
comment:6 Changed 10 years ago by
- Description modified (diff)
comment:7 Changed 10 years ago by
- Merged in set to sage-4.7.2.alpha1
- Resolution set to fixed
- Status changed from positive_review to closed
Volker,
Unfortunate - but at least it is fixed now. Passes tests in the obvious places, I'll run long tests overnight.
Comments, in decreasing order of merit.
transformation
keyword in the docstring?I don't know, just seems inefficient to go entry-by-entry. Not critical, just a thought. Long weekend here in the US, but I'll stick with this.
Rob