Opened 6 years ago
Closed 5 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) | Commit: | 6d3aeaa73c6052f691a2d3cd06cd1049cc6843ff |
Dependencies: | #20185 | Stopgaps: |
Description (last modified by )
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 6 years ago by
comment:2 Changed 6 years ago by
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 6 years ago by
- Report Upstream changed from N/A to Reported upstream. No feedback yet.
comment:4 Changed 6 years ago by
I have commented on the pull request, https://github.com/sympy/sympy/pull/9224
comment:5 Changed 6 years ago by
- Cc kcrisman added
- Description modified (diff)
- Summary changed from Symbolic integration and SymPy to missing binding for SymPy's exp_polar()
comment:6 Changed 6 years ago by
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 6 years ago by
- Branch set to u/kalvotom/missing_binding_for_sympy_s_exp_polar__
comment:8 Changed 6 years ago by
- Commit set to 0683dd3c7f980d4a2f274a200bdcd4f69073c730
- Report Upstream changed from Reported upstream. No feedback yet. to Fixed upstream, in a later stable release.
New commits:
0683dd3 | add exp_polar function
|
comment:9 follow-up: ↓ 10 Changed 6 years ago by
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 6 years ago by
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.
comment:11 Changed 6 years ago by
- Branch changed from u/kalvotom/missing_binding_for_sympy_s_exp_polar__ to u/rws/missing_binding_for_sympy_s_exp_polar__
comment:12 follow-up: ↓ 14 Changed 6 years ago by
- Commit changed from 0683dd3c7f980d4a2f274a200bdcd4f69073c730 to 6d3aeaa73c6052f691a2d3cd06cd1049cc6843ff
- Milestone changed from sage-6.6 to sage-pending
- Reviewers set to Ralf Stephan
- Status changed from new to needs_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:
ef4bdb7 | Merge branch 'develop' into t/18085/missing_binding_for_sympy_s_exp_polar__
|
6d3aeaa | 18085: reviewer's patch: remove warnings, cosmetics
|
comment:13 Changed 6 years ago by
- Status changed from needs_review to positive_review
comment:14 in reply to: ↑ 12 Changed 6 years ago by
Replying to rws: Those changes are perfectly fine. Thanks.
comment:15 Changed 5 years ago by
Why is this still sage-pending?
comment:16 Changed 5 years ago by
"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 5 years ago by
- Dependencies set to #20185
- Report Upstream changed from Fixed upstream, in a later stable release. to N/A
comment:18 Changed 5 years ago by
- Milestone changed from sage-pending to sage-7.1
comment:19 Changed 5 years ago by
- Branch changed from u/rws/missing_binding_for_sympy_s_exp_polar__ to 6d3aeaa73c6052f691a2d3cd06cd1049cc6843ff
- Resolution set to fixed
- Status changed from positive_review to closed
The problem actually does occur in Sympy code...
Would you like to make a bug report in Sympy? Either on
Vincent