refactor(Protogalaxy): Move state out of Instances (#8177)

The main goal of this PR, which is achieved, is to move all move all data except the `_data` array out of `ProverInstances`. I do additional cleanup: - Use constructors for pow polys rather than a `void` type function to update the state. - Delete commented out higher folding test, which I had been maintaining in commented out form - Move `ProtogalaxyProofConstructionState` def into `ProtogalaxyProver` - More idiomatic folding of relation parameters (loop over a zip of getters)
AztecProtocol · Aug 26, 2024 · cd5d2df · cd5d2df
1 parent 7395b95
commit cd5d2df
Show file tree

Hide file tree

Showing 15 changed files with 201 additions and 211 deletions.
diff --git a/barretenberg/cpp/src/barretenberg/polynomials/pow.bench.cpp b/barretenberg/cpp/src/barretenberg/polynomials/pow.bench.cpp
@@ -16,8 +16,7 @@ void compute_pow_poly(benchmark::State& state)
     for (auto _ : state) {
         int64_t num_betas = state.range(0);
         std::vector<bb::fr> cur_betas(betas.begin(), betas.begin() + num_betas);
-        PowPolynomial pow{ cur_betas };
-        pow.compute_values(static_cast<size_t>(num_betas));
+        PowPolynomial pow{ cur_betas, cur_betas.size() };
     }
 }
 

diff --git a/barretenberg/cpp/src/barretenberg/polynomials/pow.hpp b/barretenberg/cpp/src/barretenberg/polynomials/pow.hpp
@@ -41,9 +41,27 @@ template <typename FF> struct PowPolynomial {
      */
     FF partial_evaluation_result = FF(1);
 
-    explicit PowPolynomial(const std::vector<FF>& betas)
+    /**
+     * @brief Construct a new PowPolynomial
+     *
+     * @param betas
+     * @param log_num_monomials
+     */
+    PowPolynomial(const std::vector<FF>& betas, const size_t log_num_monomials)
         : betas(betas)
+        , pow_betas(compute_pow_betas(betas, log_num_monomials))
     {}
+
+    /**
+     * @brief Construct a new PowPolynomial object without expanding to a vector of monomials
+     * @details The sumcheck verifier does not use pow_betas
+     *
+     * @param betas
+     */
+    PowPolynomial(const std::vector<FF>& betas)
+        : betas(betas)
+    {}
+
     /**
      * @brief Retruns the element in #pow_betas at place #idx.
      *
@@ -116,15 +134,15 @@ template <typename FF> struct PowPolynomial {
      * @brief Given \f$ \vec\beta = (\beta_0,...,\beta_{d-1})\f$ compute \f$ pow_{\ell}(\vec \beta) = pow_{\beta}(\vec
      * \ell)\f$ for \f$ \ell =0,\ldots,2^{d}-1\f$.
      *
-     * @param log_circuit_size Determines the number of beta challenges used to compute pow_betas (required because when
-     * we generate CONST_SIZE_PROOF_LOG_N, currently 28, challenges but the real circuit size is less than 1 <<
+     * @param log_num_monomials Determines the number of beta challenges used to compute pow_betas (required because
+     * when we generate CONST_SIZE_PROOF_LOG_N, currently 28, challenges but the real circuit size is less than 1 <<
      * CONST_SIZE_PROOF_LOG_N, we should compute unnecessarily a vector of pow_betas of length 1 << 28 )
      */
-    BB_PROFILE void compute_values(size_t log_circuit_size)
+    BB_PROFILE static std::vector<FF> compute_pow_betas(const std::vector<FF>& betas, const size_t log_num_monomials)
     {
 
-        size_t pow_size = 1 << log_circuit_size;
-        pow_betas = std::vector<FF>(pow_size);
+        size_t pow_size = 1 << log_num_monomials;
+        std::vector<FF> pow_betas(pow_size);
 
         // Determine number of threads for multithreading.
         // Note: Multithreading is "on" for every round but we reduce the number of threads from the max available based
@@ -154,6 +172,8 @@ template <typename FF> struct PowPolynomial {
                 pow_betas[i] = res;
             }
         });
+
+        return pow_betas;
     }
 };
 /**<

diff --git a/barretenberg/cpp/src/barretenberg/polynomials/pow.test.cpp b/barretenberg/cpp/src/barretenberg/polynomials/pow.test.cpp
@@ -31,9 +31,8 @@ TEST(PowPolynomial, FullPowConsistency)
 
 TEST(PowPolynomial, PowPolynomialsOnPowers)
 {
-    auto betas = std::vector<fr>{ 2, 4, 16 };
-    auto pow = PowPolynomial(betas);
-    pow.compute_values(betas.size());
-    auto expected_values = std::vector<fr>{ 1, 2, 4, 8, 16, 32, 64, 128 };
+    std::vector<fr> betas{ 2, 4, 16 };
+    PowPolynomial<fr> pow(betas, betas.size());
+    std::vector<fr> expected_values{ 1, 2, 4, 8, 16, 32, 64, 128 };
     EXPECT_EQ(expected_values, pow.pow_betas);
 }
diff --git a/barretenberg/cpp/src/barretenberg/polynomials/univariate.hpp b/barretenberg/cpp/src/barretenberg/polynomials/univariate.hpp
@@ -28,6 +28,7 @@ template <class Fr, size_t view_domain_end, size_t view_domain_start, size_t ski
 template <class Fr, size_t domain_end, size_t domain_start = 0, size_t skip_count = 0> class Univariate {
   public:
     static constexpr size_t LENGTH = domain_end - domain_start;
+    static constexpr size_t SKIP_COUNT = skip_count;
     using View = UnivariateView<Fr, domain_end, domain_start, skip_count>;
 
     using value_type = Fr; // used to get the type of the elements consistently with std::array

diff --git a/barretenberg/cpp/src/barretenberg/protogalaxy/combiner.test.cpp b/barretenberg/cpp/src/barretenberg/protogalaxy/combiner.test.cpp
@@ -54,9 +54,13 @@ 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 = Fun::compute_combiner(instances, pow_polynomial, prover.state.univariate_accumulators);
+            prover.state.alphas.fill(bb::Univariate<FF, 12>(FF(0))); // focus on the arithmetic relation only
+            PowPolynomial<FF> pow_polynomial({ 2 }, /*log_num_monomials=*/1);
+            auto result = Fun::compute_combiner(instances,
+                                                pow_polynomial,
+                                                prover.state.relation_parameters,
+                                                prover.state.alphas,
+                                                prover.state.univariate_accumulators);
             // The expected_result values are computed by running the python script combiner_example_gen.py
             auto expected_result = Univariate<FF, 12>(std::array<FF, 12>{ 9704UL,
                                                                           13245288UL,
@@ -87,7 +91,7 @@ TEST(Protogalaxy, CombinerOn2Instances)
             }
 
             ProverInstances instances{ instance_data };
-            instances.alphas.fill(bb::Univariate<FF, 12>(FF(0))); // focus on the arithmetic relation only
+            prover.state.alphas.fill(bb::Univariate<FF, 12>(FF(0))); // focus on the arithmetic relation only
 
             const auto create_add_gate = [](auto& polys, const size_t idx, FF w_l, FF w_r) {
                 polys.w_l[idx] = w_l;
@@ -133,10 +137,17 @@ TEST(Protogalaxy, CombinerOn2Instances)
             relation value:
                       0    0    0    0    0    0    0              0    0    6   18   36   60   90      */
 
-            auto pow_polynomial = PowPolynomial(std::vector<FF>{ 2 });
-            auto result = Fun::compute_combiner(instances, pow_polynomial, prover.state.univariate_accumulators);
-            auto optimised_result =
-                Fun::compute_combiner(instances, pow_polynomial, prover.state.optimised_univariate_accumulators);
+            PowPolynomial<FF> pow_polynomial({ 2 }, /*log_num_monomials=*/1);
+            auto result = Fun::compute_combiner(instances,
+                                                pow_polynomial,
+                                                prover.state.relation_parameters,
+                                                prover.state.alphas,
+                                                prover.state.univariate_accumulators);
+            auto optimised_result = Fun::compute_combiner(instances,
+                                                          pow_polynomial,
+                                                          prover.state.optimised_relation_parameters,
+                                                          prover.state.alphas,
+                                                          prover.state.optimised_univariate_accumulators);
             auto expected_result =
                 Univariate<FF, 12>(std::array<FF, 12>{ 0, 0, 12, 36, 72, 120, 180, 252, 336, 432, 540, 660 });
 
@@ -190,10 +201,9 @@ TEST(Protogalaxy, CombinerOptimizationConsistency)
             }
 
             ProverInstances instances{ instance_data };
-            instances.alphas.fill(
+            prover.state.alphas.fill(
                 bb::Univariate<FF, UNIVARIATE_LENGTH>(FF(0))); // focus on the arithmetic relation only
-            auto pow_polynomial = PowPolynomial(std::vector<FF>{ 2 });
-            pow_polynomial.compute_values(1);
+            PowPolynomial<FF> pow_polynomial({ 2 }, /*log_num_monomials=*/1);
 
             // Relation parameters are all zeroes
             RelationParameters<FF> relation_parameters;
@@ -254,9 +264,16 @@ TEST(Protogalaxy, CombinerOptimizationConsistency)
                 precomputed_result[idx] = std::get<0>(accumulator)[0];
             }
             auto expected_result = Univariate<FF, UNIVARIATE_LENGTH>(precomputed_result);
-            auto result = Fun::compute_combiner(instances, pow_polynomial, prover.state.univariate_accumulators);
-            auto optimised_result =
-                Fun::compute_combiner(instances, pow_polynomial, prover.state.optimised_univariate_accumulators);
+            auto result = Fun::compute_combiner(instances,
+                                                pow_polynomial,
+                                                prover.state.relation_parameters,
+                                                prover.state.alphas,
+                                                prover.state.univariate_accumulators);
+            auto optimised_result = Fun::compute_combiner(instances,
+                                                          pow_polynomial,
+                                                          prover.state.optimised_relation_parameters,
+                                                          prover.state.alphas,
+                                                          prover.state.optimised_univariate_accumulators);
 
             EXPECT_EQ(result, expected_result);
             EXPECT_EQ(optimised_result, expected_result);
@@ -276,7 +293,7 @@ TEST(Protogalaxy, CombinerOptimizationConsistency)
             }
 
             ProverInstances instances{ instance_data };
-            instances.alphas.fill(bb::Univariate<FF, 12>(FF(0))); // focus on the arithmetic relation only
+            prover.state.alphas.fill(bb::Univariate<FF, 12>(FF(0))); // focus on the arithmetic relation only
 
             const auto create_add_gate = [](auto& polys, const size_t idx, FF w_l, FF w_r) {
                 polys.w_l[idx] = w_l;
@@ -322,10 +339,17 @@ TEST(Protogalaxy, CombinerOptimizationConsistency)
             relation value:
                       0    0    0    0    0    0    0              0    0    6   18   36   60   90      */
 
-            auto pow_polynomial = PowPolynomial(std::vector<FF>{ 2 });
-            auto result = Fun::compute_combiner(instances, pow_polynomial, prover.state.univariate_accumulators);
-            auto optimised_result =
-                Fun::compute_combiner(instances, pow_polynomial, prover.state.optimised_univariate_accumulators);
+            PowPolynomial<FF> pow_polynomial({ 2 }, /*log_num_monomials=*/1);
+            auto result = Fun::compute_combiner(instances,
+                                                pow_polynomial,
+                                                prover.state.relation_parameters,
+                                                prover.state.alphas,
+                                                prover.state.univariate_accumulators);
+            auto optimised_result = Fun::compute_combiner(instances,
+                                                          pow_polynomial,
+                                                          prover.state.optimised_relation_parameters,
+                                                          prover.state.alphas,
+                                                          prover.state.optimised_univariate_accumulators);
             auto expected_result =
                 Univariate<FF, 12>(std::array<FF, 12>{ 0, 0, 12, 36, 72, 120, 180, 252, 336, 432, 540, 660 });
 
@@ -336,59 +360,3 @@ TEST(Protogalaxy, CombinerOptimizationConsistency)
     run_test(true);
     run_test(false);
 };
-
-// // Tests a combiner on 4 instances, note currently we don't plan
-// // to fold with num instances > 2, this would require an additional explicit instantiation in
-// // protogalaxy_prover_ultra.cpp. Currently, we rather save the compile time.
-// TEST(Protogalaxy, DISABLED_CombinerOn4Instances)
-// {
-//     constexpr size_t NUM_INSTANCES = 4;
-//     using ProverInstance = ProverInstance_<Flavor>;
-//     using ProverInstances = ProverInstances_<Flavor, NUM_INSTANCES>;
-//     using ProtoGalaxyProver = ProtoGalaxyProver_<ProverInstances>;
-//     using Fun = ProtogalaxyProverInternal<ProverInstances>;
-
-//     const auto zero_all_selectors = [](auto& polys) {
-//         std::fill(polys.q_arith.begin(), polys.q_arith.end(), 0);
-//         std::fill(polys.q_delta_range.begin(), polys.q_delta_range.end(), 0);
-//         std::fill(polys.q_elliptic.begin(), polys.q_elliptic.end(), 0);
-//         std::fill(polys.q_aux.begin(), polys.q_aux.end(), 0);
-//         std::fill(polys.q_lookup.begin(), polys.q_lookup.end(), 0);
-//         std::fill(polys.q_4.begin(), polys.q_4.end(), 0);
-//         std::fill(polys.w_4.begin(), polys.w_4.end(), 0);
-//         std::fill(polys.w_4_shift.begin(), polys.w_4_shift.end(), 0);
-//     };
-
-//     auto run_test = [&]() {
-//         std::vector<std::shared_ptr<ProverInstance>> instance_data(NUM_INSTANCES);
-//         ProtoGalaxyProver prover;
-
-//         for (size_t idx = 0; idx < NUM_INSTANCES; idx++) {
-//             auto instance = std::make_shared<ProverInstance>();
-//             auto prover_polynomials = get_zero_prover_polynomials<Flavor>(
-//                 /*log_circuit_size=*/1);
-//             instance->proving_key.polynomials = std::move(prover_polynomials);
-//             instance->proving_key.circuit_size = 2;
-//             instance_data[idx] = instance;
-//         }
-
-//         ProverInstances instances{ instance_data };
-//         instances.alphas.fill(bb::Univariate<FF, 40>(FF(0))); // focus on the arithmetic relation only
-
-//         zero_all_selectors(instances[0]->proving_key.polynomials);
-//         zero_all_selectors(instances[1]->proving_key.polynomials);
-//         zero_all_selectors(instances[2]->proving_key.polynomials);
-//         zero_all_selectors(instances[3]->proving_key.polynomials);
-
-//         auto pow_polynomial = PowPolynomial(std::vector<FF>{ 2 });
-//         auto result = Fun::compute_combiner(instances, pow_polynomial, prover.state.univariate_accumulators);
-//         auto optimised_result =
-//             Fun::compute_combiner(instances, pow_polynomial, prover.state.optimised_univariate_accumulators);
-//         std::array<FF, 40> zeroes;
-//         std::fill(zeroes.begin(), zeroes.end(), 0);
-//         auto expected_result = Univariate<FF, 40>(zeroes);
-//         EXPECT_EQ(result, expected_result);
-//         EXPECT_EQ(optimised_result, expected_result);
-//     };
-//     run_test();
-// };
diff --git a/barretenberg/cpp/src/barretenberg/protogalaxy/protogalaxy.test.cpp b/barretenberg/cpp/src/barretenberg/protogalaxy/protogalaxy.test.cpp
@@ -98,8 +98,7 @@ template <typename Flavor> class ProtoGalaxyTests : public testing::Test {
         auto expected_honk_evals = Fun::compute_full_honk_evaluations(
             accumulator->proving_key.polynomials, accumulator->alphas, accumulator->relation_parameters);
         // Construct pow(\vec{betas*}) as in the paper
-        auto expected_pows = PowPolynomial(accumulator->gate_challenges);
-        expected_pows.compute_values(accumulator->gate_challenges.size());
+        PowPolynomial expected_pows(accumulator->gate_challenges, accumulator->gate_challenges.size());
 
         // Compute the corresponding target sum and create a dummy accumulator
         auto expected_target_sum = FF(0);
@@ -204,8 +203,7 @@ template <typename Flavor> class ProtoGalaxyTests : public testing::Test {
         }
 
         // Construct pow(\vec{betas}) as in the paper
-        auto pow_beta = bb::PowPolynomial(betas);
-        pow_beta.compute_values(log_instance_size);
+        bb::PowPolynomial pow_beta(betas, log_instance_size);
 
         // Compute the corresponding target sum and create a dummy accumulator
         auto target_sum = FF(0);
@@ -263,7 +261,7 @@ template <typename Flavor> class ProtoGalaxyTests : public testing::Test {
      * univariate, barycentrially extended to the desired number of evaluations, is performed correctly.
      *
      */
-    static void test_combine_relation_parameters()
+    static void test_compute_extended_relation_parameters()
     {
         Builder builder1;
         auto instance1 = std::make_shared<ProverInstance>(builder1);
@@ -275,22 +273,26 @@ template <typename Flavor> class ProtoGalaxyTests : public testing::Test {
         instance2->relation_parameters.eta = 3;
 
         ProverInstances instances{ { instance1, instance2 } };
-        Fun::combine_relation_parameters(instances);
+        auto relation_parameters =
+            Fun::template compute_extended_relation_parameters<typename FoldingProver::State::RelationParameters>(
+                instances);
+        auto optimised_relation_parameters = Fun::template compute_extended_relation_parameters<
+            typename FoldingProver::State::OptimisedRelationParameters>(instances);
 
         bb::Univariate<FF, 11> expected_eta{ { 1, 3, 5, 7, 9, 11, 13, 15, 17, 19, 21 } };
-        EXPECT_EQ(instances.relation_parameters.eta, expected_eta);
+        EXPECT_EQ(relation_parameters.eta, expected_eta);
         // Optimised relation parameters are the same, we just don't compute any values for non-used indices when
         // deriving values from them
         for (size_t i = 0; i < 11; i++) {
-            EXPECT_EQ(instances.optimised_relation_parameters.eta.evaluations[i], expected_eta.evaluations[i]);
+            EXPECT_EQ(optimised_relation_parameters.eta.evaluations[i], expected_eta.evaluations[i]);
         }
     }
 
     /**
      * @brief Given two dummy instances with the batching challenges alphas set (one for each subrelation) ensure
      * combining them in a univariate of desired length works as expected.
      */
-    static void test_combine_alpha()
+    static void test_compute_and_extend_alphas()
     {
         Builder builder1;
         auto instance1 = std::make_shared<ProverInstance>(builder1);
@@ -302,11 +304,11 @@ template <typename Flavor> class ProtoGalaxyTests : public testing::Test {
         instance2->alphas.fill(4);
 
         ProverInstances instances{ { instance1, instance2 } };
-        Fun::combine_alpha(instances);
+        auto alphas = Fun::compute_and_extend_alphas(instances);
 
-        bb::Univariate<FF, 12> expected_alpha{ { 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24 } };
-        for (const auto& alpha : instances.alphas) {
-            EXPECT_EQ(alpha, expected_alpha);
+        bb::Univariate<FF, 12> expected_alphas{ { 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24 } };
+        for (const auto& alpha : alphas) {
+            EXPECT_EQ(alpha, expected_alphas);
         }
     }
 
@@ -585,12 +587,12 @@ TYPED_TEST(ProtoGalaxyTests, CombinerQuotient)
 
 TYPED_TEST(ProtoGalaxyTests, CombineRelationParameters)
 {
-    TestFixture::test_combine_relation_parameters();
+    TestFixture::test_compute_extended_relation_parameters();
 }
 
-TYPED_TEST(ProtoGalaxyTests, CombineAlpha)
+TYPED_TEST(ProtoGalaxyTests, CombineAlphas)
 {
-    TestFixture::test_combine_alpha();
+    TestFixture::test_compute_and_extend_alphas();
 }
 
 TYPED_TEST(ProtoGalaxyTests, ProtogalaxyInhomogeneous)