Added LogupAtRow functionality to EvalAtRow.

[Alon-Ti]

No description provided.

[reviewable-StarkWare]

This change is 

[Alon-Ti]

• Initialize logups automatically when first used. #879 : 2 dependent PRs (#883 , #887 )
• Abstracted eval.write_fracs to eval.add_to_relations. #876
• Added LogupAtRow functionality to EvalAtRow. #875 👈 (View in Graphite)
• dev

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

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 81.03448% with 11 lines in your changes missing coverage. Please review.

Project coverage is 91.66%. Comparing base (64270dc) to head (cf4bb59).
Report is 2 commits behind head on dev.

Files with missing lines	Patch %	Lines
crates/prover/src/constraint_framework/mod.rs	71.79%	11 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #875      +/-   ##
==========================================
- Coverage   91.68%   91.66%   -0.03%     
==========================================
  Files          93       93              
  Lines       12800    12857      +57     
  Branches    12800    12857      +57     
==========================================
+ Hits        11736    11785      +49     
- Misses        951      959       +8     
  Partials      113      113              

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

[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: cf4bb59	Previous: f6214d1	Ratio
merkle throughput/simd merkle	32012073 ns/iter (± 547997)	14690867 ns/iter (± 434150)	2.18

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

CC: @shaharsamocha7

[shaharsamocha7]

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

---

crates/prover/src/examples/blake/round/mod.rs line 34 at r2 (raw file):

    }
    fn evaluate<E: EvalAtRow>(&self, mut eval: E) -> E {
        let is_first = eval.get_preprocessed_column(PreprocessedColumn::IsFirst(self.log_size()));

why did you remove that? the logup define the use in it?

Code quote:

let is_first = eval.get_preprocessed_column(PreprocessedColumn::IsFirst(self.log_size()));

---

crates/prover/src/constraint_framework/mod.rs line 169 at r2 (raw file):

        }

        fn write_frac(&mut self, fraction: Fraction<Self::EF, Self::EF>) {

maybe this should be renamed to write_logup? write_logup_frac?
WDYT?

Code quote:

fn write_frac(&mut self, fraction: Fraction<Self::EF, Self::EF>) {

---

crates/prover/src/constraint_framework/mod.rs line 202 at r2 (raw file):

    fn get_alpha_powers(&self) -> &[EF];
    fn name(&self) -> &str;
}

should this code be here?

Code quote:

pub trait RelationType<F, EF>
where
    F: Clone,
    EF: Clone + Zero + From<F> + From<SecureField> + Mul<F, Output = EF> + Sub<EF, Output = EF>,
{
    fn combine(&self, values: &[F]) -> EF {
        values
            .iter()
            .zip(self.get_alpha_powers())
            .fold(EF::zero(), |acc, (value, power)| {
                acc + power.clone() * value.clone()
            })
            - self.get_z()
    }

    fn get_z(&self) -> EF;
    fn get_alpha_powers(&self) -> &[EF];
    fn name(&self) -> &str;
}

---

crates/prover/src/examples/xor/gkr_lookups/mle_eval.rs line 928 at r2 (raw file):

                is_second_eval,
            )
        });

?

Code quote:

        });

---

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

    pub fn dummy() -> Self {
        Self {
            interaction: 0,

I think it is better not to use real values here

Suggestion:

interaction: -1,

---

crates/prover/src/constraint_framework/interaction_gen.rs line 7 at r2 (raw file):

use crate::core::poly::BitReversedOrder;

pub struct InteractionTraceGenerator<'a> {}

Do we need this?

Code quote:

use crate::core::backend::simd::SimdBackend;
use crate::core::fields::m31::BaseField;
use crate::core::pcs::TreeVec;
use crate::core::poly::circle::CircleEvaluation;
use crate::core::poly::BitReversedOrder;

pub struct InteractionTraceGenerator<'a> {}

[Alon-Ti]

Reviewable status: 8 of 19 files reviewed, 6 unresolved discussions (waiting on @ohad-starkware and @shaharsamocha7)

---

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

Previously, shaharsamocha7 wrote…

I think it is better not to use real values here

Done.

---

crates/prover/src/constraint_framework/mod.rs line 169 at r2 (raw file):

Previously, shaharsamocha7 wrote…

maybe this should be renamed to write_logup? write_logup_frac?
WDYT?

I'll rename them in the PR above where I decouple this from LogupAtRow functionality.

---

crates/prover/src/constraint_framework/mod.rs line 202 at r2 (raw file):

Previously, shaharsamocha7 wrote…

should this code be here?

It's not used in this PR, removed.

---

crates/prover/src/examples/blake/round/mod.rs line 34 at r2 (raw file):

Previously, shaharsamocha7 wrote…

why did you remove that? the logup define the use in it?

Yes, once the logup is in the eval, this becomes part of the init (especially now that it's a const column). See constraint_framework/mod.rs:157.

---

crates/prover/src/constraint_framework/interaction_gen.rs line 7 at r2 (raw file):

Previously, shaharsamocha7 wrote…

Do we need this?

Sorry, spoilers :)

---

crates/prover/src/examples/xor/gkr_lookups/mle_eval.rs line 928 at r2 (raw file):

Previously, shaharsamocha7 wrote…

?

Done.

[shaharsamocha7]

Reviewed 11 of 11 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ohad-starkware)