FRI quotients y optimization

[spapinistarkware]

This change is 

[spapinistarkware]

• FRI quotients y optimization #686 👈
• Extension FRI optimization #685
• Revert "removed computing line coeffs (#643)" #684
• dev

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

Join @spapinistarkware and the rest of your teammates on Graphite

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.19%. Comparing base (3b0a885) to head (bed00f8).

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #686      +/-   ##
==========================================
+ Coverage   90.02%   90.19%   +0.17%     
==========================================
  Files          75       75              
  Lines        9834     9862      +28     
  Branches     9834     9862      +28     
==========================================
+ Hits         8853     8895      +42     
+ Misses        900      886      -14     
  Partials       81       81              

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

[ohad-starkware]

Reviewed all commit messages.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @shaharsamocha7 and @spapinistarkware)

---

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

            let column = &columns[*column_index];
            let value = PackedSecureField::broadcast(*c) * column.data[vec_row];
            // The numerator is a line equation passing through

move the doc to the cpu code ?

---

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

                PackedSecureField::broadcast(*c) * column.data[(quad_row << 2) + i]
            });
            // y values are y,-y,-y,y.

Drop line,
also - this needs to be explained somewhere.
maybe we can implement in cpu also and doc there, this is less confusing without the interleaves.

[ohad-starkware]

Reviewed 3 of 3 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @shaharsamocha7)

[shaharsamocha7]

Reviewed all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @spapinistarkware)

---

crates/prover/src/core/backend/simd/quotients.rs line 103 at r3 (raw file):

/// Accumulates the quotients for 4 * N_LANES rows at a time.
/// spaced_ys - y values for N_LANES points in the domain, in jumps of 4.

Can you update that in the reverse order, each 4 consecutive rows are the points (P, P+H, -P, -P-H)?

Suggestion:

/// Accumulates the quotients for 4 * N_LANES rows at a time.
/// spaced_ys - y values for N_LANES points in the domain, in jumps of 4.

---

crates/prover/src/core/backend/simd/quotients.rs line 127 at r3 (raw file):

            // The numerator is the line equation:
            //   value - a * point.y - b;

?

Suggestion:

            // The numerator is the line equation:
            //   c * value - a * point.y - b;

---

crates/prover/src/examples/poseidon/mod.rs line 533 at r3 (raw file):

        // Get from environment variable:
        let log_n_instances = env::var("LOG_N_INSTANCES")
            .unwrap_or_else(|_| "10".to_string())

Doesn't take too much time? (non-blocking)
is 8 less then minimal for a component?

Code quote:

"10"

[spapinistarkware]

Reviewable status: 2 of 3 files reviewed, 2 unresolved discussions (waiting on @ohad-starkware and @shaharsamocha7)

---

crates/prover/src/core/backend/simd/quotients.rs line 103 at r3 (raw file):

Previously, shaharsamocha7 wrote…

Can you update that in the reverse order, each 4 consecutive rows are the points (P, P+H, -P, -P-H)?

Added in the body.

---

crates/prover/src/examples/poseidon/mod.rs line 533 at r3 (raw file):

Previously, shaharsamocha7 wrote…

Doesn't take too much time? (non-blocking)
is 8 less then minimal for a component?

Doesn't take too much time. 8 is too small.

[shaharsamocha7]

Reviewable status: 2 of 3 files reviewed, 4 unresolved discussions (waiting on @ohad-starkware and @spapinistarkware)

---

crates/prover/src/core/backend/simd/quotients.rs line 137 at r5 (raw file):

            // The numerator is the line equation:
            //   value - a * point.y - b;

Suggestion:

/   c * value - a * point.y - b;

---

crates/prover/src/core/backend/simd/quotients.rs line 155 at r5 (raw file):

                // Note that all summands were multiplied by coefficients (a, b or c).
                // These coefficients already hold the constraint coeff (random_coeff^i).
                // This is why we only add here.

move up

Code quote:

                // Note that all summands were multiplied by coefficients (a, b or c).
                // These coefficients already hold the constraint coeff (random_coeff^i).
                // This is why we only add here.

[spapinistarkware]

Reviewable status: 2 of 3 files reviewed, 4 unresolved discussions (waiting on @ohad-starkware and @shaharsamocha7)

---

crates/prover/src/core/backend/simd/quotients.rs line 137 at r5 (raw file):

            // The numerator is the line equation:
            //   value - a * point.y - b;

Done.

---

crates/prover/src/core/backend/simd/quotients.rs line 155 at r5 (raw file):

Previously, shaharsamocha7 wrote…

move up

Done.

[shaharsamocha7]

Reviewed 1 of 3 files at r3.
Reviewable status: 2 of 4 files reviewed, 2 unresolved discussions (waiting on @ohad-starkware and @spapinistarkware)

---

crates/prover/src/core/backend/simd/quotients.rs line 132 at r6 (raw file):

        {
            let column = &columns[*column_index];
            let values: [_; 4] = std::array::from_fn(|i| {

?

Suggestion:

let cvalues: [_; 4] = std::array::from_fn(|i| {

---

crates/prover/src/core/backend/simd/quotients.rs line 144 at r6 (raw file):

            //   G, -G, G + H, -G + H.
            // H being the half point (-1,0). The y values for these are
            //   y, -y, -y, y.

I would change that to P instead of G.
Not sure if it is better.

Suggestion:

//   P.y, -P.y, -P.y, P.y.

[shaharsamocha7]

Reviewable status: 2 of 4 files reviewed, 1 unresolved discussion (waiting on @ohad-starkware and @spapinistarkware)

---

crates/prover/src/core/backend/simd/quotients.rs line 41 at r7 (raw file):

        let span = span!(Level::INFO, "Quotient accumulation").entered();
        // TODO(spapini): bit reverse iterator.
        for quad_row in 0..1 << (subdomain.log_size() - LOG_N_LANES - 2) {

Add the comment

Suggestion:

        // Accumulate the quotients on subdomain.
        for quad_row in 0..1 << (subdomain.log_size() - LOG_N_LANES - 2) {

[shaharsamocha7]

Reviewed 1 of 2 files at r8, 1 of 1 files at r9, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @spapinistarkware)