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

feat: Small translator optimisations #6354

Merged
merged 6 commits into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -36,23 +36,22 @@ template <typename Flavor> 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)
auto ordering_function = [&](size_t i) {

// Uses the index of one of the polynomials in concatenation groups, which we copy in the concatenated polynomial
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a clearer explanation on what this function does and why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,15 @@ template <typename FF_> 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 <typename AllEntities> 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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ template <typename FF_> class GoblinTranslatorOpcodeConstraintRelationImpl {
7 // opcode constraint relation
};

/**
* @brief Returns true if the contribution from all subrelations for the provided inputs is identically zero
*
*/
template <typename AllEntities> 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
Expand Down Expand Up @@ -53,6 +58,17 @@ template <typename FF_> 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
Copy link
Contributor

Choose a reason for hiding this comment

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

why? :/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added an explanation

*
*/
template <typename AllEntities> 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ template <typename FF_> 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 <typename AllEntities> 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading