Ticket #13012 (closed defect: fixed)
MatrixSpace is too eager to construct zero matrices
| Reported by: | novoselt | Owned by: | jason, was |
|---|---|---|---|
| Priority: | major | Milestone: | sage-5.1 |
| Component: | linear algebra | Keywords: | sd40.5 |
| Cc: | vbraun, kcrisman, rbeezer | Work issues: | |
| Report Upstream: | N/A | Reviewers: | William Stein |
| Authors: | Andrey Novoseltsev | Merged in: | sage-5.1.beta5 |
| Dependencies: | Stopgaps: |
Description
This is a follow up to #10793 where it was prohibited to silently change dimensions of a matrix preserving the number of entries (e.g. 3x2 to 2x3). Here is the problem with zeros:
sage: m = zero_matrix(2, 3) sage: m [0 0 0] [0 0 0] sage: M = MatrixSpace(ZZ, 3, 5) sage: M.zero() [0 0 0 0 0] [0 0 0 0 0] [0 0 0 0 0] sage: M(m) [0 0 0 0 0] [0 0 0 0 0] [0 0 0 0 0]
The last line should have raised an exception. I'm writing a patch.
Attachments
Change History
comment:1 Changed 12 months ago by novoselt
- Status changed from new to needs_review
- Authors set to Andrey Novoseltsev
comment:2 Changed 12 months ago by novoselt
- Status changed from needs_review to needs_work
- Work issues set to doctests
comment:3 Changed 12 months ago by novoselt
Not yet finished, but I am doing the following:
- concentrate matrix constructor code in MatrixSpace.matrix (currently a lot is done in its __call__);
- quit automatic determination of "data as columns" - this is dangerous since if the code relying on it ever hits a square matrix, it will give a hard to find mistake;
- drop support for constructing matrices from arbitrary combination of matrices/vectors/scalars introduced at #10628 (and released in Sage-5.0). This was not the goal of that ticket and I think it is a dangerous functionality since it easily lets through wrong code on "corner cases": those who deliberately want to augment or stack matrices should use corresponding methods.
If you object to anything on this list, please talk to me! I think that correctness of this approach is indirectly confirmed by the fact that only tests in matrix_space that demonstrate removed functionality fail in the matrix directory (a few tests fail because of a more informative error message).
comment:4 Changed 12 months ago by novoselt
- Status changed from needs_work to needs_review
Summary of what was done:
- MatrixSpace.__call__ just calls MatrixSpace.matrix, so that the logic is the same for both ways.
- row keyword is deprecated and no guessing for row=False is done - this was used only in matrix_space for demonstration, so it is unlikely that users use it either.
- Matrices are now constructed either from a list of entries or from a list of rows, there are special methods for stacking/augmenting matrices.
Two files had to be adjusted:
- linear_code had code which was very strange, I have rewritten it.
- matrix_morphism exploited undocumented functionality - I have removed it and fixed the test.

