chore: delete IsFoldingFlavor concept by iakovenkos · Pull Request #15427 · AztecProtocol/aztec-packages

iakovenkos · 2025-07-01T15:24:50Z

Closes AztecProtocol/barretenberg#1443
Closes AztecProtocol/barretenberg#1426
Closes AztecProtocol/barretenberg#1115

iakovenkos · 2025-07-03T13:24:30Z

-// These concepts are relevant for Sumcheck, where the logic is different for BN254 and Grumpkin Flavors
+// These concept is relevant for the Sumcheck Prover, where the logic is different for BN254 and Grumpkin Flavors
 template <typename T> concept IsGrumpkinFlavor = IsAnyOf<T, ECCVMFlavor, ECCVMRecursiveFlavor_<UltraCircuitBuilder>>;
-template <typename T> concept IsECCVMRecursiveFlavor = IsAnyOf<T, ECCVMRecursiveFlavor_<UltraCircuitBuilder>>;


iakovenkos · 2025-07-03T13:24:52Z

-template <typename T> concept IsECCVMRecursiveFlavor = IsAnyOf<T, ECCVMRecursiveFlavor_<UltraCircuitBuilder>>;
-
-#ifdef STARKNET_GARAGA_FLAVORS
-template <typename T> concept IsFoldingFlavor = IsAnyOf<T, UltraFlavor,


not needed anymore

iakovenkos · 2025-07-03T13:27:23Z

        const size_t polynomial_size = polynomials.get_polynomial_size();
        Polynomial<FF> aggregated_relation_evaluations(polynomial_size);

-        const std::array<FF, NUM_SUBRELATIONS> alphas = [&alphas_]() {


don't need to prepend 1 to the array of challenges, as it's done in Oink

iakovenkos · 2025-07-03T13:28:16Z

+    static void scale_univariates(auto& tuple, const RelationSeparator& subrelation_separators)
    {
        size_t idx = 0;
-        std::array<FF, NUM_SUBRELATIONS> tmp{ current_scalar };


cleaner this way + nano optimization: Prover avoids copying and re-computing the powers of alpha in every round of Sumcheck

iakovenkos · 2025-07-03T13:28:48Z

+    static void scale_and_batch_elements(auto& tuple, const RelationSeparator& challenges, FF& result)
    {
        size_t idx = 0;
-        std::array<FF, NUM_SUBRELATIONS> tmp{ current_scalar };


same as above

iakovenkos · 2025-07-03T13:30:18Z

 class TranslatorVerifier {
  public:
    using Flavor = TranslatorFlavor;
-    using FF = typename Flavor::FF;


TranslatorVerifier is not templated on Flavor

iakovenkos · 2025-07-04T13:37:50Z

-     * @param result Batched result
-     */
-    static void scale_and_batch_elements(auto& tuple, const RelationSeparator& challenge, FF current_scalar, FF& result)
-        requires(!bb::IsFoldingFlavor<Flavor>)


@DanielKotov I removed this method

iakovenkos · 2025-07-04T17:30:38Z

@@ -191,7 +201,6 @@ template <typename Flavor, const size_t virtual_log_n = CONST_PROOF_SIZE_LOG_N>
     */
    SumcheckOutput<Flavor> prove(ProverPolynomials& full_polynomials,


I think all arguments for prove and verify methods can be moved to the constructots, cause currently it looks somewhat random

agreed, I think its always been a bit arbitrary

ledwards2225

LG! Nince cleanup. Just some minor comments

ledwards2225 · 2025-07-07T00:09:54Z

                                       avm2::AvmRecursiveFlavor_<MegaCircuitBuilder>>;

-// These concepts are relevant for Sumcheck, where the logic is different for BN254 and Grumpkin Flavors
+// These concept is relevant for the Sumcheck Prover, where the logic is different for BN254 and Grumpkin Flavors


Suggested change

// These concept is relevant for the Sumcheck Prover, where the logic is different for BN254 and Grumpkin Flavors

// This concept is relevant for the Sumcheck Prover, where the logic differs between BN254 and Grumpkin

ledwards2225 · 2025-07-07T00:14:45Z

     */
-    static void scale_univariates(auto& tuple, const RelationSeparator& challenges, FF& current_scalar)
-        requires bb::IsFoldingFlavor<Flavor>
+    static void scale_univariates(auto& tuple, const RelationSeparator& subrelation_separators)


Ah, I see you've come to the same naming :)

ledwards2225 · 2025-07-07T00:24:48Z

@@ -191,7 +201,6 @@ template <typename Flavor, const size_t virtual_log_n = CONST_PROOF_SIZE_LOG_N>
     */
    SumcheckOutput<Flavor> prove(ProverPolynomials& full_polynomials,


agreed, I think its always been a bit arbitrary

ledwards2225 · 2025-07-07T00:29:24Z

 # - Upload the compressed results: aws s3 cp bb-civc-inputs.tar.gz s3://aztec-ci-artifacts/protocol/bb-civc-inputs-[hash(0:8)].tar.gz
 # Note: In case of the "Test suite failed to run ... Unexpected token 'with' " error, need to run: docker pull aztecprotocol/build:3.0
-pinned_civc_inputs_url="https://aztec-ci-artifacts.s3.us-east-2.amazonaws.com/protocol/bb-civc-inputs-95b3f0bb.tar.gz"
+pinned_civc_inputs_url="https://aztec-ci-artifacts.s3.us-east-2.amazonaws.com/protocol/bb-civc-inputs-9a32575f.tar.gz"


I didnt catch why this is changing VKs?

…umcheckProver members to constructos

…l/aztec-packages into si/remove-is-folding-flavor

iakovenkos · 2025-07-07T15:59:03Z

                public_inputs.emplace_back(FF::from_witness(builder, native_public_input));
            }
-            for (size_t alpha_idx = 0; alpha_idx < alphas.size(); alpha_idx++) {
+            for (size_t alpha_idx = 1; alpha_idx < alphas.size(); alpha_idx++) {


@ledwards2225 this was breaking the vk pinning test, turned the first element into a circuit constant - the test is fixed

iakovenkos · 2025-07-07T22:49:35Z

            [&]<size_t relation_idx, size_t subrelation_idx, typename Element>(Element& element) {
-                using Relation = typename std::tuple_element_t<relation_idx, Relations>;
-                const Element contribution = element * challenges[idx];
-                if (subrelation_is_linearly_independent<Relation, subrelation_idx>()) {


missing constexpr - subrelation_is_linearly_independent is known at compile time, no need for runtime branching

iakovenkos · 2025-07-07T22:56:21Z

-        // avoid Montgomery allocating 0 and doing a mul. This is about 60ns per row.
-        FF linearly_independent_contribution{ 0 };
+        // Initialize result with the contribution from the first subrelation
+        FF linearly_independent_contribution = std::get<0>(evals)[0];


Initially I wanted to turn SubrelationSeparators into an array of size of NUM_SUBRELATIONS with the first entry = 1, but it made vk hashing inconsistent - so I removed the first entry of this array completely. Previously we were creating a bunch of temporary arrays of NUM_SUBRELATIONS size just to prepend 1 to the challenges.

As a nice side effect - closed another issue

…l/aztec-packages into si/remove-is-folding-flavor

conflict resolution cause my PR #15427 stopped the train --------- Co-authored-by: Jean M <132435771+jeanmon@users.noreply.github.com> Co-authored-by: Alex Gherghisan <alexghr@users.noreply.github.com> Co-authored-by: Santiago Palladino <spalladino@users.noreply.github.com> Co-authored-by: Santiago Palladino <santiago@aztec-labs.com>

In my [previous PR](#15427), I moved the `GateSeparatorPolynomial` into the SumcheckProver class as a member, which extended its lifetime and increased memory usage in CIVC by the size of this polynomial, e.g. by 21 MB in `ecdsar1+transfer_0_recursions+sponsored_fpc` bench

remove IsFoldingFlavor concept

3f7650d

iakovenkos self-assigned this Jul 1, 2025

avoid copying in scale_univariates

603f646

iakovenkos commented Jul 3, 2025

View reviewed changes

iakovenkos added 5 commits July 3, 2025 13:47

clean-up

d487edd

Merge branch 'next' into si/remove-is-folding-flavor

60bc34d

boomerang weird

2f68c6c

fix tests

1e4ed96

Merge branch 'next' into si/remove-is-folding-flavor

9ded020

iakovenkos requested a review from ledwards2225 July 3, 2025 22:53

iakovenkos added 2 commits July 4, 2025 12:26

clean-up after Jean's comments

939bcc3

more cleanup

a3303e2

iakovenkos commented Jul 4, 2025

View reviewed changes

fix vks

924bec0

iakovenkos marked this pull request as ready for review July 4, 2025 14:06

iakovenkos requested review from IlyasRidhuan, fcarreiro and jeanmon as code owners July 4, 2025 14:06

fcarreiro removed request for IlyasRidhuan, fcarreiro and jeanmon July 4, 2025 14:07

iakovenkos commented Jul 4, 2025

View reviewed changes

ledwards2225 approved these changes Jul 7, 2025

View reviewed changes

iakovenkos added 2 commits July 7, 2025 11:58

rename RelationSeparator -> SubrelationSeparators, move most of the S…

168954c

…umcheckProver members to constructos

Merge branch 'next' into si/remove-is-folding-flavor

5ca6866

iakovenkos requested a review from just-mitch as a code owner July 7, 2025 12:03

iakovenkos changed the base branch from merge-train/barretenberg to next July 7, 2025 12:34

iakovenkos and others added 3 commits July 7, 2025 15:45

Merge branch 'next' into si/remove-is-folding-flavor

728dca4

fix sumcheck tests + capture and fix the VK changes

b211275

Merge branch 'si/remove-is-folding-flavor' of github.com:AztecProtoco…

84b2911

…l/aztec-packages into si/remove-is-folding-flavor

iakovenkos commented Jul 7, 2025

View reviewed changes

iakovenkos added this pull request to the merge queue Jul 7, 2025

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 7, 2025

e2e prover full fix

3b98b2b

iakovenkos commented Jul 7, 2025

View reviewed changes

iakovenkos added this pull request to the merge queue Jul 8, 2025

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 8, 2025

iakovenkos added 2 commits July 8, 2025 09:54

Merge branch 'next' into si/remove-is-folding-flavor

68a7e00

Merge branch 'next' into si/remove-is-folding-flavor

3470bf5

iakovenkos added the ci-full Run all master checks. label Jul 8, 2025

iakovenkos and others added 2 commits July 8, 2025 13:27

docs fixes

c3a43a2

Merge branch 'next' into si/remove-is-folding-flavor

c485d00

iakovenkos added this pull request to the merge queue Jul 8, 2025

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 8, 2025

iakovenkos and others added 2 commits July 8, 2025 18:48

Merge branch 'si/remove-is-folding-flavor' of github.com:AztecProtoco…

4fadf2b

…l/aztec-packages into si/remove-is-folding-flavor

Merge branch 'next' into si/remove-is-folding-flavor

52b2701

iakovenkos added this pull request to the merge queue Jul 8, 2025

Merged via the queue into next with commit fad593a Jul 8, 2025
4 checks passed

iakovenkos deleted the si/remove-is-folding-flavor branch July 8, 2025 21:18

iakovenkos mentioned this pull request Jul 9, 2025

fix: bb merge-train conflicts #15617

Merged

iakovenkos mentioned this pull request Jul 15, 2025

fix: civc wasm memory regression #15722

Merged

		@@ -191,7 +201,6 @@ template <typename Flavor, const size_t virtual_log_n = CONST_PROOF_SIZE_LOG_N>
		*/
		SumcheckOutput<Flavor> prove(ProverPolynomials& full_polynomials,

	// These concept is relevant for the Sumcheck Prover, where the logic is different for BN254 and Grumpkin Flavors
	// This concept is relevant for the Sumcheck Prover, where the logic differs between BN254 and Grumpkin

Conversation

iakovenkos commented Jul 1, 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!

Choose a reason for hiding this comment

Uh oh!

iakovenkos Jul 3, 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!

Choose a reason for hiding this comment

Uh oh!

ledwards2225 left a comment

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!

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!

Uh oh!

iakovenkos Jul 7, 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!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

iakovenkos commented Jul 1, 2025 •

edited

Loading

iakovenkos Jul 3, 2025 •

edited

Loading

iakovenkos Jul 7, 2025 •

edited

Loading