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

refactor(Protogalaxy): Isolate some state and clarify skipped zero computation #8173

Merged
merged 6 commits into from
Aug 26, 2024

Conversation

codygunton
Copy link
Contributor

@codygunton codygunton commented Aug 24, 2024

Some steps toward clarifying state during Protogalaxy proof construction:

  • Move accumulators into the class that contains state.
  • Reduce size of Prover header. Move internal functions into a purely static class. This accounts for most of the diff.
  • Clarify the known-zero-value while removing loose coupling of template parameters.

The next step will be to reduce the amount of state in ProverInstances.

@codygunton codygunton self-assigned this Aug 24, 2024
@codygunton codygunton marked this pull request as ready for review August 24, 2024 18:21
@codygunton codygunton changed the title refactor: Clarify state in Protogalaxy refactor: Isolate some Pg state and clarify skipped zero computation Aug 25, 2024
@codygunton codygunton changed the title refactor: Isolate some Pg state and clarify skipped zero computation refacto(Protogalaxy): Isolate some state and clarify skipped zero computation Aug 25, 2024
@codygunton codygunton changed the title refacto(Protogalaxy): Isolate some state and clarify skipped zero computation refactor(Protogalaxy): Isolate some state and clarify skipped zero computation Aug 25, 2024
Copy link
Contributor

@ledwards2225 ledwards2225 left a comment

Choose a reason for hiding this comment

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

Looks good

@@ -56,7 +56,7 @@ TEST(Protogalaxy, CombinerOn2Instances)
ProverInstances instances{ instance_data };
instances.alphas.fill(bb::Univariate<FF, 12>(FF(0))); // focus on the arithmetic relation only
auto pow_polynomial = PowPolynomial(std::vector<FF>{ 2 });
auto result = prover.compute_combiner</*OptimisationEnabled=*/false>(instances, pow_polynomial);
auto result = Fun::compute_combiner(instances, pow_polynomial, prover.state.univariate_accumulators);
Copy link
Contributor

Choose a reason for hiding this comment

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

big proponent of this as you know!

@codygunton codygunton merged commit 7395b95 into master Aug 26, 2024
42 checks passed
@codygunton codygunton deleted the cg/pg-state-1 branch August 26, 2024 17:35
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.

2 participants