Opened 5 years ago
Closed 21 months ago
#15024 closed enhancement (fixed)
Add Hankel functions and make spherical Bessel and Hankel functions symbolic
Reported by:  eviatarbach  Owned by:  

Priority:  major  Milestone:  sage7.3 
Component:  symbolics  Keywords:  
Cc:  burcin, chapoton, paulmasson  Merged in:  
Authors:  Eviatar Bach, Ralf Stephan  Reviewers:  Paul Masson 
Report Upstream:  N/A  Work issues:  
Branch:  7e48705 (Commits)  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 5 years ago by
comment:1 Changed 5 years ago by
 Cc burcin added
 Status changed from new to needs_review
comment:2 Changed 5 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 5 years ago by
comment:3 Changed 4 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:4 Changed 4 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 4 years ago by
Yeah, those should be easy to add, although maybe in another ticket.
comment:6 Changed 4 years ago by
Sure, this is #16221 now. By the way, do we really want the Hankel stuff in bessel.py?
comment:7 Changed 4 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:8 Changed 4 years ago by
 Branch set to u/rws/add_hankel_functions_and_make_spherical_bessel_and_hankel_functions_symbolic
comment:9 Changed 4 years ago by
 Commit set to d0ada4badea353a69b2d49d276edbed44acaf8bc
 Status changed from needs_review to 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 4 years ago by
 Commit changed from d0ada4badea353a69b2d49d276edbed44acaf8bc to c2cd65d850b68064d226339a6a89bcf54a9646fe
comment:11 Changed 4 years ago by
 Status changed from needs_work to 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 4 years ago by
 Commit changed from c2cd65d850b68064d226339a6a89bcf54a9646fe to 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 4 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:14 Changed 4 years ago by
 Commit changed from 0e9578c1a4db7e7abfddc698faae9672a0732ceb to 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 3 years ago by
 Commit changed from 485a9ec64a55c8f1a62fa9c67188c088278780e1 to 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:16 Changed 3 years ago by
Just to let you know: this conflicts with #17130.
comment:17 Changed 3 years ago by
 Commit changed from 78e3fe10b78b19d0471fe434384c4a8470935211 to bf299a2fa7bcd53778ebe75c58158459e1ec783e
comment:18 Changed 3 years ago by
 Commit changed from bf299a2fa7bcd53778ebe75c58158459e1ec783e to e1b4ee098d002ddd794acde0833b2b3f31d0c888
comment:19 Changed 3 years ago by
 Dependencies set to #18257
#18257 makes this ticket fail doctests.
comment:20 Changed 3 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 3 years ago by
 Status changed from needs_review to 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 3 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:23 Changed 3 years ago by
#18257 is good now.
comment:24 Changed 3 years ago by
 Commit changed from e1b4ee098d002ddd794acde0833b2b3f31d0c888 to e4fbb38d5e532e07d15b825a3c6f9ccd28ce5908
comment:25 Changed 3 years ago by
 Milestone changed from sage6.4 to sage6.8
 Status changed from needs_work to needs_review
comment:26 Changed 3 years ago by
 Status changed from needs_review to needs_work
comment:27 Changed 3 years ago by
 Status changed from needs_work to needs_review
comment:28 Changed 2 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 2 years ago by
 Dependencies changed from #18257 to #18257, #19464
comment:30 Changed 2 years ago by
 Status changed from needs_review to needs_work
comment:31 Changed 2 years ago by
 Commit changed from e4fbb38d5e532e07d15b825a3c6f9ccd28ce5908 to 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 2 years ago by
 Commit changed from 5163957f72ad67374eb5310241bc3ce965e5646e to 6d1a2b12b3580aa452005284e74a2cd79d7bf772
comment:33 Changed 2 years ago by
 Milestone changed from sage6.8 to sage7.2
 Status changed from needs_work to needs_review
comment:34 Changed 2 years ago by
 Cc chapoton added
comment:35 Changed 22 months ago by
 Dependencies changed from #18257, #19464 to #18257, #19464, #12121
comment:36 Changed 22 months ago by
 Status changed from needs_review to needs_work
comment:37 followup: ↓ 38 Changed 21 months ago by
 Cc paulmasson 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 in reply to: ↑ 37 Changed 21 months ago by
 Dependencies changed from #18257, #19464, #12121 to #18257
 Milestone changed from sage7.2 to 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 21 months ago by
 Commit changed from 6d1a2b12b3580aa452005284e74a2cd79d7bf772 to 6f3fbbc5bdf5db721b1b03b0dc67547d589e45bc
comment:40 Changed 21 months ago by
 Status changed from needs_work to 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 21 months 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:42 Changed 21 months ago by
The apparently missing branch is due to server maintenance.
comment:43 in reply to: ↑ 41 Changed 21 months 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 21 months ago by
 Commit changed from 6f3fbbc5bdf5db721b1b03b0dc67547d589e45bc to 2a5c20665d38736b5f990b4f735da5fade5baade
Branch pushed to git repo; I updated commit sha1. New commits:
2a5c206  15024: revert changes not related to ticket

comment:45 Changed 21 months 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 21 months ago by
 Reviewers set to Paul Masson
comment:47 Changed 21 months ago by
 Commit changed from 2a5c20665d38736b5f990b4f735da5fade5baade to fa0d0b0effaaeffaccca661627bba232f452b773
Branch pushed to git repo; I updated commit sha1. New commits:
fa0d0b0  15024: fix previous commit

comment:48 Changed 21 months ago by
Oops, the change in airy.pyx
is necessary because meval
no longer exists.
comment:49 Changed 21 months 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 21 months ago by
 Commit changed from fa0d0b0effaaeffaccca661627bba232f452b773 to 821c307bc9a2d6b7c23929f8ee6e65e1e5a80bfa
Branch pushed to git repo; I updated commit sha1. New commits:
821c307  cosmetics

comment:51 Changed 21 months 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 21 months 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 21 months ago by
 Commit changed from 821c307bc9a2d6b7c23929f8ee6e65e1e5a80bfa to 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 in reply to: ↑ 52 Changed 21 months 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 21 months ago by
 Branch changed from u/rws/add_hankel_functions_and_make_spherical_bessel_and_hankel_functions_symbolic to u/rws/15024
comment:56 followup: ↓ 58 Changed 21 months ago by
 Commit changed from ab6006d7d04c8add865c48b1ecd0132a330bebd0 to 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 21 months ago by
 Commit changed from 28606307034dd394ec39a74508e05442cb4b4fc9 to 7e487050f152e7afdebdd05c5ba276d645f963c7
Branch pushed to git repo; I updated commit sha1. New commits:
7e48705  15024: more cosmetics

comment:58 in reply to: ↑ 56 Changed 21 months 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:60 Changed 21 months ago by
Thanks for your patient work.
comment:61 Changed 21 months ago by
 Branch changed from u/rws/15024 to 7e487050f152e7afdebdd05c5ba276d645f963c7
 Resolution set to fixed
 Status changed from positive_review to 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