Framework component

[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 99.01961% with 1 line in your changes missing coverage. Please review.

Project coverage is 93.20%. Comparing base (b06f87e) to head (a34eb4a).

Files	Patch %	Lines
...rates/prover/src/constraint_framework/component.rs	99.01%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #761      +/-   ##
==========================================
- Coverage   93.25%   93.20%   -0.06%     
==========================================
  Files          89       90       +1     
  Lines       11795    11897     +102     
  Branches    11795    11897     +102     
==========================================
+ Hits        11000    11089      +89     
- Misses        694      707      +13     
  Partials      101      101              

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

[shaharsamocha7]

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

---

.gitignore line 7 at r2 (raw file):

build
dist
target*

It's a folder, right?

Suggestion:

target/*

---

crates/prover/src/constraint_framework/component.rs line 20 at r2 (raw file):

use crate::core::{utils, InteractionElements, LookupValues};

pub trait FrameworkComponent {

Document
This framework supports components that allow only constraints inside the row right? (or lookup)
Also, it assumes that all the rows (for specific interaction) are of the same length, no?

Code quote:

pub trait FrameworkComponent {

---

crates/prover/src/constraint_framework/component.rs line 50 at r2 (raw file):

    ) -> crate::core::pcs::TreeVec<
        crate::core::ColumnVec<
            Vec<crate::core::circle::CirclePoint<crate::core::fields::qm31::SecureField>>,

In all of them

Suggestion:

SecureField

---

crates/prover/src/constraint_framework/component.rs line 65 at r2 (raw file):

                })
                .collect()
        })

You can use this:

stwo/crates/prover/src/core/air/mask.rs

Line 36 in a5e69d9

pub fn shifted_mask_points(

Code quote:

        info.mask_offsets.map(|tree_mask| {
            tree_mask
                .iter()
                .map(|col_mask| {
                    col_mask
                        .iter()
                        .map(|off| point + trace_step.mul_signed(*off).into_ef())
                        .collect()
                })
                .collect()
        })

---

crates/prover/src/constraint_framework/component.rs line 100 at r2 (raw file):

        let trace: TreeVec<
            Vec<Cow<'_, CircleEvaluation<SimdBackend, BaseField, BitReversedOrder>>>,
        > = if eval_domain.log_size() != self.log_size() + LOG_BLOWUP_FACTOR {

I think you want it only in case that the eval_domain is bigger, no?

Suggestion:

if eval_domain.log_size() > self.log_size() + LOG_BLOWUP_FACTOR

---

crates/prover/src/constraint_framework/component.rs line 151 at r2 (raw file):

    fn lookup_values(&self, _trace: &ComponentTrace<'_, SimdBackend>) -> LookupValues {
        LookupValues::default()
    }

What's that, is it unimplemented?

Code quote:

    fn lookup_values(&self, _trace: &ComponentTrace<'_, SimdBackend>) -> LookupValues {
        LookupValues::default()
    }

[spapinistarkware]

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

---

.gitignore line 7 at r2 (raw file):

Previously, shaharsamocha7 wrote…

It's a folder, right?

No, it's another target folder. I'm using targetavx for my avx compilations, so it won't interfere with the IDE.

---

crates/prover/src/constraint_framework/component.rs line 20 at r2 (raw file):

Previously, shaharsamocha7 wrote…

Document
This framework supports components that allow only constraints inside the row right? (or lookup)
Also, it assumes that all the rows (for specific interaction) are of the same length, no?

Done.

---

crates/prover/src/constraint_framework/component.rs line 50 at r2 (raw file):

Previously, shaharsamocha7 wrote…

In all of them

Done.

---

crates/prover/src/constraint_framework/component.rs line 65 at r2 (raw file):

Previously, shaharsamocha7 wrote…

You can use this:

stwo/crates/prover/src/core/air/mask.rs

Line 36 in a5e69d9

pub fn shifted_mask_points(

I'd have to createa a vector of domains though...

---

crates/prover/src/constraint_framework/component.rs line 100 at r2 (raw file):

Previously, shaharsamocha7 wrote…

I think you want it only in case that the eval_domain is bigger, no?

Theoretically, yes. But we need to do modifications to reuse the commitment evaluation in that case, which I don't want to write now. The way I wrote now, if , it's correct in that case, although, not as efficient is it could be.

---

crates/prover/src/constraint_framework/component.rs line 151 at r2 (raw file):

Previously, shaharsamocha7 wrote…

What's that, is it unimplemented?

Don't need lookup_values in this interface.

[shaharsamocha7]

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

---

crates/prover/src/constraint_framework/component.rs line 65 at r2 (raw file):

Previously, spapinistarkware (Shahar Papini) wrote…

I'd have to createa a vector of domains though...

should we change that function to get trace_step instead of domains?
Not necessarily for this pr

---

crates/prover/src/constraint_framework/component.rs line 100 at r2 (raw file):

Previously, spapinistarkware (Shahar Papini) wrote…

Theoretically, yes. But we need to do modifications to reuse the commitment evaluation in that case, which I don't want to write now. The way I wrote now, if , it's correct in that case, although, not as efficient is it could be.

Can you add a TODO to consider to optimize that?

---

crates/prover/src/constraint_framework/component.rs line 25 at r3 (raw file):

/// Implementing this trait introduces implementations for [Component] and [ComponentProver] for the
/// SIMD backend.
/// Note that the constraint framework only support components with columns of the size size.

typo

Suggestion:

/// Note that the constraint framework only support components with columns of the same size.

[spapinistarkware]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @shaharsamocha7)

---

crates/prover/src/constraint_framework/component.rs line 65 at r2 (raw file):

Previously, shaharsamocha7 wrote…

should we change that function to get trace_step instead of domains?
Not necessarily for this pr

Perhaps. It really depends on the concrete use cases of that function. For example, if we'd stop having any uses of it, we could delete it.

---

crates/prover/src/constraint_framework/component.rs line 100 at r2 (raw file):

Previously, shaharsamocha7 wrote…

Can you add a TODO to consider to optimize that?

Done.

---

crates/prover/src/constraint_framework/component.rs line 25 at r3 (raw file):

Previously, shaharsamocha7 wrote…

typo

Done.

[shaharsamocha7]

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

[spapinistarkware]

Merge activity

• Jul 30, 9:05 AM EDT: @spapinistarkware started a stack merge that includes this pull request via Graphite.
• Jul 30, 9:07 AM EDT: Graphite rebased this pull request as part of a merge.
• Jul 30, 9:11 AM EDT: @spapinistarkware merged this pull request with Graphite.