Opened 6 months ago
Closed 6 months ago
#31234 closed defect (fixed)
multiplication of matrix with zero columns or rows fails
Reported by:  ghmwageringel  Owned by:  

Priority:  major  Milestone:  sage9.3 
Component:  linear algebra  Keywords:  coercion 
Cc:  Merged in:  
Authors:  Travis Scrimshaw  Reviewers:  Markus Wageringel 
Report Upstream:  N/A  Work issues:  
Branch:  2fefdb1 (Commits, GitHub, GitLab)  Commit:  2fefdb156e00dcc589e3b48e7de17d8fd81aa223 
Dependencies:  Stopgaps: 
Description
For matrices with zero columns or rows and different base rings (at least when double fields are involved), multiplication fails:
sage: matrix.identity(QQ, 4) * matrix(RDF, 4, 0) ... ValueError: matrix is immutable; please change a copy instead (i.e., use copy(M) to change a copy of M). sage: matrix.identity(RDF, 4) * matrix(QQ, 4, 0) ... ValueError: matrix is immutable; please change a copy instead (i.e., use copy(M) to change a copy of M). sage: matrix(QQ, 0, 4) * matrix.identity(RDF, 4) ... ValueError: matrix is immutable; please change a copy instead (i.e., use copy(M) to change a copy of M).
Using copy()
, as suggested, does not work.
Change History (8)
comment:1 followup: ↓ 4 Changed 6 months ago by
comment:2 followup: ↓ 6 Changed 6 months ago by
 Branch set to public/matrix/double_zero_matrix_mult31234
 Commit set to 214e9b4bbced06cd0a09dc21f47f606a110f62e5
 Status changed from new to needs_review
I have found the problem. In matrix_double_dense.pyx
, the corresponding _matrix_times_matrix_
has the following:
if self._nrows == 0 or self._ncols == 0 or right._nrows == 0 or right._ncols == 0: return self.matrix_space(self._nrows, right._ncols).zero_matrix()
So the result is the immutable zero matrix. Compare:
sage: matrix.identity(RDF, 4) * matrix(RDF, 4, 0) [] sage: _.is_mutable() False sage: matrix.identity(QQ, 4) * matrix(QQ, 4, 0) [] sage: _.is_mutable() True
I think the most consistent thing is to return an immutable matrix. So that is what I changed it to.
New commits:
214e9b4  Return a mutable matrix for Matrix_double_dense even if one of the dimensions is 0.

comment:3 followup: ↓ 7 Changed 6 months ago by
maybe we can also take the opportunity to fix the unnecessary subdivision ?
comment:4 in reply to: ↑ 1 Changed 6 months ago by
Replying to chapoton:
sage: A = matrix.identity(QQ, 4) sage: A._subdivisions sage: A.subdivide(A.subdivisions()) sage: A._subdivisions ( [ 0, 4 ], [ 0, 4 ] )
It looks like this is not a problem.
sage: A = matrix.identity(QQ, 4) sage: A.subdivisions() ([], []) sage: A.subdivide(A.subdivisions()) sage: A.subdivisions() ([], [])
I assume that A._subdivisions
contains [0, 4]
just because it makes it more conventient to work with, internally.
comment:5 Changed 6 months ago by
 Commit changed from 214e9b4bbced06cd0a09dc21f47f606a110f62e5 to 2fefdb156e00dcc589e3b48e7de17d8fd81aa223
Branch pushed to git repo; I updated commit sha1. New commits:
2fefdb1  31234: fix trailing whitespace

comment:6 in reply to: ↑ 2 Changed 6 months ago by
 Reviewers set to Markus Wageringel
 Status changed from needs_review to positive_review
Replying to tscrim:
I think the most consistent thing is to return an immutable matrix. So that is what I changed it to.
Thank you for the fix. I have removed some trailing whitespace. Other than that, this looks good to me, so I am setting this to positive.
comment:7 in reply to: ↑ 3 Changed 6 months ago by
comment:8 Changed 6 months ago by
 Branch changed from public/matrix/double_zero_matrix_mult31234 to 2fefdb156e00dcc589e3b48e7de17d8fd81aa223
 Resolution set to fixed
 Status changed from positive_review to closed
At least one issue in
change_ring
about subdivisions:coming from here