Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move all MLE eval related constraints into same module #798

Merged

Conversation

andrewmilson
Copy link
Contributor

@andrewmilson andrewmilson commented Aug 21, 2024

This change is Reviewable

@codecov-commenter
Copy link

codecov-commenter commented Aug 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.26%. Comparing base (f976890) to head (41485b5).
Report is 1 commits behind head on dev.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #798   +/-   ##
=======================================
  Coverage   92.26%   92.26%           
=======================================
  Files          91       91           
  Lines       12442    12442           
  Branches    12442    12442           
=======================================
  Hits        11479    11479           
  Misses        857      857           
  Partials      106      106           

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

Copy link
Contributor

@spapinistarkware spapinistarkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


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

use crate::core::lookups::utils::eq;

/// Evaluates constraints that grantee a MLE evaluates to a claim at a given point.

Suggestion:

Evaluates constraints that guarantee an MLE evaluates to a claim at a given point.

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

/// `mle_coeffs_col_eval` should be the evaluation of the column containing the coefficients of the
/// MLE in the multilinear Lagrange basis. `mle_claim_shift` should equal `claim / 2^N_VARIABLES`.
pub fn eval_mle_eval_constrants<E: EvalAtRow, const N_VARIABLES: usize>(

Suggestion:

eval_mle_eval_constraints

Copy link
Contributor

@spapinistarkware spapinistarkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

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

@andrewmilson andrewmilson force-pushed the 08-20-Move_all_MLE_eval_related_constraints_into_same_module branch from e0f1063 to 41485b5 Compare August 22, 2024 14:38
Copy link
Contributor Author

@andrewmilson andrewmilson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 5 of 6 files reviewed, 1 unresolved discussion (waiting on @spapinistarkware)


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

use crate::core::lookups::utils::eq;

/// Evaluates constraints that grantee a MLE evaluates to a claim at a given point.

Done.

@andrewmilson andrewmilson merged commit f16ba08 into dev Aug 22, 2024
15 of 16 checks passed
@andrewmilson andrewmilson deleted the 08-20-Move_all_MLE_eval_related_constraints_into_same_module branch August 22, 2024 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants