Add generic FFT implementation over curve scalars#155
Add generic FFT implementation over curve scalars#155MatanHamilis wants to merge 16 commits intomasterfrom
Conversation
make power iterator public
| } | ||
| } | ||
| } | ||
| impl<T: Curve> Iterator for PowerIterator<T> { |
There was a problem hiding this comment.
Can you add a size_hint implementation? this will improve the perf of collect by allowing it to preallocate
| if self.next_idx == self.max_idx { | ||
| return None; | ||
| } |
There was a problem hiding this comment.
nit, you can replace next_idx with remaining_idx and return None when it is equal to zero (that way you save just a single usize and not 2 usizes)
| fn new(factors: &'a [(usize, usize)]) -> FactorizationIterator { | ||
| let max = factors | ||
| .iter() | ||
| .fold(1usize, |acc, (_, count)| acc * (count + 1)); | ||
| FactorizationIterator { | ||
| factorization: factors, | ||
| index: 0usize, | ||
| max, | ||
| } | ||
| } |
There was a problem hiding this comment.
Could you maybe use more a more descriptive name? what's max? it looks like the product of all the k+1?
| self.slice | ||
| .get((self.step * (self.next_index)) % self.slice.len()) | ||
| .unwrap(), |
There was a problem hiding this comment.
why get and unwrap and not indexing? did it fail due to the self.slice.len()?
| .map(|&(f, _)| { | ||
| usize::from_be_bytes( | ||
| f.to_bytes_array() | ||
| .expect("The bit length should fit in 8-byte array here!"), |
There was a problem hiding this comment.
note that usize is 4 bytes on 32bit machines
| Scalar::<T>::multiplicative_group_order_factorization() | ||
| .iter() | ||
| .filter(|(num, _)| num.bit_length() < SMALL_FACTOR_BITLENGTH) | ||
| .collect(); |
There was a problem hiding this comment.
I think you can skip the collect, as you're mapping it immediately afterwards, just keep it as an iterator
| usize::from_be_bytes( | ||
| big_int | ||
| .to_bytes_array() | ||
| .expect("The small factor should fit in a 8 byte array"), |
There was a problem hiding this comment.
usize is 4 bytes on 32 bit machines
Added a generic implementation for FFTs.
Further explanation I've written about FFTs and the implemented algorithms can be found here:
https://hackmd.io/@matan/ffts