Buffer Abstraction#260
Conversation
|
cc @recmo |
shreyas-londhe
left a comment
There was a problem hiding this comment.
Thanks for the PR @zkfriendly. Really like where this lands. Trait split is the right cut, the CPU backend stays a thin wrapper with no second implementation to drift, and the prover got simpler dropping the Any/Cow recycling for mixed_linear_combination and linear_forms_rlc. Left a few inline comments, but the shape is solid.
| fn interleaved_encode( | ||
| &self, | ||
| messages: Messages<'_, F>, | ||
| masks: &ActiveBuffer<F>, | ||
| codeword_length: usize, | ||
| ) -> ActiveBuffer<F>; |
There was a problem hiding this comment.
suggestion: ReedSolomon is a backend-abstraction trait but hardcodes the global ActiveBuffer alias in its signature, which defeats the purpose for the one trait that most needs it.
Give it an associated type Buffer: Buffer<F> and thread it through the registry (type Dyn<F> = dyn ReedSolomon<F, Buffer = ActiveBuffer<F>>); NttEngine then declares type Buffer = CpuBuffer<F>. The payoff is enforcement, not tidiness: today a second backend can't even be expressed here, since the trait forces every impl to speak CpuBuffer. With the associated type, an impl whose buffer doesn't match the build's ActiveBuffer fails to register as a type error instead of sitting as dead code that forces host round-trips if called.
There was a problem hiding this comment.
I agree with this point. But I think we can keep it simple for now, and refactor it once we introduce another backend type.
|
Superseded by #263 — same branch now pushed directly to worldfnd/whir (GitHub can't change a PR's head repo, so it had to be reopened). Review comments here have been addressed on the branch: the blocking basecase check is fixed, build_nodes caller obligation documented, and the fold_pair overrides removed as part of a larger simplification that drops the slice/view layer entirely. Replies on the individual threads to follow on #263. |
Summary
This PR introduces the buffer abstraction layer. All buffer abstraction traits are defined in the buffer crate, with a CPU implementation that essentially wraps the existing in-memory code paths.
At this moment, the buffer crate still depends on existing algebra and protocol crates. Future work can consolidate the relevant logic directly into the buffer crate as the abstraction settles.
Notes
There is also an experimental Metal implementation, but it is not included in this PR.
zk_whiris not fully ported to the buffer abstraction yet. It uses.as_slice()in places to keep the code working, so behavior is essentially unchanged compared to directly using CPU buffers. Since WHIR v3 is coming and is expected to replacezk_whir, that path is intentionally not fully ported for now.