refactor(eor)!: unify setup prefix and suffix reduction - #398
Conversation
|
Warning This PR has more than 500 changed lines and does not include a spec. Large features and architectural changes benefit from a short spec before implementation review. If this PR is a bug fix, small cleanup, or otherwise does not warrant a spec, feel free to ignore this message. |
PR SummaryHigh Risk Overview Later evaluation-trace folds can now batch a borrowed setup prefix with a compact recursive suffix in one EOR. Prefixes are sliced from the expanded setup matrix; suffix digits stream through a Terms in a group share one mapped equality factor ( Reviewed by Cursor Bugbot for commit f35a8a9. Bugbot is set up for automated code reviews on this repo. Configure here. |
Documentation blast radius (advisory)These regions may need doc/spec/book updates based on changed paths. Changed files in this PR: 28
|
PCS Profile Benchmark
13 of 13 profiles passed. Times are medians of Each sample verifies the same proof first with the configured multi-threaded pool and then with one thread. Both timings reuse the same verifier setup. Merge-base comparisons are available for Benchmark shards
Public opening statements
One-hot profiles generate deterministic witnesses with one Direct evaluates the public setup contribution during Stage 2. Recursive carries the same check through a Stage 3 setup-product sumcheck. Both modes execute the complete fold schedule and terminal verification. The chunked profiles Generated profiles may select different A, B, and D ring dimensions at different fold levels. The short profile names omit those dimensions. Each sample generates deterministic witnesses and opening points, prepares setup, commits, proves, serializes the proof, checks its size, prepares verifier setup, and verifies the claimed openings. It does not test malformed proofs. Phase time
Proof size and protocol shape
Grinding retries are rejected attempts at each fold, listed in measured-run order. Zero means the first sampled nonce was accepted. Memory and setup size
Deltas are shown only for profiles with a matching merge-base case. Negative is smaller or faster. The uploaded |
CI test timing
Run summary
Wall time spans 2 parallel nextest slice shards. Slowest tests
Regressions vs mainNo per-test regressions above the threshold. New slow testsNo new tests ≥30s vs main baseline. |
cf985c5 to
18a1250
Compare
quangvdao
changed the title
github-actions
Bot
added
spec
quangvdao
changed the title
Make dense recursive suffix tables the only tensor-packed EOR representation. Remove sparse and lazy implementations, compute hooks, tests, and the synthetic benchmark while preserving the live protocol.
Pack balanced suffix digits and contract tensor columns directly from the compact witness instead of allocating a full base-field table. Reuse row iterators for dense inputs and remove temporary coordinate allocations from tensor transposition and projection. Keep the complete suffix tensor API in its own focused module. BREAKING CHANGE: ExtField implementors must provide allocation-free coefficient construction and access. TensorColumnSource implementors must provide a row iterator.
Remove stale sparse and suffix-only wording after the EOR rebuild. Drop the unused tensor batch bound and align the packed sum-check spec with the surviving dense implementation.
871f04d to
e96f50d
Compare
quangvdao
changed the title
| where | ||
| Self: 'a; | ||
|
|
||
| /// Yield the `width` coefficients at flat tail index `tail`. |
There was a problem hiding this comment.
This public trait now relies on implementors returning exactly width items, but the fold code zips the iterator into fixed-width accumulators without checking exact length. The current implementors are fine, but a short row would silently leave trailing slots unchanged and a long row would silently drop extras. If this stays public, can we document the exact-length invariant as hard and add a debug assertion/helper to catch bad implementors in tests?
| return Err(AkitaError::InvalidInput( | ||
| "extension-opening reduction input claim mismatch".to_string(), | ||
| )); | ||
| #[cfg(debug_assertions)] |
There was a problem hiding this comment.
Moving this term-derived input-claim check behind debug assertions weakens the production boundary. I understand the full scan is expensive, but in release a mismatch between transcript-bound partials and generated dense terms will now run through proving and only fail later as a final-oracle mismatch. Can we add either a diagnostic/profile-mode consistency check or a regression that deliberately corrupts term construction and proves release-style behavior still fails clearly?
| } | ||
| } | ||
|
|
||
| let mut folded_factor = match factor { |
There was a problem hiding this comment.
This claims to share the dense group factor, but after the first challenge each term converts the shared factor into its own owned folded table. For a group with multiple polynomials, every term then folds the same transparent factor independently, so factor work/storage still scales with term count. Can we move the factor state to a group-level EOR object, or otherwise fold the common factor once per round and feed it to the per-witness accumulations?
| { | ||
| use rayon::prelude::*; | ||
|
|
||
| // Coarse subtree expansion wins at the terminal suffix sizes. |
There was a problem hiding this comment.
This comment does not quite match the branch below. For small tables with multiple Rayon threads we still build out and run a parallel final map; only the equality-table construction stays serial. Either route small tables through the fused serial mapped path, or rewrite the comment so the intended small-table behavior is clear.
| harness = false | ||
|
|
||
| [[bench]] | ||
| name = "extension_opening_reduction" |
There was a problem hiding this comment.
This PR is performance-motivated but removes the dedicated EOR benchmark target entirely. Deleting sparse coverage makes sense with the sparse path gone, but we still need a dense replacement that covers owned factors, multi-term shared factors, cylindrical padding, and fp32/fp64 extension-degree cases; otherwise regressions in the new dense/shared-factor path will be hard to see.
| &CpuBackend::DEFAULT, None, view, &logical_point | ||
| ) | ||
| .unwrap(); | ||
| let expected_partials = |
There was a problem hiding this comment.
This test is useful for setup-prefix tensor-source wiring, but it still compares against the same dense helper stack used by the implementation. The new risky path is the full mixed setup-prefix + recursive-suffix EOR: setup-prefix projection, suffix projection, shared factor construction, cylindrical padding, common rho, and final-factor replay. Can we add a protocol-level dense-oracle regression that checks input claim, per-group final factor, per-group final claim, and the batched final claim independently?
|
@RadNi I worked through the six review comments and pushed the validated revision to my fork. I no longer have LayerZero repository access, so could you please mirror this commit onto the LayerZero PR branch? Fork commit: quangvdao@dd0a9fb This is a verified fast-forward from the current PR head. The revision:
Validation passed: the full repository preflight, all four prescribed release Clippy configurations, and the complete Once it is mirrored, I will verify the LayerZero head and follow up on the individual review threads. |
2 participants
Summary
This PR replaces the old extension opening reduction, or EOR, prover with one dense implementation for every live EOR source. The same reduction now handles a borrowed setup prefix and a compact recursive suffix. Sparse witnesses and lazy sparse factors are removed.
The new implementation does the following work:
The general commitment and NTT work found during the same profiling effort now lives in PR #430. This PR contains only EOR and the tensor and equality helpers that EOR uses.
Normative records:
specs/subring-coefficient-packing.mddefines when later setup prefixes use evaluation trace and EOR.specs/packed-sumcheck.mdrecords the current scalar EOR boundary and the approved packed implementation that remains to be done.Diff metadata
ca95c81dcc268618980e22357176e22a580f5e80onmaine96f50d8715c680a419a40f01e529850061d655dWhy this change was needed
The repository had three EOR representations. It had dense tables, sparse witnesses, and a lazy sparse tensor factor. Generated production schedules did not use the sparse witness path. Direct tests and a synthetic benchmark were its only callers.
That unused path had a large cost. Every change to EOR had to preserve separate constructors, table types, compute hooks, fold logic, and tests. The sparse directory alone held more than 1,700 lines. It also hid the two source forms that the live protocol needs:
The new design keeps those source forms until the tensor boundary, then converts both into the same dense extension field reduction.
When Akita runs EOR
The opening method decides whether a fold uses EOR.
Levels 0 and 1 can use subring coefficient packing and bypass EOR. A later level can receive both a recursive witness and a setup prefix. When that level uses evaluation trace over fp32 or fp64, one EOR includes both sources.
This policy does not limit setup offloading to two levels. A later fold may create another setup prefix when the complete schedule selects that edge. The receiving fold must open the prefix and witness with the same scheduled method.
Prover data flow
flowchart TD A[Evaluation trace fold] --> B[Opening groups] B --> C[Borrowed setup prefix] B --> D[Compact suffix digits] C --> E[TensorProjectionKernel] D --> E E --> F[Opening values and tensor partials] F --> G[Add values and partials to transcript] G --> H[Sample eta and claim coefficients] H --> I[Derive the batched input claim from row partials] E --> J[Build dense packed witnesses] H --> K[Build one equality factor per group] J --> L[Create one dense term per opening] K --> L L --> M[Extend shorter groups over fixed zero coordinates] M --> N[Run one degree two sum-check] N --> O[Return one final claim per opening]Change surface
Vec<E>representationVec<F>could be built before packingi8digits are converted as each row is readAkitaExpandedSetupArc<Vec<E>>until the first foldExtensionOpeningTables::Cylindricaladds virtual zero fixed variablesTensor sources
Borrowed setup prefix
RecursiveFoldSource::SetupPrefixnow implements the same tensor projection interface as a recursive suffix.setup_prefix_base_evalschecks the frozen ring dimension, checks that the prefix length is a multiple of the ring dimension, and bounds the slice against the shared expanded setup matrix.The source borrows that checked field slice. It does not copy the prefix into a second witness. The shared dense tensor helpers then compute column partials and packed extension field evaluations.
Compact recursive suffix
The suffix keeps its balanced digits in
SuffixWitnessView. ItsTensorColumnSourceimplementation returns a row iterator that converts each digit to the base field as the contraction consumes it. The prover no longer allocates a complete padded base field table for this operation.tensor_packed_witness_evalsandtensor_column_partials_split_foldare still the canonical tensor operations. Dense slices and compact suffix rows differ only in how they supply a row.Extension field coefficient access
ExtFieldnow requiresfrom_base_fnandbase_coefficient. The existingfrom_base_sliceandto_base_vecmethods use those primitives. Tensor transposition can therefore construct and read extension field coordinates without temporary vectors.One reduction across opening groups
The prover adds the claimed openings and tensor partials to the transcript before it samples
etaand the claim batching coefficients. It derives one input claim per opening from the row partials, then combines those claims with the sampled coefficients.Each opening still creates one
ExtensionOpeningReductionTerm. Terms in the same group share the full equality factor returned byEqPolynomial::evals_mapped. Groups with fewer tail variables useExtensionOpeningTables::Cylindrical. This keeps their native witness tables small while they follow the common challenge sequence.The degree two sum-check returns one common point
rho. The prover then exposes the final witness and factor value for every original opening. The existing later protocol stage binds that complete final vector to the committed witness with a separate challenge.Dense table ownership and round work
DenseEorFactorhas two states:Sharedholds the full factor table in anArc<Vec<E>>at the start of the reduction.Ownedholds a term specific folded table after the first challenge.This is the only copy boundary. Every term in a group reads the same full factor table for the first round. Each term then owns only its half sized folded factor.
For later rounds,
fused_fold_and_accumulatefolds the witness and factor and computes the next round constant and quadratic coefficients in one pass. The next call reuses those cached coefficients.The accumulator still follows the field contract. It delays reduction only when
HasUnreducedOps::DELAYED_PRODUCT_SUM_IS_EXACTis true. Other fields reduce each product before adding it. The lossy accumulator regression test covers this fallback.Security and verifier behavior
This PR changes prover storage and computation. It does not change the statement that the verifier checks.
The release prover no longer recomputes the input claim by scanning every dense term. It uses the claim independently derived from the transcript bound row partials. Debug builds still recompute the complete table claim and reject any mismatch. This removes duplicate prover work without moving a trust decision into the verifier.
Malformed point dimensions, partial counts, table lengths, setup prefix shapes, group sizes, and virtual domain overflows return
AkitaError. The standalone dense constructor still checks that witness and factor tables have the same nonzero power of two length.Breaking API changes
This is an intentional internal API cut. The repository does not promise backward compatibility.
The PR removes these public or crate visible sparse interfaces:
SparseExtensionOpeningWitnessTensorPackedWitnessSPARSE_TENSOR_FACTOR_MAX_LAZY_ROUNDSIt also changes two traits:
ExtFieldimplementors must provide allocation free coefficient construction and access throughfrom_base_fnandbase_coefficient.TensorColumnSourceimplementors must provide a row iterator and accept the requested row width.No wrapper preserves the old sparse API. Dense standalone callers can still use
ExtensionOpeningReductionTerm::newandExtensionOpeningReductionProver::from_dense_tables.Measured effects
Local microbenchmarks collected while developing the retained source changes showed the following results:
These are focused prover measurements, not end to end proving claims. The final PR removes the temporary benchmark harnesses. The production profile jobs in CI cover complete fp32, fp64, fp128, grouped, recursive, and distributed flows.
Commit map
5e0f4df94removes the sparse EOR implementation and synthetic benchmark.e21d11572lets later evaluation trace folds include a dense setup prefix.60e2b0b0astreams compact suffix rows and adds direct extension field coefficient access.057c4b7ccbuilds mapped equality tables in one canonical operation.b3044927aprepares one transparent factor for each opening group.33a2a926eshares the full dense factor table across terms until the first fold.e96f50d87removes stale sparse wording and updates the live specifications.Validation at
e96f50d87Local validation on the exact head passed:
cargo fmt --all --checkrtk cargo test -p akita-prover --test extension_opening_reduction, with 14 tests passedrtk cargo test -p akita-prover --lib extension_opening_reduction, with 2 tests passed and 333 filtered outAt the time this description was published, 40 GitHub checks had passed and none had failed. The successful checks included Clippy, fuzzing, Jolt compatibility, documentation, security scans, the portable x86 verifier check, native AArch64 NTT tests, the emulated AVX-512 NTT job, and six of the seven production profile jobs.
The following jobs were still running:
Bench (3-fp128-base)Test (1/2)Test (2/2)Schedule table drift (all-schedules)Transcript semantics (transcript-blake2b)Transcript semantics (transcript-keccak)Remaining work
This PR completes the dense EOR consolidation. The approved packed sum-check specification is not implemented here. That work can replace the scalar extension field loops in the dense fold, accumulation, factor fold, and tensor partial contraction without changing the protocol.
The removed benchmark measured a dormant sparse representation. A future EOR benchmark should measure the surviving dense kernel boundary and should land with the packed implementation.
Reviewer map
Suggested review order:
specs/subring-coefficient-packing.mdfor the setup prefix policy.crates/akita-prover/src/protocol/core/extension_opening_reduction.rsfor transcript order and group assembly.crates/akita-prover/src/protocol/extension_opening_reduction/term.rsandtables.rsfor the canonical prover state.crates/akita-prover/src/backend/recursive/setup_prefix_source.rsfor the borrowed setup prefix.crates/akita-prover/src/backend/recursive/witness/tensor.rsfor the compact suffix source.crates/akita-types/src/extension_opening_reduction.rsandcrates/akita-algebra/src/eq_poly.rsfor the shared tensor operations.crates/akita-prover/tests/extension_opening_reduction.rsand the source local tests for correctness and rejection coverage.specs/packed-sumcheck.mdfor the separate packed follow up.