From c498861d26d0e54bc3787ae8c1bc2be5c4e4d642 Mon Sep 17 00:00:00 2001 From: lucasxia01 Date: Thu, 19 Sep 2024 15:41:45 +0000 Subject: [PATCH] fix bug and add new tests --- .../commitment_schemes/commitment_key.hpp | 8 +- .../sparse_commitment.test.cpp | 90 +++++++++++++++++++ 2 files changed, 94 insertions(+), 4 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp index 567c32c5ba4..c713e58be37 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/commitment_key.hpp @@ -124,19 +124,19 @@ template class CommitmentKey { // Extract the precomputed point table (contains raw SRS points at even indices and the corresponding // endomorphism point (\beta*x, -y) at odd indices). We offset by polynomial.start_index * 2 to align - // with our polynomial spann. + // with our polynomial span. std::span point_table = srs->get_monomial_points().subspan(polynomial.start_index * 2); // Define structures needed to multithread the extraction of non-zero inputs - const size_t num_threads = poly_size >= get_num_cpus_pow2() ? get_num_cpus_pow2() : 1; - const size_t block_size = poly_size / num_threads; + const size_t num_threads = calculate_num_threads(poly_size); + const size_t block_size = (poly_size + num_threads - 1) / num_threads; // round up std::vector> thread_scalars(num_threads); std::vector> thread_points(num_threads); // Loop over all polynomial coefficients and keep {point, scalar} pairs for which scalar != 0 parallel_for(num_threads, [&](size_t thread_idx) { const size_t start = thread_idx * block_size; - const size_t end = (thread_idx + 1) * block_size; + const size_t end = std::min(poly_size, (thread_idx + 1) * block_size); for (size_t idx = start; idx < end; ++idx) { diff --git a/barretenberg/cpp/src/barretenberg/commitment_schemes/sparse_commitment.test.cpp b/barretenberg/cpp/src/barretenberg/commitment_schemes/sparse_commitment.test.cpp index 9d7da38d893..87473bee3d5 100644 --- a/barretenberg/cpp/src/barretenberg/commitment_schemes/sparse_commitment.test.cpp +++ b/barretenberg/cpp/src/barretenberg/commitment_schemes/sparse_commitment.test.cpp @@ -66,4 +66,94 @@ TYPED_TEST(CommitmentKeyTest, CommitSparse) EXPECT_EQ(sparse_commit_result, commit_result); } +/** + * @brief Test commit_sparse on polynomial with zero start index. + * + */ +TYPED_TEST(CommitmentKeyTest, CommitSparseSmallSize) +{ + using Curve = TypeParam; + using CK = CommitmentKey; + using G1 = Curve::AffineElement; + using Fr = Curve::ScalarField; + using Polynomial = bb::Polynomial; + + const size_t num_points = 1 << 12; // large enough to ensure normal pippenger logic is used + const size_t num_nonzero = 7; + + // Construct a sparse random polynomial + Polynomial poly(num_nonzero, num_points, 0); + for (size_t i = 0; i < num_nonzero; ++i) { + poly.at(i) = Fr::random_element(); + } + + // Commit to the polynomial using both the conventional commit method and the sparse commitment method + auto key = TestFixture::template create_commitment_key(num_points); + G1 commit_result = key->commit(poly); + G1 sparse_commit_result = key->commit_sparse(poly); + + EXPECT_EQ(sparse_commit_result, commit_result); +} + +/** + * @brief Test commit_sparse on polynomial with nonzero start index. + * + */ +TYPED_TEST(CommitmentKeyTest, CommitSparseNonZeroStartIndex) +{ + using Curve = TypeParam; + using CK = CommitmentKey; + using G1 = Curve::AffineElement; + using Fr = Curve::ScalarField; + using Polynomial = bb::Polynomial; + + const size_t num_points = 1 << 12; // large enough to ensure normal pippenger logic is used + const size_t num_nonzero = 7; + const size_t offset = 1 << 11; + + // Construct a sparse random polynomial + Polynomial poly(num_nonzero, num_points, offset); + for (size_t i = offset; i < offset + num_nonzero; ++i) { + poly.at(i) = Fr::random_element(); + } + + // Commit to the polynomial using both the conventional commit method and the sparse commitment method + auto key = TestFixture::template create_commitment_key(num_points); + G1 commit_result = key->commit(poly); + G1 sparse_commit_result = key->commit_sparse(poly); + + EXPECT_EQ(sparse_commit_result, commit_result); +} + +/** + * @brief Test commit_sparse on polynomial with medium size and nonzero start index. + * @details This was used to catch a bug in commit_sparse where the number of threads was > 1 and the size was not a + * power of 2. + */ +TYPED_TEST(CommitmentKeyTest, CommitSparseMediumNonZeroStartIndex) +{ + using Curve = TypeParam; + using CK = CommitmentKey; + using G1 = Curve::AffineElement; + using Fr = Curve::ScalarField; + using Polynomial = bb::Polynomial; + + const size_t num_points = 1 << 12; // large enough to ensure normal pippenger logic is used + const size_t num_nonzero = (1 << 9) + 1; + const size_t offset = 1 << 11; + + // Construct a sparse random polynomial + Polynomial poly(num_nonzero, num_points, offset); + for (size_t i = offset; i < offset + num_nonzero; ++i) { + poly.at(i) = Fr::random_element(); + } + + // Commit to the polynomial using both the conventional commit method and the sparse commitment method + auto key = TestFixture::template create_commitment_key(num_points); + G1 commit_result = key->commit(poly); + G1 sparse_commit_result = key->commit_sparse(poly); + + EXPECT_EQ(sparse_commit_result, commit_result); +} + } // namespace bb