Fix matrix rank with complicated elements #10650
Conversation
If some expresion does not contain symbols we can do _simplify() for that,
before comparing with zero.
modified: sympy/matrices/matrices.py
modified: sympy/matrices/tests/test_matrices.py
| @@ -26,6 +26,9 @@ | |||
|
|
|||
| def _iszero(x): | |||
| """Returns True if x is zero.""" | |||
| if len(x.free_symbols) == 0: | |||
There was a problem hiding this comment.
The len function call and comparison with zero are extraneous here. This is because x.free_symbols returns a python set data type. Python allows direct testing of container data types where an empty container returns false and is otherwise true.
In this case you could use
if not x.free_symbols:
(the "not" is included so that it will return True when the set is empty)
This reduces the complexity of the statement.
|
I like the simplicity of the fix and the addition of the test case going over the code mentioned in the issue. Please add some commenting to the change in the |
|
I've fixed that @jbm950 |
| # if we do not have free symbols, we will call _simplify() | ||
| # without risks to lose some confines for domain of an expression, | ||
| # because it will be some number | ||
| x = _simplify(x) |
There was a problem hiding this comment.
In general, I always give a -1 to having simplification calls in our code base. If x is an arbitrary matrix, simplify can take inordinate amounts of computation time to complete. An iszero check should be fast. You probably want to read the conversation around issues and pull requests like #10280.
If you are going to add this simplify call, then I request that you add a benchmark to https://github.com/sympy/sympy_benchmarks so that we track how slow things start to get.
There was a problem hiding this comment.
An iszero check should be fast.
I think, iszero check should be right too.
|
Relevant issue: #10279 |
|
I don't understand, it is bad fix and it won't be merged, or I should somesing to do. I wanted to know, if some expression (e.g. determinant) equals zero. In case, when expression is complicated, _iszero() works badly. I wanted to simplify expression, if expression is number (without free symbols) and only if. So, there is check to having free symbols and, if it does not have, we call _simplify(). If this behavior is so awful, let's think of a better. |
Fix is similar to sympy/sympy#10650, thanks to @rpisarev. In case of zero-decision problem - we raise NotImplementedError. Closes diofant#288 Closes sympy/sympy#9480 (Maybe it fixes also sympy/sympy#11238) See sympy/sympy#10280 for alternative.
Fix is similar to sympy/sympy#10650, thanks to @rpisarev. In case of zero-decision problem - we raise NotImplementedError. Closes diofant#288 Closes sympy/sympy#9480 (Maybe it fixes also sympy/sympy#11238) See sympy/sympy#10280 for alternative.
|
@moorepants Can you read #11479? |
|
This has been fixed in mainline. Time to close? |
Fix matrix rank with complicated elements.
If some expresion does not contain symbols we can do _simplify() for that,
before comparing with zero.
fixes #9480