From b658acb91f00e7f0e4d4c664b73714cca17feb31 Mon Sep 17 00:00:00 2001 From: Rumata888 Date: Sun, 12 May 2024 17:51:17 +0000 Subject: [PATCH 1/6] Small translator optimisations --- .../honk/proof_system/permutation_library.hpp | 21 +++++++++---------- .../translator_decomposition_relation.hpp | 5 +++++ .../translator_extra_relations.hpp | 8 +++++-- .../translator_non_native_field_relation.hpp | 4 ++++ .../goblin_translator_prover.cpp | 8 ++++--- 5 files changed, 30 insertions(+), 16 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/honk/proof_system/permutation_library.hpp b/barretenberg/cpp/src/barretenberg/honk/proof_system/permutation_library.hpp index 8ecb9e376ce4..53c46c48e9eb 100644 --- a/barretenberg/cpp/src/barretenberg/honk/proof_system/permutation_library.hpp +++ b/barretenberg/cpp/src/barretenberg/honk/proof_system/permutation_library.hpp @@ -38,21 +38,20 @@ template void compute_concatenated_polynomials(typename Flavor // A function that produces 1 concatenated polynomial // TODO(#756): This can be rewritten to use more cores. Currently uses at maximum the number of concatenated // polynomials (4 in Goblin Translator) - auto ordering_function = [&](size_t i) { + auto ordering_function = [&](size_t index) { + size_t i = index / concatenation_groups[0].size(); + size_t j = index % concatenation_groups[0].size(); auto my_group = concatenation_groups[i]; auto& current_target = targets[i]; - // For each polynomial in group - for (size_t j = 0; j < my_group.size(); j++) { - auto starting_write_offset = current_target.begin(); - auto finishing_read_offset = my_group[j].begin(); - std::advance(starting_write_offset, j * MINI_CIRCUIT_SIZE); - std::advance(finishing_read_offset, MINI_CIRCUIT_SIZE); - // Copy into appropriate position in the concatenated polynomial - std::copy(my_group[j].begin(), finishing_read_offset, starting_write_offset); - } + auto starting_write_offset = current_target.begin(); + auto finishing_read_offset = my_group[j].begin(); + std::advance(starting_write_offset, j * MINI_CIRCUIT_SIZE); + std::advance(finishing_read_offset, MINI_CIRCUIT_SIZE); + // Copy into appropriate position in the concatenated polynomial + std::copy(my_group[j].begin(), finishing_read_offset, starting_write_offset); }; - parallel_for(concatenation_groups.size(), ordering_function); + parallel_for(concatenation_groups.size() * concatenation_groups[0].size(), ordering_function); } /** diff --git a/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_decomposition_relation.hpp b/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_decomposition_relation.hpp index e79c88500629..68e5fb1e73d4 100644 --- a/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_decomposition_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_decomposition_relation.hpp @@ -62,6 +62,11 @@ template class GoblinTranslatorDecompositionRelationImpl { 3 // decomposition of z2 into 2 limbs subrelation }; + template inline static bool skip(const AllEntities& in) + { + return in.lagrange_odd_in_minicircuit.is_zero(); + } + /** * @brief Expression for decomposition of various values into smaller limbs or microlimbs. * @details This relation enforces three types of subrelations: diff --git a/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_extra_relations.hpp b/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_extra_relations.hpp index d4d927ab4ee6..3e7c2a7e2f70 100644 --- a/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_extra_relations.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_extra_relations.hpp @@ -12,7 +12,7 @@ template class GoblinTranslatorOpcodeConstraintRelationImpl { static constexpr std::array SUBRELATION_PARTIAL_LENGTHS{ 7 // opcode constraint relation }; - + template inline static bool skip(const AllEntities& in) { return in.op.is_zero(); } /** * @brief Expression for enforcing the value of the Opcode to be {0,1,2,3,4,8} * @details This relation enforces the opcode to be one of described values. Since we don't care about even @@ -52,7 +52,11 @@ template class GoblinTranslatorAccumulatorTransferRelationImpl { 3 // accumulator limb 3 is equal to given result at the end of accumulation subrelation }; - + template inline static bool skip(const AllEntities& in) + { + return (in.lagrange_even_in_minicircuit + in.lagrange_second_to_last_in_minicircuit + in.lagrange_second) + .is_zero(); + } /** * @brief Relation enforcing non-arithmetic transitions of accumulator (value that is tracking the batched * evaluation of polynomials in non-native field) diff --git a/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_non_native_field_relation.hpp b/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_non_native_field_relation.hpp index f2e988319d64..7f2e64afaf7a 100644 --- a/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_non_native_field_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_non_native_field_relation.hpp @@ -15,6 +15,10 @@ template class GoblinTranslatorNonNativeFieldRelationImpl { 3 // Prime subrelation (checks result in native field) }; + template inline static bool skip(const AllEntities& in) + { + return in.lagrange_odd_in_minicircuit.is_zero(); + } /** * @brief Expression for the computation of Goblin Translator accumulator in integers through 68-bit limbs and * native field (prime) limb diff --git a/barretenberg/cpp/src/barretenberg/translator_vm/goblin_translator_prover.cpp b/barretenberg/cpp/src/barretenberg/translator_vm/goblin_translator_prover.cpp index 52776acbe42d..9fe6babcf77a 100644 --- a/barretenberg/cpp/src/barretenberg/translator_vm/goblin_translator_prover.cpp +++ b/barretenberg/cpp/src/barretenberg/translator_vm/goblin_translator_prover.cpp @@ -35,9 +35,11 @@ void GoblinTranslatorProver::compute_witness(CircuitBuilder& circuit_builder) // Populate the wire polynomials from the wire vectors in the circuit constructor. Note: In goblin translator wires // come as is, since they have to reflect the structure of polynomials in the first 4 wires, which we've commited to for (auto [wire_poly, wire] : zip_view(key->polynomials.get_wires(), circuit_builder.wires)) { - for (size_t i = 0; i < circuit_builder.num_gates; ++i) { - wire_poly[i] = circuit_builder.get_variable(wire[i]); - } + run_loop_in_parallel(circuit_builder.num_gates, [&](size_t start, size_t end) { + for (size_t i = start; i < end; i++) { + wire_poly[i] = circuit_builder.get_variable(wire[i]); + } + }); } // We construct concatenated versions of range constraint polynomials, where several polynomials are concatenated From fa1d14c31e72c761798cfd841be52a06ed60a210 Mon Sep 17 00:00:00 2001 From: Rumata888 Date: Sun, 12 May 2024 21:25:27 +0000 Subject: [PATCH 2/6] Add comments --- .../translator_decomposition_relation.hpp | 4 ++++ .../translator_vm/translator_extra_relations.hpp | 10 ++++++++++ .../translator_non_native_field_relation.hpp | 4 ++++ 3 files changed, 18 insertions(+) diff --git a/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_decomposition_relation.hpp b/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_decomposition_relation.hpp index 68e5fb1e73d4..519f1a4e1196 100644 --- a/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_decomposition_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_decomposition_relation.hpp @@ -62,6 +62,10 @@ template class GoblinTranslatorDecompositionRelationImpl { 3 // decomposition of z2 into 2 limbs subrelation }; + /** + * @brief Returns true if the contribution from all subrelations for the provided inputs is identically zero + * + */ template inline static bool skip(const AllEntities& in) { return in.lagrange_odd_in_minicircuit.is_zero(); diff --git a/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_extra_relations.hpp b/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_extra_relations.hpp index 3e7c2a7e2f70..ca1059e01e93 100644 --- a/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_extra_relations.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_extra_relations.hpp @@ -12,6 +12,11 @@ template class GoblinTranslatorOpcodeConstraintRelationImpl { static constexpr std::array SUBRELATION_PARTIAL_LENGTHS{ 7 // opcode constraint relation }; + + /** + * @brief Returns true if the contribution from all subrelations for the provided inputs is identically zero + * + */ template inline static bool skip(const AllEntities& in) { return in.op.is_zero(); } /** * @brief Expression for enforcing the value of the Opcode to be {0,1,2,3,4,8} @@ -52,6 +57,11 @@ template class GoblinTranslatorAccumulatorTransferRelationImpl { 3 // accumulator limb 3 is equal to given result at the end of accumulation subrelation }; + + /** + * @brief Returns true if the contribution from all subrelations for the provided inputs is identically zero + * + */ template inline static bool skip(const AllEntities& in) { return (in.lagrange_even_in_minicircuit + in.lagrange_second_to_last_in_minicircuit + in.lagrange_second) diff --git a/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_non_native_field_relation.hpp b/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_non_native_field_relation.hpp index 7f2e64afaf7a..dc94e814ad3d 100644 --- a/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_non_native_field_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_non_native_field_relation.hpp @@ -15,6 +15,10 @@ template class GoblinTranslatorNonNativeFieldRelationImpl { 3 // Prime subrelation (checks result in native field) }; + /** + * @brief Returns true if the contribution from all subrelations for the provided inputs is identically zero + * + */ template inline static bool skip(const AllEntities& in) { return in.lagrange_odd_in_minicircuit.is_zero(); From 9534763a17ca38b9c62d19fa913e3b5764f89a9b Mon Sep 17 00:00:00 2001 From: Rumata888 Date: Sun, 12 May 2024 22:03:05 +0000 Subject: [PATCH 3/6] update --- .../relations/ecc_vm/ecc_lookup_relation.hpp | 9 +++++++++ .../translator_vm/translator_extra_relations.hpp | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/relations/ecc_vm/ecc_lookup_relation.hpp b/barretenberg/cpp/src/barretenberg/relations/ecc_vm/ecc_lookup_relation.hpp index fd89cbe58197..bec4a7059dfb 100644 --- a/barretenberg/cpp/src/barretenberg/relations/ecc_vm/ecc_lookup_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/ecc_vm/ecc_lookup_relation.hpp @@ -30,6 +30,15 @@ template class ECCVMLookupRelationImpl { return (row.msm_add == 1) || (row.msm_skew == 1) || (row.precompute_select == 1); } + /** + * @brief Returns true if the contribution from all subrelations for the provided inputs is identically zero + * + */ + template inline static bool skip(const AllEntities& in) + { + return in.precompute_select.is_zero() && in.msm_add.is_zero() && in.msm_skew.is_zero(); + } + /** * @brief Get the inverse lookup polynomial * diff --git a/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_extra_relations.hpp b/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_extra_relations.hpp index ca1059e01e93..10bd8bfd2215 100644 --- a/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_extra_relations.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_extra_relations.hpp @@ -64,8 +64,8 @@ template class GoblinTranslatorAccumulatorTransferRelationImpl { */ template inline static bool skip(const AllEntities& in) { - return (in.lagrange_even_in_minicircuit + in.lagrange_second_to_last_in_minicircuit + in.lagrange_second) - .is_zero(); + return in.lagrange_even_in_minicircuit.is_zero() && in.lagrange_second_to_last_in_minicircuit.is_zero() && + in.lagrange_second.is_zero(); } /** * @brief Relation enforcing non-arithmetic transitions of accumulator (value that is tracking the batched From 1d2ac7b8e552dfa5fbd266affc879da4d540f5a1 Mon Sep 17 00:00:00 2001 From: Rumata888 Date: Sun, 12 May 2024 22:06:47 +0000 Subject: [PATCH 4/6] comment --- .../barretenberg/honk/proof_system/permutation_library.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/honk/proof_system/permutation_library.hpp b/barretenberg/cpp/src/barretenberg/honk/proof_system/permutation_library.hpp index 53c46c48e9eb..a41b9193c079 100644 --- a/barretenberg/cpp/src/barretenberg/honk/proof_system/permutation_library.hpp +++ b/barretenberg/cpp/src/barretenberg/honk/proof_system/permutation_library.hpp @@ -36,8 +36,8 @@ template void compute_concatenated_polynomials(typename Flavor const size_t MINI_CIRCUIT_SIZE = targets[0].size() / Flavor::CONCATENATION_GROUP_SIZE; ASSERT(MINI_CIRCUIT_SIZE * Flavor::CONCATENATION_GROUP_SIZE == targets[0].size()); // A function that produces 1 concatenated polynomial - // TODO(#756): This can be rewritten to use more cores. Currently uses at maximum the number of concatenated - // polynomials (4 in Goblin Translator) + + // Uses the index of one of the polynomials in concatenation groups, which we copy in the concatenated polynomial auto ordering_function = [&](size_t index) { size_t i = index / concatenation_groups[0].size(); size_t j = index % concatenation_groups[0].size(); From 239401c790bc8ed10247d4d13c2e9d0377c0312f Mon Sep 17 00:00:00 2001 From: Rumata888 Date: Sun, 12 May 2024 22:25:16 +0000 Subject: [PATCH 5/6] Restore --- .../relations/ecc_vm/ecc_lookup_relation.hpp | 9 --------- .../translator_vm/translator_extra_relations.hpp | 6 ++++-- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/relations/ecc_vm/ecc_lookup_relation.hpp b/barretenberg/cpp/src/barretenberg/relations/ecc_vm/ecc_lookup_relation.hpp index bec4a7059dfb..fd89cbe58197 100644 --- a/barretenberg/cpp/src/barretenberg/relations/ecc_vm/ecc_lookup_relation.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/ecc_vm/ecc_lookup_relation.hpp @@ -30,15 +30,6 @@ template class ECCVMLookupRelationImpl { return (row.msm_add == 1) || (row.msm_skew == 1) || (row.precompute_select == 1); } - /** - * @brief Returns true if the contribution from all subrelations for the provided inputs is identically zero - * - */ - template inline static bool skip(const AllEntities& in) - { - return in.precompute_select.is_zero() && in.msm_add.is_zero() && in.msm_skew.is_zero(); - } - /** * @brief Get the inverse lookup polynomial * diff --git a/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_extra_relations.hpp b/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_extra_relations.hpp index 10bd8bfd2215..6ad68e3261d5 100644 --- a/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_extra_relations.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_extra_relations.hpp @@ -61,11 +61,13 @@ template class GoblinTranslatorAccumulatorTransferRelationImpl { /** * @brief Returns true if the contribution from all subrelations for the provided inputs is identically zero * + * @details This has a negligible chance of failing in sumcheck + * */ template inline static bool skip(const AllEntities& in) { - return in.lagrange_even_in_minicircuit.is_zero() && in.lagrange_second_to_last_in_minicircuit.is_zero() && - in.lagrange_second.is_zero(); + return (in.lagrange_even_in_minicircuit + in.lagrange_second_to_last_in_minicircuit + in.lagrange_second) + .is_zero(); } /** * @brief Relation enforcing non-arithmetic transitions of accumulator (value that is tracking the batched From acd65a9341fe09335b11c6c7f7598777f78d50c0 Mon Sep 17 00:00:00 2001 From: Rumata888 Date: Mon, 13 May 2024 12:42:13 +0000 Subject: [PATCH 6/6] Addressed Mara's comments --- .../barretenberg/honk/proof_system/permutation_library.hpp | 7 ++++++- .../relations/translator_vm/translator_extra_relations.hpp | 4 +++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/honk/proof_system/permutation_library.hpp b/barretenberg/cpp/src/barretenberg/honk/proof_system/permutation_library.hpp index a41b9193c079..bf2f8b5274e4 100644 --- a/barretenberg/cpp/src/barretenberg/honk/proof_system/permutation_library.hpp +++ b/barretenberg/cpp/src/barretenberg/honk/proof_system/permutation_library.hpp @@ -37,9 +37,14 @@ template void compute_concatenated_polynomials(typename Flavor ASSERT(MINI_CIRCUIT_SIZE * Flavor::CONCATENATION_GROUP_SIZE == targets[0].size()); // A function that produces 1 concatenated polynomial - // Uses the index of one of the polynomials in concatenation groups, which we copy in the concatenated polynomial + // Goblin Translator uses concatenated polynomials in the permutation argument. These polynomials contain the same + // coefficients as other shorter polynomials, but we don't have to commit to them due to reusing commitments of + // shorter polynomials and updating our PCS to open using them. But the prover still needs the concatenated + // polynomials. This function constructs a chunk of the polynomial. auto ordering_function = [&](size_t index) { + // Get the index of the concatenated polynomial size_t i = index / concatenation_groups[0].size(); + // Get the index of the original polynomial size_t j = index % concatenation_groups[0].size(); auto my_group = concatenation_groups[i]; auto& current_target = targets[i]; diff --git a/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_extra_relations.hpp b/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_extra_relations.hpp index 6ad68e3261d5..3d0805af27c0 100644 --- a/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_extra_relations.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/translator_vm/translator_extra_relations.hpp @@ -61,7 +61,9 @@ template class GoblinTranslatorAccumulatorTransferRelationImpl { /** * @brief Returns true if the contribution from all subrelations for the provided inputs is identically zero * - * @details This has a negligible chance of failing in sumcheck + * @details This has a negligible chance of failing in sumcheck (not in the first round) because effectively + * transfrom original coefficients into a random linear combination. But checking each individually is noticeably + * slower. * */ template inline static bool skip(const AllEntities& in)