Skip to content

Commit

Permalink
refactor(bb): small cleanup in protogalaxy prover (#8072)
Browse files Browse the repository at this point in the history
small cleanup in protogalaxy prover (makes the polynomial structure work
a bit cleaner), bit faster runtime (-50ms client ivc
ClientIVCBench/Full/6 with the simpler multithreading), bit faster
compilation (unmeasured but I put more in the impl file), bit safer
(catch nesting parallel_for, which if ever was done looked unsafe,
luckily we did not seem to make that mistake)

- small cleanup in protogalaxy prover, introduce
`_compute_vanishing_polynomial_and_lagranges` and make methods
out-of-line
- simplify two cases `compute_next_accumulator` to not use custom
parallelization but instead use polynomial methods
- ensure we don't accidentally nest parallel_for, which didn't look
correct under recursion (global state)
  • Loading branch information
ludamad authored Aug 19, 2024
1 parent 23778c5 commit 4cb5c83
Show file tree
Hide file tree
Showing 5 changed files with 337 additions and 283 deletions.
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#include "barretenberg/common/throw_or_abort.hpp"
#ifndef NO_MULTITHREADING
#include "log.hpp"
#include "thread.hpp"
Expand Down Expand Up @@ -124,10 +125,18 @@ namespace bb {
void parallel_for_mutex_pool(size_t num_iterations, const std::function<void(size_t)>& func)
{
static ThreadPool pool(get_num_cpus() - 1);

// Note that if this is used safely, we don't need the std::atomic_bool (can use bool), but if we are catching the
// mess up case of nesting parallel_for this should be atomic
static std::atomic_bool nested = false;
// Check if we are already in a nested parallel_for_mutex_pool call
bool expected = false;
if (!nested.compare_exchange_strong(expected, true)) {
throw_or_abort("Error: Nested parallel_for_mutex_pool calls are not allowed.");
}
// info("starting job with iterations: ", num_iterations);
pool.start_tasks(num_iterations, func);
// info("done");
nested = false;
}
} // namespace bb
#endif
103 changes: 53 additions & 50 deletions barretenberg/cpp/src/barretenberg/protogalaxy/combiner.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -327,53 +327,56 @@ TEST(Protogalaxy, CombinerOptimizationConsistency)
run_test(false);
};

TEST(Protogalaxy, CombinerOn4Instances)
{
constexpr size_t NUM_INSTANCES = 4;
using ProverInstance = ProverInstance_<Flavor>;
using ProverInstances = ProverInstances_<Flavor, NUM_INSTANCES>;
using ProtoGalaxyProver = ProtoGalaxyProver_<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 = prover.compute_combiner</*OptimisationEnabled=*/false>(instances, pow_polynomial);
auto optimised_result = prover.compute_combiner(instances, pow_polynomial);
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();
};
// 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, CombinerOn4Instances)
// {
// constexpr size_t NUM_INSTANCES = 4;
// using ProverInstance = ProverInstance_<Flavor>;
// using ProverInstances = ProverInstances_<Flavor, NUM_INSTANCES>;
// using ProtoGalaxyProver = ProtoGalaxyProver_<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 = prover.compute_combiner</*OptimisationEnabled=*/false>(instances, pow_polynomial);
// auto optimised_result = prover.compute_combiner(instances, pow_polynomial);
// 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();
// };
Loading

0 comments on commit 4cb5c83

Please sign in to comment.