NTT-based univariate multiplication#174
NTT-based univariate multiplication#174erdkocak wants to merge 19 commits intoVerified-zkEVM:masterfrom
Conversation
n^2 ntt implementation, some basic tests
Refactor NTT butterfly iterations & remove redundant input arrays and padding
Improve NTT benchmark sweep and dynamic domain selection
Split univariate NTT tests by module
Minor cleanup
🤖 Gemini PR SummaryCore Components
Status of Proofs
Testing and Benchmarking
Statistics
Lean Declarations ✏️ **Added:** 41 declaration(s)
❌ **Added:** 5 `sorry`(s)
🎨 **Style Guide Adherence**There are more than 20 violations in this diff. They are grouped by rule below:
📄 **Per-File Summaries**
Last updated: 2026-03-21 19:17 UTC. |
Fix NTT test lint violations
dhsorens
left a comment
There was a problem hiding this comment.
at first glance, the code is clean and well-organized and spec looks to be right. Just a few comments for now
| def bitRevPermute (D : Domain R) (a : Array R) : Array R := | ||
| Array.ofFn (fun i : D.Idx => a.getD (bitRevNat D.logN i.1) 0) | ||
|
|
||
| /-- One butterfly stage of the iterative radix-2 transform. -/ |
There was a problem hiding this comment.
it looks like bitRevPermute, butterflyStage, and runStages are nearly identical between Forward.lean and Inverse.lean. to avoid code duplication, it's probably a good idea to factor these into a shared definition (parametric over omega/omegaInv) in a shared file. you might want to put all the shared operations in there as well (e.g. including bitRevNat)
| namespace NTT | ||
| namespace Inverse | ||
|
|
||
| variable {R : Type*} [Field R] |
There was a problem hiding this comment.
it might be worth trying to weaken the field assumption, if possible. we can always generalize results once they're proved though so don't worry too much about this right away
| /-- Required convolution length for multiplying `p` and `q`. -/ | ||
| def requiredLength (p q : CPolynomial.Raw R) : Nat := | ||
| p.trim.size + q.trim.size - 1 | ||
|
|
There was a problem hiding this comment.
note that in Nat, 0 - 1 = 0 which can lead to undesirable behavior. worth checking that this underflow won't affect the rest
forwardImpl = forwardSpecinverseImpl = inverseSpecfastMulImpl = p * q