sympy / sympy

A computer algebra system written in pure Python
https://sympy.org/
Other
12.84k stars 4.4k forks source link

Q.commutative contradicts is_commutative of Determinant #19308

Open eric-wieser opened 4 years ago

eric-wieser commented 4 years ago

I'm confused by this behavior:

>>> from sympy import *
>>> f = MatrixSymbol(Symbol('f'), Integer(2), Integer(2))
>>> x = Mul(Integer(-1), Determinant(f))
>>> x.is_commutative
False
>>> ask(Q.commutative(x))
True

Is there a bug here? Or do the two "commutative"s mean very different things?

oscarbenjamin commented 4 years ago

This is very strange:

In [1]: M = MatrixSymbol('M', 2, 2)                                                                                               

In [2]: e1 = Mul(-1, Determinant(M))                                                                                              

In [3]: e2 = - Determinant(M)                                                                                                     

In [4]: e1.is_commutative                                                                                                         
Out[4]: False

In [5]: e2.is_commutative                                                                                                         

In [6]: e1 == e2                                                                                                                  
Out[6]: True
asmeurer commented 4 years ago

The is_commutative is clearly wrong. Determinant is a number, so it is commutative. There is probably some incorrect logic somewhere that assumes that since the arguments are noncommutative, the expression is.

eric-wieser commented 4 years ago

I assume matrices of non-commutative symbols aren't supported anyway at the moment?

asmeurer commented 4 years ago

They might work with Matrix, though probably by accident. I don't know if it is tested. MatrixSymbol definitely represents a matrix of complex numbers.

eric-wieser commented 4 years ago

I ask because if not, this should be fixable with a is_commutative = True class variable?

sylee957 commented 4 years ago

I think that there are some fundamental flaws of treating commutativity based on its subexpressions. For example, for Basic, it treats whether an object is commutative or not based on whether its arguments are commutative or not.

https://github.com/sympy/sympy/blob/5d1b2c6f04f583ec0beeea4b8238d4151b497332/sympy/assumptions/handlers/common.py#L47-L51

oscarbenjamin commented 4 years ago

I don't understand what is happening because as far as I can tell e1 and e2 above are the same but they give different results for is_commutative.

eric-wieser commented 4 years ago

Note that attempting to fix this by setting Determinant.is_commutative = True, results in the error in #19327. The issue is that with that present, these lines are no longer executed:

https://github.com/sympy/sympy/blob/94fb720696f5f5d12bad8bc813699fd696afd2fb/sympy/simplify/simplify.py#L586-L587

meaning that Determinant.doit() is never called.

oscarbenjamin commented 4 years ago

I won't consider this issue fixed until I understand why e1 and e2 give different results.

eric-wieser commented 4 years ago

I won't consider this issue fixed until I understand why e1 and e2 give different results.

A partial explanation:

eric-wieser commented 4 years ago

And here's the thorough explanation:

Is is_commutative == None even meaningful?

At any rate, it's not correct for Determinant - help would be appreciated understanding why making it commutative (#19327) makes sympy crash.

oscarbenjamin commented 4 years ago

Is is_commutative == None even meaningful?

No, it's not. Either commutative is true in which case the object is known to commute with anything else or it is not in which case commutative should be False.

eric-wieser commented 4 years ago

Note that as of #19354 this is no longer an issue for Determinant, but exactly the same problem exists for any custom Basic instance with a non-commutative instance in its arguments.