-
Notifications
You must be signed in to change notification settings - Fork 153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding an extension for SymPy -> Symbolics.jl using PythonCall.jl #957
base: master
Are you sure you want to change the base?
Conversation
ext/SymPySymbolicsExt.jl
Outdated
CondaPkg.add("sympy") | ||
|
||
sp = pyimport("sympy") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That shouldn't happen in an extension
ext/SymPySymbolicsExt.jl
Outdated
# little tests | ||
PythonCall.pyconvert(Symbolics.Num, sp.sympify("t")) | ||
|
||
PythonCall.pyconvert(Symbolics.Num, sp.sympify("t**t")) | ||
|
||
PythonCall.pyconvert(Symbolics.Num, sp.sympify("t + z + d")) | ||
|
||
PythonCall.pyconvert(Symbolics.Num, sp.sympify("t*z*9")) | ||
|
||
PythonCall.pyconvert(Symbolics.Num, sp.sympify("5*t*z + 3*d + h/(b*5)")) | ||
|
||
PythonCall.pyconvert(Symbolics.Num, sp.sympify("t * n/z * t**4 * h**z + l*h - j")) | ||
|
||
PythonCall.pyconvert(Symbolics.Equation, sp.sympify("Eq(2,5, evaluate = False)")) | ||
|
||
PythonCall.pyconvert(Symbolics.Equation, sp.sympify("Eq(t*x + 5**x + 20/t, 90/t + t**4 - t*z)")) | ||
|
||
PythonCall.pyconvert(Symbolics.Num, sp.sympify("Function('f')(x)")) | ||
|
||
PythonCall.pyconvert(Symbolics.Num, sp.sympify("f(x,y)")) | ||
|
||
PythonCall.pyconvert(Symbolics.Equation, sp.sympify("Eq(f(x), 2*x +1)")) | ||
|
||
PythonCall.pyconvert(Symbolics.Num,sp.sympify("Derivative(f(x),x)")) | ||
|
||
PythonCall.pyconvert(Symbolics.Num,sp.sympify("Eq(Derivative(f(x),x), x")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add to the tests?
You might want to test this under the assumption that https://github.com/jverzani/SymPyPythonCall.jl may merge? |
That was recently tagged as a standalone package. I added a |
As far as I can tell there shouldn't be any kind of conflict, with this you have to explicitly tell |
I'm trying to mirror your work in |
How are you trying to trigger the rules? |
Here is what I think should work (please correct if I'm missing something):
If I do things directly, it seems to work as desired
This leads me to guess, the issue comes in this identification:
The class of
|
The problem could be with the |
Thanks, I'll have look. |
Just a heads up @jverzani we don't use TermInterface.jl anymore; the interface has been moved back into SymbolicUtils for some stability/compatibility reasons, but potentially in the future we can use it. |
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## master #957 +/- ##
=========================================
+ Coverage 7.93% 8.27% +0.33%
=========================================
Files 26 27 +1
Lines 3162 3298 +136
=========================================
+ Hits 251 273 +22
- Misses 2911 3025 +114
... and 16 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
This is my rough attempt at converting SymPy expressions to Symbolics.jl expressions. It uses PythonCall.jl 's support for custom rules to define conversion rules. Not sure if this should go in the same file as SymbolicsSymPyExt, but I put it in a different one and a different module since it uses PythonCall instead of PyCall.
Essentially when the pyconvert function is called to convert a SymPy
Symbol
to a SymbolicsNum
,pyconvert
returns a Symbolics variable with the same name as the SymPySymbol
. Then for any other type of expressionpyconvert
can be called recursively on the arguments to turn them in to expressions usable with Symbolics or ModelingToolkit.Some issues:
The names of the variables are not exposed when they are converted, so you can't use them... If they were
istree
I think I could go through to all of the variables and make them in to Julia variables, but they don't seem to come out asistree
objects.Each conversion from
Symbol
toNum
just makes a new variable. So if a variable already existed with the same name as something in another expression you were trying to convert, I think it would just be overwritten. Maybe solved by adding a check to see if a variable of the same name exists already?