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

Create MLE eval verifier component #805

Merged
merged 1 commit into from
Oct 1, 2024

Conversation

andrewmilson
Copy link
Contributor

@andrewmilson andrewmilson commented Aug 25, 2024

This change is Reviewable

@codecov-commenter
Copy link

codecov-commenter commented Aug 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.47%. Comparing base (cd1648e) to head (cc6929c).

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #805   +/-   ##
=======================================
  Coverage   92.47%   92.47%           
=======================================
  Files          90       90           
  Lines       12400    12400           
  Branches    12400    12400           
=======================================
  Hits        11467    11467           
  Misses        825      825           
  Partials      108      108           
Flag Coverage Δ
92.47% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@andrewmilson andrewmilson force-pushed the 08-24-Create_MLE_eval_verifier_component branch 2 times, most recently from a7d37ba to eec9cb8 Compare August 25, 2024 00:36
@andrewmilson andrewmilson force-pushed the 08-20-Create_MLE_eval_component branch from c1ac882 to 3fde6e5 Compare August 26, 2024 04:15
@andrewmilson andrewmilson force-pushed the 08-24-Create_MLE_eval_verifier_component branch from eec9cb8 to 2f7ebf2 Compare August 26, 2024 04:16
@andrewmilson andrewmilson force-pushed the 08-20-Create_MLE_eval_component branch from 3fde6e5 to bb56a26 Compare August 26, 2024 04:18
@andrewmilson andrewmilson force-pushed the 08-24-Create_MLE_eval_verifier_component branch from 2f7ebf2 to ddd533f Compare August 26, 2024 04:18
@andrewmilson andrewmilson force-pushed the 08-20-Create_MLE_eval_component branch from bb56a26 to 110a7b8 Compare August 26, 2024 17:44
@andrewmilson andrewmilson force-pushed the 08-24-Create_MLE_eval_verifier_component branch from ddd533f to 364bdc2 Compare August 26, 2024 17:45
@andrewmilson andrewmilson force-pushed the 08-20-Create_MLE_eval_component branch from 110a7b8 to 6b4cd33 Compare August 26, 2024 17:47
@andrewmilson andrewmilson force-pushed the 08-24-Create_MLE_eval_verifier_component branch from 364bdc2 to 4db6d30 Compare August 27, 2024 04:30
@andrewmilson andrewmilson force-pushed the 08-20-Create_MLE_eval_component branch from 6b4cd33 to 7af83e4 Compare August 28, 2024 02:03
@andrewmilson andrewmilson force-pushed the 08-24-Create_MLE_eval_verifier_component branch from 4db6d30 to 1bca1a7 Compare August 28, 2024 02:03
@andrewmilson andrewmilson force-pushed the 08-20-Create_MLE_eval_component branch from 7af83e4 to 3e7fdfa Compare August 28, 2024 02:11
@andrewmilson andrewmilson force-pushed the 08-24-Create_MLE_eval_verifier_component branch from 1bca1a7 to 6d88a87 Compare August 28, 2024 02:11
@andrewmilson andrewmilson force-pushed the 08-20-Create_MLE_eval_component branch from 3e7fdfa to 4e0bed5 Compare August 28, 2024 02:19
@andrewmilson andrewmilson force-pushed the 08-24-Create_MLE_eval_verifier_component branch from 6d88a87 to f06afde Compare August 28, 2024 02:19
@andrewmilson andrewmilson force-pushed the 08-20-Create_MLE_eval_component branch from 4e0bed5 to 4953abd Compare August 28, 2024 03:07
@andrewmilson andrewmilson force-pushed the 08-24-Create_MLE_eval_verifier_component branch from f06afde to 4c9bc7e Compare August 28, 2024 03:07
@andrewmilson andrewmilson force-pushed the 08-20-Create_MLE_eval_component branch from 4953abd to 283d17a Compare August 28, 2024 03:18
@andrewmilson andrewmilson force-pushed the 08-24-Create_MLE_eval_verifier_component branch from 4c9bc7e to 4f24c54 Compare August 28, 2024 03:18
@andrewmilson andrewmilson force-pushed the 08-20-Create_MLE_eval_component branch from 283d17a to 62443c5 Compare August 28, 2024 15:13
@andrewmilson andrewmilson force-pushed the 08-24-Create_MLE_eval_verifier_component branch from 4f24c54 to fff9df9 Compare August 28, 2024 15:13
@andrewmilson andrewmilson force-pushed the 08-24-Create_MLE_eval_verifier_component branch from fff9df9 to 35ba7cb Compare September 1, 2024 06:38
@andrewmilson andrewmilson force-pushed the 08-24-Create_MLE_eval_verifier_component branch from 35ba7cb to 5373213 Compare September 1, 2024 22:00
@andrewmilson andrewmilson force-pushed the 08-24-Create_MLE_eval_verifier_component branch from 5373213 to 4e92911 Compare September 3, 2024 06:18
@andrewmilson andrewmilson force-pushed the 08-24-Create_MLE_eval_verifier_component branch from 4e92911 to 14eb2a2 Compare September 9, 2024 07:05
Copy link
Collaborator

@shaharsamocha7 shaharsamocha7 left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @andrewmilson and @spapinistarkware)


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

        let mle_coeff_col_eval = self.mle_coeff_column_oracle.evaluate_at_point(point, mask);
        let carry_quotients_col_eval = eval_carry_quotient_col(&self.mle_eval_point, point);
        let is_first = eval_is_first(trace_coset, point);

Should we use this succinct version instead of committing to the is_first column in the constant tree?

Code quote:

eval_is_first(trace_coset, point)

@andrewmilson andrewmilson force-pushed the 08-24-Create_MLE_eval_verifier_component branch from 14eb2a2 to f1d17af Compare September 24, 2024 01:29
@andrewmilson andrewmilson force-pushed the 08-24-Create_MLE_eval_verifier_component branch from f1d17af to a969689 Compare September 24, 2024 01:30
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: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @shaharsamocha7 and @spapinistarkware)


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

Previously, shaharsamocha7 wrote…

Should we use this succinct version instead of committing to the is_first column in the constant tree?

We could. I remember ShaharP mentioning he thought constant column might be more efficient for the verifier. Not sure if this is just Bitcoin case or all cases

Copy link
Collaborator

@shaharsamocha7 shaharsamocha7 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 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @andrewmilson and @spapinistarkware)


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

Previously, andrewmilson (Andrew Milson) wrote…

We could. I remember ShaharP mentioning he thought constant column might be more efficient for the verifier. Not sure if this is just Bitcoin case or all cases

coset_vanishing(coset, p) / (point_vanishing(coset.initial, p) * BaseField::from(1 << coset.log_size))

Maybe it is because of the inverse but I guess that we can implement it efficiently using a hint.

Base automatically changed from 08-20-Create_MLE_eval_component to dev October 1, 2024 10:21
@andrewmilson andrewmilson force-pushed the 08-24-Create_MLE_eval_verifier_component branch from a969689 to cc6929c Compare October 1, 2024 10:22
@andrewmilson andrewmilson merged commit 6e2ec62 into dev Oct 1, 2024
15 of 16 checks passed
@andrewmilson andrewmilson deleted the 08-24-Create_MLE_eval_verifier_component branch October 1, 2024 10:26
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