Separated Xor table types.

[Alon-Ti]

No description provided.

[reviewable-StarkWare]

This change is 

[Alon-Ti]

• Remove redundant collect in framework. #896
• Separated Xor table types. #883 👈 (View in Graphite)
• Initialize logups automatically when first used. #879 : 1 other dependent PR (#887 )
• Abstracted eval.write_fracs to eval.add_to_relations. #876
• Added LogupAtRow functionality to EvalAtRow. #875
• dev

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

[ohad-starkware]

Reviewed all commit messages.
Reviewable status: 0 of 19 files reviewed, 2 unresolved discussions (waiting on @Alon-Ti and @shaharsamocha7)

---

crates/prover/src/examples/state_machine/components.rs line 51 at r1 (raw file):

        eval.add_to_relation(&[
            RelationEntry::new(&self.lookup_elements, E::EF::one(), &input_state),

unrelated to this pr, whiy is multiplicity EF and not F? wouldn't that conflict with future optimizations to these muls?

---

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

    fn add_to_relation<Relation: RelationType<Self::F, Self::EF>>(
        &mut self,
        _entries: &[RelationEntry<'_, Self::F, Self::EF, Relation>],

---

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

}

pub trait EvalAtRowWithLogup {

are you planning to kill EvalAtRow?
does it make sense to have both and still have add_to_relation for the regular Eval?
why not a super trait? (genuinely asking, don't know if it would work,)

---

crates/prover/src/constraint_framework/logup.rs line 92 at r1 (raw file):

/// Ensures that the LogupAtRow is finalized.
/// LogupAtRow should be finalized exactly once.
impl<F: Clone, EF: RelationEFTraitBound<F>> Drop for LogupAtRow<F, EF> {

non blocking

Suggestion:

 RelationEF

---

crates/prover/src/examples/blake/xor_table/gen.rs line 208 at r1 (raw file):

}

xor_table_gen!(xor12, XorElements12, 12, 4);

maybe these should be coupled with the eval macro?
non blocking

[shaharsamocha7]

Reviewed 4 of 19 files at r1, all commit messages.
Reviewable status: 4 of 19 files reviewed, 3 unresolved discussions (waiting on @Alon-Ti and @ohad-starkware)

---

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

}

pub trait EvalAtRowWithLogup {

This trait shouldn't be implemented directly from outside of this module right?
It's only for the use of the framework itself. Can you document?

Maybe we should have a utils file in this folder so mod will only contain API traits.

Code quote:

pub trait EvalAtRowWithLogup {

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.95%. Comparing base (77de4d7) to head (5a1d139).
Report is 1 commits behind head on dev.

Additional details and impacted files

@@           Coverage Diff           @@
##              dev     #883   +/-   ##
=======================================
  Coverage   91.95%   91.95%           
=======================================
  Files          93       93           
  Lines       13097    13097           
  Branches    13097    13097           
=======================================
  Hits        12043    12043           
  Misses        939      939           
  Partials      115      115           

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

---

🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests

[github-actions]

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite	Current: 174082a	Previous: f6214d1	Ratio
merkle throughput/simd merkle	31468024 ns/iter (± 424557)	14690867 ns/iter (± 434150)	2.14

This comment was automatically generated by workflow using github-action-benchmark.

CC: @shaharsamocha7

[ohad-starkware]

Reviewable status: 4 of 19 files reviewed, 4 unresolved discussions (waiting on @Alon-Ti and @shaharsamocha7)

---

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

            )
            .collect();
        self.write_logup_frac(fracs.into_iter().sum());

redundant collect

Suggestion:

        let fracs = entries.iter().map(
            |RelationEntry {
                 relation,
                 multiplicity,
                 values,
             }| { Fraction::new(multiplicity.clone(), relation.combine(values)) },
        );
        self.write_logup_frac(fracs.sum());

[ohad-starkware]

xor changes looking good

Reviewable status: 4 of 19 files reviewed, 4 unresolved discussions (waiting on @Alon-Ti and @shaharsamocha7)

[Alon-Ti]

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

---

crates/prover/src/examples/blake/xor_table/gen.rs line 208 at r1 (raw file):

Previously, ohad-starkware (Ohad) wrote…

maybe these should be coupled with the eval macro?
non blocking

Done.

---

crates/prover/src/constraint_framework/logup.rs line 92 at r1 (raw file):

Previously, ohad-starkware (Ohad) wrote…

non blocking

Reverted.

---

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

Previously, ohad-starkware (Ohad) wrote…

are you planning to kill EvalAtRow?
does it make sense to have both and still have add_to_relation for the regular Eval?
why not a super trait? (genuinely asking, don't know if it would work,)

Reverted.

---

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

Previously, shaharsamocha7 wrote…

This trait shouldn't be implemented directly from outside of this module right?
It's only for the use of the framework itself. Can you document?

Maybe we should have a utils file in this folder so mod will only contain API traits.

Reverted.

---

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

Previously, ohad-starkware (Ohad) wrote…

redundant collect

Reverted.

---

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

    fn add_to_relation<Relation: RelationType<Self::F, Self::EF>>(
        &mut self,
        _entries: &[RelationEntry<'_, Self::F, Self::EF, Relation>],

Reverted.

[ohad-starkware]

Reviewable status: 0 of 20 files reviewed, 1 unresolved discussion (waiting on @shaharsamocha7)

[shaharsamocha7]

Reviewed 1 of 1 files at r2, 2 of 6 files at r3, 15 of 17 files at r4, 2 of 2 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Alon-Ti)

[Alon-Ti]

Merge activity

• Nov 22, 11:20 PM EST: A user merged this pull request with Graphite.