-
Notifications
You must be signed in to change notification settings - Fork 92
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
Make CommitmentSchemeProver::prove_values take ownership #852
Make CommitmentSchemeProver::prove_values take ownership #852
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #852 +/- ##
==========================================
+ Coverage 91.93% 91.95% +0.02%
==========================================
Files 93 93
Lines 13312 13308 -4
Branches 13312 13308 -4
==========================================
- Hits 12238 12237 -1
+ Misses 957 954 -3
Partials 117 117 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
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.
Reviewed 9 of 9 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @andrewmilson and @shaharsamocha7)
crates/prover/src/core/prover/mod.rs
line 150 at r1 (raw file):
fn extract_composition_oods_eval(&self) -> Result<SecureField, InvalidOodsSampleStructure> { // TODO(andrew): `[.., composition_mask, _quotients_mask]` when add quotients commitment. let [_traces_mask @ .., composition_mask] = &**self.sampled_values else {
Why are you binding traces_mask
?
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.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Alon-Ti and @shaharsamocha7)
crates/prover/src/core/prover/mod.rs
line 150 at r1 (raw file):
Previously, Alon-Ti wrote…
Why are you binding
traces_mask
?
I would have done _ @ ..
but it errors strangely.
OHHH. Wow didn't realise you can just do ..
SMH, tyty
798f58a
to
a46c994
Compare
4de2301
to
64b9479
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.
Reviewed 6 of 6 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @shaharsamocha7)
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.
Reviewed 3 of 9 files at r1, 6 of 6 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @andrewmilson)
a46c994
to
19e4fac
Compare
64b9479
to
16f372f
Compare
16f372f
to
e5089ad
Compare
19e4fac
to
deca512
Compare
e5089ad
to
02fe2b1
Compare
deca512
to
b32c8bc
Compare
02fe2b1
to
557345d
Compare
b32c8bc
to
e5fe5d9
Compare
557345d
to
78be733
Compare
e5fe5d9
to
840ed3e
Compare
78be733
to
c46d77b
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.
⚠️ 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: bbe8f96 | Previous: cd8b37b | Ratio |
---|---|---|---|
merkle throughput/simd merkle |
27701539 ns/iter (± 437610 ) |
13712527 ns/iter (± 579195 ) |
2.02 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @shaharsamocha7
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.
Reviewed 9 of 9 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @andrewmilson)
crates/prover/src/examples/plonk/mod.rs
line 301 at r3 (raw file):
let sizes = component.trace_log_degree_bounds(); // Constant columns.
rename
Suggestion:
// Preprocessed columns.
c46d77b
to
ba15488
Compare
ba15488
to
bbe8f96
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.
Reviewable status: 8 of 11 files reviewed, all discussions resolved (waiting on @shaharsamocha7)
crates/prover/src/examples/plonk/mod.rs
line 301 at r3 (raw file):
Previously, shaharsamocha7 wrote…
rename
Done thanks
Simplifies the implementation of quotient commitment tree in following PR
This change is