Opened 6 months ago

Closed 6 months ago

multiplication of matrix with zero columns or rows fails

Reported by: Owned by: gh-mwageringel major sage-9.3 linear algebra coercion Travis Scrimshaw Markus Wageringel N/A 2fefdb1 2fefdb156e00dcc589e3b48e7de17d8fd81aa223

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.

comment:1 follow-up: ↓ 4 Changed 6 months ago by chapoton

At least one issue in `change_ring` about subdivisions:

```sage: A = matrix.identity(QQ, 4)
sage: A._subdivisions
sage: A2 = A.change_ring(RDF)
sage: A2._subdivisions
( [ 0, 4 ], [ 0, 4 ] )
```

coming from here

```sage: A = matrix.identity(QQ, 4)
sage: A._subdivisions
sage: A.subdivide(A.subdivisions())
sage: A._subdivisions
( [ 0, 4 ], [ 0, 4 ] )
```
Last edited 6 months ago by chapoton (previous) (diff)

comment:2 follow-up: ↓ 6 Changed 6 months ago by tscrim

• Authors set to Travis Scrimshaw
• Branch set to public/matrix/double_zero_matrix_mult-31234
• 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 follow-up: ↓ 7 Changed 6 months ago by chapoton

maybe we can also take the opportunity to fix the un-necessary subdivision ?

comment:4 in reply to: ↑ 1 Changed 6 months ago by gh-mwageringel

```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 git

• 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 gh-mwageringel

• Reviewers set to Markus Wageringel
• Status changed from needs_review to positive_review

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.