-
Notifications
You must be signed in to change notification settings - Fork 93
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
DomainAccumulator allows accumulating a row at once #480
DomainAccumulator allows accumulating a row at once #480
Conversation
Your org has enabled the Graphite merge queue for merging into devAdd the label “merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. You must have a Graphite account in order to use the merge queue. Sign up using this link. |
3ccd166
to
93e0bb7
Compare
b41c0d0
to
fc7d13f
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #480 +/- ##
==========================================
+ Coverage 95.03% 95.06% +0.02%
==========================================
Files 61 61
Lines 9123 9130 +7
Branches 9123 9130 +7
==========================================
+ Hits 8670 8679 +9
+ Misses 396 393 -3
- Partials 57 58 +1 ☔ View full report in Codecov by Sentry. |
d10a9f2
to
8a09ec1
Compare
d933282
to
d48bcc2
Compare
8a09ec1
to
adbae30
Compare
d48bcc2
to
a5106b1
Compare
adbae30
to
3ce5d2f
Compare
a5106b1
to
383eb39
Compare
3ce5d2f
to
c8e1e19
Compare
383eb39
to
9ff3956
Compare
c8e1e19
to
91180ed
Compare
9ff3956
to
84adff0
Compare
91180ed
to
a8e2c16
Compare
84adff0
to
47f75b0
Compare
a8e2c16
to
6e91b9c
Compare
47f75b0
to
94653ae
Compare
6e91b9c
to
21b9764
Compare
94653ae
to
fea9b26
Compare
21b9764
to
e207427
Compare
fea9b26
to
83ecff8
Compare
e207427
to
2bbdee2
Compare
83ecff8
to
09f6436
Compare
2bbdee2
to
69525bb
Compare
09f6436
to
7eaa305
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 all commit messages.
Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @spapinistarkware)
src/core/air/accumulation.rs
line 107 at r3 (raw file):
.unwrap_or_else(|e| panic!("invalid log_sizes: {}", e)) .map(|c| ColumnAccumulator { random_coeff_pow: self.random_coeff,
I don't understand this, seems that it always get the same element.
Why not give the ColumnAccumulator the correct pow in advance?
(I thought this change reduces the need of exposing the random coeff from this struct)
Code quote:
random_coeff_pow: self.random_coeff,
src/core/air/accumulation.rs
line 112 at r3 (raw file):
for i in 0..N { res[i].random_coeff_pow = self.random_coeff.pow(n_cols_per_size[i].1 as u128); }
Don't we want to calculate the powers more efficiently?
Code quote:
for i in 0..N {
res[i].random_coeff_pow = self.random_coeff.pow(n_cols_per_size[i].1 as u128);
}
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: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @shaharsamocha7)
src/core/air/accumulation.rs
line 107 at r3 (raw file):
Previously, shaharsamocha7 wrote…
I don't understand this, seems that it always get the same element.
Why not give the ColumnAccumulator the correct pow in advance?(I thought this change reduces the need of exposing the random coeff from this struct)
Done.
src/core/air/accumulation.rs
line 112 at r3 (raw file):
Previously, shaharsamocha7 wrote…
Don't we want to calculate the powers more efficiently?
Done.
7eaa305
to
760ccc2
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 1 of 2 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @spapinistarkware)
Merge activity
|
This change is