perf(stark): migrate end_exemptions_poly to evaluation form#621
perf(stark): migrate end_exemptions_poly to evaluation form#621diegokingston wants to merge 1 commit into
Conversation
Replaces the coefficient-form `end_exemptions_poly: Polynomial<F>` with two eval-form helpers, removing transition.rs's last dependency on `Polynomial` arithmetic. Stacks on #604 (which removed boundary.rs's `Polynomial` dependency). With both landed, no production code in the prover uses `Polynomial` operators. - `end_exemptions_roots` returns the roots r_i of prod(x - r_i) (<= 2 in the example AIRs, 0 in every VM table). - `end_exemptions_lde_evaluations(domain)` evaluates the product over the precomputed LDE coset directly: an O(N * end_exemptions) loop replaces an O(N log N) FFT. - `evaluate_zerofier`'s OOD path computes prod(z - r_i) directly instead of `Polynomial::evaluate`. Performance: the VM's dominant path (end_exemptions == 0) is unchanged - the existing else-branch early-return is preserved bit-for-bit, so that case still returns the short cyclic vector instead of expanding it. The k > 0 path (example AIRs only) goes from FFT to direct product, strictly faster. Correctness verified by `cargo test -p stark` (125/125, includes the fibonacci and read-only-memory AIRs with end_exemptions = 1 and 2 - exactly the eval-form k > 0 path). VM prover lib test counts are identical to the #604 baseline (273 pass, 77 pre-existing failures unrelated to constraints).
Codex Code ReviewNo issues found in the PR diff. I was not able to run the Rust tests: cargo/rustup tried to write under read-only |
Code ReviewClean refactoring. The eval-form replacement is correct and the logic is preserved faithfully. A few items worth noting: Medium — Latent correctness bug: offset != 0 with end_exemptions > 0 The FIXME on line 122 is carried over from the original, but now has sharper consequences: the roots vector is the sole source of truth for which rows are exempt, so a wrong root silently produces a wrong proof for any future constraint with both offset != 0 and end_exemptions > 0. No existing AIR exercises this combination today, but it is easy to miss. The correct roots should be the last k elements of the offset periodic domain — currently decrement = g^(-period) ignores the offset entirely. The correct value for the last root in the offset domain is g^(offset + (trace_length/period - 1)*period) = g^(trace_length - period + offset). Low — exemptions_period branch does not skip LDE expansion when end_exemptions == 0 The else branch has a fast-path early return for end_exemptions == 0 (preserving the cyclic vector). The exemptions_period branch calls end_exemptions_lde_evaluations unconditionally: when roots are empty the fold produces a vector of N ones and multiplies through pointlessly. Not a regression (old code also evaluated one_poly over the full domain here), but worth a matching guard if this path ever gets a VM caller. Nit — double-negation in evaluate_zerofier The expression -(root.clone() - z.clone()) is a workaround for the subfield/superfield subtraction direction. The comment explains it, but if the math library exposes to_extension(), z.clone() - root.to_extension() reads more directly. Overall the change is correct, the described performance improvement holds, and tests cover the exercised paths. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| // FIXME: when offset != 0 the roots may need to be scaled by | ||
| // trace_root^(offset * trace_length / period) — carried over unresolved | ||
| // from the original coefficient-form end_exemptions_poly. | ||
| let decrement = trace_primitive_root.pow(trace_length - self.period()); |
There was a problem hiding this comment.
The FIXME is pre-existing but the impact is now higher: decrement = g^(trace_length - period) = g^(-period), which gives the correct last root only when offset == 0. When offset != 0, the last row in the offset periodic domain is at g^(trace_length - period + offset), so the roots should start from g^(offset * trace_length / period - (trace_length / period - end_exemptions) * trace_length / period) — or more simply, enumerate the last end_exemptions elements of { g^(offset + j*period) | j = 0..trace_length/period-1 }. A wrong root here produces a silently invalid proof, not an error.
| coset_offset, | ||
| ) | ||
| .unwrap(); | ||
| let end_exemption_evaluations = self.end_exemptions_lde_evaluations(domain); |
There was a problem hiding this comment.
The else branch has a guard (if self.end_exemptions() == 0 { return evaluations; }) that avoids expanding the full LDE domain when there are no exemptions. This exemptions_period branch lacks the same guard: when end_exemptions == 0, end_exemptions_lde_evaluations iterates over the entire LDE coset to produce a vector of ones and then multiplies through pointlessly. Not a regression from the old code, but worth adding a symmetric guard here for consistency and to close the gap if a VM table ever hits this path.
|
/bench 5 |
Benchmark — fib_iterative_8M (median of 5)Table parallelism: auto (cores / 3)
Commit: ccab5dc · Baseline: cached · Runner: self-hosted bench |
1 participant
Follow-up to #604. Removes transition.rs's last dependency on
Polynomialarithmetic — combined with #604 (already merged) no production code in the prover usesPolynomialoperators anymore.Summary
Replaces the coefficient-form
end_exemptions_poly: Polynomial<F>incrypto/stark/src/constraints/transition.rswith two eval-form helpers:end_exemptions_roots(trace_primitive_root, trace_length) -> Vec<FieldElement<F>>— the rootsrᵢof∏(x − rᵢ).end_exemptions_lde_evaluations(domain) -> Vec<FieldElement<F>>— the product evaluated over the precomputed LDE coset directly.The OOD path in
evaluate_zerofiernow computes∏(z − rᵢ)directly instead ofPolynomial::evaluate. ThePolynomialandevaluate_polynomial_on_lde_domainimports are dropped fromtransition.rs.Performance
end_exemptions == 0, the only case in every VM table): unchanged. The else-branch's existing fast-path early-return is preserved bit-for-bit, so the function still returns the short cyclic vector instead of expanding it to the full LDE domain.O(N · end_exemptions)direct product replaces anO(N log N)FFT. Strictly faster forend_exemptions ≤ 2.Test plan
cargo build --workspace— clean.make lint— clean (all three clippy configs).cargo test -p stark— 125/125 passing, including the example AIRs that exercise the eval-formk > 0path (end_exemptions = 1, 2in fibonacci variants and read-only-memory).cargo test -p lambda-vm-prover --lib— 273 pass / 77 fail; identical counts on the pre-cleanup(stark/constraints): drop coefficient-form leftovers, dedupe eval #604 baseline (verified by reverting justtransition.rs). All 77 failures are pre-existing on the base and unrelated to constraints (UnknownSyscall(5)etc.).