-
Notifications
You must be signed in to change notification settings - Fork 9
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
Inconsistent results for pure JAX implementation of inverse Wigner transformation #209
Comments
Thanks for your comments @ElisR . We'll try to look into this soon. We've tested the Wigner transforms and they should be working fine but there is a known issue that they are only stable for relative low azimuthal bandlimit N at present (around O(10)). I believe a warning/error is displayed if one tries to go beyond that (is that the case @CosmoMatt?). We have plans to fix this in the very near future. Is this the issue you were running into or is this something different (apologies, I haven't had a chance to look at your code example yet)? |
Thanks for your response @jasonmcewen and apologies for my late reply. You're right that the issue must be coming from the large azimuthal bandlimit Perhaps as a temporary measure I will make a PR that adds this note to the docstring of I'll be curious to know what's causing the numerical instability at large |
Thanks for catching this @ElisR ! We're getting back into further development of s2fft again now after some other commitments. If you could create a PR to add a warning that would be fantastic. We'll then add you to all contributors for the code. In terms of the numerical stability, it's related to the recursions that we use. We've already implemented alternative recursions that don't suffer from this instability, although they may not distribute over GPUs quite so efficiently. We just need to integrate these into the harmonic transforms and will try to get to that soon. Let us know if you're interested in helping out with this and we could help to support that? Otherwise we'll try to get to it soon but we have a few other todos first. |
Hi @jasonmcewen - made a PR #220 to acknowledge the low accuracy beyond I'd be happy to try and help bring over the stable recursions into the transforms, if you could point me to the existing work. Is there a branch with these recursions, or are they already in |
Thanks for this @ElisR . We'll review this shorty and get into main. Great that you're interested in getting involved in the integration of the alternative recursions. Let's pick up the discussion about that in another issue shortly... |
Closing this issue since the documentation updates regarding warnings were added in this PR and we'll pick up the discussion about integrating other recursions for stability for high azimuthal bandlimits in this issue. |
Hi! Firstly, thanks a lot for developing this package! I'm not in astro, but the package has been just what I needed in a project I'm working on.
The issue I'm having is related to the JAX implementation of the inverse Wigner transformation
s2fft.wigner.inverse_jax
. I was lucky to stumble upon the SSHT version of this function that allowed me to implement what I needed, albeit on the CPU.Below, I have added a minimum (non-)working example to illustrate the kind of use case I have.
Apologies that it is still slightly verbose.
I haven't delved deep into the library code to try and debug, but from the scales it looks like some catastrophic floating point errors are happening (beyond aliasing artefacts, which is what I originally assumed).
The text was updated successfully, but these errors were encountered: