ejmahler
Require `Float` trait for `FftNum` supertrait
Based on its doc comment, FftNum was intended to only work with f32s and f64s. (As a side note, is this comment correct in that other forms of Float (like f128) aren't implemented?)
I'm hardly an expert on FFTs, so maybe there's some use case for this that I'm not aware of, but the values left after processing an FFT with integer types seem to be just garbage. This PR is intended to prevent users inexperienced with DFTs (like me) from doing something nonsensical.
This PR prevents the following code from compiling:
use rustfft::{FftPlanner, num_complex::Complex};
let mut planner = FftPlanner::<i32>::new();
let fft = planner.plan_fft_forward(1234);
let mut buffer = vec![Complex{ re: 0, im: 0 }; 1234];
I would add a test to ensure that the above code fails to compile, but compile_fail seems to be the only option without depending on an external crate like trybuild, and I wasn't sure where a doc comment like that should live within this crate.
With the more lax trait bounds, the implementation can also be used to calculate FFTs of dual numbers (or generalizations thereof) that are used in forward automatic differentiation.
Good point. I wonder if adding Float (or similar) to num_dual would be feasible?
That would require redesigning around requiring the Copy trait bound that is implied by Float. Not impossible, but necessary? I would like to hear the maintainer's view on the issue first.
The Float trait has a lot of methods that are not needed by RustFFT, see: https://docs.rs/num-traits/latest/num_traits/float/trait.Float.html So if Float is a requirement, all those need to be implemented, even if they won't be used. And some of them probably don't quite work for things like dual numbers.
I'm afraid I have to decline this PR. In rust, it's idiomatic to only request wheat you need, which is what we do now. Requesting the entirety of Float would give us sin() and cos() which would be helpful, but we'd also be requesting a dozen other things we don't need.
