Skip to content

Commit

Permalink
chore: use RelationChecker in relation correctness tests and add Tran…
Browse files Browse the repository at this point in the history
…slator interleaving test (#11878)

The RelationChecker was introduced as a debugging utility in a previous
PR but was not actually used in relevant tests, leading to duplicated
code. This PR fixes that and aims to refine the check function in the
utility. It also includes refactoring of stale code and adds a small
sequential test that changing the interleaving strategy in translator
will not break the PermutationRelation and DeltaRangeConstraintRelation
(the two relations that now operate on the concatenated polynomials)
  • Loading branch information
maramihali authored Feb 12, 2025
1 parent f54db75 commit ed215e8
Show file tree
Hide file tree
Showing 10 changed files with 223 additions and 450 deletions.
5 changes: 0 additions & 5 deletions barretenberg/cpp/src/barretenberg/eccvm/eccvm_prover.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,6 @@ class ECCVMProver {
CommitmentLabels commitment_labels;
ZKData zk_sumcheck_data;

Polynomial batched_quotient_Q; // batched quotient poly computed by Shplonk
FF nu_challenge; // needed in both Shplonk rounds

Polynomial quotient_W;

FF evaluation_challenge_x;
FF translation_batching_challenge_v; // to be rederived by the translator verifier

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,29 +24,36 @@ template <typename Flavor> class RelationChecker {

/**
* @brief Check that a single specified relation is satisfied for a set of polynomials
*
* @tparam Relation a linearly independent Relation to be checked
* @param polynomials prover polynomials
* @param params a RelationParameters instance
*/
template <typename Relation>
static void check(const auto& polynomials,
const auto& params,
bool is_linearly_independent,
std::string label = "Relation")
template <typename Relation, bool has_linearly_dependent = false>
static void check(const auto& polynomials, const auto& params, std::string label = "Relation")
{

// Define the appropriate accumulator type for the relation and initialize to zero
typename Relation::SumcheckArrayOfValuesOverSubrelations result;
for (auto& element : result) {
element = 0;
}

for (size_t i = 0; i < polynomials.w_l.virtual_size(); i++) {
if (is_linearly_independent) {
// Evaluate each constraint in the relation and check that each is satisfied
Relation::accumulate(result, polynomials.get_row(i), params, 1);
size_t subrelation_idx = 0;
for (auto& element : result) {
for (size_t i = 0; i < polynomials.get_polynomial_size(); i++) {

Relation::accumulate(result, polynomials.get_row(i), params, 1);
size_t subrelation_idx = 0;

// Iterate over all the subrelation results and report if a linearly independent one failed
for (auto& element : result) {
if constexpr (has_linearly_dependent) {
if (element != 0 && Relation::SUBRELATION_LINEARLY_INDEPENDENT[subrelation_idx]) {
info("RelationChecker: ",
label,
" relation (subrelation idx: ",
subrelation_idx,
") failed at row idx: ",
i,
".");
ASSERT(false);
}
} else {
if (element != 0) {
info("RelationChecker: ",
label,
Expand All @@ -57,21 +64,27 @@ template <typename Flavor> class RelationChecker {
".");
ASSERT(false);
}
subrelation_idx++;
}
subrelation_idx++;
}
}

if (!is_linearly_independent) {
// Result accumulated across entire execution trace should be zero
if constexpr (has_linearly_dependent) {
size_t subrelation_idx = 0;
for (auto& element : result) {
if (element != 0) {
info("RelationChecker: ", label, " relation (linearly indep.) failed.");
// Check that linearly dependent subrelation result is 0 over the entire execution trace
if (element != 0 && Relation::SUBRELATION_LINEARLY_INDEPENDENT[subrelation_idx]) {
info("RelationChecker: ",
label,
" linearly dependent subrelation idx: ",
subrelation_idx,
" failed.");
ASSERT(false);
}
subrelation_idx++;
}
}
}
};
};

// Specialization for Ultra
Expand All @@ -84,16 +97,16 @@ template <> class RelationChecker<bb::UltraFlavor> : public RelationChecker<void
using FF = UltraFlavor::FF;

// Linearly independent relations (must be satisfied at each row)
Base::check<UltraArithmeticRelation<FF>>(polynomials, params, true, "UltraArithmetic");
Base::check<UltraPermutationRelation<FF>>(polynomials, params, true, "UltraPermutation");
Base::check<DeltaRangeConstraintRelation<FF>>(polynomials, params, true, "DeltaRangeConstraint");
Base::check<EllipticRelation<FF>>(polynomials, params, true, "Elliptic");
Base::check<AuxiliaryRelation<FF>>(polynomials, params, true, "Auxiliary");
Base::check<Poseidon2ExternalRelation<FF>>(polynomials, params, true, "Poseidon2External");
Base::check<Poseidon2InternalRelation<FF>>(polynomials, params, true, "Poseidon2Internal");
Base::check<UltraArithmeticRelation<FF>>(polynomials, params, "UltraArithmetic");
Base::check<UltraPermutationRelation<FF>>(polynomials, params, "UltraPermutation");
Base::check<DeltaRangeConstraintRelation<FF>>(polynomials, params, "DeltaRangeConstraint");
Base::check<EllipticRelation<FF>>(polynomials, params, "Elliptic");
Base::check<AuxiliaryRelation<FF>>(polynomials, params, "Auxiliary");
Base::check<Poseidon2ExternalRelation<FF>>(polynomials, params, "Poseidon2External");
Base::check<Poseidon2InternalRelation<FF>>(polynomials, params, "Poseidon2Internal");

// Linearly dependent relations (must be satisfied as a sum across all rows)
Base::check<LogDerivLookupRelation<FF>>(polynomials, params, false, "LogDerivLookup");
Base::check<LogDerivLookupRelation<FF>, true>(polynomials, params, "LogDerivLookup");
}
};

Expand All @@ -108,13 +121,10 @@ template <> class RelationChecker<MegaFlavor> : public RelationChecker<void> {
RelationChecker<UltraFlavor>::check_all(polynomials, params);

using FF = MegaFlavor::FF;

// Linearly independent relations (must be satisfied at each row)
Base::check<EccOpQueueRelation<FF>>(polynomials, params, true, "EccOpQueue");

// Linearly dependent relations (must be satisfied as a sum across all rows)
Base::check<DatabusLookupRelation<FF>>(polynomials, params, false, "DatabusLookup");
Base::check<EccOpQueueRelation<FF>>(polynomials, params, "EccOpQueue");
Base::check<DatabusLookupRelation<FF>, true>(polynomials, params, "DatabusLookup");
}
};
} // namespace bb

} // namespace bb
// namespace bb
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,6 @@ template <typename BuilderType> class ECCVMRecursiveFlavor_ {
// Reuse the Relations from ECCVM
using Relations = ECCVMFlavor::Relations_<FF>;

// think these two are not needed for recursive verifier land
// using GrandProductRelations = std::tuple<ECCVMSetRelation<FF>>;
// using LookupRelation = ECCVMLookupRelation<FF>;
static constexpr size_t MAX_PARTIAL_RELATION_LENGTH = ECCVMFlavor::MAX_PARTIAL_RELATION_LENGTH;

// BATCHED_RELATION_PARTIAL_LENGTH = algebraic degree of sumcheck relation *after* multiplying by the `pow_zeta`
Expand Down
Loading

1 comment on commit ed215e8

@AztecBot
Copy link
Collaborator

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'C++ Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.05.

Benchmark suite Current: ed215e8 Previous: 3980f6c Ratio
commit(t) 2637187705 ns/iter 2385448085 ns/iter 1.11

This comment was automatically generated by workflow using github-action-benchmark.

CC: @ludamad @codygunton

Please sign in to comment.