-
-
Notifications
You must be signed in to change notification settings - Fork 487
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
bessel_K function is broken #3426
Comments
Attachment: kbessel_fixes.patch.gz |
Changed keywords from bessel, bessel_K to bessel, bessel_K, editor_gfurnish |
comment:4
Regarding bessel_K being real for real argument and real or imaginary order, see, e.g. the appendix to H. Then, Maass cusp forms for large eigenvalues, Math. Comp. Volume 74, Number 249, pp. 363 - 381: "The K-Bessel function K_ir(x) is ... real for real arguments x and real or imaginary order ir." |
comment:5
I can say that I agree that this is 100% correct. |
comment:9
I think a solution of the following type would be better.
|
comment:10
Probably the correct code would be
|
comment:11
Since Rishi commented on this it might be a good idea to discuss his comments. Cheers, Michael |
comment:23
rishi's code does not prevent brokenness at all (in fact it is 100% equivalent to attempting to trying to return RR(answer, prec). The patch as is makes the answer "more correct," and then we can go back and write code (that makes use of this patch) to make it 100% correct. Alternatively, if someone wants to make a new patch that checks for real(z)>=0 in all cases and throws an error otherwise, I would give that a positive review. However, the modification of real(z)>0 is not sufficient to ensure correctness. |
comment:24
This ticket has been sitting around for a while without any movement. Change the title so that the reports pick up this ticket correctly. Cheers, Michael |
comment:26
I note that Alex added himself to the CC for this. Whatever is done for this issue absolutely must take into account the work done for #4096, so at the least I suggest that the author of this patch looks at that one and reworks this. Anything relying on Sage/pari precision questions is likely to be useless otherwise. |
comment:27
I looked up the definition and properties of the Bessel functions in I uploaded a brand new patch that implements the behavior described In the process I uncovered a couple of unrelated issues with The patch is made against 3.1.3.alpha2. |
comment:29
Thanks for catching this. It actually comes from a bug in Pari:
I've reported it upstream, but I will post a patch with a workaround while we wait. |
apply instead of the previous patch |
comment:30
Attachment: trac3426-fix-bessel-fns.patch.gz OK, I've replaced my patch with one that fixes the issue reported by Dan. |
comment:33
See #4626, which at least fixes the |
comment:34
This ticket is a huge mess :) I now think that we should just use mpmath to evaluate Bessel functions, see http://mpmath.googlecode.com/svn/trunk/doc/build/functions/bessel.html For the examples that Dan gave:
|
Changed keywords from bessel, bessel_K, editor_gfurnish to bessel, bessel_K |
comment:35
From my quick experiments with the issues Bober was dealing with a year ago, I see that do not arise if we use mpmath, even when I set the precision to 5000. I agree with Alex Ghitza. I should say that the bessels functions for non integer indices have always bothered me. I believe computing will involve a log, and how do you consistently choose a branch. |
comment:37
With some experiments, I saw that the branch of the log taken is the negative real axis. We should mention this in the documentation when it is implemented. I believe ddrake is working up a patch. |
comment:41
This would most likely be fixed by #4102. |
comment:42
Yes, it will be fixed by #4102. I'll make a note to add a related doctest to that effect. |
Reviewer: Karl-Dieter Crisman, Benjamin Jones |
comment:43
Everything here is now in a doctest in #4102, including the stuff in the thread from three (!) years ago. |
Currently we have
In this case the result actually should be a real number, so we fix this by discarding the imaginary part of the result from pari. In other cases, however, the result is actually a complex number, and we shouldn't always be attempting to cast it to a real number (which the attached patch also fixes).
CC: @burcin @kcrisman @benjaminfjones
Component: calculus
Keywords: bessel, bessel_K
Reviewer: Karl-Dieter Crisman, Benjamin Jones
Issue created by migration from https://trac.sagemath.org/ticket/3426
The text was updated successfully, but these errors were encountered: