Removed batching from LogupAtRow.

[Alon-Ti]

This change is 

[Alon-Ti]

• Removed batching from LogupAtRow. #817 👈
• dev

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

Join @Alon-Ti and the rest of your teammates on Graphite

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.58%. Comparing base (215b1cd) to head (c9d43aa).

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #817      +/-   ##
==========================================
- Coverage   92.60%   92.58%   -0.03%     
==========================================
  Files          89       89              
  Lines       12055    12017      -38     
  Branches    12055    12017      -38     
==========================================
- Hits        11164    11126      -38     
  Misses        784      784              
  Partials      107      107              

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

[shaharsamocha7]

Reviewed 2 of 6 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status: 5 of 6 files reviewed, 3 unresolved discussions (waiting on @Alon-Ti)

---

crates/prover/src/constraint_framework/logup.rs line 34 at r2 (raw file):

    /// The evaluation of the last cumulative sum column.
    pub prev_col_cumsum: E::EF,
    cur_frac: Option<Fraction<E::EF, E::EF>>,

Why not initialize with (zero(), one())?
and remove the Option

Code quote:

cur_frac: Option<Fraction<E::EF, E::EF>>,

---

crates/prover/src/examples/blake/scheduler/constraints.rs line 45 at r2 (raw file):

    // TODO(spapini): Support multiplicities.
    // TODO(spapini): Change to -1.

Can we fix those todos or remove the lookup if not needed?

Code quote:

    // TODO(spapini): Support multiplicities.
    // TODO(spapini): Change to -1.

---

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

        // Provide state lookups.
        let final_state_denom: E::EF = lookup_elements.combine(&state);
        // (1 / denom1) - (1 / denom1) = (denom1 - denom0) / (denom0 * denom1).

(1 / denom1) - (1 / denom1) = 0

Suggestion:

// (1 / denom0) - (1 / denom1) = (denom1 - denom0) / (denom0 * denom1).

[Alon-Ti]

Reviewable status: 5 of 7 files reviewed, 3 unresolved discussions (waiting on @shaharsamocha7)

---

crates/prover/src/constraint_framework/logup.rs line 34 at r2 (raw file):

Previously, shaharsamocha7 wrote…

Why not initialize with (zero(), one())?
and remove the Option

Because of the finalize logic I only add a constraint on the next call for each frac, I need a way to identify the uninitialized state anyway, checking if there was a previous frac does that.

---

crates/prover/src/examples/blake/scheduler/constraints.rs line 45 at r2 (raw file):

Previously, shaharsamocha7 wrote…

Can we fix those todos or remove the lookup if not needed?

I'm not sure, I just mechanically converted the lookups. Let's go over all spapini TODOs next week to see what should be done now and change ownership.

---

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

Previously, shaharsamocha7 wrote…

(1 / denom1) - (1 / denom1) = 0

Done.

[shaharsamocha7]

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: 5 of 7 files reviewed, 1 unresolved discussion (waiting on @Alon-Ti)

---

crates/prover/src/examples/blake/xor_table/constraints.rs line 30 at r4 (raw file):

        let frac_chunks: [Fraction<E::EF, E::EF>; 1 << (2 * EXPAND_BITS)] =
            std::array::from_fn(|i| {
                let (i, j) = ((i >> EXPAND_BITS) as u32, (i % (1 << EXPAND_BITS)) as u32);

@andrewmilson Do you have another alternative?
This is pretty awful :(

Code quote:

        let frac_chunks: [Fraction<E::EF, E::EF>; 1 << (2 * EXPAND_BITS)] =
            std::array::from_fn(|i| {
                let (i, j) = ((i >> EXPAND_BITS) as u32, (i % (1 << EXPAND_BITS)) as u32);

[shaharsamocha7]

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Alon-Ti)