Skip to content
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

Optimize prover boundary evaluations in construction of composition polynomial #22

Merged
merged 5 commits into from Jun 6, 2023

Conversation

ajgara
Copy link
Contributor

@ajgara ajgara commented Jun 5, 2023

Improves 2 times the prover speed.

  • Computes outside of the forloop some polynomial evaluations and inverses.
  • Change lagrange evaluation to FFT.
  • Uses batch inverse.

Copy link
Member

@Oppen Oppen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@Oppen
Copy link
Member

Oppen commented Jun 5, 2023

Do we have some benchmark results?

Comment on lines 90 to 114
let mut boundary_polys_evaluations = Vec::with_capacity(boundary_polys.len());
for poly in boundary_polys.iter() {
let evaluations = evaluate_polynomial_on_lde_domain(
poly,
domain.blowup_factor,
domain.interpolation_domain_size,
&domain.coset_offset,
)
.unwrap();
boundary_polys_evaluations.push(evaluations);
}

let mut boundary_zerofiers_inverse_evaluations =
Vec::with_capacity(boundary_zerofiers.len());
for poly in boundary_zerofiers.iter() {
let mut evaluations = evaluate_polynomial_on_lde_domain(
poly,
domain.blowup_factor,
domain.interpolation_domain_size,
&domain.coset_offset,
)
.unwrap();
FieldElement::inplace_batch_inverse(&mut evaluations);
boundary_zerofiers_inverse_evaluations.push(evaluations);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can evaluate the polynomials before collecting the original vectors, so we halve the memory taken by the whole operation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. Done!

@codecov-commenter
Copy link

Codecov Report

Merging #22 (03dc0ba) into main (3850002) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #22      +/-   ##
==========================================
+ Coverage   97.92%   97.93%   +0.01%     
==========================================
  Files          33       33              
  Lines        4872     4902      +30     
==========================================
+ Hits         4771     4801      +30     
  Misses        101      101              
Impacted Files Coverage Δ
src/air/constraints/evaluator.rs 100.00% <100.00%> (ø)
src/prover.rs 99.83% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@MauroToscano
Copy link
Collaborator

Good results
Screenshot 2023-06-06 at 14 18 54

@MauroToscano MauroToscano added this pull request to the merge queue Jun 6, 2023
Merged via the queue into main with commit b1eaefb Jun 6, 2023
3 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants