Optimize Bivariate poly math#576
Open
dvdplm wants to merge 24 commits into
Open
Conversation
sprinkled a few quesitions/notices for reviewers, added some validation where strictly needed, stop returning Result when code is infallible
add: DoublePoly::from_bivariate helper
Consolidated Tests Results 2026-05-21 - 10:08:25Test ResultsDetails
test-reporter: Run #2318
🎉 All tests passed!TestsView All Tests
🍂 No flaky tests in this run. Github Test Reporter by CTRF 💚 🔄 This comment has been updated |
…efault` derivation
chore: moved compute_powers* to matrix
opt: do "outer Horner" for full_eval opt: see if avoiding indexing is faster (avoids bounds checks)
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors bivariate polynomial evaluation to remove the prior generic/ndarray-driven evaluation path and replace it with specialized, inlined evaluation routines for the shapes used in production, while relocating the remaining generic matmul utilities into a new matrix module.
Changes:
- Reimplemented
BivariatePolystorage/evaluation to use a row-major coefficientVecplus specializedpartial_x_eval,partial_y_eval,full_eval, and fusedpartial_evals. - Moved generic
MatrixMul+ power precomputation helpers (compute_powers,compute_powers_list) intocore/threshold-algebra/src/matrix.rsand updated imports/call sites. - Updated threshold execution VSS/PRSS code to use the new infallible bivariate APIs and introduced
DoublePoly::from_bivariateto leverage fused partial evaluations.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| dylint.toml | Removes trailing formatting artifact in the config. |
| core/threshold-execution/src/small_execution/prss.rs | Switches power-precompute + matmul imports to algebra::matrix. |
| core/threshold-execution/src/malicious_execution/large_execution/malicious_vss.rs | Updates to infallible bivariate API and uses DoublePoly::from_bivariate. |
| core/threshold-execution/src/large_execution/vss.rs | Updates evaluation calls to new bivariate API and adds DoublePoly::from_bivariate. |
| core/threshold-execution/src/large_execution/single_sharing.rs | Replaces compute_powers usage with an inlined power-generation loop; updates matmul import. |
| core/threshold-execution/src/large_execution/double_sharing.rs | Updates matmul import to algebra::matrix. |
| core/threshold-algebra/src/syndrome.rs | Switches compute_powers_list import to matrix module. |
| core/threshold-algebra/src/matrix.rs | New module containing MatrixMul + compute_powers* helpers and associated tests. |
| core/threshold-algebra/src/lib.rs | Exposes the new matrix module. |
| core/threshold-algebra/src/galois_rings/degree_3.rs | Switches compute_powers import to matrix module. |
| core/threshold-algebra/src/galois_rings/degree_4.rs | Switches compute_powers import to matrix module. |
| core/threshold-algebra/src/galois_rings/degree_5.rs | Switches compute_powers import to matrix module. |
| core/threshold-algebra/src/galois_rings/degree_6.rs | Switches compute_powers import to matrix module. |
| core/threshold-algebra/src/galois_rings/degree_7.rs | Switches compute_powers import to matrix module. |
| core/threshold-algebra/src/galois_rings/degree_8.rs | Switches compute_powers import to matrix module. |
| core/threshold-algebra/src/galois_rings/common.rs | Switches compute_powers_list import to matrix module. |
| core/threshold-algebra/src/bivariate.rs | Replaces ndarray-based bivariate representation/eval with specialized vector-based routines and new tests. |
| core/threshold-algebra/Cargo.toml | Adds Criterion dev-dependency and registers the new bivariate benchmark. |
| core/threshold-algebra/benches/bivariate.rs | New Criterion benchmark suite for bivariate sampling/evaluation paths. |
| Cargo.lock | Adds Criterion to the lockfile due to new benchmark dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refactor the
BivariatePolymaths to avoid generic impl and instead provide only optimized code for the shapes in actual use: vector x matrix (partial_x_eval) and matrix x vector (partial_y_eval). These are now inlined (along with the vector x vector case).Also in this PR:
pubmethods)Resultpartial_evalsthat compute both partial evals in one go (do note theTODO(dp)in the code though).Note: In this PR the
ndarraycrate is still present and the generic matmul code is moved tomatrix.rs. The follow-up PR #585 removes it entirely along with a second batch of optimizations that are unrelated to bivariate polys.Added a few benchmarks and new tests.
This PR is ~3x faster than
mainon my machine for the deg 4 (i.e. 5x5 grid) case that we use in production.