chore: use RelationChecker in relation correctness tests and add Translator interleaving test by maramihali · Pull Request #11878 · AztecProtocol/aztec-packages

maramihali · 2025-02-10T14:07:22Z

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)

…leaving

…on-checker

maramihali · 2025-02-10T19:00:23Z

barretenberg/cpp/src/barretenberg/translator_vm/translator_proving_key.test.cpp

+ * @brief Ensures the concatenation by interleaving function still preserves the correctness of relations.
+ *
+ */
+TEST_F(TranslatorProvingKeyTests, InterleaveFull)


if you think i shouldn't merge this because it's very preliminary work i can leave it out, I basically wanted to establish that interleaving doesn't break Permutation and DeltaRangeConstraint, which operate on the concatenated polynomials (and their ordered version) in an isolated way

I think it's fine. I very much prefer incremental PRs and given that you're planning to work primarily on this it shouldn't result in stale unused code

ledwards2225

Looks great, long overdue cleanup. Left a couple of minor suggestions

barretenberg/cpp/src/barretenberg/translator_vm/relation_correctness.test.cpp

ledwards2225 · 2025-02-11T13:27:33Z

barretenberg/cpp/src/barretenberg/translator_vm/relation_correctness.test.cpp

-    fill_polynomial_with_random_14_bit_values(prover_polynomials.relation_wide_limbs_range_constraint_2);
-    fill_polynomial_with_random_14_bit_values(prover_polynomials.relation_wide_limbs_range_constraint_3);
+
+    for (const auto& group : prover_polynomials.get_groups_to_be_concatenated()) {


haha wow, this was long overdue

yay i did something useful last week

ledwards2225 · 2025-02-11T13:28:13Z

barretenberg/cpp/src/barretenberg/translator_vm/relation_correctness.test.cpp

-        polynomial_get_all[shifted_id] = polynomial_container[to_be_shifted_id].shifted();
-    }
-
+    ProverPolynomials prover_polynomials = TranslatorFlavor::ProverPolynomials(circuit_size);


same as above

ledwards2225 · 2025-02-11T13:28:45Z

barretenberg/cpp/src/barretenberg/translator_vm/relation_correctness.test.cpp

-        // Ensure we can shift this
-        polynomial_get_all[shifted_id] = polynomial_container[to_be_shifted_id].shifted();
-    }
+    ProverPolynomials prover_polynomials = TranslatorFlavor::ProverPolynomials(circuit_size);


ledwards2225 · 2025-02-11T13:31:56Z

barretenberg/cpp/src/barretenberg/translator_vm/translator_proving_key.cpp

+
+    // Targets have to be full-sized proving_key->polynomials. We can compute the mini circuit size from them by
+    // dividing by concatenation index
+    const size_t MINI_CIRCUIT_SIZE = concatenated_polynomials[0].size() / Flavor::CONCATENATION_GROUP_SIZE;


Isnt MINI_CIRCUIT_SIZE a fixed flavor constant anyway? Maybe not always?

there is a minimum_mini_circuit_size for which the translator is optimised but technically it can have another size as well, if I understand correctly it's given by the number of gates in the translator circuit

barretenberg/cpp/src/barretenberg/translator_vm/translator_proving_key.cpp

ledwards2225 · 2025-02-11T14:02:31Z

barretenberg/cpp/src/barretenberg/translator_vm/translator_proving_key.cpp

+{
+
+    const size_t group_size = group.size();
+    const size_t group_polynomial_size = result.size() / group_size;


Its a little hard to follow how these sizes work out. group_polynomial_size is computed as the the size (size of the memory block) of the destination poly divided by number of small polys, but then we only iterate from result.start_index() to group_polynomial_size. If result had a start index > 0, wouldn't this not iterate over the full size? Perhaps some checks within this method would add safety and also clarify what the sizes are expected to be?

size() is computed as end_index() - start_index()

but yes you are right, will add some checks

maybe we're saying the same thing but just to be sure: I agree size is end - start, but the loop below goes from idx = start to idx = size, not idx = end, i.e. it does not loop over size-many elements. Oh but group_polynomial_size is not being computed group_polynomial.size() so maybe its somehow doing the right thing?

yeah so the issue here is that the group polynomials are initialised over the full domain (i.e. the same domain as the result polynomial) rather than they're real domain (mini_circuit_size) even though past their real domain the coeff are 0. A follow on is to refine the sizes so we can call group.size(). I started doing it in this PR but subtle changes to sizes deserve a PR of their own so I added an issue for now.

ledwards2225 · 2025-02-11T14:06:49Z

barretenberg/cpp/src/barretenberg/ultra_honk/relation_correctness.test.cpp

@@ -25,67 +26,6 @@ void ensure_non_zero(auto& polynomial)
    ASSERT_TRUE(has_non_zero_coefficient);
 }



nice, thanks for doing this

ledwards2225 · 2025-02-11T14:08:19Z

barretenberg/cpp/src/barretenberg/translator_vm/translator_proving_key.test.cpp

+ * @brief Ensures the concatenation by interleaving function still preserves the correctness of relations.
+ *
+ */
+TEST_F(TranslatorProvingKeyTests, InterleaveFull)


I think it's fine. I very much prefer incremental PRs and given that you're planning to work primarily on this it shouldn't result in stale unused code

ledwards2225 · 2025-02-11T14:25:59Z

barretenberg/cpp/src/barretenberg/plonk_honk_shared/relation_checker.hpp

+
+            // Iterate over all the subrelation results and report if a linearly independent one failed
+            for (auto& element : result) {
+                if constexpr (has_linearly_dependent) {


Took me a while to see why this logic has to be written this way but I guess its because only relations that have linearly dependent subrelations specify the array SUBRELATION_LINEARLY_INDEPENDENT. This could be simplified quite a bit if all relations provided this. Might be worth doing but doesn't need to be here.

I think we used to have some sort of boolean specifying whether a relation has a linearly dependent contribution, i'll add an issue

🤖 I have created a release *beep* *boop* --- <details><summary>aztec-package: 0.76.3</summary> ## [0.76.3](aztec-package-v0.76.2...aztec-package-v0.76.3) (2025-02-12) ### Features * Add undici ([#11818](#11818)) ([8503c7a](8503c7a)) * Initial multi-proof test ([#11779](#11779)) ([f54db75](f54db75)) </details> <details><summary>barretenberg.js: 0.76.3</summary> ## [0.76.3](barretenberg.js-v0.76.2...barretenberg.js-v0.76.3) (2025-02-12) ### Features * **perf:** Speed up construction of bbjs Frs & cache zero hashes in ephemeral trees (redo) ([#11894](#11894)) ([e093acf](e093acf)) ### Bug Fixes * Memory fragmentation fixes to cut UltraHonk memory usage by 26% ([#11895](#11895)) ([b4e2264](b4e2264)) </details> <details><summary>aztec-packages: 0.76.3</summary> ## [0.76.3](aztec-packages-v0.76.2...aztec-packages-v0.76.3) (2025-02-12) ### Features * Add undici ([#11818](#11818)) ([8503c7a](8503c7a)) * **avm:** Sequential lookup resolution ([#11769](#11769)) ([3980f6c](3980f6c)) * Enable ws for reth on devnet ([#11922](#11922)) ([7124664](7124664)), closes [#11921](#11921) * Initial multi-proof test ([#11779](#11779)) ([f54db75](f54db75)) * Native world state now supports checkpointing ([#11739](#11739)) ([6464059](6464059)) * **perf:** Speed up construction of bbjs Frs & cache zero hashes in ephemeral trees (redo) ([#11894](#11894)) ([e093acf](e093acf)) ### Bug Fixes * Correctly configure batch queue ([#11934](#11934)) ([4908df8](4908df8)) * De-dup pubkey conversion of cli-wallet param ([#11948](#11948)) ([5529871](5529871)) * **docs:** Update the token bridge tutorial ([#11578](#11578)) ([aaf42a7](aaf42a7)) * Don't restart kind control plane automatically ([#11923](#11923)) ([c23c0f9](c23c0f9)) * Empty blocks can now be unwound ([#11920](#11920)) ([fdc2042](fdc2042)) * Gcloud logs ([#11944](#11944)) ([c53b1c5](c53b1c5)), closes [#11887](#11887) * Lmdb cmake race condition ([#11959](#11959)) ([031200d](031200d)) * Memory fragmentation fixes to cut UltraHonk memory usage by 26% ([#11895](#11895)) ([b4e2264](b4e2264)) * Npm packages noir resolution ([#11946](#11946)) ([d3e3f20](d3e3f20)) * Set resource limits on Loki pods ([#11940](#11940)) ([6999982](6999982)) ### Miscellaneous * Only take FF (and not Flavor) in compute_logderivative_inverse ([#11938](#11938)) ([bbbded3](bbbded3)) * Op queue cleanup ([#11925](#11925)) ([082ed66](082ed66)) * Renaming `pxe_db.nr` as `capsules.nr` ([#11905](#11905)) ([14d873c](14d873c)) * Replace relative paths to noir-protocol-circuits ([9ce401a](9ce401a)) * Use realistic size Client IVC proofs during simulations ([#11692](#11692)) ([90b9fbf](90b9fbf)) * Use RelationChecker in relation correctness tests and add Translator interleaving test ([#11878](#11878)) ([ed215e8](ed215e8)) </details> <details><summary>barretenberg: 0.76.3</summary> ## [0.76.3](barretenberg-v0.76.2...barretenberg-v0.76.3) (2025-02-12) ### Features * **avm:** Sequential lookup resolution ([#11769](#11769)) ([3980f6c](3980f6c)) * Native world state now supports checkpointing ([#11739](#11739)) ([6464059](6464059)) ### Bug Fixes * Empty blocks can now be unwound ([#11920](#11920)) ([fdc2042](fdc2042)) * Lmdb cmake race condition ([#11959](#11959)) ([031200d](031200d)) * Memory fragmentation fixes to cut UltraHonk memory usage by 26% ([#11895](#11895)) ([b4e2264](b4e2264)) ### Miscellaneous * Only take FF (and not Flavor) in compute_logderivative_inverse ([#11938](#11938)) ([bbbded3](bbbded3)) * Op queue cleanup ([#11925](#11925)) ([082ed66](082ed66)) * Use RelationChecker in relation correctness tests and add Translator interleaving test ([#11878](#11878)) ([ed215e8](ed215e8)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

🤖 I have created a release *beep* *boop* --- <details><summary>aztec-package: 0.76.3</summary> ## [0.76.3](AztecProtocol/aztec-packages@aztec-package-v0.76.2...aztec-package-v0.76.3) (2025-02-12) ### Features * Add undici ([#11818](AztecProtocol/aztec-packages#11818)) ([8503c7a](AztecProtocol/aztec-packages@8503c7a)) * Initial multi-proof test ([#11779](AztecProtocol/aztec-packages#11779)) ([f54db75](AztecProtocol/aztec-packages@f54db75)) </details> <details><summary>barretenberg.js: 0.76.3</summary> ## [0.76.3](AztecProtocol/aztec-packages@barretenberg.js-v0.76.2...barretenberg.js-v0.76.3) (2025-02-12) ### Features * **perf:** Speed up construction of bbjs Frs & cache zero hashes in ephemeral trees (redo) ([#11894](AztecProtocol/aztec-packages#11894)) ([e093acf](AztecProtocol/aztec-packages@e093acf)) ### Bug Fixes * Memory fragmentation fixes to cut UltraHonk memory usage by 26% ([#11895](AztecProtocol/aztec-packages#11895)) ([b4e2264](AztecProtocol/aztec-packages@b4e2264)) </details> <details><summary>aztec-packages: 0.76.3</summary> ## [0.76.3](AztecProtocol/aztec-packages@aztec-packages-v0.76.2...aztec-packages-v0.76.3) (2025-02-12) ### Features * Add undici ([#11818](AztecProtocol/aztec-packages#11818)) ([8503c7a](AztecProtocol/aztec-packages@8503c7a)) * **avm:** Sequential lookup resolution ([#11769](AztecProtocol/aztec-packages#11769)) ([3980f6c](AztecProtocol/aztec-packages@3980f6c)) * Enable ws for reth on devnet ([#11922](AztecProtocol/aztec-packages#11922)) ([7124664](AztecProtocol/aztec-packages@7124664)), closes [#11921](AztecProtocol/aztec-packages#11921) * Initial multi-proof test ([#11779](AztecProtocol/aztec-packages#11779)) ([f54db75](AztecProtocol/aztec-packages@f54db75)) * Native world state now supports checkpointing ([#11739](AztecProtocol/aztec-packages#11739)) ([6464059](AztecProtocol/aztec-packages@6464059)) * **perf:** Speed up construction of bbjs Frs & cache zero hashes in ephemeral trees (redo) ([#11894](AztecProtocol/aztec-packages#11894)) ([e093acf](AztecProtocol/aztec-packages@e093acf)) ### Bug Fixes * Correctly configure batch queue ([#11934](AztecProtocol/aztec-packages#11934)) ([4908df8](AztecProtocol/aztec-packages@4908df8)) * De-dup pubkey conversion of cli-wallet param ([#11948](AztecProtocol/aztec-packages#11948)) ([5529871](AztecProtocol/aztec-packages@5529871)) * **docs:** Update the token bridge tutorial ([#11578](AztecProtocol/aztec-packages#11578)) ([aaf42a7](AztecProtocol/aztec-packages@aaf42a7)) * Don't restart kind control plane automatically ([#11923](AztecProtocol/aztec-packages#11923)) ([c23c0f9](AztecProtocol/aztec-packages@c23c0f9)) * Empty blocks can now be unwound ([#11920](AztecProtocol/aztec-packages#11920)) ([fdc2042](AztecProtocol/aztec-packages@fdc2042)) * Gcloud logs ([#11944](AztecProtocol/aztec-packages#11944)) ([c53b1c5](AztecProtocol/aztec-packages@c53b1c5)), closes [#11887](AztecProtocol/aztec-packages#11887) * Lmdb cmake race condition ([#11959](AztecProtocol/aztec-packages#11959)) ([031200d](AztecProtocol/aztec-packages@031200d)) * Memory fragmentation fixes to cut UltraHonk memory usage by 26% ([#11895](AztecProtocol/aztec-packages#11895)) ([b4e2264](AztecProtocol/aztec-packages@b4e2264)) * Npm packages noir resolution ([#11946](AztecProtocol/aztec-packages#11946)) ([d3e3f20](AztecProtocol/aztec-packages@d3e3f20)) * Set resource limits on Loki pods ([#11940](AztecProtocol/aztec-packages#11940)) ([6999982](AztecProtocol/aztec-packages@6999982)) ### Miscellaneous * Only take FF (and not Flavor) in compute_logderivative_inverse ([#11938](AztecProtocol/aztec-packages#11938)) ([bbbded3](AztecProtocol/aztec-packages@bbbded3)) * Op queue cleanup ([#11925](AztecProtocol/aztec-packages#11925)) ([082ed66](AztecProtocol/aztec-packages@082ed66)) * Renaming `pxe_db.nr` as `capsules.nr` ([#11905](AztecProtocol/aztec-packages#11905)) ([14d873c](AztecProtocol/aztec-packages@14d873c)) * Replace relative paths to noir-protocol-circuits ([9ce401a](AztecProtocol/aztec-packages@9ce401a)) * Use realistic size Client IVC proofs during simulations ([#11692](AztecProtocol/aztec-packages#11692)) ([90b9fbf](AztecProtocol/aztec-packages@90b9fbf)) * Use RelationChecker in relation correctness tests and add Translator interleaving test ([#11878](AztecProtocol/aztec-packages#11878)) ([ed215e8](AztecProtocol/aztec-packages@ed215e8)) </details> <details><summary>barretenberg: 0.76.3</summary> ## [0.76.3](AztecProtocol/aztec-packages@barretenberg-v0.76.2...barretenberg-v0.76.3) (2025-02-12) ### Features * **avm:** Sequential lookup resolution ([#11769](AztecProtocol/aztec-packages#11769)) ([3980f6c](AztecProtocol/aztec-packages@3980f6c)) * Native world state now supports checkpointing ([#11739](AztecProtocol/aztec-packages#11739)) ([6464059](AztecProtocol/aztec-packages@6464059)) ### Bug Fixes * Empty blocks can now be unwound ([#11920](AztecProtocol/aztec-packages#11920)) ([fdc2042](AztecProtocol/aztec-packages@fdc2042)) * Lmdb cmake race condition ([#11959](AztecProtocol/aztec-packages#11959)) ([031200d](AztecProtocol/aztec-packages@031200d)) * Memory fragmentation fixes to cut UltraHonk memory usage by 26% ([#11895](AztecProtocol/aztec-packages#11895)) ([b4e2264](AztecProtocol/aztec-packages@b4e2264)) ### Miscellaneous * Only take FF (and not Flavor) in compute_logderivative_inverse ([#11938](AztecProtocol/aztec-packages#11938)) ([bbbded3](AztecProtocol/aztec-packages@bbbded3)) * Op queue cleanup ([#11925](AztecProtocol/aztec-packages#11925)) ([082ed66](AztecProtocol/aztec-packages@082ed66)) * Use RelationChecker in relation correctness tests and add Translator interleaving test ([#11878](AztecProtocol/aztec-packages#11878)) ([ed215e8](AztecProtocol/aztec-packages@ed215e8)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

maramihali added 11 commits February 6, 2025 14:56

translator simple interleaving and relation correctness refactoring

2671e22

Merge remote-tracking branch 'origin/master' into mm/translator-inter…

f1c2012

…leaving

stuff

f203055

cleanup

15b01cf

fix return values

8a5495f

fix

a44d1ef

now it work

22f5621

Merge branch 'mm/translator-interleaving' into mm/actually-use-relati…

ce89a3b

…on-checker

translator uses relation checker

66b73a5

reduce scope and cleanup

fe4509b

cleanup

94798f5

maramihali changed the title ~~chore: actually use the RelationChecker for Mega and Ultra~~ chore: use RelationChecker in relation correctness test and add Translator interleaving test Feb 10, 2025

maramihali changed the title ~~chore: use RelationChecker in relation correctness test and add Translator interleaving test~~ chore: use RelationChecker in relation correctness tests and add Translator interleaving test Feb 10, 2025

maramihali commented Feb 10, 2025

View reviewed changes

complete test

48b1b6e

maramihali marked this pull request as ready for review February 10, 2025 19:02

maramihali self-assigned this Feb 10, 2025

maramihali added the crypto cryptography label Feb 10, 2025

maramihali requested a review from ledwards2225 February 11, 2025 11:05

ledwards2225 approved these changes Feb 11, 2025

View reviewed changes

ludamad mentioned this pull request Feb 11, 2025

do not merge test(ci3): chore: use RelationChecker in relation correctness tests and add Translator interleaving test #11932

Closed

updates from PR review

51689f6

maramihali enabled auto-merge (squash) February 12, 2025 13:38

maramihali disabled auto-merge February 12, 2025 13:49

Merge branch 'master' into mm/actually-use-relation-checker

dc8759b

maramihali merged commit ed215e8 into master Feb 12, 2025
25 checks passed

maramihali deleted the mm/actually-use-relation-checker branch February 12, 2025 15:17

AztecBot mentioned this pull request Feb 12, 2025

chore(master): Release 0.76.3 #11924

Merged

		@@ -25,67 +26,6 @@ void ensure_non_zero(auto& polynomial)
		ASSERT_TRUE(has_non_zero_coefficient);
		}

Conversation

maramihali commented Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ledwards2225 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maramihali Feb 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

maramihali commented Feb 10, 2025 •

edited

Loading

maramihali Feb 12, 2025 •

edited

Loading