Adding process_immutable_with_scratch method

[michaelciraci]

Follow on for #150

I found it easier to follow up #151 (comment) with a fresh PR, so this is instead of #151.

I implemented the immutable method for scalar, AVX, SSE, Neon, and wasm, so no default method was actually needed.

[ejmahler]

It's great to see so much progress! I think this is close, but there are still a few things we need to work on. As I mentioned in my comment above, the biggest thing is to just make sure that the amount of scratch we're requesting is sound. I know from experience that it's difficult to wrap my head around and easy to get wrong, so it's worth paying extra attention to it.

If you want a specific example where this will go wrong with Mixed Radix and Good-Thomas in this PR, try constructing this in a local test: A MixedRadix instance (or good-thomas instance), with the children being a Butterfly7 on one side, and a BluesteinsAlgorithm with a length of 100 and inner FFT of 256 on the other side, and then compute an immutable-input FFT. The current code will not distribute scratch correctly and I bet it will panic.

This is also making me realize that Radix4 could use some more testing. Last year, I added a PR to allow Radix4 to take arbitrary FFTs as the base, which is a fantastic expansion of functionality, but opens the door for similar scratch bugs like this. I should have added the big scratch test I linked above, but it didn't occur to me.

Since scratch size is a concern again, especially with this new code, would you mind implementing big scratch tests for the various Radix4 implementations, similar to the one that exists for MixedRadix? That way we could get code coverage on this new pathway's scratch logic for Radix4.

[ejmahler]

One more thing: The check_fft_algorithm method should have a new section at the bottom that tests the new immutable FFT method. I imagine it can largely be copied from the out of place version, just with immutable input. Every single FFT instance in the project uses that as a part of their unit tests, so it'll ensure that all of our new code gets test coverage.

[michaelciraci]

Sounds good. I was leaving it up to the planner to chose an algorithm for each length, but I'll use that method for coverage for all the different algorithms.

[michaelciraci]

Corrected the scalar implementation, and tested the permutation of all features. Pipeline should pass now.

[michaelciraci]

Also updated benches.

[michaelciraci]

@ejmahler Just a friendly follow up. Should be good :)

[ejmahler]

Thanks for waiting. Since it's so big, I was waiting for some free time to do a more thorough review, and I have some now.

[michaelciraci]

Actually, I think we're going to have an issue with the autogen check because SSE and Neon have different imports

[michaelciraci]

Admittedly, I'm not entirely sure why there would be an issue here but I can try to dig into it: https://github.com/ejmahler/RustFFT/actions/runs/15455339790/job/43506452392?pr=157

Perhaps got overly excited about being able to merge this in today...

[ejmahler]

If you're having trouble with the workflow of the autogenerated files, the way is works is that you edit the template, and then you run the autogen for sse, neon, and wasm simd.

I recommend adding fft_error_immut back to the list of imports in the template, then run the script for all 3 files. Then you can use it in the macro the same way everything else does.

[ejmahler]

I'm not sure what's going on with the wasm simd error. If you need help with that, I'll investigate that myself later tonight and push a fix myself once I figure out what's going on.

[michaelciraci]

If you don't mind taking a look, I'd definitely accept the offer.

[ejmahler]

I found the problem - in the latest nightly, whatever is responsible for executing doctests was finding false positive doctests and trying to execute them, and they were of course failing because they weren't designed to be doctests. I made a separate PR (#160) to bypass the error for now, and merged it into this branch.

[ejmahler]

I did one more pass to make things as consistent as possible with RustFFT's existing style. It's a lot of changes, but each change is small.

I'm ready to merge this. Since this is such a big change, would you mind doing the same as me, and looking over everything one last time to make sure there's nothing else the two of us missed?

Once you tell me everything's ok I'll merge.

[michaelciraci]

All the changes look good and make sense to me.

[ejmahler]

Excellent! You put a ton of work into this, and I really appreciate it.

I have some small refactoring I want to do before making a release, which I will be able to do this weekend, so the release will probably be at the end of the weekend or next week.

[michaelciraci]

Sounds good.

And likewise, thanks for your assistance with during this merge request.

[ejmahler]

This change is live: https://crates.io/crates/rustfft/6.4.0

Thanks again for all your hard work.