-
-
Notifications
You must be signed in to change notification settings - Fork 10.9k
Support norm="unnormalized"
for FFT functions?
#16126
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
Comments
Checked the issue and I'm about to start working on it; is somebody else working on that at the moment? I understand that we work without assigning issues, so I thought I'd check with you first. I've been using the I'm going to introduce an additional |
Hi @cvav-git I am not aware of anyone working on this. cc: @mattvend who was working on the CuPy counterpart. |
Thanks for your quick reply @leofang! Hi @anirudh2290 @charris my current plan is to modify the Let me know if you have any thoughts or comments so that we try to find the best solution possible; and of course the new norm name is still open to discussion, I'm just using the tentative name cc @mattvend |
I've pushed my code changes and opened a pull request regarding this issue; I've cc'd you on the request description; let me know how to proceed, thanks! |
The kwarg option norm=inverse leads to scaling of the transforms inverse (opposite) to that of the default option norm=None; i.e. the forward transform is normalized with 1/n whereas the backward one with 1. The fft routines and their tests have been modified to reflect the changes; all tests have been passed successfully. Closes #16126 * modified _unitary, fft & ifft and their tests; tests passed * modified rfft & irfft and their tests; tests passed * modified hfft & ihfft and all tests; all modifications are now done * correction in ihfft; all 79 fft tests passed successfully * undo unnecessary docstrings changes made in my previous commits * use norm=forward kwarg value name * Update numpy/fft/__init__.py Co-authored-by: Leo Fang <leofang@bnl.gov> * add code review suggestions Co-authored-by: Leo Fang <leofang@bnl.gov> * add default norm=None alias norm=backward * streamline private normalization functions * modify hermitian FFTs * add review suggestions Co-authored-by: Leo Fang <leofang@bnl.gov> * add review suggestions v2 (dict as module const) * make review suggestions v3 * Apply suggestions from code review Co-authored-by: Leo Fang <leofang@bnl.gov>
I tried searching in the issue tracker here but didn't find any, so let me know if this is a duplicate.
This issue was first brought up in CuPy (cupy/cupy#944) and has regained some interest, and I believe it's better to be implemented/considered first in upstream, which downstream libraries like SciPy and CuPy can follow.
The desired effect is to do the normalization opposite to
norm=None
; that is, the backward/inverse transform is unscaled, and the forward transform is scaled by 1/n. For a perfectionist's point of view adding this option makes a lot of sense, as it would cover every possible normalization scheme.In particular, for GPU libraries like CuPy this could be performance critical for certain applications, as kernel launching is more costly in the GPU world. People usually don't do extra work whenever possible, especially if the normalization is of no interest.
The name
"unnormalized"
can be changed to a more informative one. I don't have any preference (in fact I'm just a messenger bringing this to NumPy...)The text was updated successfully, but these errors were encountered: