chore: cleanup polynomials module by nishatkoti · Pull Request #20585 · AztecProtocol/aztec-packages

nishatkoti · 2026-02-17T12:48:37Z

Cleanup and minor changes in the polynomial module.

Changes:

Delete unused methods in univariate_coefficient_basis.hpp
- is_zero
- random_element
- to_buffer
- serialize_from_buffer
- size
- get_random
Delete unused methods in polynomial.hpp/cpp
- clear()
- right_shifted()
- debug_hash()
- compute_barycentric_evaluation()
- in_place_operation_viable
- overload of evaluate evaluate(const Fr& z, size_t target_size)
Delete unused methods in polynomial_arithmetic.hpp/cpp
- compute_multiplicative_subgroup
- compute_barycentric_evaluation
- fft, coset_fft, unused overloads of fft_inner_parallel and ifft
Delete unused compute_generator_table and four_inverse in evaluation_domain.hpp
Fixed _allocate_aligned_memory call to pass element count (n_l) instead of byte size.
Add new tests in barycentric.test.cpp covering the stdlib runtime path
Deduplicate compile-time and runtime BarycentricData logic via a shared base class (resolves TODO #674)

nishatkoti · 2026-02-25T12:31:36Z

+    static constexpr bool value = true;
+};

-   3) There should be more thorough testing of this class in isolation.


More tests included to cover the stdlib runtime path

nishatkoti · 2026-02-25T12:34:50Z

    , domain_inverse(domain.invert())
    , generator(Fr::template coset_generator<0>())
    , generator_inverse(Fr::template coset_generator<0>().invert())
-    , four_inverse(Fr(4).invert())


nishatkoti · 2026-02-25T12:36:08Z

    , generator_size(other.generator_size)
-    , root(Fr::get_root_of_unity(log2_size))
-    , root_inverse(root.invert())
+    , root(other.root)


Use other's root parameters, to avoid computing root_of_unity for Grumpkin, similar to the behaviour of the primary constructor.

nishatkoti · 2026-02-25T12:39:11Z

-     * corresponds to the start_index of the destination polynomial and also that the number of elements we want to copy
-     * corresponds to the size of the polynomial. This is quirky behavior and we might want to improve the UX.
-     *
-     * @todo https://github.com/AztecProtocol/barretenberg/issues/1292


Removed the todo as it is marked as closed. Also clarified the function’s usage in the comment.

nishatkoti · 2026-02-25T12:42:34Z


-    // hard code exception for when the domain size is tiny - we won't execute the next loop, so need to manually
-    // reduce + copy
-    if (domain.size <= 2) {


Removed this copying back into coeffs as for domain.size <= 2 the final output is already in target, andcoeffs should not be modified in this overload of fft_inner_parallel. This was probably copied from the previous overload.

ledwards2225

Overall LGTM, made some small suggestions. Would you mind doing a follow on here where you're really ruthless about removing unused code? E.g. I was under the impression that the only FFT related method we use is ifft in SSIPA

ledwards2225 · 2026-02-25T13:50:41Z

-// pre-computable arrays in BarycentricData need to be constexpr and it takes some trickery to share these functions
-// with the non-constexpr setting. Right now everything is more or less duplicated across BarycentricDataCompileTime and
-// BarycentricDataRunTime. There should be a way to share more of the logic.
+/* Future improvements (see https://github.com/AztecProtocol/barretenberg/issues/10): The code works for its intended


Maybe (2) here is something we leave as a note but (1) and (3) should probably be handled. Can you evaluate whether there is something needed to address them, do it if so, then remove?

For (1), added compile-time guards for domain_size, num_evals. This should take care of invalid sizes. For (3), I had already added stdlib runtime tests which were missing earlier. Updated the note accordingly.

ledwards2225 · 2026-02-25T13:54:30Z

-            if (coeffs[i] == 0) {
-                skipped[i] = true;
+            // For native field types, == 0 is a plain constexpr comparison. For stdlib field_t,
+            // operator== would create circuit constraints and return bool_t, so we use get_value()


suggestion: add some protection around the get_value call to reflect the assumption stated in comments e.g. BB_ASSERT(coeffs[i].is_constant());

ledwards2225 · 2026-02-25T14:19:46Z

-    // TODO(https://github.com/AztecProtocol/barretenberg/issues/1096): Make this a Polynomial with
-    // DontZeroMemory::FLAG
-    auto tmp_ptr = _allocate_aligned_memory<Fr_>(sizeof(Fr_) * n_l);
+    // IMPROVEMENT: tmp is fully overwritten. We could make this a Polynomial with DontZeroMemory::FLAG. See


Can you make an assessment one way or the other on this one too?

although tmp is fully overwritten, switching it to Polynomial<Fr_> with DontZeroMemory may not reduce work here because it would still allocate backing memory and introduce additional overhead for setting up the metadata for SharedShiftedVirtualZeroesArray. The current allocation already avoids zeroing the memory. Also, the linked issue (#1096) is about get_row copying and doesn’t look related to this mle temporary buffer. So I have removed the improvement comment and the issue reference to avoid confusion.

ledwards2225 · 2026-02-25T14:23:06Z

                result.value_at(idx + 1) = result.value_at(idx) + delta;
            }
        } else if constexpr (LENGTH == 3) {
+            static constexpr Fr inverse_two = Fr(2).invert();


is this used?

yes, it is used to compute a = (f(2) + f(0) - 2f(1)) / 2 in the lines following it

ledwards2225 · 2026-02-25T14:23:36Z

            }
        } else if constexpr (LENGTH == 3) {
+            static constexpr Fr inverse_two = Fr(2).invert();
            // Based off https://hackmd.io/@aztec-network/SyR45cmOq?type=view


This wont be accessible to most - can you see if this hackmd says anything useful and if so port some version of that to the comments here?

it slipped my mind that the hackmd may not be accessible. The hackmd doesn’t have anything additional except for some rough cost accounting. So I have removed the link and added a short comment note that this special case path is cheaper than the generic method.

ledwards2225 · 2026-02-25T14:24:24Z

        return output;
    };
-
+    /*


was this meant to be deleted altogether?

yes, deleted.

… todo#674

…ltiplicative_subgroup, unused coset_fft overload, compute_barycentric_evaluation, unnecessary if block from fft_inner_parallel, update array declarations, add test for coset_fft scaling.

…e, and update constructors for consistent root handling

BEGIN_COMMIT_OVERRIDE chore: cleanup polynomials module (#20585) fix: splitting scalars edge case (#20686) feat: parallelize ACIR parsing and decompression (#20873) END_COMMIT_OVERRIDE

…#20988) - Replace `ITERATE_OVER_DOMAIN_START` and `ITERATE_OVER_DOMAIN_END` macros with a `parallel_for` loop in `ifft()` - The macro was only used in `ifft()`. Replacing it with an inline loop makes the control flow clearer and allows removing the unused header. - Delete the now-unused `iterate_over_domain.hpp` - Address race condition in `try_allocate_file_backed` in `backing_memory.hpp` - Remove old improvement note in `barycentric.hpp`, as it is addressed in PR [#20585](#20585)

Cleanup and minor changes in the polynomial module. Changes: - Delete unused methods in `univariate_coefficient_basis.hpp` - `is_zero` - `random_element` - `to_buffer` - `serialize_from_buffer` - `size` - `get_random` - Delete unused methods in `polynomial.hpp/cpp` - `clear()` - `right_shifted()` - `debug_hash()` - `compute_barycentric_evaluation()` - `in_place_operation_viable` - overload of evaluate `evaluate(const Fr& z, size_t target_size)` - Delete unused methods in `polynomial_arithmetic.hpp/cpp` - `compute_multiplicative_subgroup` - `compute_barycentric_evaluation ` - `fft`, `coset_fft`, unused overloads of `fft_inner_parallel` and `ifft` - Delete unused `compute_generator_table` and `four_inverse` in `evaluation_domain.hpp` - Fixed `_allocate_aligned_memory` call to pass element count (`n_l`) instead of byte size. - Add new tests in `barycentric.test.cpp` covering the `stdlib` runtime path - Deduplicate compile-time and runtime `BarycentricData` logic via a shared base class (resolves TODO [#674](AztecProtocol/barretenberg#674))

…#20988) - Replace `ITERATE_OVER_DOMAIN_START` and `ITERATE_OVER_DOMAIN_END` macros with a `parallel_for` loop in `ifft()` - The macro was only used in `ifft()`. Replacing it with an inline loop makes the control flow clearer and allows removing the unused header. - Delete the now-unused `iterate_over_domain.hpp` - Address race condition in `try_allocate_file_backed` in `backing_memory.hpp` - Remove old improvement note in `barycentric.hpp`, as it is addressed in PR [#20585](#20585)

nishatkoti marked this pull request as ready for review February 17, 2026 12:49

nishatkoti marked this pull request as draft February 17, 2026 13:05

nishatkoti marked this pull request as ready for review February 18, 2026 18:09

nishatkoti marked this pull request as draft February 18, 2026 18:38

nishatkoti force-pushed the nk/polynomials-pass-one branch from 8283495 to d1e9cce Compare February 23, 2026 08:33

nishatkoti marked this pull request as ready for review February 23, 2026 08:34

nishatkoti marked this pull request as draft February 23, 2026 11:26

nishatkoti marked this pull request as ready for review February 24, 2026 08:42

nishatkoti marked this pull request as draft February 24, 2026 08:59

nishatkoti marked this pull request as ready for review February 25, 2026 12:29

nishatkoti commented Feb 25, 2026

View reviewed changes

nishatkoti requested review from federicobarbacovi, kashbrti and ledwards2225 February 25, 2026 13:03

ledwards2225 approved these changes Feb 25, 2026

View reviewed changes

nishatkoti added 7 commits February 27, 2026 06:08

remove unused code and tests, fix mle tmp allocation

d5d0bfc

add tests for run-time version of BarycentricData

8b8567e

deduplicate compile-time and runtime BarycentricData logic, resolving…

4e13db0

… todo#674

polynomial_arithmetic: fix fft,coset_fft overloads, delete compute_mu…

0f1b224

…ltiplicative_subgroup, unused coset_fft overload, compute_barycentric_evaluation, unnecessary if block from fft_inner_parallel, update array declarations, add test for coset_fft scaling.

evaluation_domain: remove unused compute_generator_table, four_invers…

c237358

…e, and update constructors for consistent root handling

change eval domain min size to 2 and use bb_assert in eq_poly

f43d1ab

address review comments

64adc29

nishatkoti force-pushed the nk/polynomials-pass-one branch from 0e9cb38 to 64adc29 Compare February 27, 2026 06:10

nishatkoti merged commit 663a7f1 into merge-train/barretenberg Feb 27, 2026
14 checks passed

nishatkoti deleted the nk/polynomials-pass-one branch February 27, 2026 08:38

AztecBot mentioned this pull request Feb 27, 2026

feat: merge-train/barretenberg #20941

Merged

This was referenced Mar 2, 2026

chore: remove domain iteration macros and address backing memory race #20988

Merged

Improve BarycentricData class AztecProtocol/barretenberg#10

Closed

Conversation

nishatkoti commented Feb 17, 2026 • 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!

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!

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!

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

nishatkoti commented Feb 17, 2026 •

edited

Loading