MatrixSpace is too eager to construct zero matrices

[novoselt]

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.

CC: @vbraun @kcrisman @rbeezer

Component: linear algebra

Keywords: sd40.5

Author: Andrey Novoseltsev

Reviewer: William Stein

Merged: sage-5.1.beta5

Issue created by migration from https://trac.sagemath.org/ticket/13012

[novoselt]

Author: Andrey Novoseltsev

[novoselt]

Work Issues: doctests

[novoselt]

comment:3

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 initialization of matrices from vectors or list of lists can be way faster #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).

[novoselt]

Attachment: trac_13012_bug_with_zero_matrices.patch.gz

[novoselt]

comment:4

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.

[williamstein]

comment:5

LGTM

[jdemeyer]

comment:6

Please fill in your real name in the Author / Reviewer fields.

[novoselt]

Reviewer: William Stein

[jdemeyer]

Changed work issues from doctests to none

[jdemeyer]

Merged: sage-5.1.beta5