Fall back to cpu in small fft size.

[alonh5]

This change is 

[alonh5]

• Assert enough aplha powers for combine. #850
• Fall back to CPU in small FRI. #840
• Fall back to CPU in small constraint eval. #838
• Fall back to cpu in small fft size. #827 👈
• dev

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @alonh5 and the rest of your teammates on Graphite

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 36.84211% with 12 lines in your changes missing coverage. Please review.

Project coverage is 91.82%. Comparing base (cdf66f3) to head (77b7cdd).
Report is 2 commits behind head on dev.

Files with missing lines	Patch %	Lines
crates/prover/src/core/backend/simd/circle.rs	46.66%	8 Missing ⚠️
crates/prover/src/core/poly/circle/evaluation.rs	0.00%	3 Missing ⚠️
crates/prover/src/core/backend/simd/quotients.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #827      +/-   ##
==========================================
- Coverage   91.86%   91.82%   -0.04%     
==========================================
  Files          89       89              
  Lines       12080    12102      +22     
  Branches    12080    12102      +22     
==========================================
+ Hits        11097    11113      +16     
- Misses        876      882       +6     
  Partials      107      107              

Flag	Coverage Δ
	91.82% <36.84%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[andrewmilson]

Can all relevant tests be added please

Reviewable status: 0 of 4 files reviewed, 3 unresolved discussions (waiting on @alonh5 and @shaharsamocha7)

---

crates/prover/src/core/backend/simd/circle.rs line 148 at r1 (raw file):

        let log_size = eval.values.length.ilog2();
        if log_size < MIN_FFT_LOG_SIZE {
            let cpu_poly = CpuBackend::interpolate(eval.to_cpu(), &twiddles.to_cpu());

twiddles.to_cpu() could be very expensive.

Suggestion:

let cpu_poly = eval.to_cpu().interpolate();

---

crates/prover/src/core/backend/simd/circle.rs line 241 at r1 (raw file):

                &CirclePoly::new(poly.coeffs.to_cpu()),
                domain,
                &twiddles.to_cpu(),

Same here.

---

crates/prover/src/core/backend/simd/circle.rs line 244 at r1 (raw file):

            );
            return CircleEvaluation::new(
                cpu_eval.domain,

Won't this be expensive if cpu_domain is large?

[alonh5]

Reviewable status: 0 of 4 files reviewed, 3 unresolved discussions (waiting on @andrewmilson and @shaharsamocha7)

---

crates/prover/src/core/backend/simd/circle.rs line 148 at r1 (raw file):

Previously, andrewmilson (Andrew Milson) wrote…

twiddles.to_cpu() could be very expensive.

Done.

---

crates/prover/src/core/backend/simd/circle.rs line 241 at r1 (raw file):

Previously, andrewmilson (Andrew Milson) wrote…

Same here.

Done.

---

crates/prover/src/core/backend/simd/circle.rs line 244 at r1 (raw file):

Previously, andrewmilson (Andrew Milson) wrote…

Won't this be expensive if cpu_domain is large?

The worst it will be is 4 + log_blowup_factor. So it shouldn't be that bad, right?

[andrewmilson]

with comment

Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @alonh5 and @shaharsamocha7)

---

crates/prover/src/core/backend/simd/circle.rs line 229 at r2 (raw file):

    ) -> CircleEvaluation<Self, BaseField, BitReversedOrder> {
        // TODO(spapini): Precompute twiddles.
        // TODO(spapini): Handle small cases.

I think these can be removed now.

Code quote:

        // TODO(spapini): Precompute twiddles.
        // TODO(spapini): Handle small cases.

---

crates/prover/src/core/poly/twiddles.rs line 36 at r2 (raw file):

        }
    }
}

Can this be removed?

Code quote:

impl TwiddleTree<SimdBackend> {
    pub fn to_cpu(&self) -> TwiddleTree<CpuBackend> {
        TwiddleTree {
            root_coset: self.root_coset,
            twiddles: self
                .twiddles
                .iter()
                .map(|x| BaseField::from(x >> 1))
                .collect_vec(),
            itwiddles: self
                .itwiddles
                .iter()
                .map(|x| BaseField::from(x >> 1))
                .collect_vec(),
        }
    }
}

[alonh5]

Fixed comments. Regarding the tests, there are still places where the minimum bound needs to be removed. I'm fixing it and will add tests in the next PR.

Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @andrewmilson and @shaharsamocha7)

---

crates/prover/src/core/backend/simd/circle.rs line 229 at r2 (raw file):

Previously, andrewmilson (Andrew Milson) wrote…

I think these can be removed now.

Done.

---

crates/prover/src/core/poly/twiddles.rs line 36 at r2 (raw file):

Previously, andrewmilson (Andrew Milson) wrote…

Can this be removed?

Done.

[andrewmilson]

Reviewable status: 0 of 4 files reviewed, all discussions resolved (waiting on @shaharsamocha7)

[alonh5]

Merge activity

• Sep 24, 4:47 AM EDT: @alonh5 started a stack merge that includes this pull request via Graphite.
• Sep 24, 4:47 AM EDT: @alonh5 merged this pull request with Graphite.