Skip to content
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

Implement real FFT #50

Closed
wants to merge 6 commits into from
Closed

Implement real FFT #50

wants to merge 6 commits into from

Conversation

ejmahler
Copy link
Owner

@ejmahler ejmahler commented Jan 8, 2021

This isn't ready to merge yet, but it's in a previewable state.

I used the RealFFT crate as a starting point for the new structs, and I was able to make a few optimizations to bring down the overhead by about a third.

Todo:

  • Find a paper or something for odd-sized real FFTs
  • Implement an AVX version of c2r and r2c
  • Clean up unit tests
  • Write some integration tests
  • Benchmark and write some c2r and r2c butterflies, so that using r2c is always faster than or equal to a normal FFT
  • Decide what to do with the planner. Should this stuff go in the current planner? Should I make a new planner that wraps the current one? If we made a new planner, it would make it harder to integrate AVX. But I'm worried about a slippery slope where we bloat the planner struct until it's this giant monolith.

@ejmahler
Copy link
Owner Author

ejmahler commented Jan 8, 2021

cc: #28

@HEnquist
Copy link
Contributor

HEnquist commented Jan 8, 2021

Very nice! For the first point, I have tried and failed. The only thing I have found is the implementation in fftw, but it's very hard to read. And they don't provide any references (that I have been able to find at least).
When you look under the hood of most other implementations, they turn out to cheat and just use a C2C transform of the same length.

For the planner, I also think adding more and more things in the current planner is risky. But the stuff added in this PR are reasonable, just reusing the existing things and wraps them up as a R2C or C2R. But that simplicity disappears if there are special R2C butterflies.. Probably needs some more thinking.

@HEnquist HEnquist mentioned this pull request Jan 12, 2021
@micwoj92
Copy link

This branch has conflicts that must be resolved

@ejmahler
Copy link
Owner Author

Given what I've learned about this since opening the PR (IE that implementing odd R2C essentially requires fully rewriting the FFT stack from the ground up), I don't think I'll ever be doing it this way, so I'm closing this.

If I ever fully implement r2c etc FFTs, it'll be in another crate,

@ejmahler ejmahler closed this Mar 12, 2023
@ejmahler ejmahler deleted the realfft branch March 12, 2023 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants