matrix.__add__ vs. matrix.__mul__ #3004

Open
asmeurer opened this Issue Feb 19, 2013 · 5 comments

Comments

Projects
None yet
2 participants
In [64]: import numpy

In [65]: numpy.__version__
Out[65]: 1.7.0

In [66]: from sympy import Matrix

In [67]: type(numpy.matrix([[1, 2], [3, 4]]) + Matrix([[1, 2], [3, 4]]))
Out[67]: numpy.matrixlib.defmatrix.matrix

In [68]: type(numpy.matrix([[1, 2], [3, 4]])*Matrix([[1, 2], [3, 4]]))
Out[68]: sympy.matrices.dense.MutableDenseMatrix

In other words, matrix.__add__ gives a matrix, but matrix.__mul__ returns NotImplemented (or it just isn't implemented), and so falls back to sympy.Matrix.__rmul__.

I don't particularly care which behavior is used, but it should be consistant. Note that this is new in numpy 1.7.0. Previously, both of the above would return a SymPy Matrix.

Member

seberg commented Feb 26, 2013

I don't see through the depths of this problem or what might be right or wrong (the issue has something to do with using __array_priority__ logic for types that provide __array__, I guess). Now I do not know if the use of __array_priority__ to decide operator precedence for array-likes (or even subclasses?) makes a lot of sense in the first place, but I think sympy might want to boost its __array_priority__ to somewhat above 10 in any case (i.e. scipy.sparse uses 10.1).

Member

seberg commented Feb 27, 2013

@asmeurer just wanted to ping. Is this just an annoyance, or something that seriously breaks people's code? It does seem to me that changing operator precedence (which obviously in some way or another happened here) of array-likes in ufuncs, can potentially break quite a bit code out there. But then probably most either set their array_priority clear enough or do not use the __array__ way of speaking with numpy (while also wanting precedence over numpy arrays).

I don't know if it breaks people's code. It just came up as a failure in our test suite.

I can ask on our mailing list what people would prefer if you want. I suppose the easiest fix for us would then be to set the priority accordingly. Currently it is set to 10.0.

My whole point of opening the issue here was that + and * should be consistant in numpy. I guess __array_priority__ is ignored for * on matrices? numpy.matrix*sympy.Matrix gives sympy.Matrix regardless of what sympy.Matrix.__array_priority__ is.

Member

seberg commented Feb 28, 2013

Ok, just wondered if this needed to be investigated soon for 1.7, seems its not pressing. Yes, * is different since the operator is overloaded for matrix in python code (you can easily check that part if you like). Had a second look and I guess the actual change was really a bug fix, since it only happens for object array (sympy returns an object array with __array__). As to changing things here, not sure exactly what would be best in the first place to be honest, so if someone figures something out great, or maybe you are happy enough for now with changing that priority...

asmeurer added a commit to asmeurer/sympy that referenced this issue Feb 28, 2013

fishbeinb added a commit to fishbeinb/sympy that referenced this issue Mar 10, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment