#12964 closed defect (fixed)
multiply matrix of matrices by a scalar: boom
Reported by: | jhpalmieri | Owned by: | jason, was |
---|---|---|---|
Priority: | major | Milestone: | sage-5.1 |
Component: | linear algebra | Keywords: | |
Cc: | Merged in: | sage-5.1.beta6 | |
Authors: | Robert Bradshaw | Reviewers: | Mike Hansen |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
From stackoverflow: Define a
to be a matrix whose entries are matrices, and multiply a
by a scalar:
a = matrix([[matrix([[ 1, 2], [ 3, 4]]), matrix([[ 5, 6], [ 7, 8]])], [matrix([[ 9, 10], [11, 12]]), matrix([[13, 14], [15, 16]])]]) 3 * a
This results in the following error message:
TypeError: unsupported operand parent(s) for '*': 'Full MatrixSpace of 2 by 2 dense matrices over Integer Ring' and 'Full MatrixSpace of 2 by 2 dense matrices over Integer Ring'
which is clearly nonsense. Multiplying a
by, for example, 3 * identity_matrix(2)
works, by the way.
Apply only 12964-fix-mul.patch.
Attachments (2)
Change History (11)
Changed 4 years ago by
comment:1 Changed 4 years ago by
comment:2 Changed 4 years ago by
- Status changed from new to needs_review
comment:3 Changed 4 years ago by
Another option would be to define _mul_
on matrices to do _matrix_times_matrix_
.
comment:4 Changed 4 years ago by
My first thought when reading this patch (before even reading your comments) was also to add a _mul_ method on matrices. _rmul_ should always be safe to call with identical ring elements, and this assumption is used in many other places.
comment:5 follow-up: ↓ 9 Changed 4 years ago by
Robert: after your patch, regarding __mul__
and _mul_
for matrices in element.pyx
: what are their roles? In what order do they get called?
comment:6 Changed 4 years ago by
- Reviewers set to Mike Hansen
- Status changed from needs_review to positive_review
The matrix code (changed in the first patch) assumed that you can always call _mul_
on the entries of the matrix in order to do multiplication. However, this is not the case if the entries of the matrix are in fact matrices themselves since they bypass the coercion system, implement __mul__
directly (which calls _matrix_times_matrix_
), and don't implement _mul_
. Robert's patch just implements _mul_
to do "what it should" in case there are other places that assume that you can always use _mul_
directly.
Positive review for Robert's patch.
Apply only 12964-fix-mul.patch .
comment:7 Changed 4 years ago by
- Description modified (diff)
comment:8 Changed 4 years ago by
- Merged in set to sage-5.1.beta6
- Resolution set to fixed
- Status changed from positive_review to closed
comment:9 in reply to: ↑ 5 Changed 4 years ago by
Thanks, Mike, for revewing this.
Replying to jhpalmieri:
Robert: after your patch, regarding
__mul__
and_mul_
for matrices inelement.pyx
: what are their roles? In what order do they get called?
To close the loop on this, a.__mul__(b)
is called by Python when a * b is encountered. a._mul_(b)
is called by the coercion model when parent(a) is parent(b)
to actually do the arithmetic. Full details at http://www.sagemath.org/doc/reference/coercion.html
This happens because matrices don't really use
_mul_
. The end result is less efficient than it could be (since you're doing matrix multiplications for each entry); however, I think this would be for another ticket.