compiler: Patch premature lowering of EvalDerivative #1979
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1979 +/- ##
=======================================
Coverage 87.90% 87.90%
=======================================
Files 214 214
Lines 36213 36232 +19
Branches 5461 5465 +4
=======================================
+ Hits 31832 31849 +17
- Misses 3866 3868 +2
Partials 515 515
Help us with your feedback. Take ten seconds to tell us how you rate us. |
| @@ -423,14 +423,30 @@ def __new__(cls, *args, **kwargs): | |||
| # a*1 -> a | |||
| args = [i for i in args if i != 1] | |||
|
|
|||
| # a*-1 -> a | |||
There was a problem hiding this comment.
good catch. will put a*-1 (see below)
| # a*-1*-1 -> a | ||
| # a*-1*-1*-1 -> a*-1 |
There was a problem hiding this comment.
the arguments are a and -1 (unless a it's an integer) that's why it's not factored out
| # will preserve the integrity of `EvalDerivative`s, if any. | ||
| try: | ||
| a, b = args | ||
| if not a.is_zero and a.is_Rational: |
There was a problem hiding this comment.
a.is_zero is un-necessary would have been cought line 420 already.
| a, b = args | ||
| if not a.is_zero and a.is_Rational: | ||
| r, b = b.as_coeff_Mul() | ||
| if r is sympy.S.One and type(b) is Add: |
There was a problem hiding this comment.
Can this be generalized to like r is sympy.Number or r.is_Symbol and merge it with a?
There was a problem hiding this comment.
for the record, I lifted it from sympy
https://github.com/sympy/sympy/blob/master/sympy/core/mul.py#L285
tho admittedly a slightly older version than the one I have installed locally
I will try to improve it
|
pushed edits |
|
note latest pep8-related commit necessary due to https://github.com/devitocodes/devito/runs/7605926638?check_suite_focus=true |
fixes #1978