fix complex sph_bessel_j kernel sqrt branch#96
fix complex sph_bessel_j kernel sqrt branch#96lucascolley wants to merge 2 commits intoscipy:mainfrom
sph_bessel_j kernel sqrt branch#96Conversation
include/xsf/sph_bessel.h
Outdated
| } | ||
|
|
||
| std::complex<T> out = std::sqrt(static_cast<T>(M_PI_2) / z) * cyl_bessel_j(n + 1 / static_cast<T>(2), z); | ||
| std::complex<T> out = std::sqrt(static_cast<T>(M_PI_2)) * cyl_bessel_j(n + static_cast<T>(0.5), z) / std::sqrt(z); |
There was a problem hiding this comment.
This looks viable. I suggest putting
inline constexpr double SQRT_PI_2 = 1.2533141373155003;in a detail namespace in this file, and then replacing the above with
std::complex<T> out = static_cast<T>(detail::SQRT_PI_2) * cyl_bessel_j(n + static_cast<T>(0.5), z) / std::sqrt(z);using inline constexpr let's us sprinkle constants in whichever headers they are needed without running afoul of the one definition rule.
|
Thanks @lucascolley. I think this will work. See the comment about defining a constant to use here. Also, the fix is needed in the similar line in each of the spherical bessel functions defined in this header. |
|
Do we have the relevant test coverage in this repo? |
Yes, all cases from the |
|
I did not expect that. I'll have to look into why the relevant cases aren't there. |
|
I found that It shouldn't be too bad to add the tables. I might have time over the weekend. |
scipy/scipy#23700 (comment) @steppi
AI tools used: ChatGPT
The SciPy tests seem to pass on osx-arm64 with this change.