-
Notifications
You must be signed in to change notification settings - Fork 79
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
PolyOps interpolate columns refactor #823
PolyOps interpolate columns refactor #823
Conversation
da84f11
to
b1365c1
Compare
b1365c1
to
a8a1ee3
Compare
4b06605
to
5398d91
Compare
1d62aaa
to
dc89e3b
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, 3 unresolved discussions (waiting on @jarnesino)
crates/prover/src/core/pcs/prover.rs
line 174 at r1 (raw file):
let span = span!(Level::INFO, "Interpolation for commitment").entered(); let polys: Vec<CirclePoly<B>> = B::interpolate_columns(columns, self.commitment_scheme.twiddles);
Suggestion:
let polys = B::interpolate_columns(columns, self.commitment_scheme.twiddles);
crates/prover/src/core/poly/circle/ops.rs
line 31 at r1 (raw file):
/// Computes minimal polynomials that evaluate to the same values as each evaluation in `columns`. /// Used by the [`TreeBuilder::extend_evals()`] function.
discussed with team,
this doc is redundant; 'interpolate' is clear enough.
Code quote:
/// Computes minimal polynomials that evaluate to the same values as each evaluation in `columns`.
/// Used by the [`TreeBuilder::extend_evals()`] function.
crates/prover/src/core/poly/circle/ops.rs
line 38 at r1 (raw file):
columns .into_iter() .map(|eval| eval.interpolate_with_twiddles(&twiddles))
clippy hates the ref
try to run scripts/clippy.sh
Suggestion:
.map(|eval| eval.interpolate_with_twiddles(twiddles))
dc89e3b
to
8437012
Compare
Changes made. |
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 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @jarnesino and @ohad-starkware)
This PR introduces a new PolyOps trait function allows any backend to optimize interpolations of many columns.
This change is