-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
math: add tests for corner cases of Yn Bessel function #19130
Comments
I think we are already testing almost all the edge cases here. We are testing Yn(n,a) for combinations of n=2,-3 and a= positive finite number, 0, -Inf, +Inf, NaN. |
CL https://golang.org/cl/37310 mentions this issue. |
The special cases listed for Yn are: Yn(n, +Inf) = 0 also Y0(+Inf) = 0 and Y1(+Inf) = 0 [Note that Y0(x) = Yn(0, x) and Y1(x) = Yn(1, x)] Yn(n, x) is already tested for n ∈ {0, 1, 2, -3} and x ∈ {+∞, -∞, 0, NaN}, covering each of the above cases at least once. There are also several test cases over the same values of n with finite x>0. Note that the tests for n=0 and n=1 actually test Y0 and Y1 but that's equivalent. The only test that might be seen as missing is finite x<0 but I don't think it's needed. |
I think we are in agreement about which which cases are already tested. The only case not tested at all is negative finite number for which I added a test with -1. This a usefull addition imo. Y0(0) is already tested, like you said. So with the extra test Yn(0,0) we are only testing that Yn is calling Y0 in that case. I could see that this is something we might not need. |
Oh, sorry, I missed that you had already sent a CL. I don't think that Yn(0, x) is needed unless we also need Yn(1, x) (and Jn(0, x) and Jn(1, x)). In my opinion the case Yn(n, x<0) is already covered by Yn(n, -Inf), so I don't think we actually need any change at all here. Nevertheless, extra tests aren't going to hurt. Since a CL is already provided, should the HelpWanted label be removed from this issue? |
Follow-up on #18823 and corresponding change https://go-review.googlesource.com/#/c/35970/ (see Rob's comment in that change).
The text was updated successfully, but these errors were encountered: