Skip to content
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

refactor: clean out prover instance and remove instance from oink #5314

Merged
merged 19 commits into from
Mar 22, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
d132a10
moved sorted_polynomials to proving key
lucasxia01 Mar 19, 2024
eec17c6
make prover polys constructor from proving key
lucasxia01 Mar 20, 2024
45c5ee2
eliminated initialize_prover_polynomials
lucasxia01 Mar 20, 2024
810d369
moved compute_sorted_list_accumulator to proving_key
lucasxia01 Mar 20, 2024
e76c972
test fix
lucasxia01 Mar 20, 2024
6078832
moved compute_sorted_accumulator_polynomials and made updates to rela…
lucasxia01 Mar 20, 2024
b0855d5
test fix (prover polys wasn't being updated correctly)
lucasxia01 Mar 20, 2024
f4aa979
add OinkProverOutput and make one prove() function
lucasxia01 Mar 21, 2024
1f4e307
proverpolynomials now is just a temporary object during oink
lucasxia01 Mar 21, 2024
3fe93cc
Merge remote-tracking branch 'origin/master' into lx/refactor-prover-…
lucasxia01 Mar 21, 2024
2798eaf
moved compute_logderivative_inverse() from instance to proving_key
lucasxia01 Mar 21, 2024
4030744
moved compute_grand_product_polynomials from instance to proving key
lucasxia01 Mar 21, 2024
e11bb7e
removed the last instance reference that is non instance->proving_key!
lucasxia01 Mar 21, 2024
b4c4354
oink says bye bye to instance 👋
lucasxia01 Mar 21, 2024
2b67229
small fixes to tests
lucasxia01 Mar 21, 2024
abd0bd7
removed stupid shared_ptr calling copy constructor of proving_key (ad…
lucasxia01 Mar 21, 2024
8923c8c
deleting copy constructor of proving key to prevent copying
lucasxia01 Mar 22, 2024
c425e3d
updated based on comments
lucasxia01 Mar 22, 2024
25f870e
Merge branch 'master' into lx/refactor-prover-instance-2
lucasxia01 Mar 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion barretenberg/cpp/scripts/analyze_client_ivc_bench.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
"ProtogalaxyProver::fold_instances(t)",
"Decider::construct_proof(t)",
"ECCVMComposer::create_prover(t)",
"GoblinTranslatorComposer::create_prover(t)",
"ECCVMProver::construct_proof(t)",
"GoblinTranslatorProver::construct_proof(t)",
"Goblin::merge(t)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ BB_PROFILE static void test_round_inner(State& state, GoblinUltraProver& prover,
time_if_index(SORTED_LIST_ACCUMULATOR, [&] { prover.oink_prover.execute_sorted_list_accumulator_round(); });
time_if_index(LOG_DERIVATIVE_INVERSE, [&] { prover.oink_prover.execute_log_derivative_inverse_round(); });
time_if_index(GRAND_PRODUCT_COMPUTATION, [&] { prover.oink_prover.execute_grand_product_computation_round(); });
// we need to get the relation_parameters and prover_polynomials from the oink_prover
prover.instance->relation_parameters = prover.oink_prover.relation_parameters;
prover.instance->prover_polynomials = GoblinUltraFlavor::ProverPolynomials(prover.instance->proving_key);
time_if_index(RELATION_CHECK, [&] { prover.execute_relation_check_rounds(); });
time_if_index(ZEROMORPH, [&] { prover.execute_zeromorph_rounds(); });
}
Expand Down
1 change: 1 addition & 0 deletions barretenberg/cpp/src/barretenberg/flavor/flavor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ class ProvingKey_ : public PrecomputedPolynomials, public WitnessPolynomials {
std::vector<uint32_t> recursive_proof_public_input_indices;
bb::EvaluationDomain<FF> evaluation_domain;
std::shared_ptr<CommitmentKey_> commitment_key;
std::array<Polynomial, 4> sorted_polynomials;
lucasxia01 marked this conversation as resolved.
Show resolved Hide resolved

// offset due to placing zero wires at the start of execution trace
// non-zero for Instances constructed from circuits, this concept doesn't exist for accumulated
Expand Down
137 changes: 137 additions & 0 deletions barretenberg/cpp/src/barretenberg/flavor/goblin_ultra.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
#include "barretenberg/honk/proof_system/types/proof.hpp"
#include "barretenberg/polynomials/univariate.hpp"
#include "barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp"
#include "barretenberg/proof_system/library/grand_product_delta.hpp"
#include "barretenberg/proof_system/library/grand_product_library.hpp"
#include "barretenberg/relations/auxiliary_relation.hpp"
#include "barretenberg/relations/databus_lookup_relation.hpp"
#include "barretenberg/relations/ecc_op_queue_relation.hpp"
Expand Down Expand Up @@ -276,6 +278,119 @@ class GoblinUltraFlavor {
};
// The plookup wires that store plookup read data.
auto get_table_column_wires() { return RefArray{ w_l, w_r, w_o }; };

/**
* @brief Construct sorted list accumulator polynomial 's'.
*
* @details Compute s = s_1 + η*s_2 + η²*s_3 + η³*s_4 (via Horner) where s_i are the
* sorted concatenated witness/table polynomials
*
* @param key proving key
* @param sorted_list_polynomials sorted concatenated witness/table polynomials
* @param eta random challenge
* @return Polynomial
*/
void compute_sorted_list_accumulator(FF eta)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copied from instnace

{
const size_t circuit_size = this->circuit_size;

auto sorted_list_accumulator = Polynomial{ circuit_size };

// Construct s via Horner, i.e. s = s_1 + η(s_2 + η(s_3 + η*s_4))
for (size_t i = 0; i < circuit_size; ++i) {
FF T0 = this->sorted_polynomials[3][i];
T0 *= eta;
T0 += this->sorted_polynomials[2][i];
T0 *= eta;
T0 += this->sorted_polynomials[1][i];
T0 *= eta;
T0 += this->sorted_polynomials[0][i];
sorted_list_accumulator[i] = T0;
}
this->sorted_accum = sorted_list_accumulator.share();
}

void compute_sorted_accumulator_polynomials(FF eta)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mostly copied from instance, removing some unnecessary relation parameter and prover poly stuff

{
// Compute sorted witness-table accumulator
this->compute_sorted_list_accumulator(eta);

// Finalize fourth wire polynomial by adding lookup memory records
add_plookup_memory_records_to_wire_4(eta);
}

/**
* @brief Add plookup memory records to the fourth wire polynomial
*
* @details This operation must be performed after the first three wires have been committed to, hence the
* dependence on the `eta` challenge.
*
* @tparam Flavor
* @param eta challenge produced after commitment to first three wire polynomials
*/
void add_plookup_memory_records_to_wire_4(FF eta)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copied from instance

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can also pass eta by const&

{
// The plookup memory record values are computed at the indicated indices as
// w4 = w3 * eta^3 + w2 * eta^2 + w1 * eta + read_write_flag;
// (See plookup_auxiliary_widget.hpp for details)
auto wires = this->get_wires();

// Compute read record values
for (const auto& gate_idx : this->memory_read_records) {
wires[3][gate_idx] += wires[2][gate_idx];
wires[3][gate_idx] *= eta;
wires[3][gate_idx] += wires[1][gate_idx];
wires[3][gate_idx] *= eta;
wires[3][gate_idx] += wires[0][gate_idx];
wires[3][gate_idx] *= eta;
}

// Compute write record values
for (const auto& gate_idx : this->memory_write_records) {
wires[3][gate_idx] += wires[2][gate_idx];
wires[3][gate_idx] *= eta;
wires[3][gate_idx] += wires[1][gate_idx];
wires[3][gate_idx] *= eta;
wires[3][gate_idx] += wires[0][gate_idx];
wires[3][gate_idx] *= eta;
wires[3][gate_idx] += 1;
}
}

/**
* @brief Compute the inverse polynomial used in the log derivative lookup argument
*
* @tparam Flavor
* @param beta
* @param gamma
*/
void compute_logderivative_inverse(RelationParameters<FF>& relation_parameters)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: I made the parameters a relation_parameters reference, instead of just beta and gamma

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const too yeah?

{
auto prover_polynomials = ProverPolynomials(this);
// Compute permutation and lookup grand product polynomials
bb::compute_logderivative_inverse<GoblinUltraFlavor, typename GoblinUltraFlavor::LogDerivLookupRelation>(
prover_polynomials, relation_parameters, this->circuit_size);
this->lookup_inverses = prover_polynomials.lookup_inverses;
}

void compute_grand_product_polynomials(RelationParameters<FF>& relation_parameters)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: I made the parameters a relation_parameters reference, instead of just beta and gamma. We modify relation_parameters in this function

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, probably needs a comment

{
auto public_input_delta = compute_public_input_delta<GoblinUltraFlavor>(this->public_inputs,
relation_parameters.beta,
relation_parameters.gamma,
this->circuit_size,
this->pub_inputs_offset);
relation_parameters.public_input_delta = public_input_delta;
auto lookup_grand_product_delta = compute_lookup_grand_product_delta(
relation_parameters.beta, relation_parameters.gamma, this->circuit_size);
relation_parameters.lookup_grand_product_delta = lookup_grand_product_delta;

// Compute permutation and lookup grand product polynomials
auto prover_polynomials = ProverPolynomials(this);
compute_grand_products<GoblinUltraFlavor>(*this, prover_polynomials, relation_parameters);
this->z_perm = prover_polynomials.z_perm;
this->z_lookup = prover_polynomials.z_lookup;
}
};

/**
Expand Down Expand Up @@ -331,6 +446,28 @@ class GoblinUltraFlavor {
*/
class ProverPolynomials : public AllEntities<Polynomial> {
public:
ProverPolynomials(ProvingKey* proving_key)
{
for (auto [prover_poly, key_poly] : zip_view(this->get_unshifted(), proving_key->get_all())) {
ASSERT(flavor_get_label(*this, prover_poly) == flavor_get_label(*proving_key, key_poly));
prover_poly = key_poly.share();
}
for (auto [prover_poly, key_poly] : zip_view(this->get_shifted(), proving_key->get_to_be_shifted())) {
ASSERT(flavor_get_label(*this, prover_poly) == (flavor_get_label(*proving_key, key_poly) + "_shift"));
prover_poly = key_poly.shifted();
}
}
ProverPolynomials(std::shared_ptr<ProvingKey>& proving_key)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new prover polynomials constructor from proving key (same as in ultra.hpp)

{
for (auto [prover_poly, key_poly] : zip_view(this->get_unshifted(), proving_key->get_all())) {
ASSERT(flavor_get_label(*this, prover_poly) == flavor_get_label(*proving_key, key_poly));
prover_poly = key_poly.share();
}
for (auto [prover_poly, key_poly] : zip_view(this->get_shifted(), proving_key->get_to_be_shifted())) {
ASSERT(flavor_get_label(*this, prover_poly) == (flavor_get_label(*proving_key, key_poly) + "_shift"));
prover_poly = key_poly.shifted();
}
}
// Define all operations as default, except move construction/assignment
ProverPolynomials() = default;
ProverPolynomials& operator=(const ProverPolynomials&) = delete;
Expand Down
122 changes: 122 additions & 0 deletions barretenberg/cpp/src/barretenberg/flavor/ultra.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,14 @@
#include "barretenberg/polynomials/polynomial.hpp"
#include "barretenberg/polynomials/univariate.hpp"
#include "barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp"
#include "barretenberg/proof_system/library/grand_product_delta.hpp"
#include "barretenberg/proof_system/library/grand_product_library.hpp"
#include "barretenberg/relations/auxiliary_relation.hpp"
#include "barretenberg/relations/elliptic_relation.hpp"
#include "barretenberg/relations/gen_perm_sort_relation.hpp"
#include "barretenberg/relations/lookup_relation.hpp"
#include "barretenberg/relations/permutation_relation.hpp"
#include "barretenberg/relations/relation_parameters.hpp"
#include "barretenberg/relations/ultra_arithmetic_relation.hpp"
#include "barretenberg/transcript/transcript.hpp"

Expand Down Expand Up @@ -280,6 +283,103 @@ class UltraFlavor {
};
// The plookup wires that store plookup read data.
auto get_table_column_wires() { return RefArray{ w_l, w_r, w_o }; };

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these functions here should be identical to the ones in GoblinUltraFlavor

/**
* @brief Construct sorted list accumulator polynomial 's'.
*
* @details Compute s = s_1 + η*s_2 + η²*s_3 + η³*s_4 (via Horner) where s_i are the
* sorted concatenated witness/table polynomials
*
* @param key proving key
* @param sorted_list_polynomials sorted concatenated witness/table polynomials
* @param eta random challenge
* @return Polynomial
*/
void compute_sorted_list_accumulator(FF eta)
{
const size_t circuit_size = this->circuit_size;

auto sorted_list_accumulator = Polynomial{ circuit_size };

// Construct s via Horner, i.e. s = s_1 + η(s_2 + η(s_3 + η*s_4))
for (size_t i = 0; i < circuit_size; ++i) {
FF T0 = this->sorted_polynomials[3][i];
T0 *= eta;
T0 += this->sorted_polynomials[2][i];
T0 *= eta;
T0 += this->sorted_polynomials[1][i];
T0 *= eta;
T0 += this->sorted_polynomials[0][i];
sorted_list_accumulator[i] = T0;
}
this->sorted_accum = sorted_list_accumulator.share();
}

void compute_sorted_accumulator_polynomials(FF eta)
{
// Compute sorted witness-table accumulator
this->compute_sorted_list_accumulator(eta);

// Finalize fourth wire polynomial by adding lookup memory records
add_plookup_memory_records_to_wire_4(eta);
}

/**
* @brief Add plookup memory records to the fourth wire polynomial
*
* @details This operation must be performed after the first three wires have been committed to, hence the
* dependence on the `eta` challenge.
*
* @tparam Flavor
* @param eta challenge produced after commitment to first three wire polynomials
*/
void add_plookup_memory_records_to_wire_4(FF eta)
{
// The plookup memory record values are computed at the indicated indices as
// w4 = w3 * eta^3 + w2 * eta^2 + w1 * eta + read_write_flag;
// (See plookup_auxiliary_widget.hpp for details)
auto wires = this->get_wires();

// Compute read record values
for (const auto& gate_idx : this->memory_read_records) {
wires[3][gate_idx] += wires[2][gate_idx];
wires[3][gate_idx] *= eta;
wires[3][gate_idx] += wires[1][gate_idx];
wires[3][gate_idx] *= eta;
wires[3][gate_idx] += wires[0][gate_idx];
wires[3][gate_idx] *= eta;
}

// Compute write record values
for (const auto& gate_idx : this->memory_write_records) {
wires[3][gate_idx] += wires[2][gate_idx];
wires[3][gate_idx] *= eta;
wires[3][gate_idx] += wires[1][gate_idx];
wires[3][gate_idx] *= eta;
wires[3][gate_idx] += wires[0][gate_idx];
wires[3][gate_idx] *= eta;
wires[3][gate_idx] += 1;
}
}

void compute_grand_product_polynomials(RelationParameters<FF>& relation_parameters)
{
auto public_input_delta = compute_public_input_delta<UltraFlavor>(this->public_inputs,
relation_parameters.beta,
relation_parameters.gamma,
this->circuit_size,
this->pub_inputs_offset);
relation_parameters.public_input_delta = public_input_delta;
auto lookup_grand_product_delta = compute_lookup_grand_product_delta(
relation_parameters.beta, relation_parameters.gamma, this->circuit_size);
relation_parameters.lookup_grand_product_delta = lookup_grand_product_delta;

// Compute permutation and lookup grand product polynomials
auto prover_polynomials = ProverPolynomials(this);
compute_grand_products<UltraFlavor>(*this, prover_polynomials, relation_parameters);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with the exception of UltraFlavor here (and above in compute_public_input_delta ) instead of GoblinUltraFlavor

this->z_perm = prover_polynomials.z_perm;
this->z_lookup = prover_polynomials.z_lookup;
}
};

/**
Expand Down Expand Up @@ -307,6 +407,28 @@ class UltraFlavor {
*/
class ProverPolynomials : public AllEntities<Polynomial> {
public:
ProverPolynomials(ProvingKey* proving_key)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe should just be a reference here (maybe even const?)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah esp once ProvingKey is not handled through a shared pointer

{
for (auto [prover_poly, key_poly] : zip_view(this->get_unshifted(), proving_key->get_all())) {
ASSERT(flavor_get_label(*this, prover_poly) == flavor_get_label(*proving_key, key_poly));
prover_poly = key_poly.share();
}
for (auto [prover_poly, key_poly] : zip_view(this->get_shifted(), proving_key->get_to_be_shifted())) {
ASSERT(flavor_get_label(*this, prover_poly) == (flavor_get_label(*proving_key, key_poly) + "_shift"));
prover_poly = key_poly.shifted();
}
}
ProverPolynomials(std::shared_ptr<ProvingKey>& proving_key)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ideally, this one goes away as we destroy shared ptr usage.

{
for (auto [prover_poly, key_poly] : zip_view(this->get_unshifted(), proving_key->get_all())) {
ASSERT(flavor_get_label(*this, prover_poly) == flavor_get_label(*proving_key, key_poly));
prover_poly = key_poly.share();
}
for (auto [prover_poly, key_poly] : zip_view(this->get_shifted(), proving_key->get_to_be_shifted())) {
ASSERT(flavor_get_label(*this, prover_poly) == (flavor_get_label(*proving_key, key_poly) + "_shift"));
prover_poly = key_poly.shifted();
}
}
// Define all operations as default, except move construction/assignment
ProverPolynomials() = default;
ProverPolynomials& operator=(const ProverPolynomials&) = delete;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ void compute_grand_product(const size_t circuit_size,
}

template <typename Flavor>
void compute_grand_products(std::shared_ptr<typename Flavor::ProvingKey>& key,
void compute_grand_products(const typename Flavor::ProvingKey& key,
typename Flavor::ProverPolynomials& full_polynomials,
bb::RelationParameters<typename Flavor::FF>& relation_parameters)
{
Expand All @@ -157,10 +157,10 @@ void compute_grand_products(std::shared_ptr<typename Flavor::ProvingKey>& key,
// For example, for UltraPermutationRelation, this will be `full_polynomials.z_perm`
// For example, for LookupRelation, this will be `full_polynomials.z_lookup`
bb::Polynomial<FF>& full_polynomial = GrandProdRelation::get_grand_product_polynomial(full_polynomials);
auto& key_polynomial = GrandProdRelation::get_grand_product_polynomial(*key);
auto& key_polynomial = GrandProdRelation::get_grand_product_polynomial(key);
full_polynomial = key_polynomial.share();

compute_grand_product<Flavor, GrandProdRelation>(key->circuit_size, full_polynomials, relation_parameters);
compute_grand_product<Flavor, GrandProdRelation>(key.circuit_size, full_polynomials, relation_parameters);
bb::Polynomial<FF>& full_polynomial_shift =
GrandProdRelation::get_shifted_grand_product_polynomial(full_polynomials);
full_polynomial_shift = key_polynomial.shifted();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,16 +117,17 @@ template <typename Flavor> class ProtoGalaxyTests : public testing::Test {
construct_circuit(builder);

auto instance = std::make_shared<ProverInstance>(builder);
instance->initialize_prover_polynomials();

auto eta = FF::random_element();
auto beta = FF::random_element();
auto gamma = FF::random_element();
instance->compute_sorted_accumulator_polynomials(eta);
instance->relation_parameters.eta = FF::random_element();
instance->relation_parameters.beta = FF::random_element();
instance->relation_parameters.gamma = FF::random_element();

instance->proving_key->compute_sorted_accumulator_polynomials(instance->relation_parameters.eta);
if constexpr (IsGoblinFlavor<Flavor>) {
instance->compute_logderivative_inverse(beta, gamma);
instance->proving_key->compute_logderivative_inverse(instance->relation_parameters);
}
instance->compute_grand_product_polynomials(beta, gamma);
instance->proving_key->compute_grand_product_polynomials(instance->relation_parameters);
instance->prover_polynomials = ProverPolynomials(instance->proving_key);

for (auto& alpha : instance->alphas) {
alpha = FF::random_element();
Expand Down
Loading
Loading