-
Notifications
You must be signed in to change notification settings - Fork 5
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
Str-700: Preparation work for the prover CI performance metrics #546
Conversation
c4b49d5
to
bd3e6ea
Compare
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## main #546 +/- ##
==========================================
- Coverage 56.73% 56.72% -0.02%
==========================================
Files 308 309 +1
Lines 31316 31387 +71
==========================================
+ Hits 17768 17805 +37
- Misses 13548 13582 +34
|
bd3e6ea
to
b672e9a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cosmetic nits.
Otherwise looks good and will ACK it.
Nice work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct me if my understanding of how this works is incorrect but I would think that the proof generators should have the input be based on the block ranges so that it uniquely refers to what we're making the proof about. So that if we run the proof gen with the same input twice we always get the same proof.
Otherwise, concur with Jose's comments but looks well-reasoned.
@delbonis Yes, you are correct. Isn't it the case for the current prover generators in the tests? I don't quite get your concerns, would appreciate if you unwrap your thinking more. Generators are used in tests and operate on the test data (test btc block segment, ee block segment, etc). The inputs are defined directly in test cases. Generators input currently work as a convertor into the underlying prover input (for the underlying prover). |
Yeah I'm seeing it in the other PR now. Not an issue that can be solved here. |
8ad96ae
to
55ac98c
Compare
…in adapters and test provers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 55ac98c
0aa4b56
55ac98c
to
0aa4b56
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 0aa4b56
e978582
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK e978582
Description
Preparation work for the prover CI performance metrics. Includes:
ProofReport
simple structurebin/prover-client/zkvm
into a separate crate. Reuse this crate inprovers/tests
and de-duplicate some code. Later use it for in prover CI performance metrics.Cargo.lock
files inprovers/
Type of Change
Notes to Reviewers
Checklist
Related Issues
Unblocks STR-651.