Lambdify doesn't recognize derivative symbol if cse is enabled
Here is a minimal reproducer:
>>> import sympy as sm
>>> t = sm.symbols("t")
>>> x = sm.Function("x")(t)
>>> xd = x.diff(t)
>>> sm.lambdify((xd, x), xd + x)(1, 1)
2
>>> sm.lambdify((xd, x), xd, cse=True)(1, 1)
1
>>> sm.lambdify((xd, x), xd + x, cse=True)(1, 1)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "...\sympy\utilities\lambdify.py", line 875, in lambdify
funcstr = funcprinter.doprint(funcname, iterable_args, _expr, cses=cses)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "...\sympy\utilities\lambdify.py", line 1166, in doprint
str_expr = _recursive_to_string(self._exprrepr, expr)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "...\sympy\utilities\lambdify.py", line 961, in _recursive_to_string
return doprint(arg)
^^^^^^^^^^^^
File "...\sympy\printing\codeprinter.py", line 172, in doprint
lines = self._print(expr).splitlines()
^^^^^^^^^^^^^^^^^
File "...\sympy\printing\printer.py", line 331, in _print
return printmethod(expr, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "...\sympy\printing\str.py", line 57, in _print_Add
t = self._print(term)
^^^^^^^^^^^^^^^^^
File "...\sympy\printing\printer.py", line 331, in _print
return printmethod(expr, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "...\sympy\printing\codeprinter.py", line 582, in _print_not_supported
raise PrintMethodNotImplementedError("Unsupported by %s: %s" % (str(type(self)), str(type(expr))) + \
sympy.printing.codeprinter.PrintMethodNotImplementedError: Unsupported by <class 'sympy.printing.numpy.SciPyPrinter'>: <class 'sympy.core.function.Derivative'>
Set the printer option 'strict' to False in order to generate partially printed code.
I am not sure if this issue is easily solvable as the problem seems to be that it tries to print the expression x0 + Derivative(x0, t) with the [(x0, x(t))] as subexpressions.
I thought this would work, but it doesn't:
In [1]: import sympy as sm
In [2]: t = sm.symbols("t")
In [3]: x = sm.Function("x")(t)
In [4]: xd = x.diff(t)
In [5]: sm.lambdify((xd, x), xd + x, cse=True, dummify=True)(1, 1)
---------------------------------------------------------------------------
NameError Traceback (most recent call last)
Cell In[5], line 1
----> 1 sm.lambdify((xd, x), xd + x, cse=True, dummify=True)(1, 1)
File <lambdifygenerated-1>:5, in _lambdifygenerated(_Dummy_36, _Dummy_37)
1 def _lambdifygenerated(_Dummy_36, _Dummy_37):
2 x0 = _Dummy_37
3 return ( # Not supported in Python with SciPy and NumPy:
4 # Derivative
----> 5 x0 + Derivative(x0, t))
NameError: name 'Derivative' is not defined
cse is called on the expressions and then the cse results are passed to the printer which runs a preprocessor on the results that do the dummifying. I'm guessing those operations should be swapped for things to work correctly.
Hello,
I've looked a bit into the code, I think the issue is specific to using xd = x.diff(t). This case is not tested(referring to test_lambdify.py) and with the current implementation, the Dummy_XY are not placed correctly in the expression calculated by cse since you would need to look recursively and replace recursively in the Derivative(x0, t) when x0 is replaced with a dummy (although looking recursively might not be needed if we could implement a simple way to link them) .
I'll look into it more and indeed, if we call cse after the printer it works perfectly, but then the results are not used.
The dummy replacement works recursively already. Maybe just look into having the preprocessor apply the dummy subs to all of the cse output.
Actually this code looks like the preprocessor is applied to all outputs of cse:
https://github.com/sympy/sympy/blob/master/sympy/utilities/lambdify.py#L1133-L1140
It looks like in the CSE process, which happens first, the argument of the derivative gets swapped out:
ipdb> expr
[x0 + Derivative(x0, t), x(t)]
This would then cause the dummy replacement to be missed because it is looking for Derivative(x, t) (probably). So making the dummy replacement happen first might be better.
In [3]: x + xd
Out[3]: x(t) + Derivative(x(t), t)
In [4]: sm.cse(x + xd)
Out[4]: ([(x0, x(t))], [x0 + Derivative(x0, t)])
One option would be a flag to cse() to have it skip Derivative(x, t) type terms.
Maybe a pre and post optimization could be used for cse, something like:
In [27]: def pre(expr): return expr.xreplace({xd: sm.Symbol('a')})
In [28]: def post(expr): return expr.xreplace({sm.Symbol('a'): xd})
In [29]: sm.cse(x + xd, optimizations=[(pre, post)])
Out[29]: ([], [x(t) + Derivative(x(t), t)])
In [31]: sm.cse(x + x*x + xd, optimizations=[(pre, post)])
Out[31]: ([(x0, x(t))], [x0**2 + x0 + Derivative(x(t), t)])
This would then cause the dummy replacement to be missed because it is looking for Derivative(x, t) (probably).
That's what I ended up thinking to
One option would be a flag to cse() to have it skip Derivative(x, t) type terms.
A flag could work, I was also thinking that we could consider the derivative (Derivative(x0, t)) like a replacement but I think the flag would be more practical.
Maybe a pre and post optimization could be used for cse
It would work to, but what are the advantages to this compared to the flag possibility?(since with this possibility, we would go trhough the expression twice and still ignore the derivatives)
It would work to, but what are the advantages to this compared to the flag possibility?(since with this possibility, we would go trhough the expression twice and still ignore the derivatives)
It is a flag that is already present, so we would not need to implement a new kwarg in cse.
It is a flag that is already present, so we would not need to implement a new kwarg in cse.
But isn't there some cases where we want the arguments of the derivative also changed accordingly?( instead of Derivative(x,t), we would want Derivative(x0,t))
I've been trying to implement the change(not using pre-prost method but a flad indicating cse is called inside lambdify), but I'm running into a difficulty and I don't know how to resolve it.
I've added an optionnal parameter in cse that indicates that it is called inside a lambdify function. When I simply used it as a non-optionnal parameter (put right after expr), the function worked perfectly and the bug was resolved. However, when put as an optionnal parameter, the value of flag_lambdify(name of the flag) isn't modified, and I've noticed that it is the same for the optionnal parameter list.
To see this problem, add the new flag variable, add a print(list, flag_lambdify) under the header of the function cse and enter the lines of code above that produced the bug. The print is also executed 2 times( you'll have a first print with the right values, and a second one with the default values).
If you want, you can also see the modified files in my sympy repo(although I'm not sure I left the print(list, flag_lambdify) when I pushed).
Also, I've not put the flag_lamdbify as a non-optionnal parameter because I think it is used in other files, but if it is not it would solve the problem( but not the problem with the list variable)
This seems to be fixed in master:
moorepants@nandi:sympy(master)$ ipython
Python 3.11.9 | packaged by conda-forge | (main, Apr 19 2024, 18:36:13) [GCC 12.3.0]
Type 'copyright', 'credits' or 'license' for more information
IPython 8.26.0 -- An enhanced Interactive Python. Type '?' for help.
In [1]: %paste
>>> import sympy as sm
>>> t = sm.symbols("t")
>>> x = sm.Function("x")(t)
>>> xd = x.diff(t)
>>> sm.lambdify((xd, x), xd + x)(1, 1)
## -- End pasted text --
Out[1]: 2
In [2]: sm.lambdify((xd, x), xd, cse=True)(1, 1)
Out[2]: 1
In [3]: sm.lambdify((xd, x), xd + x, cse=True)(1, 1)
Out[3]: 2
test script:
import sympy as sm
t = sm.symbols("t")
x = sm.Function("x")(t)
xd = x.diff(t)
print(sm.lambdify((xd, x), xd + x)(1, 1))
print(sm.lambdify((xd, x), xd, cse=True)(1, 1))
print(sm.lambdify((xd, x), xd + x, cse=True)(1, 1))
print(sm.lambdify((xd, x), xd + x, cse=True, dummify=True)(1, 1))
I tried to git bisect this against a version tag, e.g. sympy-1.13.0 and master but I get this error:
Some good revs are not ancestors of the bad rev.
git bisect cannot work properly in this case.
Maybe you mistook good and bad revs?
Closing as fixed, not quite sure what PR fixed it though.
I tried to git bisect this against a version tag, e.g.
sympy-1.13.0and master but I get this error:
It is definitely possible to bisect between a version tag and master. You might need to do something like git fetch upstream to make sure you have all the commits/tags:
$ git checkout master
$ git bisect bad
You need to start by "git bisect start"
Do you want me to do it for you [Y/n]?
$ git checkout sympy-1.13.0
...
HEAD is now at 5f58c25d47 Merge pull request #26763 from oscarbenjamin/pr_bump_versions_1.13.0_final
$ git bisect good
Bisecting: 361 revisions left to test after this (roughly 9 steps)
[0d2f438a4af34e1595194acd5d76a017e82519af] Update test_actuator.py
I asked in the other issue whether this is a regression in 1.13 and whether it warrants a backport to a 1.13.3 release. Obviously we can't backport if we don't know what PR fixed it though.
I tried this:
moorepants@nandi:sympy(master)$ git pull upstream --tags
remote: Enumerating objects: 133, done.
remote: Counting objects: 100% (133/133), done.
remote: Compressing objects: 100% (56/56), done.
remote: Total 133 (delta 79), reused 107 (delta 75), pack-reused 0 (from 0)
Receiving objects: 100% (133/133), 78.12 KiB | 930.00 KiB/s, done.
Resolving deltas: 100% (79/79), completed with 26 local objects.
From github.com:sympy/sympy
06bc81eafc..2197797741 1.13 -> upstream/1.13
* [new branch] revert-26358-26338 -> upstream/revert-26358-26338
* [new tag] 1.13.2 -> 1.13.2
* [new tag] sympy-1.13.1 -> sympy-1.13.1
* [new tag] sympy-1.13.2 -> sympy-1.13.2
You asked to pull from the remote 'upstream', but did not specify
a branch. Because this is not the default configured remote
for your current branch, you must specify a branch on the command line.
moorepants@nandi:sympy(master)$ git checkout sympy-1.12
sympy-1.12 sympy-1.12.1 sympy-1.12.1rc1 sympy-1.12rc1
moorepants@nandi:sympy(master)$ git checkout sympy-1.12.1
Note: switching to 'sympy-1.12.1'.
You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.
If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:
git switch -c <new-branch-name>
Or undo this operation with:
git switch -
Turn off this advice by setting config variable advice.detachedHead to false
HEAD is now at a36a8b23de Merge pull request #26635 from oscarbenjamin/pr_ci_update
moorepants@nandi:sympy((sympy-1.12.1))$ python lam_diff.py
2
1
Traceback (most recent call last):
File "/home/moorepants/src/sympy/lam_diff.py", line 7, in <module>
print(sm.lambdify((xd, x), xd + x, cse=True)(1, 1))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "<lambdifygenerated-3>", line 5, in _lambdifygenerated
NameError: name 'Derivative' is not defined
moorepants@nandi:sympy((sympy-1.12.1))$ git bisect start
status: waiting for both good and bad commits
moorepants@nandi:sympy((sympy-1.12.1)|BISECTING)$ git bisect bad
status: waiting for good commit(s), bad commit known
moorepants@nandi:sympy((sympy-1.12.1)|BISECTING)$ git checkout master
Previous HEAD position was a36a8b23de Merge pull request #26635 from oscarbenjamin/pr_ci_update
Switched to branch 'master'
Your branch is ahead of 'origin/master' by 2162 commits.
(use "git push" to publish your local commits)
moorepants@nandi:sympy(master|BISECTING)$ git bisect good
Some good revs are not ancestors of the bad rev.
git bisect cannot work properly in this case.
Maybe you mistook good and bad revs?
Okay, you can't use git bisect like that.
There is no way with git bisect to say that you are looking for a fix rather than a break: you have to say that a newer commit is the bad one and an older commit is the good one. Then you have to invert the logic in your test script so that it asserts that the bug is not fixed like:
try:
func()
except Exception:
print('failed')
else:
assert False, "did not fail"
Hence the error message from git:
Some good revs are not ancestors of the bad rev. git bisect cannot work properly in this case. Maybe you mistook good and bad revs?
Thanks, I guess I've never tried this inversion because I'm usually looking for where something broke.
#26678 seems to have fixed this issue.