-
Notifications
You must be signed in to change notification settings - Fork 615
feat: simple sparse commitment #7488
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
Changes from 11 commits
7811d58
958a682
c62842b
6e940fd
170ac63
c8055e0
2cdd476
e8c3002
5b3926a
df99d1d
0696f08
75fbea8
a339ff5
9fecf59
d89771f
ff801fe
6139b4a
cd3ed2f
e503e55
1789be2
982480d
199faca
3ee1bab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,31 +1,159 @@ | ||
|
|
||
| #include "barretenberg/commitment_schemes/commitment_key.hpp" | ||
| #include "barretenberg/common/zip_view.hpp" | ||
| #include "barretenberg/srs/factories/mem_bn254_crs_factory.hpp" | ||
| #include <algorithm> | ||
| #include <benchmark/benchmark.h> | ||
| #include <iostream> | ||
| #include <ranges> | ||
| #include <vector> | ||
|
|
||
| namespace bb { | ||
|
|
||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This suite was previously just benchmarking committing to zero polynomials of various sizes. I'm assuming it was just a WiP and never used but who knows. I've updated it to include a number of different scenarios, including committing various types of sparse polynomials with the traditional |
||
| template <typename Curve> std::shared_ptr<CommitmentKey<Curve>> create_commitment_key(const size_t num_points) | ||
| { | ||
| bb::srs::init_crs_factory("../srs_db/ignition"); | ||
| std::string srs_path; | ||
| return std::make_shared<CommitmentKey<Curve>>(num_points); | ||
| } | ||
|
|
||
| constexpr size_t MAX_LOG_NUM_POINTS = 24; | ||
| constexpr size_t MAX_NUM_POINTS = 1 << MAX_LOG_NUM_POINTS; | ||
| template <typename FF> Polynomial<FF> sparse_random_poly(const size_t size, const size_t num_nonzero) | ||
| { | ||
| auto polynomial = Polynomial<FF>(size); | ||
|
|
||
| for (size_t i = 0; i < num_nonzero; i++) { | ||
| polynomial[i] = FF::random_element(); | ||
| } | ||
|
|
||
| auto key = create_commitment_key<curve::BN254>(MAX_NUM_POINTS); | ||
| return polynomial; | ||
| } | ||
|
|
||
| template <typename Curve> void bench_commit(::benchmark::State& state) | ||
| template <typename Curve> | ||
| std::vector<typename Curve::AffineElement> extract_srs(std::shared_ptr<CommitmentKey<Curve>> commitment_key, | ||
| const size_t num_points) | ||
| { | ||
| using G1 = typename Curve::AffineElement; | ||
| std::vector<G1> monomials; | ||
| size_t idx = 0; | ||
| std::span<G1> point_table(commitment_key->srs->get_monomial_points(), 2 * num_points); | ||
|
|
||
| for (auto& element : point_table) { | ||
| if (idx % 2 == 0) { | ||
| monomials.emplace_back(element); | ||
| } | ||
| idx++; | ||
| if (monomials.size() == num_points) { | ||
| break; | ||
| } | ||
| } | ||
| return monomials; | ||
| } | ||
|
|
||
| constexpr size_t MAX_LOG_NUM_POINTS = 18; | ||
| constexpr size_t MAX_NUM_POINTS = 1 << MAX_LOG_NUM_POINTS; | ||
|
Comment on lines
+30
to
+31
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW we prob do care about 2^24 since that's the ClientIVC recursive verifier size right now.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah easy enough to change for an interested party but I was running these a lot and anything larger was taking too long |
||
|
|
||
| template <typename Curve> void bench_commit_zero(::benchmark::State& state) | ||
| { | ||
| auto key = create_commitment_key<Curve>(MAX_NUM_POINTS); | ||
|
|
||
| const size_t num_points = 1 << state.range(0); | ||
| const auto polynomial = Polynomial<typename Curve::ScalarField>(num_points); | ||
| for (auto _ : state) { | ||
| benchmark::DoNotOptimize(key->commit(polynomial)); | ||
| key->commit(polynomial); | ||
| } | ||
| } | ||
|
|
||
| template <typename Curve> void bench_commit_sparse(::benchmark::State& state) | ||
| { | ||
| using Fr = typename Curve::ScalarField; | ||
| auto key = create_commitment_key<Curve>(MAX_NUM_POINTS); | ||
|
|
||
| const size_t num_points = 1 << state.range(0); | ||
| const size_t num_nonzero = 2; | ||
|
|
||
| auto polynomial = Polynomial<Fr>(num_points); | ||
| for (size_t i = 0; i < num_nonzero; i++) { | ||
| polynomial[i] = 1; | ||
| } | ||
|
|
||
| for (auto _ : state) { | ||
| key->commit(polynomial); | ||
| } | ||
| } | ||
|
|
||
| template <typename Curve> void bench_commit_sparse_preprocessed(::benchmark::State& state) | ||
| { | ||
| using Fr = typename Curve::ScalarField; | ||
| auto key = create_commitment_key<Curve>(MAX_NUM_POINTS); | ||
|
|
||
| const size_t num_points = 1 << state.range(0); | ||
| const size_t num_nonzero = 2; | ||
|
|
||
| auto polynomial = Polynomial<Fr>(num_points); | ||
| for (size_t i = 0; i < num_nonzero; i++) { | ||
| polynomial[i] = 1; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe a follow-on question: if these are at random locations, do we have the same performance?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the random sparse polys (i.e. the ones for which I was showing bench results) the locations are randomized, but I did not see a difference before and after randomizing the locations. |
||
| } | ||
|
|
||
| for (auto _ : state) { | ||
| key->commit_sparse(polynomial); | ||
| } | ||
| } | ||
|
|
||
| template <typename Curve> void bench_commit_sparse_random(::benchmark::State& state) | ||
| { | ||
| using Fr = typename Curve::ScalarField; | ||
| auto key = create_commitment_key<Curve>(MAX_NUM_POINTS); | ||
|
|
||
| const size_t num_points = 1 << state.range(0); | ||
| const size_t num_nonzero = 5; | ||
|
|
||
| auto polynomial = sparse_random_poly<Fr>(num_points, num_nonzero); | ||
|
|
||
| for (auto _ : state) { | ||
| key->commit(polynomial); | ||
| } | ||
| } | ||
|
|
||
| template <typename Curve> void bench_commit_sparse_random_preprocessed(::benchmark::State& state) | ||
| { | ||
| using Fr = typename Curve::ScalarField; | ||
| auto key = create_commitment_key<Curve>(MAX_NUM_POINTS); | ||
|
|
||
| const size_t num_points = 1 << state.range(0); | ||
| const size_t num_nonzero = 5; | ||
|
|
||
| auto polynomial = sparse_random_poly<Fr>(num_points, num_nonzero); | ||
|
|
||
| for (auto _ : state) { | ||
| key->commit_sparse(polynomial); | ||
| } | ||
| } | ||
|
|
||
| template <typename Curve> void bench_commit_random(::benchmark::State& state) | ||
| { | ||
| using Fr = typename Curve::ScalarField; | ||
| auto key = create_commitment_key<Curve>(MAX_NUM_POINTS); | ||
|
|
||
| const size_t num_points = 1 << state.range(0); | ||
| auto polynomial = Polynomial<Fr>(num_points); | ||
| for (auto& coeff : polynomial) { | ||
| coeff = Fr::random_element(); | ||
| } | ||
| for (auto _ : state) { | ||
| key->commit(polynomial); | ||
| } | ||
| } | ||
|
|
||
| BENCHMARK(bench_commit<curve::BN254>)->DenseRange(10, MAX_LOG_NUM_POINTS)->Unit(benchmark::kMillisecond); | ||
| BENCHMARK(bench_commit_zero<curve::BN254>)->DenseRange(14, MAX_LOG_NUM_POINTS)->Unit(benchmark::kMillisecond); | ||
| BENCHMARK(bench_commit_sparse<curve::BN254>)->DenseRange(14, MAX_LOG_NUM_POINTS)->Unit(benchmark::kMillisecond); | ||
| BENCHMARK(bench_commit_sparse_preprocessed<curve::BN254>) | ||
| ->DenseRange(14, MAX_LOG_NUM_POINTS) | ||
| ->Unit(benchmark::kMillisecond); | ||
| BENCHMARK(bench_commit_sparse_random<curve::BN254>)->DenseRange(14, MAX_LOG_NUM_POINTS)->Unit(benchmark::kMillisecond); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NB the weird asymptotics including a sudden huge jump. |
||
| BENCHMARK(bench_commit_sparse_random_preprocessed<curve::BN254>) | ||
| ->DenseRange(14, MAX_LOG_NUM_POINTS) | ||
| ->Unit(benchmark::kMillisecond); | ||
| BENCHMARK(bench_commit_random<curve::BN254>)->DenseRange(14, MAX_LOG_NUM_POINTS)->Unit(benchmark::kMillisecond); | ||
|
|
||
| } // namespace bb | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -18,6 +18,7 @@ | |||
|
|
||||
| #include <cstddef> | ||||
| #include <memory> | ||||
| #include <ranges> | ||||
| #include <string_view> | ||||
|
|
||||
| namespace bb { | ||||
|
|
@@ -34,6 +35,7 @@ template <class Curve> class CommitmentKey { | |||
|
|
||||
| using Fr = typename Curve::ScalarField; | ||||
| using Commitment = typename Curve::AffineElement; | ||||
| using G1 = typename Curve::AffineElement; | ||||
|
|
||||
| public: | ||||
| scalar_multiplication::pippenger_runtime_state<Curve> pippenger_runtime_state; | ||||
|
|
@@ -81,6 +83,47 @@ template <class Curve> class CommitmentKey { | |||
| return scalar_multiplication::pippenger_unsafe<Curve>( | ||||
| const_cast<Fr*>(polynomial.data()), srs->get_monomial_points(), degree, pippenger_runtime_state); | ||||
| }; | ||||
|
|
||||
| Commitment commit_sparse(std::span<const Fr> polynomial) | ||||
| { | ||||
| // BB_OP_COUNT_TIME(); | ||||
| const size_t degree = polynomial.size(); | ||||
| ASSERT(degree <= srs->get_monomial_size()); | ||||
|
|
||||
| G1* point_table = srs->get_monomial_points(); | ||||
|
|
||||
| std::vector<Fr> scalars; | ||||
| std::vector<G1> points; | ||||
|
|
||||
| const size_t num_threads = degree >= get_num_cpus_pow2() ? get_num_cpus_pow2() : 1; | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is weird for small degree but we don't care in practice?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIUC these shouldn't (aren't) the number of threads, just the number of things you need to iterate. If you look closely at the current implementation of aztec-packages/barretenberg/cpp/src/barretenberg/common/parallel_for_mutex_pool.cpp Line 124 in 6035595
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess I'm a little unclear what your suggestion is here. Is your objection about the name
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What I mean is that the value passed to Maybe the way you set it up does make it coincide with the number of threads though. @ludamad would be the best to ask. I'm commenting because I had to use
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah I see what you mean. In my case the "num_iterations" input will always be <= actual num threads but your point stands. I don't really love that - seems like the parallel_for interface should allow you to specify how it should multithread. |
||||
| const size_t block_size = degree / num_threads; | ||||
|
|
||||
| std::vector<std::vector<Fr>> thread_scalars(num_threads); | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My above comment also means that you might get fewer real threads than the number of elements. IIUC, this means that your division of vectors here is still thread-safe, but less efficient than you might think (i.e., not one "bucket" per thread). |
||||
| std::vector<std::vector<G1>> thread_points(num_threads); | ||||
|
|
||||
| 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; | ||||
|
|
||||
| for (size_t idx = start; idx < end; ++idx) { | ||||
|
|
||||
| const G1& point = point_table[idx * 2]; | ||||
| const Fr& scalar = polynomial[idx]; | ||||
| if (!scalar.is_zero()) { | ||||
| thread_scalars[thread_idx].emplace_back(scalar); | ||||
| thread_points[thread_idx].emplace_back(point); | ||||
| } | ||||
| } | ||||
| }); | ||||
|
|
||||
| for (size_t idx = 0; idx < num_threads; ++idx) { | ||||
| scalars.insert(scalars.end(), thread_scalars[idx].begin(), thread_scalars[idx].end()); | ||||
| points.insert(points.end(), thread_points[idx].begin(), thread_points[idx].end()); | ||||
| } | ||||
|
|
||||
| return scalar_multiplication::pippenger_without_endomorphism_basis_points<Curve>( | ||||
| scalars.data(), points.data(), scalars.size(), pippenger_runtime_state); | ||||
| } | ||||
| }; | ||||
|
|
||||
| } // namespace bb | ||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| #include "barretenberg/commitment_schemes/commitment_key.hpp" | ||
| #include "barretenberg/polynomials/polynomial.hpp" | ||
| #include "barretenberg/srs/factories/file_crs_factory.hpp" | ||
|
|
||
| #include <gtest/gtest.h> | ||
|
|
||
| namespace bb { | ||
|
|
||
| template <typename Curve> class CommitmentKeyTest : public ::testing::Test { | ||
| using CK = CommitmentKey<Curve>; | ||
|
|
||
| using Fr = typename Curve::ScalarField; | ||
| using Commitment = typename Curve::AffineElement; | ||
| using Polynomial = bb::Polynomial<Fr>; | ||
|
|
||
| public: | ||
| template <class CK> inline std::shared_ptr<CK> create_commitment_key(size_t num_points); | ||
| }; | ||
|
|
||
| template <> | ||
| template <> | ||
| std::shared_ptr<CommitmentKey<curve::BN254>> CommitmentKeyTest<curve::BN254>::create_commitment_key< | ||
| CommitmentKey<curve::BN254>>(const size_t num_points) | ||
| { | ||
| srs::init_crs_factory("../srs_db/ignition"); | ||
| return std::make_shared<CommitmentKey<curve::BN254>>(num_points); | ||
| } | ||
|
|
||
| template <> | ||
| template <> | ||
| std::shared_ptr<CommitmentKey<curve::Grumpkin>> CommitmentKeyTest<curve::Grumpkin>::create_commitment_key< | ||
| CommitmentKey<curve::Grumpkin>>(const size_t num_points) | ||
| { | ||
| srs::init_grumpkin_crs_factory("../srs_db/grumpkin"); | ||
| return std::make_shared<CommitmentKey<curve::Grumpkin>>(num_points); | ||
| } | ||
|
|
||
| using Curves = ::testing::Types<curve::BN254, curve::Grumpkin>; | ||
|
|
||
| TYPED_TEST_SUITE(CommitmentKeyTest, Curves); | ||
|
|
||
| // Check that commit and commit_sparse return the same result for a random sparse polynomial | ||
| TYPED_TEST(CommitmentKeyTest, CommitSparse) | ||
| { | ||
| using Curve = TypeParam; | ||
| using CK = CommitmentKey<Curve>; | ||
| using G1 = Curve::AffineElement; | ||
| using Fr = Curve::ScalarField; | ||
| using Polynomial = bb::Polynomial<Fr>; | ||
|
|
||
| const size_t num_points = 1 << 10; | ||
| const size_t num_nonzero = 7; | ||
|
|
||
| // Construct a sparse random polynomial | ||
| Polynomial poly{ num_points }; | ||
| for (size_t i = 0; i < num_nonzero; ++i) { | ||
| size_t idx = (i + 1) * (i + 1) % num_points; | ||
| poly[idx] = 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<CK>(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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was logic previously used only to process bench data for the Relations but I wanted to reuse it for commitments so I just made it a method