-
Notifications
You must be signed in to change notification settings - Fork 149
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add compress
to compiler
#752
Conversation
16daa4d
to
7f9d305
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #752 +/- ##
==========================================
- Coverage 85.33% 85.07% -0.26%
==========================================
Files 54 56 +2
Lines 4166 4343 +177
==========================================
+ Hits 3555 3695 +140
- Misses 611 648 +37
... and 8 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
709413a
to
2a36291
Compare
2a36291
to
ebe3a22
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall it lgtm, just some requests for comments here and there.
The PR still needs an issue though
src/composer/compiler/scalar.rs
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Imo it would be nice to have 'compressor' somewhere in the module path. Maybe add a compressor module with scalar and witness in it?
src/composer/compiler/scalar.rs
Outdated
]; | ||
|
||
#[cfg(test)] | ||
mod tests { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am missing tests for compressing and decompressing scalars that are bigger than i8
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests are here:
plonk/src/composer/compiler/scalar.rs
Lines 1049 to 1073 in 0862d97
#[test] | |
fn compress_decompress_works() { | |
let compressor = ScalarCompressor::default(); | |
let mut buffer: Vec<u8> = vec![]; | |
// check negatives | |
for v in 0..=u8::MAX { | |
buffer.clear(); | |
let x = BlsScalar::zero() - BlsScalar::from(v as u64); | |
let n = compressor.compress(&x, &mut buffer); | |
let (p, y) = ScalarCompressor::decompress(&buffer[..n]); | |
assert_eq!(n, p); | |
assert_eq!(x, y); | |
} | |
// check positives | |
for v in 0..=u8::MAX { | |
buffer.clear(); | |
let x = BlsScalar::from(v as u64); | |
let n = compressor.compress(&x, &mut buffer); | |
let (p, y) = ScalarCompressor::decompress(&buffer[..n]); | |
assert_eq!(n, p); | |
assert_eq!(x, y); | |
} | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And for scalars bigger than u8
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
11b690f
to
5d3d862
Compare
This commit introduces `Compiler::compress` and `Compiler::decompress`, two functions that will create a compressed representation of a circuit that can be used to generate prover and verifier keys. The compression strategy takes advantage of the fact that circuit representations are sparse; meaning, most of the scalars are zeroes. We also have a higher incidence of `1` and `-1`. This will result in expressive gains in terms of storage. For instance, a `2^16` circuit can be compressed into roughly 250Kb.
5d3d862
to
c4fda09
Compare
This commit introduces
Compiler::compress
andCompiler::decompress
, two functions that will create a compressed representation of a circuit that can be used to generate prover and verifier keys.The compression strategy takes advantage of the fact that circuit representations are sparse; meaning, most of the scalars are zeroes. We also have a higher incidence of
1
and-1
, so any value that is equivalent to ai8
is compressed into a single byte, instead of the regular32
bytes of a bls12-381.This will result in expressive gains in terms of storage. For instance, a
2^12
circuit can be compressed into roughly 100Kb.