-
Notifications
You must be signed in to change notification settings - Fork 10
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
Comparison to RustFFT is misleading #16
Comments
Is padding with zeros not working for you? What kind of functionality do you need for non-powers of 2? |
I can't use padding with zeros because it does not give an equivalent result. It works for some use cases, but it's really not a general solution. |
Somewhat related, but should perhaps be a separate issue. The documentation for the planner does not mention the power of two requirement, for example here: Line 36 in 01aed51
|
I just realised that the benchmarks create new planner and FFT instances for every iteration. This is incredibly inefficient and does not reflect how FFTs are typically used. With FFTW and RustFFT, planning and creating an FFT is expensive, and they will both look very slow if you do not reuse the same FFT for more than one iteration. |
@HEnquist You seem upset. From what I gather, your disappointment stems from a fundamental misunderstanding between the FFT and the DFT. The FFT is nothing more than an algorithm to compute the DFT in special cases. You should consider taking a glance at this writeup by NI. Of note:
Yes,
In accordance with the original paper by Cooley & Tukey, you can't use the FFT for your use case. What you need is either Bluestein's algorithm, the basic DFT, or something else entirely.
A third party has already taken this into account in their independently developed benchmarks. I'll let the results speak for themselves. Remarkably, Thank you for your insight. Best, |
No not upset, sorry if it came across that way.
There is no misunderstanding. But now I see that the problem with the documentation comes from how FFT is defined. More on that below.
Yes, Bluesteins, Raders, Mixed Radix and Good-Thomas are all there to support non-power-of-two lengths, and they are all described as FFT algorithms in literature. Most of the logic in the planner is for selecting between these.
That definition of what FFT is was only relevant in 1965! Sure you can still find texts where only power-of-two algorithms are counted as FFTs (like in that note from NI) but those are a tiny minority. There are many later algorithms that support other lengths, that are referred to as FFTs everywhere except in those few bad examples. Some of these are almost as old as Cooley-Tukey.
Those results look good and they are much more relevant than the set included in the PhastFT repo. There is still some unnecessary overhead from the planners, but it's probably not affecting the results very much (except for at very short lengths). |
@HEnquist Thank you for getting back to me. Since Bluestein's Algorithm would be able to handle your use case of arbitrary length sequences (without the need for padding), we'll open up an issue, and we'll add it into the planned features section of the readme. I found a simple implementation of the Bluestein's Algo (e.g., the Chirp Z-transform) here. Judging by the following snippet (taken from the previous link), we already have from scipy import *
def czt(x, m=None, w=None, a=None):
# Translated from GNU Octave's czt.m
n = len(x)
if m is None: m = n
if w is None: w = exp(-2j * pi / m)
if a is None: a = 1
chirp = w ** (arange(1 - n, max(m, n)) ** 2 / 2.0)
N2 = int(2 ** ceil(log2(m + n - 1))) # next power of 2
xp = append(x * a ** -arange(n) * chirp[n - 1 : n + n - 1], zeros(N2 - n))
ichirpp = append(1 / chirp[: m + n - 1], zeros(N2 - (m + n - 1)))
r = ifft(fft(xp) * fft(ichirpp))
return r[n - 1 : m + n - 1] * chirp[n - 1 : m + n - 1]
@vectorize # Rounds complex numbers
def cround(z, d=None): return round(z.real, d) + 1j * round(z.imag, d)
arr = [1.0, 2.0, 3.0]
print cround(czt(arr), 4) # [ 6.0+0.j -1.5+0.866j -1.5-0.866j]
print cround(fft(arr), 4) # [ 6.0+0.j -1.5+0.866j -1.5-0.866j]
With the author's permission, the benchmarks I linked will supplant the current benchmarking "framework". Unfortunately, the file output for
Yes, thank you. We're not at v1.0.0 yet, so there were bound to be deficiencies such as the one you came across. If you don't mind, please open up a separate issue to point out anything else that needs fixing in the readme, docs, etc. I will open up a PR now, and I'll link it to the issue you open. Thank you for your help. Best, |
Adding Bluesteins is a very good idea. That will remove the limitation for what lengths can be processed, and is pretty simple to implement. There is a nicely readable one here, already in the right language:
I wouldn't spend too much time on optimising stuff here! In practice, nearly all the time is spent doing the inner FFT and iFFT steps anyway. |
Hi @HEnquist, We released v0.2.0 with the fixes that you suggested here and in PR #20. The implementation of Bluestein's algorithm will have its own PR opened, shortly. Please let me know if there is anything else you'd like to see amended prior to this issue being closed. I will add a changelog shortly that will acknowledge contributions, suggestions, etc. Thank you! Best, |
Thanks, yes I think this can be closed. |
The readme contains a comparison with RustFFT. This is helpful, but it leaves out a very important aspect. PhastFT only supports power of two lengths, while RustFFT supports arbitrary length. For my own uses of FFT I absolutely need arbitrary length so this difference is critical.
Also, the readme mentions a "single, general-purpose FFT algorithm". Can an algorithm that only supports power of two lengths truly be called general purpose?
The text was updated successfully, but these errors were encountered: