Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Hankel functions and make spherical Bessel and Hankel functions symbolic #15024

Closed
eviatarbach opened this issue Aug 9, 2013 · 73 comments
Closed

Comments

@eviatarbach
Copy link
Contributor

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.

Depends on #18257

CC: @burcin @fchapoton @paulmasson

Component: symbolics

Author: Eviatar Bach, Ralf Stephan

Branch/Commit: 7e48705

Reviewer: Paul Masson

Issue created by migration from https://trac.sagemath.org/ticket/15024

@eviatarbach eviatarbach added this to the sage-6.1 milestone Aug 9, 2013
@eviatarbach
Copy link
Contributor Author

Attachment: trac15024.patch.gz

@eviatarbach
Copy link
Contributor Author

comment:1

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

@eviatarbach
Copy link
Contributor Author

comment:2

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

@sagetrac-vbraun-spam
Copy link
Mannequin

sagetrac-vbraun-spam mannequin commented Jan 30, 2014

comment:3

Attachment: trac15024_2.patch.gz

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@kcrisman
Copy link
Member

comment:4

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...

@eviatarbach
Copy link
Contributor Author

comment:5

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

@kcrisman
Copy link
Member

comment:6

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

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@rwst
Copy link
Contributor

rwst commented May 12, 2014

@rwst
Copy link
Contributor

rwst commented May 12, 2014

comment:9

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.

@rwst
Copy link
Contributor

rwst commented May 12, 2014

Commit: d0ada4b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 17, 2014

Changed commit from d0ada4b to c2cd65d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 17, 2014

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

@rwst
Copy link
Contributor

rwst commented Jul 17, 2014

comment:11

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

@rwst
Copy link
Contributor

rwst commented Jul 17, 2014

Author: Eviatar Bach, Ralf Stephan

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 6, 2014

Changed commit from c2cd65d to 0e9578c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 6, 2014

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

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin removed this from the sage-6.3 milestone Aug 10, 2014
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 6, 2016

Changed commit from 6f3fbbc to 2a5c206

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Jul 7, 2016

comment:45

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

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Jul 7, 2016

Reviewer: Paul Masson

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 7, 2016

Changed commit from 2a5c206 to fa0d0b0

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 7, 2016

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

fa0d0b015024: fix previous commit

@rwst
Copy link
Contributor

rwst commented Jul 7, 2016

comment:48

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

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Jul 8, 2016

comment:49

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!

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 8, 2016

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

821c307cosmetics

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 8, 2016

Changed commit from fa0d0b0 to 821c307

@rwst
Copy link
Contributor

rwst commented Jul 8, 2016

comment:51

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

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Jul 8, 2016

comment:52

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?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 9, 2016

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 9, 2016

Changed commit from 821c307 to ab6006d

@rwst
Copy link
Contributor

rwst commented Jul 9, 2016

comment:54

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.

@rwst
Copy link
Contributor

rwst commented Jul 9, 2016

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Jul 10, 2016

Changed commit from ab6006d to 2860630

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Jul 10, 2016

comment:56

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 10, 2016

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

7e4870515024: more cosmetics

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 10, 2016

Changed commit from 2860630 to 7e48705

@rwst
Copy link
Contributor

rwst commented Jul 10, 2016

comment:58

Replying to @paulmasson:

  1. The evaluation of spherical harmonics has changed. I'm assuming this is because meval is gone, but please confirm.

Yes.

  1. 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.

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Jul 10, 2016

comment:59

Good to go.

@rwst
Copy link
Contributor

rwst commented Jul 11, 2016

comment:60

Thanks for your patient work.

@vbraun
Copy link
Member

vbraun commented Jul 12, 2016

Changed branch from u/rws/15024 to 7e48705

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants