Eval framework point and domain

[spapinistarkware]

This change is 

[spapinistarkware]

• Blake statement #781
• Blake air #773
• Blake scheduler #770
• Blake round #767
• Optimized lookup combine #765
• Blake xor table #764
• Framework component #761
• Poseidon with logup #712
• Logup trace generation #711
• is_first constant column #710
• Poseidon plain proof #708
• ProverCommitmentScheme::commit_on_evals() #707
• Eval framework info #715
• Eval framework asserts #714
• Eval framework point and domain #713 👈
• Eval framework #706
• Eval framework preperation #705
• Generalize AirExt::mask_points() #704
• Wrap mask with TreeVec #702
• Generalize component_trace() #701
• 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

Attention: Patch coverage is 78.88889% with 19 lines in your changes missing coverage. Please review.

Project coverage is 90.42%. Comparing base (5c9fb36) to head (fe24074).

Files	Patch %	Lines
...tes/prover/src/constraint_framework/simd_domain.rs	71.42%	16 Missing ⚠️
crates/prover/src/constraint_framework/point.rs	91.17%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #713      +/-   ##
==========================================
- Coverage   90.48%   90.42%   -0.07%     
==========================================
  Files          77       79       +2     
  Lines       10184    10274      +90     
  Branches    10184    10274      +90     
==========================================
+ Hits         9215     9290      +75     
- Misses        886      901      +15     
  Partials       83       83              

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

[shaharsamocha7]

Reviewed 1 of 5 files at r1, all commit messages.
Reviewable status: 1 of 5 files reviewed, 4 unresolved discussions (waiting on @spapinistarkware)

---

crates/prover/src/constraint_framework/domain.rs line 1 at r1 (raw file):

use std::ops::Mul;

Consider to rename to simd_domain.rs

---

crates/prover/src/constraint_framework/domain.rs line 17 at r1 (raw file):

use crate::core::utils::offset_bit_reversed_circle_domain_index;

/// Evaluates expressions at an evaluation domain rows.

shouldn't we use here constraint notation?

Suggestion:

/// Evaluates constraints at an evaluation domain points.

---

crates/prover/src/constraint_framework/domain.rs line 21 at r1 (raw file):

    pub trace_eval:
        &'a TreeVec<Vec<&'a CircleEvaluation<SimdBackend, BaseField, BitReversedOrder>>>,
    pub col_index: Vec<usize>,

rename as discussed

Code quote:

    pub col_index: Vec<usize>,

---

crates/prover/src/constraint_framework/domain.rs line 66 at r1 (raw file):

                return self.trace_eval[interaction][col_index].data[self.vec_row];
            }
            PackedBaseField::from_array(std::array::from_fn(|i| {

Add a comment to explain this

Code quote:

            // TODO(spapini): Optimize.
            if off == 0 {
                return self.trace_eval[interaction][col_index].data[self.vec_row];
            }
            PackedBaseField::from_array(std::array::from_fn(|i| {

[spapinistarkware]

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

---

crates/prover/src/constraint_framework/domain.rs line 1 at r1 (raw file):

Previously, shaharsamocha7 wrote…

Consider to rename to simd_domain.rs

Done.

---

crates/prover/src/constraint_framework/domain.rs line 17 at r1 (raw file):

Previously, shaharsamocha7 wrote…

shouldn't we use here constraint notation?

Done.

---

crates/prover/src/constraint_framework/domain.rs line 21 at r1 (raw file):

Previously, shaharsamocha7 wrote…

rename as discussed

Done.

---

crates/prover/src/constraint_framework/domain.rs line 66 at r1 (raw file):

Previously, shaharsamocha7 wrote…

Add a comment to explain this

Done.

[shaharsamocha7]

Reviewed 2 of 5 files at r1, 1 of 3 files at r2.
Reviewable status: 2 of 5 files reviewed, 7 unresolved discussions (waiting on @spapinistarkware)

---

crates/prover/src/constraint_framework/mod.rs line 63 at r1 (raw file):

    /// Combines 4 base field values into a single extension field value.
    fn combine_ef(values: [Self::F; 4]) -> Self::EF;

SECURE_EXTENSION_DEGREE

In all places

Code quote:

4

---

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

#[derive(Clone)]
pub struct PoseidonComponent {
    pub log_n_rows: u32,

Isn't it better to store log_n_instances?

Code quote:

pub log_n_rows: u32,

---

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

        let denom = coset_vanishing(constraint_zero_domain, point);
        let denom_inverse = denom.inverse();
        let mut eval = PoseidonEval {

Rename

Suggestion:

let mut poseidon_eval = PoseidonEval {

---

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

            .as_cols_ref()
            .map_cols(|col| col.evaluate_with_twiddles(eval_domain, &twiddles));
        let trace_eval_ref = trace_eval.as_ref().map(|t| t.iter().collect_vec());

Isn't this effectively a clone?

Code quote:

let trace_eval_ref = trace_eval.as_ref().map(|t| t.iter().collect_vec());

---

crates/prover/src/constraint_framework/simd_domain.rs line 22 at r2 (raw file):

        &'a TreeVec<Vec<&'a CircleEvaluation<SimdBackend, BaseField, BitReversedOrder>>>,
    pub column_index_per_interaction: Vec<usize>,
    pub vec_row: usize,

what's vec_row stands for?

Suggestion:

pub row_index: usize,

---

crates/prover/src/constraint_framework/simd_domain.rs line 27 at r2 (raw file):

    pub constraint_index: usize,
    pub domain_log_size: u32,
    pub eval_log_size: u32,

Rename

Suggestion:

pub eval_domain_log_size: u32,

---

crates/prover/src/constraint_framework/simd_domain.rs line 70 at r2 (raw file):

            // at the bit-reversed natural order index at an offset.
            PackedBaseField::from_array(std::array::from_fn(|i| {
                let index = offset_bit_reversed_circle_domain_index(

Rename

Suggestion:

let row_index = offset_bit_reversed_circle_domain_index(

[spapinistarkware]

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

---

crates/prover/src/constraint_framework/mod.rs line 63 at r1 (raw file):

Previously, shaharsamocha7 wrote…

SECURE_EXTENSION_DEGREE

In all places

Done.

---

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

Previously, shaharsamocha7 wrote…

Isn't it better to store log_n_instances?

log_n_rows is a lot more useful. And log_n_instances is not always integer (e.g. if we have 3 repetitions).

---

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

Previously, shaharsamocha7 wrote…

Rename

Done.

---

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

Previously, shaharsamocha7 wrote…

Isn't this effectively a clone?

No. It's just references.

---

crates/prover/src/constraint_framework/simd_domain.rs line 22 at r2 (raw file):

Previously, shaharsamocha7 wrote…

what's vec_row stands for?

It's not the row, it's the row of the vector-of rows table. Since this is SIMD. We currently use vec_row everywhere in the project. We can change them all together in a PR if you like.

---

crates/prover/src/constraint_framework/simd_domain.rs line 27 at r2 (raw file):

Previously, shaharsamocha7 wrote…

Rename

Done.

---

crates/prover/src/constraint_framework/simd_domain.rs line 70 at r2 (raw file):

Previously, shaharsamocha7 wrote…

Rename

Done.

[shaharsamocha7]

Reviewed 5 of 5 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @spapinistarkware)

---

crates/prover/src/constraint_framework/mod.rs line 2 at r3 (raw file):

mod point;
/// ! This module contains helpers to express and use constraints for components.

shouldn't it be the first line?

Code quote:

/// ! This module contains helpers to express and use constraints for components.

---

crates/prover/src/constraint_framework/simd_domain.rs line 22 at r2 (raw file):

Previously, spapinistarkware (Shahar Papini) wrote…

It's not the row, it's the row of the vector-of rows table. Since this is SIMD. We currently use vec_row everywhere in the project. We can change them all together in a PR if you like.

Ok,
Let's consider changing it to packed_row (in a different pr as you said)

[shaharsamocha7]

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

[spapinistarkware]

Merge activity

• Jul 14, 8:05 AM EDT: @spapinistarkware merged this pull request with Graphite.