8651: Addressing review comments

barretenberg/cpp/src/barretenberg/polynomials/polynomial.cpp

@@ -2,6 +2,7 @@
 #include "barretenberg/common/assert.hpp"
 #include "barretenberg/common/slab_allocator.hpp"
 #include "barretenberg/common/thread.hpp"
+#include "barretenberg/numeric/bitop/get_msb.hpp"
 #include "barretenberg/numeric/bitop/pow.hpp"
 #include "barretenberg/polynomials/shared_shifted_virtual_zeroes_array.hpp"
 #include "polynomial_arithmetic.hpp"
@@ -186,16 +187,19 @@ template <typename Fr> Fr Polynomial<Fr>::evaluate(const Fr& z) const
 
 template <typename Fr> Fr Polynomial<Fr>::evaluate_mle(std::span<const Fr> evaluation_points, bool shift) const
 {
-    const size_t m = evaluation_points.size();
+    const size_t n = evaluation_points.size();
+
+    ASSERT(size() > 0);                                       // Avoid trivial polynomial and ensure end_index() >= 1
+    const size_t dim = numeric::get_msb(end_index() - 1) + 1; // Round up to next power of 2
 
     // To simplify handling of edge cases, we assume that the index space is always a power of 2
-    ASSERT(virtual_size() == static_cast<size_t>(1 << m));
+    ASSERT(virtual_size() == static_cast<size_t>(1 << n));
 
-    // we do m rounds l = 0,...,m-1.
+    // We first fold over dim rounds l = 0,...,dim-1.
     // in round l, n_l is the size of the buffer containing the Polynomial partially evaluated
     // at u₀,..., u_l.
-    // in round 0, this is half the size of n
-    size_t n_l = 1 << (m - 1);
+    // In round 0, this is half the size of dim
+    size_t n_l = 1 << (dim - 1);
 
     // temporary buffer of half the size of the Polynomial
     // TODO(https://github.com/AztecProtocol/barretenberg/issues/1096): Make this a Polynomial with DontZeroMemory::FLAG
@@ -216,32 +220,23 @@ template <typename Fr> Fr Polynomial<Fr>::evaluate_mle(std::span<const Fr> evalu
         const size_t ALLOW_ONE_PAST_READ = 1;
         tmp[i] = get(i * 2 + offset) + u_l * (get(i * 2 + 1 + offset, ALLOW_ONE_PAST_READ) - get(i * 2 + offset));
     }
-    // partially evaluate the m-1 remaining points
-    for (size_t l = 1; l < m; ++l) {
-        n_l = 1 << (m - l - 1);
+
+    // partially evaluate the dim-1 remaining points
+    for (size_t l = 1; l < dim; ++l) {
+        n_l = 1 << (dim - l - 1);
         u_l = evaluation_points[l];
         for (size_t i = 0; i < n_l; ++i) {
             tmp[i] = tmp[i * 2] + u_l * (tmp[(i * 2) + 1] - tmp[i * 2]);
         }
     }
-    Fr result = tmp[0];
-    return result;
-}
-
-template <typename Fr> Fr Polynomial<Fr>::evaluate_bounded_mle(std::span<const Fr> evaluation_points, size_t dim) const
-{
-    const size_t n = evaluation_points.size();
-    ASSERT(dim <= n);
-
-    std::span<const Fr, std::dynamic_extent> truncated_points = evaluation_points.first(dim);
+    auto result = tmp[0];
 
-    // In evaluate_mle, an assertion enforces truncated_points.size() == virtual_size()
-    Fr mle_res = evaluate_mle(truncated_points);
+    // We handle the "trivial" dimensions which are full of zeros.
     for (size_t i = dim; i < n; i++) {
-        mle_res *= (Fr(1) - evaluation_points[i]);
+        result *= (Fr(1) - evaluation_points[i]);
     }
 
-    return mle_res;
+    return result;
 }
 
 template <typename Fr> Polynomial<Fr> Polynomial<Fr>::partial_evaluate_mle(std::span<const Fr> evaluation_points) const

barretenberg/cpp/src/barretenberg/polynomials/polynomial.hpp

@@ -142,34 +142,22 @@ template <typename Fr> class Polynomial {
      */
     Polynomial shifted() const;
 
-    /**
-     * @brief evaluates p(X) = ∑ᵢ aᵢ⋅Xⁱ considered as multi-linear extension p(X₀,…,Xₘ₋₁) = ∑ᵢ aᵢ⋅Lᵢ(X₀,…,Xₘ₋₁)
-     * at u = (u₀,…,uₘ₋₁)
-     *
-     * @details this function allocates a temporary buffer of size n/2
-     *
-     * @param evaluation_points an MLE evaluation point u = (u₀,…,uₘ₋₁)
-     * @param shift evaluates p'(X₀,…,Xₘ₋₁) = 1⋅L₀(X₀,…,Xₘ₋₁) + ∑ᵢ˲₁ aᵢ₋₁⋅Lᵢ(X₀,…,Xₘ₋₁) if true
-     * @return Fr p(u₀,…,uₘ₋₁)
-     */
-    Fr evaluate_mle(std::span<const Fr> evaluation_points, bool shift = false) const;
-
     /**
      * @brief evaluate multi-linear extension p(X_0,…,X_{n-1}) = \sum_i a_i*L_i(X_0,…,X_{n-1}) at u = (u_0,…,u_{n-1})
-     *        in a more efficient way if a_j == 0 for any j >= 2^k where k is less or equal
-     *        to n. In this case, we fold over k dimensions and then multiply the result by
-     *        (1 - u_k) * (1 - u_{k+1}) ... * (1 - u_{n-1}). Note that in this case, for any
+     *        If the polynomial is embedded into a lower dimension k<n, i.e, start_index + size <= 2^k,
+     *        we evaluate it in a more efficient way. Note that a_j == 0 for any j >= 2^k.
+     *        We fold over k dimensions and then multiply the result by
+     *        (1 - u_k) * (1 - u_{k+1}) ... * (1 - u_{n-1}). In this case, for any
      *        i < 2^k, L_i is a multiple of (1 - X_k) * (1 - X_{k+1}) ... * (1 - X_{n-1}). Dividing
      *        p by this monomial leads to a multilinear extension over variables X_0, X_1, ..X_{k-1}.
      *
-     * @param evaluation_points evaluation vector of size n
-     * @param dim dimension of hypercube, i.e., value k above
+     * @details this function allocates a temporary buffer of size 2^(k-1)
      *
-     * @details The current polynomial is asssumed to have virtual size equal to 2^k with k <= n. This means
-     *          that the virtual size will determine the number of dimensions to fold. Superfluous virtual
-     *          size will have superfluous extra cost.
+     * @param evaluation_points evaluation vector of size n
+     * @param shift a boolean and when set to true, we evaluate the shifted counterpart polynomial:
+     *              enforce a_0 == 0 and compute \sum_i a_{i+1}*L_i(X_0,…,X_{n-1})
      */
-    Fr evaluate_bounded_mle(std::span<const Fr> evaluation_points, size_t dim) const;
+    Fr evaluate_mle(std::span<const Fr> evaluation_points, bool shift = false) const;
 
     /**
      * @brief Partially evaluates in the last k variables a polynomial interpreted as a multilinear extension.

barretenberg/cpp/src/barretenberg/polynomials/polynomial_arithmetic.test.cpp

@@ -1091,7 +1091,8 @@ TYPED_TEST(PolynomialTests, interpolation_constructor)
     }
 }
 
-TYPED_TEST(PolynomialTests, evaluate_mle)
+// LegacyPolynomials MLE
+TYPED_TEST(PolynomialTests, evaluate_mle_legacy)
 {
     using FF = TypeParam;
 
@@ -1149,37 +1150,44 @@ TYPED_TEST(PolynomialTests, evaluate_mle)
 }
 
 /*
- * @brief Compare that the bounded mle evaluation is equal to the mle evaluation.
+ * @brief Compare that the mle evaluation over Polynomials match the mle evaluation of
+ *        LegacyPolynomials.
  */
-TYPED_TEST(PolynomialTests, evaluate_bounded_mle)
+TYPED_TEST(PolynomialTests, evaluate_mle)
 {
     using FF = TypeParam;
 
     auto test_case = [](size_t n, size_t dim) {
         auto& engine = numeric::get_debug_randomness();
+        size_t k = 1 << dim;
 
         std::vector<FF> evaluation_points;
         evaluation_points.resize(n);
         for (size_t i = 0; i < n; i++) {
             evaluation_points[i] = FF::random_element(&engine);
         }
 
-        const size_t bounded_size = 1 << dim;
-        Polynomial<FF> bounded_poly(bounded_size);
-        for (size_t i = 0; i < bounded_size; ++i) {
-            bounded_poly.at(i) = FF::random_element(&engine);
-        }
-
-        const size_t size = 1 << n;
-        Polynomial<FF> poly(size);
-        for (size_t i = 0; i < bounded_size; ++i) {
-            poly.at(i) = bounded_poly.at(i);
+        LegacyPolynomial<FF> legacy_poly(1 << n);
+        Polynomial<FF> poly(k, 1 << n);
+        for (size_t i = 0; i < k; ++i) {
+            auto const rnd = FF::random_element(&engine);
+            legacy_poly[i] = rnd;
+            poly.at(i) = rnd;
         }
 
-        const FF bounded_res = bounded_poly.evaluate_bounded_mle(evaluation_points, dim);
+        const FF legacy_res = legacy_poly.evaluate_mle(evaluation_points);
         const FF res = poly.evaluate_mle(evaluation_points);
 
-        EXPECT_EQ(bounded_res, res);
+        EXPECT_EQ(legacy_res, res);
+
+        // Same with shifted polynomials
+        legacy_poly[0] = FF(0);
+        poly.at(0) = FF(0);
+
+        const FF legacy_shift_res = legacy_poly.evaluate_mle(evaluation_points, true);
+        const FF shift_res = poly.evaluate_mle(evaluation_points, true);
+
+        EXPECT_EQ(legacy_shift_res, shift_res);
     };
 
     test_case(9, 3);

barretenberg/cpp/src/barretenberg/vm/avm/generated/verifier.cpp

@@ -28,11 +28,10 @@ AvmVerifier& AvmVerifier::operator=(AvmVerifier&& other) noexcept
 using FF = AvmFlavor::FF;
 
 // Evaluate the given public input column over the multivariate challenge points
-inline FF evaluate_public_input_column(const std::vector<FF>& points, std::vector<FF> challenges)
+inline FF AvmVerifier::evaluate_public_input_column(const std::vector<FF>& points, std::vector<FF> challenges)
 {
-    constexpr size_t MAX_COL_SIZE = 1 << AVM_PUBLIC_COLUMN_MAX_SIZE_LOG;
-    Polynomial<FF> polynomial(points, MAX_COL_SIZE);
-    return polynomial.evaluate_bounded_mle(challenges, AVM_PUBLIC_COLUMN_MAX_SIZE_LOG);
+    Polynomial<FF> polynomial(points, key->circuit_size);
+    return polynomial.evaluate_mle(challenges);
 }
 
 /**

barretenberg/cpp/src/barretenberg/vm/avm/generated/verifier.hpp

@@ -28,6 +28,9 @@ class AvmVerifier {
     std::shared_ptr<VerificationKey> key;
     std::map<std::string, Commitment> commitments;
     std::shared_ptr<Transcript> transcript;
+
+  private:
+    FF evaluate_public_input_column(const std::vector<FF>& points, std::vector<FF> challenges);
 };
 
 } // namespace bb

barretenberg/cpp/src/barretenberg/vm/aztec_constants.hpp

@@ -35,7 +35,7 @@
 #define PUBLIC_CONTEXT_INPUTS_LENGTH 42
 #define AVM_VERIFICATION_KEY_LENGTH_IN_FIELDS 66
 #define AVM_PROOF_LENGTH_IN_FIELDS 3817
-#define AVM_PUBLIC_COLUMN_MAX_SIZE_LOG 8
+#define AVM_PUBLIC_COLUMN_MAX_SIZE_LOG2 8
 #define MEM_TAG_U1 1
 #define MEM_TAG_U8 2
 #define MEM_TAG_U16 3

bb-pilcom/bb-pil-backend/templates/verifier.cpp.hbs

@@ -28,11 +28,10 @@ namespace bb {
 using FF = {{name}}Flavor::FF;
 
 // Evaluate the given public input column over the multivariate challenge points
-inline FF evaluate_public_input_column(const std::vector<FF>& points, std::vector<FF> challenges)
+inline FF AvmVerifier::evaluate_public_input_column(const std::vector<FF>& points, std::vector<FF> challenges)
 {
-    constexpr size_t MAX_COL_SIZE = 1 << AVM_PUBLIC_COLUMN_MAX_SIZE_LOG;
-    Polynomial<FF> polynomial(points, MAX_COL_SIZE);
-    return polynomial.evaluate_bounded_mle(challenges, AVM_PUBLIC_COLUMN_MAX_SIZE_LOG);
+    Polynomial<FF> polynomial(points, key->circuit_size);
+    return polynomial.evaluate_mle(challenges);
 }
 
 /**

bb-pilcom/bb-pil-backend/templates/verifier.hpp.hbs

@@ -28,6 +28,9 @@ class {{name}}Verifier {
     std::shared_ptr<VerificationKey> key;
     std::map<std::string, Commitment> commitments;
     std::shared_ptr<Transcript> transcript;
+
+  private:
+    FF evaluate_public_input_column(const std::vector<FF>& points, std::vector<FF> challenges);
 };
 
 } // namespace bb

noir-projects/noir-protocol-circuits/crates/types/src/constants.nr

@@ -295,7 +295,7 @@ global AVM_VERIFICATION_KEY_LENGTH_IN_FIELDS: u32 = 2 + 16 * 4;
 // To determine latest value, hover `COMPUTED_AVM_PROOF_LENGTH_IN_FIELDS`
 // in barretenberg/cpp/src/barretenberg/vm/avm/generated/flavor.hpp
 global AVM_PROOF_LENGTH_IN_FIELDS: u32 = 3817;
-global AVM_PUBLIC_COLUMN_MAX_SIZE_LOG = 8;
+global AVM_PUBLIC_COLUMN_MAX_SIZE_LOG2 = 8;
 /**
  * Enumerate the hash_indices which are used for pedersen hashing.
  * We start from 1 to avoid the default generators. The generator indices are listed

yarn-project/circuits.js/src/constants.gen.ts

@@ -201,7 +201,7 @@ export const TUBE_PROOF_LENGTH = 439;
 export const VERIFICATION_KEY_LENGTH_IN_FIELDS = 128;
 export const AVM_VERIFICATION_KEY_LENGTH_IN_FIELDS = 66;
 export const AVM_PROOF_LENGTH_IN_FIELDS = 3817;
-export const AVM_PUBLIC_COLUMN_MAX_SIZE_LOG = 8;
+export const AVM_PUBLIC_COLUMN_MAX_SIZE_LOG2 = 8;
 export const MEM_TAG_U1 = 1;
 export const MEM_TAG_U8 = 2;
 export const MEM_TAG_U16 = 3;