Opened 11 years ago
Closed 11 years ago
#10974 closed defect (fixed)
Overhaul matrix stack, augment
| Reported by: | rbeezer | Owned by: | jason, was |
|---|---|---|---|
| Priority: | minor | Milestone: | sage-4.7 |
| Component: | linear algebra | Keywords: | |
| Cc: | Merged in: | sage-4.7.alpha4 | |
| Authors: | Rob Beezer | Reviewers: | Keshav Kini |
| Report Upstream: | N/A | Work issues: | |
| Branch: | Commit: | ||
| Dependencies: | Stopgaps: |
Status badges
Description (last modified by )
This patch overhauls, consolidates, fixes and expands matrix augmentation and stacking.
Fixes:
- subdivisions of matrices are generally not preserved under these operations, but now are
Features:
- a vector may now be supplied as a second argument across all routines
Consolidation:
- there is some standardization of variable names and techniques across the routines
Expansion:
- augment() for dense integer matrices is uncommented and working now
Documentation:
- new features (vector argument, subdivisions) have been documented where added
- a general upgrade in documentation would be valuable, on another ticket
To Do:
- work to have result formed over a common ring
"general" patch adds vector argument and subdivision support to the generic stack() method. Subdivision management is split out as helper methods for use with routines in the "special" patch.
"special" patch works on routines for dense integer matrices, mod 2 matrices, and generic sparse matrices. It also moves the generic matrix augment() to a better location in the source (it was left in place so changes could be seen more readily on the "general" patch).
Apply:
Attachments (4)
Change History (14)
Changed 11 years ago by
Changed 11 years ago by
comment:1 Changed 11 years ago by
- Description modified (diff)
- Status changed from new to needs_review
comment:2 Changed 11 years ago by
- Reviewers set to Keshav Kini
- Status changed from needs_review to positive_review
comment:4 Changed 11 years ago by
Keshav,
Thanks for the review! The changes look good - thanks for those as well.
Rob
comment:5 Changed 11 years ago by
- Status changed from positive_review to needs_work
Change the commit message of the last patch such that it contains the ticket number.
Changed 11 years ago by
comment:6 Changed 11 years ago by
- Description modified (diff)
- Status changed from needs_work to positive_review
Commit message fixed in v2 version of reviewer patch.
comment:7 Changed 11 years ago by
- Description modified (diff)
comment:8 Changed 11 years ago by
- Status changed from positive_review to needs_work
This patch conflicts with #4983.
comment:9 Changed 11 years ago by
- Status changed from needs_work to positive_review
Let's leave this one alone. I'll rebase #4983. I'm setting this back to "positive review".
comment:10 Changed 11 years ago by
- Merged in set to sage-4.7.alpha4
- Resolution set to fixed
- Status changed from positive_review to closed
Three doctests failed on sage.math. I reran them and they failed to fail (one of them was devel/sage/sage/tests/startup.py ), so everything looks good. I did some minor formatting to keep line lengths down according to PEP 8 and will attach a reviewer patch accordingly, but feel free to veto it. Positive review!