Add parallel coset shift method (speeds up coset_fft), add fft correctness tests #125

ValarDragon · 2020-12-09T23:29:50Z

Description

This PR adds parallelization to computing the coset shift, which is important for coset_ffts. Furthermore, it adds tests to show that all radix 2 FFTs are actually correct. (This previously was not here)

The speed improvements of this change on a 48 core benchmark server:

"bls12_381 - radix2" - coset_fft_in_place/131072
                        time:   [33.022 ms 33.577 ms 34.128 ms]
                        change: [-31.177% -29.858% -28.472%] (p = 0.00 < 0.05)
                        Performance has improved.
"bls12_381 - radix2" - coset_fft_in_place/262144
                        time:   [60.894 ms 61.374 ms 61.877 ms]
                        change: [-20.505% -19.577% -18.747%] (p = 0.00 < 0.05)
                        Performance has improved.
"bls12_381 - radix2" - coset_fft_in_place/524288
                        time:   [110.27 ms 110.95 ms 111.67 ms]
                        change: [-19.993% -19.415% -18.737%] (p = 0.00 < 0.05)
                        Performance has improved.
"bls12_381 - radix2" - coset_fft_in_place/1048576
                        time:   [212.02 ms 212.96 ms 213.91 ms]
                        change: [-19.769% -19.319% -18.870%] (p = 0.00 < 0.05)
                        Performance has improved.

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.

Targeted PR against correct branch (main)
Linked to Github issue with discussion and accepted design OR have an explanation in the PR that describes this work.
Wrote unit tests
Updated relevant documentation in the code
Added a relevant changelog entry to the Pending section in CHANGELOG.md
Re-reviewed Files changed in the Github PR explorer

ValarDragon · 2020-12-09T23:30:53Z

poly-benches/benches/fft.rs

@@ -49,7 +49,7 @@ fn setup_bench(c: &mut Criterion, name: &str, bench_fn: fn(&mut Bencher, &usize)
 fn fft_common_setup<F: FftField, D: EvaluationDomain<F>>(degree: usize) -> (D, Vec<F>) {
    let mut rng = &mut rand::thread_rng();
    let domain = D::new(degree).unwrap();
-    let a = DensePolynomial::<F>::rand(degree, &mut rng)
+    let a = DensePolynomial::<F>::rand(degree - 1, &mut rng)


Driveby bug fix, this shouldn't have affected the FFT time, but causes the meaning of the bench to have previously been incorrect.

ValarDragon · 2020-12-09T23:32:13Z

poly/src/polynomial/univariate/dense.rs

-            assert!((poly + neg).is_zero());
+            let result = poly + neg;
+            assert!(result.is_zero());
+            assert_eq!(result.degree(), 0);


drive by improvement to the additive identity tests. It would have caught the bug fixed by #119

Add parallel coset shift method, add fft correctness tests

7ee2dae

ValarDragon commented Dec 9, 2020

View reviewed changes

Add Changelog entry

37d0287

ValarDragon requested a review from Pratyush December 9, 2020 23:33

Merge branch 'master' into parallel_coset_shift

e014a14

Pratyush approved these changes Dec 9, 2020

View reviewed changes

ValarDragon merged commit 299eac3 into master Dec 9, 2020

ValarDragon deleted the parallel_coset_shift branch December 9, 2020 23:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add parallel coset shift method (speeds up coset_fft), add fft correctness tests #125

Add parallel coset shift method (speeds up coset_fft), add fft correctness tests #125

ValarDragon commented Dec 9, 2020 •

edited

Loading

ValarDragon Dec 9, 2020 •

edited

Loading

ValarDragon Dec 9, 2020

Add parallel coset shift method (speeds up coset_fft), add fft correctness tests #125

Add parallel coset shift method (speeds up coset_fft), add fft correctness tests #125

Conversation

ValarDragon commented Dec 9, 2020 • edited Loading

Description

ValarDragon Dec 9, 2020 • edited Loading

Choose a reason for hiding this comment

ValarDragon Dec 9, 2020

Choose a reason for hiding this comment

ValarDragon commented Dec 9, 2020 •

edited

Loading

ValarDragon Dec 9, 2020 •

edited

Loading