Improve radix-2 FFTs #169
Improve radix-2 FFTs #169
Conversation
|
Should we add rsw to the |
Pratyush
force-pushed
the
faster-radix2
branch
from
bc76a8a to
cdbe01e
Compare
|
@kwantam, how would you like to be credited? (fantastic work btw) |
|
I think at minimum, there should be a header at the top of the file explaining it came from |
|
Hi folks! Glad the code is useful, and thanks for the kind words! Feel free to credit however makes sense for your project. The suggested comment at the top of a file with a pointer to the fffft repo/crate seems like a nice idea. If you list names in an authors file, my real name and/or GitHub user-id would be cool. Cool work, by the way! |
|
Just out of severe curiosity, any story on the GitHub user-id |
Not much of one---I figured that the correct spelling would be unavailable as a username, whereas a misspelling might not (and indeed it seems like that's usually the case). :) |
Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
| @@ -54,7 +50,11 @@ impl<F: FftField> EvaluationDomain<F> for Radix2EvaluationDomain<F> { | |||
| /// having `num_coeffs` coefficients. | |||
| fn new(num_coeffs: usize) -> Option<Self> { | |||
| // Compute the size of our evaluation domain | |||
| let size = num_coeffs.next_power_of_two() as u64; | |||
| let size = if num_coeffs.is_power_of_two() { | |||
There was a problem hiding this comment.
This avoids bumping up to the next power of two in an edge case.
Pratyush
force-pushed
the
faster-radix2
branch
from
82e23b4 to
f1aed69
Compare
There was a problem hiding this comment.
The code looks good. But it is good if @ValarDragon takes a final pass.
Description
Improves radix-2 FFTs by moving to the algorithm described in https://github.com/kwantam/fffft. The implementation matches the one there almost verbatim, except for tweaks specific to
ark-polyAPIs. The speedups provided by this new algorithm are really nice: between 25-50% on even moderate sizes.Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
Pendingsection inCHANGELOG.mdFiles changedin the Github PR explorer