Opened 9 years ago
Closed 6 years ago
#15024 closed enhancement (fixed)
Add Hankel functions and make spherical Bessel and Hankel functions symbolic
Reported by:  Eviatar Bach  Owned by:  

Priority:  major  Milestone:  sage7.3 
Component:  symbolics  Keywords:  
Cc:  Burcin Erocal, Frédéric Chapoton, Paul Masson  Merged in:  
Authors:  Eviatar Bach, Ralf Stephan  Reviewers:  Paul Masson 
Report Upstream:  N/A  Work issues:  
Branch:  7e48705 (Commits, GitHub, GitLab)  Commit:  7e487050f152e7afdebdd05c5ba276d645f963c7 
Dependencies:  #18257  Stopgaps: 
Description
For some reason Sage has spherical Hankel functions but not regular ones. This patch adds Hankel functions and makes spherical Hankel and Bessel functions symbolic, as well as adding arbitraryprecision numeric evaluation.
Attachments (2)
Change History (63)
Changed 9 years ago by
Attachment:  trac15024.patch added 

comment:1 Changed 9 years ago by
Cc:  Burcin Erocal added 

Status:  new → needs_review 
comment:2 Changed 9 years ago by
New patch gets coverage to 100%, adds _latex_
methods (for getting a LaTeX representation for the function without arguments), and adds \left
and \right
to _print_latex_
, which I forgot before.
Patchbot apply trac15024_2.patch
Changed 9 years ago by
Attachment:  trac15024_2.patch added 

comment:3 Changed 9 years ago by
Milestone:  sage6.1 → sage6.2 

comment:4 Changed 9 years ago by
Note also the Struve functions
struve_h (v,z) Struve H function struve_l (v,z) Struve L function
noted at the symbolics wiki on Trac, which seem to go with the Hankels...
comment:5 Changed 9 years ago by
Yeah, those should be easy to add, although maybe in another ticket.
comment:6 Changed 9 years ago by
Sure, this is #16221 now. By the way, do we really want the Hankel stuff in bessel.py?
comment:7 Changed 9 years ago by
Milestone:  sage6.2 → sage6.3 

comment:8 Changed 9 years ago by
Branch:  → u/rws/add_hankel_functions_and_make_spherical_bessel_and_hankel_functions_symbolic 

comment:9 Changed 9 years ago by
Commit:  → d0ada4badea353a69b2d49d276edbed44acaf8bc 

Status:  needs_review → needs_work 
Made commit from patch, removed merge conflicts. However:
sage t long src/sage/functions/special.py # 2 doctests failed sage t long src/sage/functions/bessel.py # 12 doctests failed
New commits:
d0ada4b  Adding Hankel functions, making spherical Bessel and Hankel functions symbolic.

comment:10 Changed 8 years ago by
Commit:  d0ada4badea353a69b2d49d276edbed44acaf8bc → c2cd65d850b68064d226339a6a89bcf54a9646fe 

comment:11 Changed 8 years ago by
Authors:  → Eviatar Bach, Ralf Stephan 

Status:  needs_work → needs_review 
Please review. As to which category Hankel functions are in, if not "bessel", I would propose this hierarchy for orientation: special functions>holonomic functions>Bessel functions
comment:12 Changed 8 years ago by
Commit:  c2cd65d850b68064d226339a6a89bcf54a9646fe → 0e9578c1a4db7e7abfddc698faae9672a0732ceb 

Branch pushed to git repo; I updated commit sha1. New commits:
0e9578c  Merge branch 'develop' into t/15024/add_hankel_functions_and_make_spherical_bessel_and_hankel_functions_symbolic

comment:13 Changed 8 years ago by
Milestone:  sage6.3 → sage6.4 

comment:14 Changed 8 years ago by
Commit:  0e9578c1a4db7e7abfddc698faae9672a0732ceb → 485a9ec64a55c8f1a62fa9c67188c088278780e1 

Branch pushed to git repo; I updated commit sha1. New commits:
485a9ec  Merge branch 'develop' into t/15024/add_hankel_functions_and_make_spherical_bessel_and_hankel_functions_symbolic

comment:15 Changed 8 years ago by
Commit:  485a9ec64a55c8f1a62fa9c67188c088278780e1 → 78e3fe10b78b19d0471fe434384c4a8470935211 

Branch pushed to git repo; I updated commit sha1. New commits:
78e3fe1  Merge branch 'develop' into t/15024/add_hankel_functions_and_make_spherical_bessel_and_hankel_functions_symbolic

comment:17 Changed 8 years ago by
Commit:  78e3fe10b78b19d0471fe434384c4a8470935211 → bf299a2fa7bcd53778ebe75c58158459e1ec783e 

comment:18 Changed 8 years ago by
Commit:  bf299a2fa7bcd53778ebe75c58158459e1ec783e → e1b4ee098d002ddd794acde0833b2b3f31d0c888 

comment:20 Changed 8 years ago by
Possibly not a bug with this ticket but in Maxima (actually I think it's in our exponential integral), a branch cut difference between us and Mma in the integral:
sage: f(x) = Ei(I*x)  6*gamma(1, I*x)  15*gamma(2, I*x)  15*gamma(3, I*x) sage: f(3) Ei(3*I)  6*gamma(1, 3*I)  15*gamma(2, 3*I)  15*gamma(3, 3*I) sage: f(3.) 0.529089163583971  3.36665821311941*I sage: g(x) = e^(i*x)/x^3*(i*x^2+5*x5*i) sage: g(3.)
Also, the spherical Hankels don't make it into the top matter in the reference page  a very brief mention should suffice.
comment:21 Changed 8 years ago by
Status:  needs_review → needs_work 

Wikipedia and DLMF agree that the derivative formulas for the various spherical functions only apply if the index is an integer. I'm not sure how this connects to WA's solution to that.
Also
sage: integrate(spherical_bessel_J(1,x)^2,(x,0,oo)) 1/6*pi sage: integrate(spherical_bessel_Y(1,x)^2,(x,0,oo)) 1/6*pi
is just too cool not to include.
This is very nicely put together and other than these comments mostly needs just pseudorandom testing that mpmath or whoever doesn't have any weird error spots. 'Needs work' for updating updating documentation, otherwise 'needs info' for the other points raised.
comment:22 Changed 8 years ago by
(By the way, I don't know if I want to make #18257 a prereq for adding more symbolic special functions; I totally agree that it should not be ignored, but in the meantime making that arbitrary +100
bigger again seems better than waiting for someone to figure out a longterm solution to that and then holding various tickets hostage.)
comment:24 Changed 7 years ago by
Commit:  e1b4ee098d002ddd794acde0833b2b3f31d0c888 → e4fbb38d5e532e07d15b825a3c6f9ccd28ce5908 

comment:25 Changed 7 years ago by
Milestone:  sage6.4 → sage6.8 

Status:  needs_work → needs_review 
comment:26 Changed 7 years ago by
Status:  needs_review → needs_work 

comment:27 Changed 7 years ago by
Status:  needs_work → needs_review 

comment:28 Changed 7 years ago by
The failing doctest is because with this ticket the expression returned by random_expr(20, nvars=2)
suddenly contains the nonsymbolic floor
function which triggers the fail via
sage: floor(x,hold=True)  TypeError Traceback (most recent call last) <ipythoninput8fc5809e0a430> in <module>() > 1 floor(x,hold=True) TypeError: __call__() got an unexpected keyword argument 'hold'
Since this is completely unrelated I'll open a separate ticket. EDIT: See #19464.
comment:29 Changed 7 years ago by
Dependencies:  #18257 → #18257, #19464 

comment:30 Changed 7 years ago by
Status:  needs_review → needs_work 

comment:31 Changed 7 years ago by
Commit:  e4fbb38d5e532e07d15b825a3c6f9ccd28ce5908 → 5163957f72ad67374eb5310241bc3ce965e5646e 

Branch pushed to git repo; I updated commit sha1. New commits:
4198c28  fix 19464 by allowing a hold keyword on floor/ceil

ddfa8fa  Merge branch 'u/rws/add_hankel_functions_and_make_spherical_bessel_and_hankel_functions_symbolic' of git://trac.sagemath.org/sage into tmp02

d2afc44  19464: fix bug

5163957  Merge branch 'u/rws/expressiontreewalker_fails_on_some_functions' of git://trac.sagemath.org/sage into tmp02

comment:32 Changed 7 years ago by
Commit:  5163957f72ad67374eb5310241bc3ce965e5646e → 6d1a2b12b3580aa452005284e74a2cd79d7bf772 

comment:33 Changed 7 years ago by
Milestone:  sage6.8 → sage7.2 

Status:  needs_work → needs_review 
comment:34 Changed 7 years ago by
Cc:  Frédéric Chapoton added 

comment:35 Changed 6 years ago by
Dependencies:  #18257, #19464 → #18257, #19464, #12121 

comment:36 Changed 6 years ago by
Status:  needs_review → needs_work 

comment:37 followup: 38 Changed 6 years ago by
Cc:  Paul Masson added 

Symbolic floor
currently accepts hold=True
, as does ceil
. Why is the ticket still dependent on #12121?
Also, in investigating inaccuracies in plotting spherical Bessel functions for arguments less than one I came across this error:
sage: spherical_bessel_J(3,.1) 0.951851972086556*l  5
This was supposedly fixed in#2494 but it's back. Will it be fixed when this ticket is merged?
I'd be happy to review this as soon as it's ready to go.
comment:38 Changed 6 years ago by
Dependencies:  #18257, #19464, #12121 → #18257 

Milestone:  sage7.2 → sage7.3 
Replying to paulmasson:
Symbolic
floor
currently acceptshold=True
, as doesceil
. Why is the ticket still dependent on #12121?
You're right. That somehow changed in the meantime.
Also, in investigating inaccuracies in plotting spherical Bessel functions for arguments less than one I came across this error:
sage: spherical_bessel_J(3,.1) 0.951851972086556*l  5
I resolved merge conflicts and, based on beta6, I get
sage: spherical_bessel_J(3,.1) 9.51851972086557e6
comment:39 Changed 6 years ago by
Commit:  6d1a2b12b3580aa452005284e74a2cd79d7bf772 → 6f3fbbc5bdf5db721b1b03b0dc67547d589e45bc 

comment:40 Changed 6 years ago by
Status:  needs_work → needs_review 

I also finally removed unused code in special.py
and moved associated doctests to better locations. Hope it didn't increase your task too much.
comment:41 followup: 43 Changed 6 years ago by
Updating special.py
is appropriate to this ticket. In response to a comment above, Hankel stuff most certainly belongs in bessel.py since they are Bessel functions.
Unfortunately I'm now seeing changes to airy.py
and jacobi.py
that are not appropriate to this ticket. Please fix that.
Also, the branch is missing: can't checkout what's not there!
comment:43 Changed 6 years ago by
Replying to paulmasson:
Unfortunately I'm now seeing changes to
airy.py
andjacobi.py
that are not appropriate to this ticket. Please fix that.
I was just moving doctests away from special.py
. But I can add them to those files in another ticket, no problem, as long as you don't mind that I'm removing them in this ticket.
comment:44 Changed 6 years ago by
Commit:  6f3fbbc5bdf5db721b1b03b0dc67547d589e45bc → 2a5c20665d38736b5f990b4f735da5fade5baade 

Branch pushed to git repo; I updated commit sha1. New commits:
2a5c206  15024: revert changes not related to ticket

comment:45 Changed 6 years ago by
When running doctests I get the message
ImportError: cannot import name meval
Similar error when trying to build documentation. Sage crashes on startup, with message in crash log:
> 56 from sage.functions.special import meval ... ImportError: cannot import name meval
comment:46 Changed 6 years ago by
Reviewers:  → Paul Masson 

comment:47 Changed 6 years ago by
Commit:  2a5c20665d38736b5f990b4f735da5fade5baade → fa0d0b0effaaeffaccca661627bba232f452b773 

Branch pushed to git repo; I updated commit sha1. New commits:
fa0d0b0  15024: fix previous commit

comment:48 Changed 6 years ago by
Oops, the change in airy.pyx
is necessary because meval
no longer exists.
comment:49 Changed 6 years ago by
Doctests all pass. Documentation builds and has multiple preexisting issues, but that should be on a separate ticket.
Random numeric testing of new Hankel functions is accurate. Both functions plot. Symbolics behave as expected.
Per #20496 we are now supposed to escape abbreviated first names in references. There are three instances in the current branch that don't do that. If you fix those then I think it's ready to go.
There are changes in here I was planning to make, so I'm glad I read your list of open symbolics tickets beforehand!
comment:50 Changed 6 years ago by
Commit:  fa0d0b0effaaeffaccca661627bba232f452b773 → 821c307bc9a2d6b7c23929f8ee6e65e1e5a80bfa 

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

comment:51 Changed 6 years ago by
Thanks for the review. It's good to see again someone is interested in special functions. I am motivated in having at least the most known holonomic functions implemented symbolically (for experimental symbolics) but had stopped because very few tickets were reviewed.
New commits:
821c307  cosmetics

comment:52 followup: 54 Changed 6 years ago by
Positive review after cosmetic changes but there is now a merge conflict with 7.3.beta7.
Do I need to review again after conflict is resolved? What's the protocol on this?
comment:53 Changed 6 years ago by
Commit:  821c307bc9a2d6b7c23929f8ee6e65e1e5a80bfa → ab6006d7d04c8add865c48b1ecd0132a330bebd0 

Branch pushed to git repo; I updated commit sha1. New commits:
ab6006d  Merge branch 'develop' into t/15024/add_hankel_functions_and_make_spherical_bessel_and_hankel_functions_symbolic

comment:54 Changed 6 years ago by
Replying to paulmasson:
Do I need to review again after conflict is resolved? What's the protocol on this?
In principle I could have introduced any possible change with it, so yes. But it is easy to check this, either per eyeball by skimming the diff again, or automatically (I just tried this the first time ever):
git diff develop >diff1 git co 821c307 (the commit before the merge) git diff 7.3.beta6 >diff2 diff u diff2 diff1
The output of the last diff should not contain lines that start with ++/+/+/
. This way I found I somehow introduced some orig
files into the branch so I'll delete those files. But as introducing and removing these files create completely unnecessary data I'll upload a new branch with an improved merge commit.
comment:55 Changed 6 years ago by
Branch:  u/rws/add_hankel_functions_and_make_spherical_bessel_and_hankel_functions_symbolic → u/rws/15024 

comment:56 followup: 58 Changed 6 years ago by
Commit:  ab6006d7d04c8add865c48b1ecd0132a330bebd0 → 28606307034dd394ec39a74508e05442cb4b4fc9 

Doctests pass and documentation builds. Numerics look good. A few problems resulting from the merge conflict resolution:
1) A line has been modified in hypergeometric.py
. This modification appears to be inaccurate and has the wrong variable anyway. Please remove the condition x>0
on hypergeometric_U
.
2) The definition of the spherical Bessel function of the second kind has been left behind in special.py
after the definition of spherical harmonics. This one needs to be deleted since it's already in bessel.py
.
3) The evaluation of spherical harmonics has changed. I'm assuming this is because meval
is gone, but please confirm. This change has no apparent problems.
4) There are doctests added to the complete elliptic integral which are outside this ticket. Since they pass I'm fine leaving them here, but let me know that they are included just to avoid an extra ticket.
New commits:
2860630  Merge branch 'develop' into tmp02

comment:57 Changed 6 years ago by
Commit:  28606307034dd394ec39a74508e05442cb4b4fc9 → 7e487050f152e7afdebdd05c5ba276d645f963c7 

Branch pushed to git repo; I updated commit sha1. New commits:
7e48705  15024: more cosmetics

comment:58 Changed 6 years ago by
Replying to paulmasson:
3) The evaluation of spherical harmonics has changed. I'm assuming this is because
meval
is gone, but please confirm.
Yes.
4) There are doctests added to the complete elliptic integral which are outside this ticket. Since they pass I'm fine leaving them here, but let me know that they are included just to avoid an extra ticket.
I cannot avoid the extra ticket but would ask you to leave this part in for now.
comment:61 Changed 6 years ago by
Branch:  u/rws/15024 → 7e487050f152e7afdebdd05c5ba276d645f963c7 

Resolution:  → fixed 
Status:  positive_review → closed 
Note that this also changes the LaTeX representation for the other Bessel functions by removing
\operatorname
, since in all sources I've seen the function name is italicized.Patchbot apply trac15024.patch