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

feat: Protogalaxy Combiner #2436

Merged
merged 66 commits into from
Oct 17, 2023
Merged

feat: Protogalaxy Combiner #2436

merged 66 commits into from
Oct 17, 2023

Conversation

codygunton
Copy link
Contributor

@codygunton codygunton commented Sep 20, 2023

This work introduces the Protogalaxy combiner polynomial ($G$ in the paper). I reuse structures from our sumcheck code, and also try to reflect the sumcheck code's organization, as much as possible. The main result is a test file that tests the combiner construction in three special cases, touching on 2- and 4-instance folding.

Major points

  • Generalize tuple of tuples containers to Protogalaxy, where they need to handle a varying number of instances.
  • Introduce compute_combiner function.

Minor points

  • Move extend and evaluate out of BarycentricData and into Univariate for convenience.
  • Move shared sumcheck functions (e.g. for operating on tuples of tuples) into a utilities class.
  • Add get_random method to RelationParameters.

@codygunton codygunton mentioned this pull request Sep 20, 2023
4 tasks
@codygunton codygunton changed the title Cg/pg combiner bb moved feat: Protogalaxy Combiner Sep 20, 2023
@codygunton codygunton changed the base branch from master to cg/pg-combiner September 20, 2023 16:31
@codygunton codygunton changed the base branch from cg/pg-combiner to master September 20, 2023 16:32
@codygunton codygunton requested a review from maramihali October 16, 2023 13:40
@codygunton codygunton marked this pull request as ready for review October 16, 2023 13:42
using TupleOfTuplesOfUnivariates = decltype(create_relation_univariates_container<FF, Relations>());
using TupleOfArraysOfValues = decltype(create_relation_values_container<FF, Relations>());
using SumcheckTupleOfTuplesOfUnivariates = decltype(create_sumcheck_tuple_of_tuples_of_univariates<Relations>());
using SumcheckTupleOfArraysOfValues = decltype(create_sumcheck_tuple_of_arrays_of_values<Relations>());

Copy link
Contributor

Choose a reason for hiding this comment

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

these are much clear names to me, also thanks for the comments :)

@@ -712,12 +712,16 @@ template <typename CycleGroup_T, typename Curve_T, typename PCS_T> class ECCVMBa
};

/**
* @brief A container for univariates produced during the hot loop in sumcheck.
* @brief A container for univariates used during Protogalaxy folding and sumcheck.
Copy link
Contributor

Choose a reason for hiding this comment

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

we should probably have no mention of ProtoGalaxy in here since we are not going to fold ECC VM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed

@@ -54,15 +54,18 @@ class Ultra {
proof_system::AuxiliaryRelation<FF>>;

static constexpr size_t MAX_RELATION_LENGTH = get_max_relation_length<Relations>();
static_assert(MAX_RELATION_LENGTH == 6);
Copy link
Contributor

Choose a reason for hiding this comment

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

why is there a static_assert only here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh this was just me checking what I thought was true. Will remove.

@@ -315,12 +318,16 @@ class Ultra {
};

/**
* @brief A container for univariates produced during the hot loop in sumcheck.
* @brief A container for univariates used during Protogalaxy folding and sumcheck.
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe one sentence explainer of what these univariates are? here and in the other flavors

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -5,22 +5,54 @@ namespace proof_system::honk {

template <typename Flavor_, size_t NUM_> struct ProverInstances_ {
using Flavor = Flavor_;
using FF = typename Flavor_::FF;
Copy link
Contributor

Choose a reason for hiding this comment

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

is this preferred to Flavor::FF?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No there's no difference

* Relation contributes a tuple with num-identities many Univariates and there are num-relations many tuples in the
* outer tuple.
* @brief Recursive utility function to construct a container for the subrelation accumulators of Protogalaxy folding.
* @details Each relation contributes a tuple with num-identities many Univariates and there are num-relations many
Copy link
Contributor

Choose a reason for hiding this comment

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

can you pleasea define what num-identities is here?

Copy link
Contributor Author

@codygunton codygunton Oct 17, 2023

Choose a reason for hiding this comment

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

I didn't write this but I added a clarification

* @brief Recursive utility function to construct a container for the subrelation accumulators of Protogalaxy folding.
* @details Each relation contributes a tuple with num-identities many Univariates and there are num-relations many
* tuples in the outer tuple. The lengths of the univariaties are determined by subrelation lengths and the number of
* instances to be folded.
Copy link
Contributor

Choose a reason for hiding this comment

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

worth giving the mathematical formula here to make it more clear

Copy link
Contributor Author

@codygunton codygunton Oct 17, 2023

Choose a reason for hiding this comment

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

IMO more detail than we should give here. Very easy to find the function that computes this.

template <typename Flavor>
std::pair<std::array<barretenberg::Polynomial<typename Flavor::FF>, Flavor::NUM_ALL_ENTITIES>,
typename Flavor::ProverPolynomials>
get_sequential_prover_polynomials(const size_t log_circuit_size, const size_t starting_value)
Copy link
Contributor

Choose a reason for hiding this comment

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

can you briefly document this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure


public:
using FF = typename Flavor::FF;
template <size_t univariate_length>
using ExtendedEdges = typename Flavor::template ExtendedEdges<univariate_length>;
using ExtendedEdges = typename Flavor::ExtendedEdges;
Copy link
Contributor

Choose a reason for hiding this comment

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

formatting weirdness?

Copy link
Contributor Author

@codygunton codygunton Oct 17, 2023

Choose a reason for hiding this comment

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

No, MAX_RELATION_LENGTH is computed at compile time. This didn't need to be a template.

using FF = typename Flavor::FF;
using RelationParameters = proof_system::RelationParameters<FF>;

TEST(Protogalaxy, Combiner2Instances)
Copy link
Contributor

Choose a reason for hiding this comment

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

CombinerOnTwoInstances maybe? ditto on the other test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@maramihali maramihali left a comment

Choose a reason for hiding this comment

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

great work :)

@codygunton codygunton merged commit a60c70d into master Oct 17, 2023
3 checks passed
@codygunton codygunton deleted the cg/pg-combiner-bb-moved branch October 17, 2023 21:29
rahul-kothari pushed a commit that referenced this pull request Oct 24, 2023
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-packages: 0.10.0</summary>

##
[0.10.0](aztec-packages-v0.9.0...aztec-packages-v0.10.0)
(2023-10-24)


### ⚠ BREAKING CHANGES

* Emitting encrypted log by default
([#2926](#2926))

### Features

* Added register-account command to cli
([#2980](#2980))
([0977a90](0977a90))
* **docs:** Fix portals tutorial formatting
([#2929](#2929))
([ab19b67](ab19b67))
* Emitting encrypted log by default
([#2926](#2926))
([1ea2d4f](1ea2d4f)),
closes
[#2912](#2912)
* Goblin translator non-native field relation (Goblin Translator part 6)
([#2871](#2871))
([c4d8d96](c4d8d96))
* Honk profiling by pass, tsan preset
([#2982](#2982))
([a1592fd](a1592fd))
* Incorporate docs feedback and add "intermediate" level intros to some
pages
([#2598](#2598))
([78f9f52](78f9f52))
* Nuking `Pokeable` contract
([#2939](#2939))
([583d6fb](583d6fb))
* Protogalaxy Combiner
([#2436](#2436))
([a60c70d](a60c70d))
* Protogalaxy perturbator!
([#2624](#2624))
([509dee6](509dee6))
* Refactor pedersen hash standard
([#2592](#2592))
([3085676](3085676))
* Widget benchmarking
([#2897](#2897))
([0e927e9](0e927e9))


### Bug Fixes

* Add @jest/types to box deps
([#2903](#2903))
([db3fa62](db3fa62))
* Add lint rule for focused tests
([#2901](#2901))
([fd1a1a8](fd1a1a8))
* Avoid tsc OOM by unignoring an old contract artifact
([#2932](#2932))
([7310600](7310600))
* Bad it.only in tests
([#2900](#2900))
([a1f3af1](a1f3af1))
* Boxes boostrap dont use ts-node directly and add .prettierignore
([#2890](#2890))
([a3b1804](a3b1804))
* Confusing "Unknown complete address" error
([#2967](#2967))
([3a8f54a](3a8f54a))
* Force jest to quit, otherwise CI can rack up to 3hrs of credits per
job.
([#2899](#2899))
([ba2f671](ba2f671))
* Honk sumcheck performance
([#2925](#2925))
([5fbfe6e](5fbfe6e))
* Pending commitments contract using the wrong number of arguments
([#2959](#2959))
([655c322](655c322))
* Prettierignore in boxes
([#2902](#2902))
([8f7a200](8f7a200))
* Randomness in `AddressNote`
([#2965](#2965))
([4dc49a9](4dc49a9))
* Yarn lock
([#2923](#2923))
([7042bc6](7042bc6))


### Miscellaneous

* `Private Data Tree` --&gt; `Note Hash Tree`
([#2945](#2945))
([abaec9c](abaec9c)),
closes
[#2906](#2906)
* Apply hash abstraction over aztec-nr
([#2958](#2958))
([52f01ae](52f01ae))
* **docs:** Add Singleton and ImmutableSingleton `view_note` methods
([#2934](#2934))
([c1497f8](c1497f8))
* Fix box frontend styling
([#2919](#2919))
([7e9e8cc](7e9e8cc))
* Less noisy benchmark reports
([#2916](#2916))
([0df166c](0df166c))
* Remove unused nix files
([#2933](#2933))
([3174f84](3174f84))
* Run all e2e tests against sandbox
([#2891](#2891))
([6c4e26c](6c4e26c))
* Token box copies noir source files from noir-contracts on bootstrap
([#2940](#2940))
([a467b96](a467b96))


### Documentation

* Fix: update cheat codes to connect to ethRpcUrl
([#2922](#2922))
([4ffe9be](4ffe9be))
</details>

<details><summary>barretenberg.js: 0.10.0</summary>

##
[0.10.0](barretenberg.js-v0.9.0...barretenberg.js-v0.10.0)
(2023-10-24)


### Features

* Refactor pedersen hash standard
([#2592](#2592))
([3085676](3085676))
</details>

<details><summary>barretenberg: 0.10.0</summary>

##
[0.10.0](barretenberg-v0.9.0...barretenberg-v0.10.0)
(2023-10-24)


### Features

* Goblin translator non-native field relation (Goblin Translator part 6)
([#2871](#2871))
([c4d8d96](c4d8d96))
* Honk profiling by pass, tsan preset
([#2982](#2982))
([a1592fd](a1592fd))
* Protogalaxy Combiner
([#2436](#2436))
([a60c70d](a60c70d))
* Protogalaxy perturbator!
([#2624](#2624))
([509dee6](509dee6))
* Refactor pedersen hash standard
([#2592](#2592))
([3085676](3085676))
* Widget benchmarking
([#2897](#2897))
([0e927e9](0e927e9))


### Bug Fixes

* Honk sumcheck performance
([#2925](#2925))
([5fbfe6e](5fbfe6e))


### Miscellaneous

* Remove unused nix files
([#2933](#2933))
([3174f84](3174f84))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
AztecBot added a commit to AztecProtocol/barretenberg that referenced this pull request Oct 27, 2023
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-packages: 0.10.0</summary>

##
[0.10.0](AztecProtocol/aztec-packages@aztec-packages-v0.9.0...aztec-packages-v0.10.0)
(2023-10-24)


### ⚠ BREAKING CHANGES

* Emitting encrypted log by default
([#2926](AztecProtocol/aztec-packages#2926))

### Features

* Added register-account command to cli
([#2980](AztecProtocol/aztec-packages#2980))
([0977a90](AztecProtocol/aztec-packages@0977a90))
* **docs:** Fix portals tutorial formatting
([#2929](AztecProtocol/aztec-packages#2929))
([ab19b67](AztecProtocol/aztec-packages@ab19b67))
* Emitting encrypted log by default
([#2926](AztecProtocol/aztec-packages#2926))
([1ea2d4f](AztecProtocol/aztec-packages@1ea2d4f)),
closes
[#2912](AztecProtocol/aztec-packages#2912)
* Goblin translator non-native field relation (Goblin Translator part 6)
([#2871](AztecProtocol/aztec-packages#2871))
([c4d8d96](AztecProtocol/aztec-packages@c4d8d96))
* Honk profiling by pass, tsan preset
([#2982](AztecProtocol/aztec-packages#2982))
([a1592fd](AztecProtocol/aztec-packages@a1592fd))
* Incorporate docs feedback and add "intermediate" level intros to some
pages
([#2598](AztecProtocol/aztec-packages#2598))
([78f9f52](AztecProtocol/aztec-packages@78f9f52))
* Nuking `Pokeable` contract
([#2939](AztecProtocol/aztec-packages#2939))
([583d6fb](AztecProtocol/aztec-packages@583d6fb))
* Protogalaxy Combiner
([#2436](AztecProtocol/aztec-packages#2436))
([a60c70d](AztecProtocol/aztec-packages@a60c70d))
* Protogalaxy perturbator!
([#2624](AztecProtocol/aztec-packages#2624))
([509dee6](AztecProtocol/aztec-packages@509dee6))
* Refactor pedersen hash standard
([#2592](AztecProtocol/aztec-packages#2592))
([3085676](AztecProtocol/aztec-packages@3085676))
* Widget benchmarking
([#2897](AztecProtocol/aztec-packages#2897))
([0e927e9](AztecProtocol/aztec-packages@0e927e9))


### Bug Fixes

* Add @jest/types to box deps
([#2903](AztecProtocol/aztec-packages#2903))
([db3fa62](AztecProtocol/aztec-packages@db3fa62))
* Add lint rule for focused tests
([#2901](AztecProtocol/aztec-packages#2901))
([fd1a1a8](AztecProtocol/aztec-packages@fd1a1a8))
* Avoid tsc OOM by unignoring an old contract artifact
([#2932](AztecProtocol/aztec-packages#2932))
([7310600](AztecProtocol/aztec-packages@7310600))
* Bad it.only in tests
([#2900](AztecProtocol/aztec-packages#2900))
([a1f3af1](AztecProtocol/aztec-packages@a1f3af1))
* Boxes boostrap dont use ts-node directly and add .prettierignore
([#2890](AztecProtocol/aztec-packages#2890))
([a3b1804](AztecProtocol/aztec-packages@a3b1804))
* Confusing "Unknown complete address" error
([#2967](AztecProtocol/aztec-packages#2967))
([3a8f54a](AztecProtocol/aztec-packages@3a8f54a))
* Force jest to quit, otherwise CI can rack up to 3hrs of credits per
job.
([#2899](AztecProtocol/aztec-packages#2899))
([ba2f671](AztecProtocol/aztec-packages@ba2f671))
* Honk sumcheck performance
([#2925](AztecProtocol/aztec-packages#2925))
([5fbfe6e](AztecProtocol/aztec-packages@5fbfe6e))
* Pending commitments contract using the wrong number of arguments
([#2959](AztecProtocol/aztec-packages#2959))
([655c322](AztecProtocol/aztec-packages@655c322))
* Prettierignore in boxes
([#2902](AztecProtocol/aztec-packages#2902))
([8f7a200](AztecProtocol/aztec-packages@8f7a200))
* Randomness in `AddressNote`
([#2965](AztecProtocol/aztec-packages#2965))
([4dc49a9](AztecProtocol/aztec-packages@4dc49a9))
* Yarn lock
([#2923](AztecProtocol/aztec-packages#2923))
([7042bc6](AztecProtocol/aztec-packages@7042bc6))


### Miscellaneous

* `Private Data Tree` --&gt; `Note Hash Tree`
([#2945](AztecProtocol/aztec-packages#2945))
([abaec9c](AztecProtocol/aztec-packages@abaec9c)),
closes
[#2906](AztecProtocol/aztec-packages#2906)
* Apply hash abstraction over aztec-nr
([#2958](AztecProtocol/aztec-packages#2958))
([52f01ae](AztecProtocol/aztec-packages@52f01ae))
* **docs:** Add Singleton and ImmutableSingleton `view_note` methods
([#2934](AztecProtocol/aztec-packages#2934))
([c1497f8](AztecProtocol/aztec-packages@c1497f8))
* Fix box frontend styling
([#2919](AztecProtocol/aztec-packages#2919))
([7e9e8cc](AztecProtocol/aztec-packages@7e9e8cc))
* Less noisy benchmark reports
([#2916](AztecProtocol/aztec-packages#2916))
([0df166c](AztecProtocol/aztec-packages@0df166c))
* Remove unused nix files
([#2933](AztecProtocol/aztec-packages#2933))
([3174f84](AztecProtocol/aztec-packages@3174f84))
* Run all e2e tests against sandbox
([#2891](AztecProtocol/aztec-packages#2891))
([6c4e26c](AztecProtocol/aztec-packages@6c4e26c))
* Token box copies noir source files from noir-contracts on bootstrap
([#2940](AztecProtocol/aztec-packages#2940))
([a467b96](AztecProtocol/aztec-packages@a467b96))


### Documentation

* Fix: update cheat codes to connect to ethRpcUrl
([#2922](AztecProtocol/aztec-packages#2922))
([4ffe9be](AztecProtocol/aztec-packages@4ffe9be))
</details>

<details><summary>barretenberg.js: 0.10.0</summary>

##
[0.10.0](AztecProtocol/aztec-packages@barretenberg.js-v0.9.0...barretenberg.js-v0.10.0)
(2023-10-24)


### Features

* Refactor pedersen hash standard
([#2592](AztecProtocol/aztec-packages#2592))
([3085676](AztecProtocol/aztec-packages@3085676))
</details>

<details><summary>barretenberg: 0.10.0</summary>

##
[0.10.0](AztecProtocol/aztec-packages@barretenberg-v0.9.0...barretenberg-v0.10.0)
(2023-10-24)


### Features

* Goblin translator non-native field relation (Goblin Translator part 6)
([#2871](AztecProtocol/aztec-packages#2871))
([c4d8d96](AztecProtocol/aztec-packages@c4d8d96))
* Honk profiling by pass, tsan preset
([#2982](AztecProtocol/aztec-packages#2982))
([a1592fd](AztecProtocol/aztec-packages@a1592fd))
* Protogalaxy Combiner
([#2436](AztecProtocol/aztec-packages#2436))
([a60c70d](AztecProtocol/aztec-packages@a60c70d))
* Protogalaxy perturbator!
([#2624](AztecProtocol/aztec-packages#2624))
([509dee6](AztecProtocol/aztec-packages@509dee6))
* Refactor pedersen hash standard
([#2592](AztecProtocol/aztec-packages#2592))
([3085676](AztecProtocol/aztec-packages@3085676))
* Widget benchmarking
([#2897](AztecProtocol/aztec-packages#2897))
([0e927e9](AztecProtocol/aztec-packages@0e927e9))


### Bug Fixes

* Honk sumcheck performance
([#2925](AztecProtocol/aztec-packages#2925))
([5fbfe6e](AztecProtocol/aztec-packages@5fbfe6e))


### Miscellaneous

* Remove unused nix files
([#2933](AztecProtocol/aztec-packages#2933))
([3174f84](AztecProtocol/aztec-packages@3174f84))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants