Opened 8 years ago

Closed 7 years ago

#18085 closed defect (fixed)

missing binding for SymPy's exp_polar()

Reported by: kalvotom Owned by:
Priority: minor Milestone: sage-7.1
Component: symbolics Keywords: sd66
Cc: vdelecroix, sstarosta, kcrisman Merged in:
Authors: Tomáš Kalvoda Reviewers: Ralf Stephan
Report Upstream: N/A Work issues:
Branch: 6d3aeaa (Commits, GitHub, GitLab) Commit: 6d3aeaa73c6052f691a2d3cd06cd1049cc6843ff
Dependencies: #20185 Stopgaps:

GitHub link to the corresponding issue

Description (last modified by rws)

The following integral cannot be evaluated by maxima:

sage: integrate(1/sqrt(1+x^3),x)
integrate(1/sqrt(x^3 + 1), x)

With 'sympy' algorithm the computation fails:

sage: integrate(1/sqrt(1+x^3),x,algorithm='sympy')
...
AttributeError: 'gamma' object has no attribute '_sage_'

However, SymPy? can compute the integral and gives the result in terms of gamma and hypergeometric functions:

sage: import sympy
sage: sympy.integrate(1/sqrt(1+x**3))
x*gamma(1/3)*hyper((1/3, 1/2), (4/3,), x**3*exp_polar(I*pi))/(3*gamma(4/3))

It can be seen that not only gamma is a problem (already fixed in sympy master) but also exp_polar which Sage does not know.

This ticket should track the status of the Sympy pull request fixing the exp_polar issue, and it should implement a skeleton exp_polar on the Sage side.

Change History (19)

comment:1 Changed 8 years ago by vdelecroix

The problem actually does occur in Sympy code...

sage: import sympy
sage: s = sympy.gamma(1/3)
sage: s._sage_()
Traceback (most recent call last):
...
AttributeError: 'gamma' object has no attribute '_sage_'

Would you like to make a bug report in Sympy? Either on

Vincent

comment:2 Changed 8 years ago by kalvotom

After some more digging it turned out the this is almost OK in the most recent git version of Sympy (not 0.7.6 which is shipped with Sage). Class Function has a _sage_ method and hypergeometric function hyper is also correctly dealt with.

However, the example mentioned above still fails because Sage does not know exp_polar function. I have created this pull request which adds _sage_ method to exp_polar and returns the usual exponential.

Tomáš

comment:3 Changed 8 years ago by kalvotom

Report Upstream: N/AReported upstream. No feedback yet.

comment:4 Changed 8 years ago by rws

I have commented on the pull request, https://github.com/sympy/sympy/pull/9224

comment:5 Changed 8 years ago by rws

Cc: kcrisman added
Description: modified (diff)
Summary: Symbolic integration and SymPymissing binding for SymPy's exp_polar()

comment:6 Changed 8 years ago by rws

If you want to do try your hands at it, see http://trac.sagemath.org/wiki/symbolics/functions for symbolic function tickets, and https://github.com/sagemath/sage/compare/develop...rwst:special for a collection of most recent code examples.

comment:7 Changed 8 years ago by kalvotom

Branch: u/kalvotom/missing_binding_for_sympy_s_exp_polar__

comment:8 Changed 8 years ago by kalvotom

Authors: Tomáš Kalvoda
Commit: 0683dd3c7f980d4a2f274a200bdcd4f69073c730
Report Upstream: Reported upstream. No feedback yet.Fixed upstream, in a later stable release.

New commits:

0683dd3add exp_polar function

comment:9 Changed 8 years ago by kalvotom

I have doctested the function with git version of Sympy and all tests passed (especially the one with integration). I guess we have to wait for a new version of Sympy to arrive in Sage (probably Sympy 0.7.7).

Anyway, this is my first attempt to contribute to Sage, so feel free to knitpick.

I have one question. How complicated it would be to force Sage to perform simplifications like

sage: exp_polar(2 + 5*I)*exp_polar(5 - I)
exp_polar(7 + 4*I)

My current understanding is that such simplifications are taken care of by an external library ( Pynac ) and there is no way how to do this in Sage alone, am I right?

comment:10 in reply to:  9 Changed 8 years ago by rws

Replying to kalvotom:

I have doctested the function with git version of Sympy and all tests passed (especially the one with integration). I guess we have to wait for a new version of Sympy to arrive in Sage (probably Sympy 0.7.7).

Yes, other tickets are waiting for that too.

Anyway, this is my first attempt to contribute to Sage, so feel free to knitpick.

Will get to it.

I have one question. How complicated it would be to force Sage to perform simplifications like

sage: exp_polar(2 + 5*I)*exp_polar(5 - I)
exp_polar(7 + 4*I)

My current understanding is that such simplifications are taken care of by an external library ( Pynac ) and there is no way how to do this in Sage alone, am I right?

I think so, but I'm not much into Pynac. Anyway, it could be part of Python Sage by including a function that does it in one of the Expression.simplify* methods.

Last edited 8 years ago by rws (previous) (diff)

comment:11 Changed 8 years ago by rws

Branch: u/kalvotom/missing_binding_for_sympy_s_exp_polar__u/rws/missing_binding_for_sympy_s_exp_polar__

comment:12 Changed 8 years ago by rws

Commit: 0683dd3c7f980d4a2f274a200bdcd4f69073c7306d3aeaa73c6052f691a2d3cd06cd1049cc6843ff
Milestone: sage-6.6sage-pending
Reviewers: Ralf Stephan
Status: newneeds_review

Thanks for your contribution. There were some minor problems requiring changes which I have added: removed the warning (because this ticket is pending Sympy-0.7.7 anyway), and adapted a bit to PEP 8. Documentation is fine, tests pass, so there is no reason to hold this back. If you agree with my changes you can set positive review.


New commits:

ef4bdb7Merge branch 'develop' into t/18085/missing_binding_for_sympy_s_exp_polar__
6d3aeaa18085: reviewer's patch: remove warnings, cosmetics

comment:13 Changed 8 years ago by kalvotom

Status: needs_reviewpositive_review

comment:14 in reply to:  12 Changed 8 years ago by kalvotom

Replying to rws: Those changes are perfectly fine. Thanks.

comment:15 Changed 7 years ago by vbraun

Why is this still sage-pending?

comment:16 Changed 7 years ago by vdelecroix

"this ticket is pending Sympy-0.7.7" (see In comment:12) but

sage: import sympy
sage: sympy.__version__
'0.7.6.1'

comment:17 Changed 7 years ago by rws

Dependencies: #20185
Report Upstream: Fixed upstream, in a later stable release.N/A

comment:18 Changed 7 years ago by rws

Milestone: sage-pendingsage-7.1

comment:19 Changed 7 years ago by vbraun

Branch: u/rws/missing_binding_for_sympy_s_exp_polar__6d3aeaa73c6052f691a2d3cd06cd1049cc6843ff
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.