Skip to content

chore: circuit to polys cleanup#23013

Open
ledwards2225 wants to merge 5 commits intomerge-train/barretenbergfrom
lde/circuit-to-polys
Open

chore: circuit to polys cleanup#23013
ledwards2225 wants to merge 5 commits intomerge-train/barretenbergfrom
lde/circuit-to-polys

Conversation

@ledwards2225
Copy link
Copy Markdown
Contributor

@ledwards2225 ledwards2225 commented May 7, 2026

Two cleanups in trace-to-polynomials land. Originally motivated by small perf wins but is attractive for the code simplification:

1. Simpler permutation polynomial construction. The old code built a large intermediate table and walked it to produce the sigma/id polynomials. The new code
writes those polynomials directly in three steps: identity init, cycle linkages, public-input update.

2. Each block has only the non-gate selectors plus (at most) one gate selector. Previously every block declared all eight or nine possible gate selectors using the hacky ZeroSelector. ZeroSelector class is gone plus lots of simplification in the block classes.

@ledwards2225 ledwards2225 marked this pull request as ready for review May 7, 2026 00:51
@ledwards2225 ledwards2225 force-pushed the lde/circuit-to-polys branch from 0554c68 to 62b9931 Compare May 7, 2026 19:26
@ledwards2225
Copy link
Copy Markdown
Contributor Author

Bench: realistic Chonk flow, dedicated remote (HARDWARE_CONCURRENCY=16)

Flow: schnorr+deploy_tokenContract_with_registration+sponsored_fpc, merge-train pinned inputs 341842bf. Single run on the isolated EC2 box.

Region before (ms) after (ms) Δ
ChonkAPI::prove (total) 6711.6 6643.1 −1.0%
ChonkAccumulate 4345.6 4291.9 −1.2%
construct_trace_data 138.5 133.0 −4.0%
compute_permutation_argument_polynomials 56.8 45.2 −20.4%
populate_wires_and_emit_cycles 21.8 19.3 −11.5%
populate_selectors 16.3 14.6 −10.6%
fill_copy_cycles 19.0 17.1 −9.7%

The regions the PR directly touches all show double-digit improvements: dropping the intermediate PermutationMapping struct buys −20% on compute_permutation_argument_polynomials, and de-virtualising the selector reads/writes (single concrete Selector<FF> + kind tag instead of a Selector / ZeroSelector / SlabVectorSelector hierarchy) buys ~10% across the trace-construction loop. Total proving wall-clock is −1%. Sub-percent moves elsewhere are within single-run noise on this machine.

* @details We check that for the arithmetic, delta_range, elliptic, memory, nnf, lookup, busread, poseidon2_external,
* poseidon2_internal blocks, and the other selectors are zero on that block.
*/
TEST(MegaCircuitBuilder, CompleteSelectorPartitioningCheck)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the condition this test is checking is now structurally impossible to deviate from

EXPECT_TRUE(CircuitChecker::check(builder));

// Verify that in the NNF block, all non-NNF selectors are zero
for (size_t i = 0; i < builder.blocks.nnf.size(); ++i) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ditto

* @brief Stores permutation mapping data for a single wire column.
*
*/
struct Mapping {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

massive code simplification here primarily due to constructing the permutation polys directly rather than passing through the intermediate Mapping/PermutationMapping. also a perf improvement since it avoids many small allocations

@ledwards2225 ledwards2225 requested a review from ludamad May 8, 2026 16:29
@ledwards2225 ledwards2225 force-pushed the lde/circuit-to-polys branch from 1f76369 to 17b314c Compare May 8, 2026 21:56
@ledwards2225 ledwards2225 added the ci-full Run all master checks. label May 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-full Run all master checks.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants