Add safe simplify for expressions that compares random assignments before and after.

[Alon-Ti]

before and after.

[reviewable-StarkWare]

This change is 

[Alon-Ti]

• Added add_intermediate and add_secure_intermediate to eval API. #919
• Add safe simplify for expressions that compares random assignments before and after. #918 👈 (View in Graphite)
• Added test for expression simplifier. #908
• Added eval on exprs. #904
• Separated Base and Extension field expressions. #903
• Added From<G> in add_constraint. #901
• Simplify expressions. #894
• Exported shared code to temp variables in ExprEvaluator. #893
• Added total_ and claimed_sums as formal variables in ExprEvaluator. #892
• State machine constraint string. #889
• Expr formatting. #888
• Formal Expr logup variables. #887
• Initialize logups automatically when first used. #879 : 1 other dependent PR (#883 )
• 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.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 98.97959% with 1 line in your changes missing coverage. Please review.

Project coverage is 91.48%. Comparing base (df4a642) to head (f475a85).
Report is 1 commits behind head on dev.

Files with missing lines	Patch %	Lines
crates/prover/src/constraint_framework/expr.rs	98.97%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #918      +/-   ##
==========================================
+ Coverage   91.42%   91.48%   +0.05%     
==========================================
  Files          94       94              
  Lines       13581    13671      +90     
  Branches    13581    13671      +90     
==========================================
+ Hits        12417    12507      +90     
  Misses       1049     1049              
  Partials      115      115              

☔ 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: f475a85	Previous: cd8b37b	Ratio
iffts/simd ifft/28	1289757786 ns/iter (± 27177876)	643771030 ns/iter (± 19147376)	2.00
merkle throughput/simd merkle	29234038 ns/iter (± 430219)	13712527 ns/iter (± 579195)	2.13

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

CC: @shaharsamocha7

[shaharsamocha7]

Reviewable status: 0 of 1 files reviewed, 6 unresolved discussions (waiting on @Alon-Ti)

---

crates/prover/src/constraint_framework/expr.rs line 196 at r2 (raw file):

    }

    pub fn simplify(&self) -> Self {

Document
And put note that

Code quote:

 pub fn simplify(&self) -> Self {

---

crates/prover/src/constraint_framework/expr.rs line 391 at r2 (raw file):

    HashMap<String, BaseField>,
    HashMap<String, SecureField>,
);

move above ExprVariables

Code quote:

pub type ExprVarAssignment = (
    HashMap<(usize, usize, isize), BaseField>,
    HashMap<String, BaseField>,
    HashMap<String, SecureField>,
);

---

crates/prover/src/constraint_framework/expr.rs line 418 at r2 (raw file):

    }

    pub fn randomize(&self) -> ExprVarAssignment {

Document
and maybe rename to random_assignment? WDYT?

Code quote:

pub fn randomize(&self) -> ExprVarAssignment {

---

crates/prover/src/constraint_framework/expr.rs line 181 at r1 (raw file):

    }

    pub fn simplify(&self) -> Self {

rename to unsafe
and document that this shouldn't be used and refer to the simplify function

Code quote:

pub fn simplify(&self) -> Self {

---

crates/prover/src/constraint_framework/expr.rs line 248 at r1 (raw file):

    pub fn random_eval(&self) -> BaseField {
        let assignment = self.collect_variables().randomize();

Assert that assignment.2 is empty?

Code quote:

let assignment = self.collect_variables().randomize();

---

crates/prover/src/constraint_framework/expr.rs line 381 at r1 (raw file):

#[derive(Default)]
pub struct ExprVariables {

Document

Code quote:

pub struct ExprVariables {

[Alon-Ti]

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

---

crates/prover/src/constraint_framework/expr.rs line 181 at r1 (raw file):

Previously, shaharsamocha7 wrote…

rename to unsafe
and document that this shouldn't be used and refer to the simplify function

Done.

---

crates/prover/src/constraint_framework/expr.rs line 248 at r1 (raw file):

Previously, shaharsamocha7 wrote…

Assert that assignment.2 is empty?

Done.

---

crates/prover/src/constraint_framework/expr.rs line 381 at r1 (raw file):

Previously, shaharsamocha7 wrote…

Document

Done.

---

crates/prover/src/constraint_framework/expr.rs line 196 at r2 (raw file):

Previously, shaharsamocha7 wrote…

Document
And put note that

Done.

---

crates/prover/src/constraint_framework/expr.rs line 391 at r2 (raw file):

Previously, shaharsamocha7 wrote…

move above ExprVariables

Done.

---

crates/prover/src/constraint_framework/expr.rs line 418 at r2 (raw file):

Previously, shaharsamocha7 wrote…

Document
and maybe rename to random_assignment? WDYT?

Done.

[shaharsamocha7]

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

---

crates/prover/src/constraint_framework/expr.rs line 418 at r2 (raw file):

Previously, Alon-Ti wrote…

Done.

Also write that this function uses only for tests.
Could we define it with cfg(tests) (not sure If I already suggested this)

---

crates/prover/src/constraint_framework/expr.rs line 1043 at r3 (raw file):

        let full_eval = expr.eval_expr::<AssertEvaluator<'_>, _, _, _>(&columns, &vars, &ext_vars);
        let simplified_eval = expr
            .unchecked_simplify()

why not just call simplify here?

Code quote:

unchecked_simplify

[shaharsamocha7]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Alon-Ti)

[Alon-Ti]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @shaharsamocha7)

---

crates/prover/src/constraint_framework/expr.rs line 418 at r2 (raw file):

Previously, shaharsamocha7 wrote…

Also write that this function uses only for tests.
Could we define it with cfg(tests) (not sure If I already suggested this)

It's not just for tests though, it should run every time an expression is simplified, including (especially) real use cases.

[shaharsamocha7]

Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @Alon-Ti)

[Alon-Ti]

Merge activity

• Dec 3, 4:02 AM EST: A user merged this pull request with Graphite.