Implement an iterative FFT algorithm #21
Conversation
|
Currently the new code isn't integrated into proof generation/verification. There is a unit test that confirms that the algorithms output the same thing, and I've tested that the rest of the tests pass with the new algorithm. If you'd like, we can integrate the new algorithm as part of this PR. |
cjpatton
force-pushed
the
iter-fft
branch
3 times, most recently
from
a9fb223
to
a415368
Compare
src/fft.rs
Outdated
| } | ||
|
|
||
| /// Sets `outp` to the inverse of the DFT of `inp`. | ||
| pub fn dft_inv<F: FieldElement>(outp: &mut [F], inp: &[F]) { |
There was a problem hiding this comment.
Similar comments to the dft function:
- I'd prefer a longer, more explicit function name like
inverse_discrete_fourier_transform - Would prefer that this return the output as
[F]instead of taking a mutable slice, unless there's a specific reason to do it this way.
There was a problem hiding this comment.
1. I'd prefer a longer, more explicit function name like `inverse_discrete_fourier_transform`
Changed to discrete_fourier_transform_inv, in order to align with FieldElement::inv.
2. Would prefer that this return the output as `[F]` instead of taking a mutable slice, unless there's a specific reason to do it this way.
See above comment (I'm trying to avoid nedless malloc).
src/fft.rs
Outdated
| /// Sets `outp` to the inverse of the DFT of `inp`. | ||
| pub fn dft_inv<F: FieldElement>(outp: &mut [F], inp: &[F]) { | ||
| let n = inp.len(); | ||
| let m: F = F::from_usize(n).unwrap().inv(); |
There was a problem hiding this comment.
Similarly to dft, dft_inv should return an error and avoid unwrap().
There was a problem hiding this comment.
Here again, I think panicking is more appropriate. The function will only panic if discrete_fourier_transform() isn't implemented properly. (See the new input length checks in that function.)
| + Display | ||
| { | ||
| /// The integer representation of the field element. | ||
| type Integer; | ||
|
|
||
| /// Modular exponentation, i.e., `self^exp (mod p)`. | ||
| fn pow(&self, exp: Self) -> Self; | ||
| fn pow(&self, exp: Self) -> Self; // TODO(cjpatton) exp should have type Self::Integer |
There was a problem hiding this comment.
Why? Because I might want to raise self to some exponent that's greater than the field prime?
There was a problem hiding this comment.
I'm not sure if one would ever want to do that. The reason is more mathematically pedantic: Raising an element of a group (or field) to the power of an element of that group (or field) is not always well-defined. In general, a^x only makes sense if x is an integer.
There was a problem hiding this comment.
Note that I plan to solve this TODO in the next PR.
cjpatton
force-pushed
the
iter-fft
branch
3 times, most recently
from
d440206
to
6fd32fe
Compare
This change adds an alternative algorithm for computing the discrete Fourier transform. It also adds a new module, benchmarked, for components of the crate that we want to benchmark, but don't want to expose in the public API. Finally, it adds a benchmark for comparing the speed of iterative FFT and recursive FFT on various input lengths.
This adds an alternative FFT algorithm for doing the discrete Fourier transform. The main advantage is that you don't have to set up auxiliary state (
PolyAuxMemoryandPolyFFTTempMemory) that gets carried around through the code. It's also quite a bit faster, even compared to amortizing the cost of setting up the auxiliary state across FFT operations. This is demonstrated by the benchmarks in the last commit. The commit is marked "WIP" because I'm not sure we actually want to merge it, since it requires making the thepolynomialmodule pubic.