-
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
Add arithmetic op counts to InfoEvaluator #832
Add arithmetic op counts to InfoEvaluator #832
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 #832 +/- ##
==========================================
- Coverage 91.94% 91.93% -0.02%
==========================================
Files 93 93
Lines 13092 13312 +220
Branches 13092 13312 +220
==========================================
+ Hits 12038 12238 +200
- Misses 939 957 +18
- Partials 115 117 +2 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
4c79a3d
to
d79f369
Compare
d79f369
to
798f58a
Compare
798f58a
to
a46c994
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 2 of 15 files at r1, 14 of 14 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @andrewmilson and @shaharsamocha7)
crates/prover/src/examples/poseidon/mod.rs
line 30 at r3 (raw file):
use crate::core::ColumnVec; const N_LOG_INSTANCES_PER_ROW: usize = 0;
It still works with 3 though, right?
a46c994
to
19e4fac
Compare
deca512
to
b32c8bc
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: 3 of 19 files reviewed, 1 unresolved discussion (waiting on @Alon-Ti and @shaharsamocha7)
crates/prover/src/examples/poseidon/mod.rs
line 30 at r3 (raw file):
Previously, Alon-Ti wrote…
It still works with 3 though, right?
It does. Reverted back
b32c8bc
to
e5fe5d9
Compare
e5fe5d9
to
840ed3e
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: e5fe5d9 | Previous: f6214d1 | Ratio |
---|---|---|---|
merkle throughput/simd merkle |
29850012 ns/iter (± 871197 ) |
14690867 ns/iter (± 434150 ) |
2.03 |
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 14 of 16 files at r4, 2 of 2 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @shaharsamocha7)
Couldn't see a way to add the counting types without removing
Copy
from a few places.Some constraint code is a bit more ugly now :(
This change is