Opened 4 years ago

Closed 15 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: sage-7.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 arbitrary-precision numeric evaluation.

Attachments (2)

trac15024.patch (19.1 KB) - added by eviatarbach 4 years ago.
trac15024_2.patch (25.7 KB) - added by eviatarbach 4 years ago.

Download all attachments as: .zip

Change History (63)

Changed 4 years ago by eviatarbach

comment:1 Changed 4 years ago by eviatarbach

  • Cc burcin added
  • Status changed from new to needs_review

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

comment:2 Changed 4 years ago by eviatarbach

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 4 years ago by eviatarbach

comment:3 Changed 4 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:4 Changed 3 years ago by kcrisman

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 3 years ago by eviatarbach

Yeah, those should be easy to add, although maybe in another ticket.

comment:6 Changed 3 years ago by kcrisman

Sure, this is #16221 now. By the way, do we really want the Hankel stuff in bessel.py?

Last edited 3 years ago by kcrisman (previous) (diff)

comment:7 Changed 3 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:8 Changed 3 years ago by rws

  • Branch set to u/rws/add_hankel_functions_and_make_spherical_bessel_and_hankel_functions_symbolic

comment:9 Changed 3 years ago by rws

  • 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:

d0ada4bAdding Hankel functions, making spherical Bessel and Hankel functions symbolic.

comment:10 Changed 3 years ago by git

  • Commit changed from d0ada4badea353a69b2d49d276edbed44acaf8bc to c2cd65d850b68064d226339a6a89bcf54a9646fe

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

57aa3d9Merge branch 'develop' into t/15024/add_hankel_functions_and_make_spherical_bessel_and_hankel_functions_symbolic
c2cd65d15024: fix doctests

comment:11 Changed 3 years ago by rws

  • Authors set to Eviatar Bach, Ralf Stephan
  • 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 3 years ago by git

  • Commit changed from c2cd65d850b68064d226339a6a89bcf54a9646fe to 0e9578c1a4db7e7abfddc698faae9672a0732ceb

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

0e9578cMerge branch 'develop' into t/15024/add_hankel_functions_and_make_spherical_bessel_and_hankel_functions_symbolic

comment:13 Changed 3 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:14 Changed 3 years ago by git

  • Commit changed from 0e9578c1a4db7e7abfddc698faae9672a0732ceb to 485a9ec64a55c8f1a62fa9c67188c088278780e1

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

485a9ecMerge branch 'develop' into t/15024/add_hankel_functions_and_make_spherical_bessel_and_hankel_functions_symbolic

comment:15 Changed 3 years ago by git

  • Commit changed from 485a9ec64a55c8f1a62fa9c67188c088278780e1 to 78e3fe10b78b19d0471fe434384c4a8470935211

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

78e3fe1Merge branch 'develop' into t/15024/add_hankel_functions_and_make_spherical_bessel_and_hankel_functions_symbolic

comment:16 Changed 3 years ago by jdemeyer

Just to let you know: this conflicts with #17130.

comment:17 Changed 3 years ago by git

  • Commit changed from 78e3fe10b78b19d0471fe434384c4a8470935211 to bf299a2fa7bcd53778ebe75c58158459e1ec783e

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

8c43c75Merge branch 'develop' into t/15024/add_hankel_functions_and_make_spherical_bessel_and_hankel_functions_symbolic
bf299a215024: simplifications due to new BuiltinFunction code

comment:18 Changed 3 years ago by git

  • Commit changed from bf299a2fa7bcd53778ebe75c58158459e1ec783e to e1b4ee098d002ddd794acde0833b2b3f31d0c888

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

bb20d85Merge branch 'develop' into t/15024/add_hankel_functions_and_make_spherical_bessel_and_hankel_functions_symbolic
e1b4ee015024: fix doctest

comment:19 Changed 3 years ago by rws

  • Dependencies set to #18257

#18257 makes this ticket fail doctests.

comment:20 Changed 2 years ago by kcrisman

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*x-5*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 2 years ago by kcrisman

  • 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 W|A'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 pseudo-random 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 2 years ago by kcrisman

(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 long-term solution to that and then holding various tickets hostage.)

comment:23 Changed 2 years ago by kcrisman

#18257 is good now.

comment:24 Changed 2 years ago by git

  • Commit changed from e1b4ee098d002ddd794acde0833b2b3f31d0c888 to e4fbb38d5e532e07d15b825a3c6f9ccd28ce5908

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

7a0c36eMerge branch 'develop' into t/15024/add_hankel_functions_and_make_spherical_bessel_and_hankel_functions_symbolic
e4fbb3815024: improve documentation

comment:25 Changed 2 years ago by rws

  • Milestone changed from sage-6.4 to sage-6.8
  • Status changed from needs_work to needs_review

comment:26 Changed 2 years ago by rws

  • Status changed from needs_review to needs_work

comment:27 Changed 2 years ago by rws

  • Status changed from needs_work to needs_review

comment:28 Changed 2 years ago by rws

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)
<ipython-input-8-fc5809e0a430> 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.

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

comment:29 Changed 2 years ago by rws

  • Dependencies changed from #18257 to #18257, #19464

comment:30 Changed 19 months ago by rws

  • Status changed from needs_review to needs_work

comment:31 Changed 19 months ago by git

  • Commit changed from e4fbb38d5e532e07d15b825a3c6f9ccd28ce5908 to 5163957f72ad67374eb5310241bc3ce965e5646e

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

4198c28fix 19464 by allowing a hold keyword on floor/ceil
ddfa8faMerge branch 'u/rws/add_hankel_functions_and_make_spherical_bessel_and_hankel_functions_symbolic' of git://trac.sagemath.org/sage into tmp02
d2afc4419464: fix bug
5163957Merge branch 'u/rws/expressiontreewalker_fails_on_some_functions' of git://trac.sagemath.org/sage into tmp02

comment:32 Changed 19 months ago by git

  • Commit changed from 5163957f72ad67374eb5310241bc3ce965e5646e to 6d1a2b12b3580aa452005284e74a2cd79d7bf772

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

4e5e90115024: address reviewers comments; cosmetics
6d1a2b115024: add references

comment:33 Changed 19 months ago by rws

  • Milestone changed from sage-6.8 to sage-7.2
  • Status changed from needs_work to needs_review

comment:34 Changed 19 months ago by rws

  • Cc chapoton added

comment:35 Changed 16 months ago by rws

  • Dependencies changed from #18257, #19464 to #18257, #19464, #12121

comment:36 Changed 16 months ago by rws

  • Status changed from needs_review to needs_work

comment:37 follow-up: Changed 16 months ago by paulmasson

  • Cc paulmasson added

Symbolic floorcurrently 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 16 months ago by rws

  • Dependencies changed from #18257, #19464, #12121 to #18257
  • Milestone changed from sage-7.2 to sage-7.3

Replying to paulmasson:

Symbolic floorcurrently accepts hold=True, as does ceil. 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.51851972086557e-6

comment:39 Changed 16 months ago by git

  • Commit changed from 6d1a2b12b3580aa452005284e74a2cd79d7bf772 to 6f3fbbc5bdf5db721b1b03b0dc67547d589e45bc

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

0c83678Merge branch 'develop' into t/15024/add_hankel_functions_and_make_spherical_bessel_and_hankel_functions_symbolic
7dcdd9815024: revert changes coming from the 19464 branch
6f3fbbc15024: cleanup in special.py

comment:40 Changed 16 months ago by rws

  • 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 follow-up: Changed 16 months ago by paulmasson

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 16 months ago by paulmasson

The apparently missing branch is due to server maintenance.

comment:43 in reply to: ↑ 41 Changed 16 months ago by rws

Replying to paulmasson:

Unfortunately I'm now seeing changes to airy.py and jacobi.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 16 months ago by git

  • Commit changed from 6f3fbbc5bdf5db721b1b03b0dc67547d589e45bc to 2a5c20665d38736b5f990b4f735da5fade5baade

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

2a5c20615024: revert changes not related to ticket

comment:45 Changed 16 months ago by paulmasson

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 16 months ago by paulmasson

  • Reviewers set to Paul Masson

comment:47 Changed 16 months ago by git

  • Commit changed from 2a5c20665d38736b5f990b4f735da5fade5baade to fa0d0b0effaaeffaccca661627bba232f452b773

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

fa0d0b015024: fix previous commit

comment:48 Changed 16 months ago by rws

Oops, the change in airy.pyx is necessary because meval no longer exists.

comment:49 Changed 16 months ago by paulmasson

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 16 months ago by git

  • Commit changed from fa0d0b0effaaeffaccca661627bba232f452b773 to 821c307bc9a2d6b7c23929f8ee6e65e1e5a80bfa

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

821c307cosmetics

comment:51 Changed 16 months ago by rws

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:

821c307cosmetics

comment:52 follow-up: Changed 16 months ago by paulmasson

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 16 months ago by git

  • Commit changed from 821c307bc9a2d6b7c23929f8ee6e65e1e5a80bfa to ab6006d7d04c8add865c48b1ecd0132a330bebd0

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

ab6006dMerge branch 'develop' into t/15024/add_hankel_functions_and_make_spherical_bessel_and_hankel_functions_symbolic

comment:54 in reply to: ↑ 52 Changed 16 months ago by rws

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 16 months ago by rws

  • Branch changed from u/rws/add_hankel_functions_and_make_spherical_bessel_and_hankel_functions_symbolic to u/rws/15024

comment:56 follow-up: Changed 16 months ago by paulmasson

  • 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:

2860630Merge branch 'develop' into tmp02

comment:57 Changed 16 months ago by git

  • Commit changed from 28606307034dd394ec39a74508e05442cb4b4fc9 to 7e487050f152e7afdebdd05c5ba276d645f963c7

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

7e4870515024: more cosmetics

comment:58 in reply to: ↑ 56 Changed 16 months ago by rws

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:59 Changed 16 months ago by paulmasson

  • Status changed from needs_review to positive_review

Good to go.

comment:60 Changed 16 months ago by rws

Thanks for your patient work.

comment:61 Changed 15 months ago by vbraun

  • Branch changed from u/rws/15024 to 7e487050f152e7afdebdd05c5ba276d645f963c7
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.