Symbolic to SymPy convertion for generic function

[mmancini]

Implement generic function coversion from Symbolic to SymPy?

sage: a = function('A')(x,y)

sage: type(A)
<class 'sage.symbolic.function_factory.NewSymbolicFunction'>

sage: type(a)
<type 'sage.symbolic.expression.Expression'>

sage: a._sympy_()
---------------------------------------------------------------------------
NotImplementedError                       Traceback (most recent call last)
<ipython-input-101-eeb95aeda6e6> in <module>()
----> 1 a._sympy_()

[mmancini]

New commits:

​b494a4f	Added sympy symbolic method for manifolds.
​3da5712	merged with develop

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​ff2c44d

Small change towards symbolic=>sympy conversion of generic functions

[tscrim]

Is this ready for review (other than removing the commented out debugging code)?

[mmancini]

No, the work is still not finished : composed functions are not coverted.

[mmancini]

The inverse transformation was submitted to sympy in the pull request:

"sympy to sage conversion : Abstract function." ​https://github.com/sympy/sympy/pull/12826

[mforets]

@mmancini: Thanks for the link to that sympy's thread, this is good news! how is that patch incorporated into 'sage's sympy'?

Please note that there is i think some conflict of commit ff2c44d here, with the branch in #20204.

[rws]

We can do either a patch ticket (i.e., adding it to build/pkgs/sympy/patches/) or wait for SymPy to accept the PR, then do an upgrade ticket here.

[mmancini]

The PR on sympy si passed , I added tests and resolved some problems. Add the page will be a great thing but I don't know if it is necessary for the moment.

Concerning the conflicts with #20204 : I did not see this ticket before, but yes, some problems could arrive.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​df4dc4c

symbolic=>sympy conversion for derivation of generic functions

[mmancini]

The conversion of generic functions (also derivatives and composed functions) between sage and sympy works fine. For the full operabilty changes in sympy (​https://github.com/sympy/sympy/pull/12826) are needed

[tscrim]

If you need upstream changes, you will have to add the corresponding patch to our version of sympy. For better granularity, it might be better to do the patching on a separate ticket.

[mmancini]

How can I patch the sage version of sympy it is not versioned in git. Have you some doc?

[tscrim]

You create a patch from the git commit. I think the command is something like git diff -p <commit>, but it should be easy enough to find examples via Google. From there, you simply add the patch to the $SAGE_ROOT/build/pkgs/sympy/patches folder, bump the corresponding package-version.txt to 1.0.p3, and commit. To test, you should simply need to run make build and then test as usual.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​eb19598	added patch for sympy
​6eaf35d	Better sympy patch taken from #12826 PR in sympy

[mmancini]

Hello, I don't know if the procedure is correct: in $SYMPY

git diff -p 6929ca35a33ba7813cd6d5828d96bc339ea06cb6 > 02_undeffun_sage.patch

then

mv 02_undeffun_sage.patch $SAGE_ROOT/build/pkgs/sympy/patches/
cat 1.0.p2 > $SAGE_ROOT/build/pkgs/sympy/package-version.txt

The test are ok. I had to modify again sympy : now if the function is known in sympy (like fresneln in calculus.py tests) but not in sage, then the AttributeError is raised from sympy and catched in sage as before.

[tscrim]

It looks okay to me, but I would prefer the sympy patch to go on a separate ticket that this ticket will then depend on. You can just cherry-pick (and squash if desired) your last two commits into a new branch and then merge that branch into this one here.

[mmancini]

Changes done as required. I hope it is ok.

[tscrim]

LGTM (with manually increased my sympy patch level for testing, but that is a change that needs to be done on #23496).

[git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

​b764ebe	Better sympy patch taken from #12826 PR in sympy
​9d16a70	Lost package-version.txt modifications : restored
​d5d6b67	Corrected error on patcj version
​462bd68	test is not corrected at this point
​daa2f2c	Error on sympy branch corrected with a new patch
​4872425	forgetten to delete gg in doctest
​b6fec4d	merge with sympy patch

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​721eed7	removed whitespace
​e46345b	changed sympy patch version
​46ebd72	Merge branch 'public/23496_patch_sympy_abstact_function' of git://trac.sagemath.org/sage into public/23496_patch_sympy_abstact_function
​068d495	Merge branch 'public/22802_sympy_abstact_function' of git://trac.sagemath.org/sage into public/22802_sympy_abstact_function

[tscrim]

Just a merge in of the latest #23496. Tests still pass, LGTM, thanks.

[mforets]

while this is "on-hold", could derivative be revised? the docstring has some defects right now:

• no INPUT
• extra spaces that are not conventional in sage library code
• what is convertion :: ?

and why should the debug lines # print(...) be kept?

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​078532f

docstring modifications

[mmancini]

Changed name because typo error

[mmancini]

Last 10 new commits:

​721eed7	removed whitespace
​e46345b	changed sympy patch version
​46ebd72	Merge branch 'public/23496_patch_sympy_abstact_function' of git://trac.sagemath.org/sage into public/23496_patch_sympy_abstact_function
​ff2c44d	Small change towards symbolic=>sympy conversion of generic functions
​df4dc4c	symbolic=>sympy conversion for derivation of generic functions
​eb19598	added patch for sympy
​6eaf35d	Better sympy patch taken from #12826 PR in sympy
​b6fec4d	merge with sympy patch
​068d495	Merge branch 'public/22802_sympy_abstact_function' of git://trac.sagemath.org/sage into public/22802_sympy_abstact_function
​078532f	docstring modifications

[git]

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

​721eed7	removed whitespace
​e46345b	changed sympy patch version
​46ebd72	Merge branch 'public/23496_patch_sympy_abstact_function' of git://trac.sagemath.org/sage into public/23496_patch_sympy_abstact_function
​ff2c44d	Small change towards symbolic=>sympy conversion of generic functions
​df4dc4c	symbolic=>sympy conversion for derivation of generic functions
​eb19598	added patch for sympy
​6eaf35d	Better sympy patch taken from #12826 PR in sympy
​b6fec4d	merge with sympy patch
​068d495	Merge branch 'public/22802_sympy_abstact_function' of git://trac.sagemath.org/sage into public/22802_sympy_abstact_function
​078532f	docstring modifications

[mmancini]

The name of the branch was corrected

[mforets]

could you please merge again with #23496 ? i haven't checked in detail, but there seem to be differences in the composition function,

​https://git.sagemath.org/sage.git/diff?id=eedf6da36d74e1dd3cc996201a4160d101c3b0a6

and

​https://git.sagemath.org/sage.git/diff?id2=037272ccbac814ddcacd7f0df60cc68d357d3fa0&id=bc21300122019b04a30dba02aaa2cd728ff7453f

of course in case of conflict we want to keep the changes introduced by #23496.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​67f7f46	integrated modifications to composition. But an error remains in french_book.recequadiff tests
​1b5227d	Error in french_book.recequadiff tests corrected
​018d753	Merge branch 'develop' into t/23496/public/23496_patch_sympy_abstact_function
​94af74b	#23496 : (temp) revert order in rsolve recequadiff.tex test
​d76caea	#23496 : add message for change in doctest
​bc21300	#23496 : fix a _sympy_init_ doctest
​0be1d90	Merge branch 'public/23496_patch_sympy_abstract_function' into 22802_sympy_abstract_function

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​7a6cb0a	Merge branch 'public/22802_sympy_abstract_function' of git://trac.sagemath.org/sage into public/22802_sympy_abstract_function
​137d715	Trivial reviwer changes.

[tscrim]

I did some trivial reviewer changes, but I am getting the same two real failures as on the patchbot:

sage -t src/sage/symbolic/expression_conversions.py
**********************************************************************
File "src/sage/symbolic/expression_conversions.py", line 792, in sage.symbolic.expression_conversions.SympyConverter.derivative
Failed example:
    df_sympy = df_sage._sympy_(); df_sympy
Exception raised:
    Traceback (most recent call last):
      File "/home/travis/sage-build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 515, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/travis/sage-build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 885, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.symbolic.expression_conversions.SympyConverter.derivative[4]>", line 1, in <module>
        df_sympy = df_sage._sympy_(); df_sympy
      File "sage/symbolic/expression.pyx", line 1445, in sage.symbolic.expression.Expression._sympy_ (/home/travis/sage-build/src/build/cythonized/sage/symbolic/expression.cpp:11785)
        return sympy(self)
      File "/home/travis/sage-build/local/lib/python2.7/site-packages/sage/symbolic/expression_conversions.py", line 222, in __call__
        return self.derivative(ex, operator)
      File "/home/travis/sage-build/local/lib/python2.7/site-packages/sage/symbolic/expression_conversions.py", line 815, in derivative
        return f_sympy.diff(*sympy_arg)
    AttributeError: 'NoneType' object has no attribute 'diff'
**********************************************************************
File "src/sage/symbolic/expression_conversions.py", line 794, in sage.symbolic.expression_conversions.SympyConverter.derivative
Failed example:
    df_sympy == f_sympy.diff(x, 2, y, 1)
Exception raised:
    Traceback (most recent call last):
      File "/home/travis/sage-build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 515, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/travis/sage-build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 885, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.symbolic.expression_conversions.SympyConverter.derivative[5]>", line 1, in <module>
        df_sympy == f_sympy.diff(x, Integer(2), y, Integer(1))
    NameError: name 'df_sympy' is not defined
**********************************************************************
1 item had failures:
   2 of   7 in sage.symbolic.expression_conversions.SympyConverter.derivative
    [489 tests, 2 failures, 2.37 s]
----------------------------------------------------------------------
sage -t src/sage/symbolic/expression_conversions.py  # 2 doctests failed
----------------------------------------------------------------------

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​5a53e12

Problem SympyConverter.composition: returned to previous implementation

[tscrim]

I do not get any of the errors reported by the patchbot. Positive review.

[mforets]

there is at least a merge conflict with its dependency, introduced in ​5a53e12

[tscrim]

This is already based on the branch on #23496.

[mforets]

the branches do have different code for composition (since commit ​5a53e12 came after the merge).

anyway, sorry for my confusion. i didn't realize that it will just overwrite the code in the dependency, but without merge conflict,

$ git checkout t/22802/public/22802_sympy_abstract_function
Switched to branch 't/22802/public/22802_sympy_abstract_function'
Your branch is up-to-date with 'trac/public/22802_sympy_abstract_function'.
$ git merge t/23496/public/23496_patch_sympy_abstract_function
Already up-to-date. 

[mmancini]

I had remove the changes in composition because it caused problems in this branch and it was not influent in #23496