Opened 10 years ago
Closed 9 years ago
#11306 closed enhancement (fixed)
Upgrade unitary check for RDF/CDF matrices
Reported by: | rbeezer | Owned by: | jason, was |
---|---|---|---|
Priority: | minor | Milestone: | sage-5.0 |
Component: | linear algebra | Keywords: | days30 |
Cc: | jason | Merged in: | sage-5.0.beta9 |
Authors: | Rob Beezer | Reviewers: | David Loeffler |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #11027, #10848, #11277 | Stopgaps: |
Description (last modified by )
This is an upgrade of the is_unitary()
method, based on experience building is_normal()
, is_hermitian()
. (#10848, #11104)
One test is discovering a bug elsewhere (#11248), so needs to be adjusted slightly to preserve that discovery, though at this writing, the test is disabled (#11277).
(a) Adds a "orthonormal" variant, which is now the default, based on the Schur decomposition, an idea used in the other two methods, but not considered here previously.
(b) Makes the existing naive algorithm a bit more efficent by using the break command twice.
(c) Fixes an ommission in the naive algorithm where the loop on i
previously did not start at zero.
(d) Upgraded error-checking on tolerance parameter.
(e) Docstring upgraded to reflect changes above.
Depends on:
Apply:
Attachments (2)
Change History (13)
Changed 10 years ago by
comment:1 Changed 10 years ago by
- Dependencies set to 11027, 10848, 11277
- Description modified (diff)
- Status changed from new to needs_review
comment:2 Changed 10 years ago by
- Keywords days30 added
comment:3 Changed 10 years ago by
- Dependencies changed from 11027, 10848, 11277 to #11027, #10848, #11277
comment:4 Changed 9 years ago by
- Cc jason added
comment:5 Changed 9 years ago by
- Reviewers set to David Loeffler
- Status changed from needs_review to positive_review
Looks good and passes doctests
comment:6 Changed 9 years ago by
- Status changed from positive_review to needs_work
There is some misformatting of the documentation (check http://sagemath.org/doc/developer/conventions.html#documentation-strings for a template):
dochtml.log:docstring of sage.matrix.matrix_double_dense.Matrix_double_dense.is_normal:29: WARNING: Literal block expected; none found. dochtml.log:docstring of sage.matrix.matrix_double_dense.Matrix_double_dense.is_normal:46: WARNING: Literal block expected; none found. dochtml.log:docstring of sage.matrix.matrix_double_dense.Matrix_double_dense.is_unitary:32: WARNING: Literal block expected; none found.
Changed 9 years ago by
comment:7 Changed 9 years ago by
- Status changed from needs_work to needs_review
Two of these aren't related to this ticket (I guess you were testing this and #11104 at the same time). The other one is fixed by the single-character patch above.
comment:8 Changed 9 years ago by
Dear David,
Gotta love those one-character patches. I'll get this reviewed as well.
Thanks for plowing though the "needs_rewview" backlog.
Rob
comment:9 Changed 9 years ago by
- Description modified (diff)
comment:10 Changed 9 years ago by
- Status changed from needs_review to positive_review
Positive review. I'm inclined to just leave the author/reviewer fields as-is, but if David wants to double them up, that's fine too.
Thanks for the review, David.
comment:11 Changed 9 years ago by
- Merged in set to sage-5.0.beta9
- Resolution set to fixed
- Status changed from positive_review to closed
For the patchbot:
Depends on 11027, 10848, 11277