From 04f4296bcbe08f580e95c024a67d5d4997e1c641 Mon Sep 17 00:00:00 2001 From: Hossein Moghaddas Date: Tue, 14 Nov 2023 00:52:38 +0100 Subject: [PATCH 1/5] Add `col_major` to see what happens --- poly-commit/src/linear_codes/mod.rs | 9 ++++-- poly-commit/src/utils.rs | 49 ++++++++++++++++++----------- 2 files changed, 38 insertions(+), 20 deletions(-) diff --git a/poly-commit/src/linear_codes/mod.rs b/poly-commit/src/linear_codes/mod.rs index e6628dd8..f90e8dc6 100644 --- a/poly-commit/src/linear_codes/mod.rs +++ b/poly-commit/src/linear_codes/mod.rs @@ -553,10 +553,15 @@ where H::Output: Into, C::Leaf: Default + Clone + Send, { - let ext_mat_cols = ext_mat.cols(); + let ext_mat_cols = ext_mat.cols().clone(); let mut col_hashes: Vec = cfg_into_iter!(ext_mat_cols) - .map(|col| hash_column::(col, &col_hash_params).unwrap()) + .map(|col| { + H::evaluate(col_hash_params, col) + .map_err(|_| Error::HashingError) + .map(|x| x.into()) + .unwrap() + }) .collect(); // pad the column hashes with zeroes diff --git a/poly-commit/src/utils.rs b/poly-commit/src/utils.rs index 34e41197..95125379 100644 --- a/poly-commit/src/utils.rs +++ b/poly-commit/src/utils.rs @@ -54,7 +54,8 @@ pub(crate) fn ceil_div(x: usize, y: usize) -> usize { pub struct Matrix { pub(crate) n: usize, pub(crate) m: usize, - entries: Vec>, + row_major: Vec>, + col_major: Vec>, } impl Matrix { @@ -74,32 +75,48 @@ impl Matrix { ); // TODO more efficient to run linearly? - let entries: Vec> = (0..n) + let row_major: Vec> = (0..n) .map(|row| (0..m).map(|col| entry_list[m * row + col]).collect()) .collect(); + let col_major = (0..m) + .map(|col| (0..n).map(|row| row_major[row][col]).collect()) + .collect(); - Self { n, m, entries } + Self { + n, + m, + row_major, + col_major, + } } /// Returns a Matrix given a list of its rows, each in turn represented as a list of field elements. /// /// # Panics /// Panics if the sub-lists do not all have the same length. - pub(crate) fn new_from_rows(row_list: Vec>) -> Self { - let m = row_list[0].len(); + pub(crate) fn new_from_rows(row_major: Vec>) -> Self { + let m = row_major[0].len(); - for row in row_list.iter().skip(1) { + for row in row_major.iter().skip(1) { assert_eq!( row.len(), m, "Invalid matrix construction: not all rows have the same length" ); } + let col_major = (0..m) + .map(|col| { + (0..row_major.len()) + .map(|row| row_major[row][col]) + .collect() + }) + .collect(); Self { - n: row_list.len(), + n: row_major.len(), m, - entries: row_list, + row_major, + col_major, } } @@ -110,19 +127,17 @@ impl Matrix { /// Index bound checks are waived for efficiency and behaviour under invalid indexing is undefined #[cfg(test)] pub(crate) fn entry(&self, i: usize, j: usize) -> F { - self.entries[i][j] + self.row_major[i][j] } /// Returns self as a list of rows - pub(crate) fn rows(&self) -> Vec> { - self.entries.clone() + pub(crate) fn rows(&self) -> &Vec> { + &self.row_major } /// Returns self as a list of columns - pub(crate) fn cols(&self) -> Vec> { - (0..self.m) - .map(|col| (0..self.n).map(|row| self.entries[row][col]).collect()) - .collect() + pub(crate) fn cols(&self) -> &Vec> { + &self.col_major } /// Returns the product v * self, where v is interpreted as a row vector. In other words, @@ -142,9 +157,7 @@ impl Matrix { .map(|col| { inner_product( v, - &(0..self.n) - .map(|row| self.entries[row][col]) - .collect::>(), + &self.col_major[col], ) }) .collect() From ddfd74d7669e405d15fe5285d1fc0f6e0b5cbd92 Mon Sep 17 00:00:00 2001 From: Hossein Moghaddas Date: Tue, 14 Nov 2023 01:09:46 +0100 Subject: [PATCH 2/5] Fix monir bug --- poly-commit/src/linear_codes/mod.rs | 7 +------ poly-commit/src/utils.rs | 7 +------ 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/poly-commit/src/linear_codes/mod.rs b/poly-commit/src/linear_codes/mod.rs index f90e8dc6..9d47f44f 100644 --- a/poly-commit/src/linear_codes/mod.rs +++ b/poly-commit/src/linear_codes/mod.rs @@ -556,12 +556,7 @@ where let ext_mat_cols = ext_mat.cols().clone(); let mut col_hashes: Vec = cfg_into_iter!(ext_mat_cols) - .map(|col| { - H::evaluate(col_hash_params, col) - .map_err(|_| Error::HashingError) - .map(|x| x.into()) - .unwrap() - }) + .map(|col| hash_column::(col, &col_hash_params).unwrap()) .collect(); // pad the column hashes with zeroes diff --git a/poly-commit/src/utils.rs b/poly-commit/src/utils.rs index 95125379..d67b453c 100644 --- a/poly-commit/src/utils.rs +++ b/poly-commit/src/utils.rs @@ -154,12 +154,7 @@ impl Matrix { ); (0..self.m) - .map(|col| { - inner_product( - v, - &self.col_major[col], - ) - }) + .map(|col| inner_product(v, &self.col_major[col])) .collect() } } From 453df5b8e4ccc9d3a654114d8b87417ecb5a738c Mon Sep 17 00:00:00 2001 From: mmagician Date: Tue, 12 Mar 2024 12:24:17 +0100 Subject: [PATCH 3/5] fix the missing clone --- poly-commit/src/linear_codes/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/poly-commit/src/linear_codes/mod.rs b/poly-commit/src/linear_codes/mod.rs index 30b524c0..decb7047 100644 --- a/poly-commit/src/linear_codes/mod.rs +++ b/poly-commit/src/linear_codes/mod.rs @@ -254,7 +254,7 @@ where let ext_mat_cols = ext_mat.cols(); let leaves: Vec = cfg_into_iter!(ext_mat_cols) .map(|col| { - H::evaluate(ck.col_hash_params(), col) + H::evaluate(ck.col_hash_params(), col.clone()) .map_err(|_| Error::HashingError) .unwrap() }) From e20f67e8122b26f1adea8847d491c65d41588ab1 Mon Sep 17 00:00:00 2001 From: mmagician Date: Tue, 12 Mar 2024 12:14:42 +0100 Subject: [PATCH 4/5] replace clone with a borrow --- poly-commit/src/linear_codes/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/poly-commit/src/linear_codes/mod.rs b/poly-commit/src/linear_codes/mod.rs index decb7047..96b494d3 100644 --- a/poly-commit/src/linear_codes/mod.rs +++ b/poly-commit/src/linear_codes/mod.rs @@ -19,7 +19,7 @@ use ark_std::string::ToString; use ark_std::vec::Vec; #[cfg(feature = "parallel")] -use rayon::iter::{IntoParallelIterator, IntoParallelRefIterator, ParallelIterator}; +use rayon::iter::{IntoParallelRefIterator, ParallelIterator}; mod utils; @@ -252,9 +252,9 @@ where // 2. Create the Merkle tree from the hashes of each column. let ext_mat_cols = ext_mat.cols(); - let leaves: Vec = cfg_into_iter!(ext_mat_cols) + let leaves: Vec = cfg_iter!(ext_mat_cols) .map(|col| { - H::evaluate(ck.col_hash_params(), col.clone()) + H::evaluate(ck.col_hash_params(), col.borrow()) .map_err(|_| Error::HashingError) .unwrap() }) From cf2af292f36f75fd5d5097440b474530e8c4d89f Mon Sep 17 00:00:00 2001 From: Dimitris Date: Tue, 26 Mar 2024 11:17:25 +0100 Subject: [PATCH 5/5] Split Matrix into two separate representations. --- bench-templates/src/lib.rs | 3 + poly-commit/benches/brakedown_ml_times.rs | 2 +- .../src/linear_codes/data_structures.rs | 6 +- poly-commit/src/linear_codes/mod.rs | 15 +- poly-commit/src/utils.rs | 136 +++++++++++------- 5 files changed, 99 insertions(+), 63 deletions(-) diff --git a/bench-templates/src/lib.rs b/bench-templates/src/lib.rs index c3211c7f..c825ba82 100644 --- a/bench-templates/src/lib.rs +++ b/bench-templates/src/lib.rs @@ -48,6 +48,9 @@ pub fn bench_pcs_method< let pp = PCS::setup(num_vars, Some(num_vars), rng).unwrap(); let (ck, vk) = PCS::trim(&pp, num_vars, num_vars, None).unwrap(); + // Modify to alter sample size and/or significance level + group.significance_level(0.1).sample_size(10); + group.bench_with_input( BenchmarkId::from_parameter(num_vars), &num_vars, diff --git a/poly-commit/benches/brakedown_ml_times.rs b/poly-commit/benches/brakedown_ml_times.rs index 55ebfa7f..dbfd5452 100644 --- a/poly-commit/benches/brakedown_ml_times.rs +++ b/poly-commit/benches/brakedown_ml_times.rs @@ -53,7 +53,7 @@ fn rand_point_brakedown_ml(num_vars: usize, rng: &mut ChaCha20Rng } const MIN_NUM_VARS: usize = 12; -const MAX_NUM_VARS: usize = 22; +const MAX_NUM_VARS: usize = 22; // 30 bench!( Brakedown, diff --git a/poly-commit/src/linear_codes/data_structures.rs b/poly-commit/src/linear_codes/data_structures.rs index 0ceab21b..5332dd45 100644 --- a/poly-commit/src/linear_codes/data_structures.rs +++ b/poly-commit/src/linear_codes/data_structures.rs @@ -1,5 +1,5 @@ use super::utils::SprsMat; -use crate::{utils::Matrix, PCCommitment, PCCommitmentState}; +use crate::{utils::ColumnMajorMatrix, utils::RowMajorMatrix, PCCommitment, PCCommitmentState}; use ark_crypto_primitives::{ crh::CRHScheme, merkle_tree::{Config, LeafParam, Path, TwoToOneParam}, @@ -95,8 +95,8 @@ where F: PrimeField, H: CRHScheme, { - pub(crate) mat: Matrix, - pub(crate) ext_mat: Matrix, + pub(crate) mat: RowMajorMatrix, + pub(crate) ext_mat: ColumnMajorMatrix, pub(crate) leaves: Vec, } diff --git a/poly-commit/src/linear_codes/mod.rs b/poly-commit/src/linear_codes/mod.rs index 96b494d3..244b92a7 100644 --- a/poly-commit/src/linear_codes/mod.rs +++ b/poly-commit/src/linear_codes/mod.rs @@ -1,4 +1,4 @@ -use crate::utils::{inner_product, Matrix}; +use crate::utils::{inner_product, ColumnMajorMatrix, RowMajorMatrix}; use crate::{ to_bytes, Error, LabeledCommitment, LabeledPolynomial, PCCommitterKey, PCUniversalParams, PCVerifierKey, PolynomialCommitment, @@ -111,7 +111,10 @@ where /// Arrange the coefficients of the polynomial into a matrix, /// and apply encoding to each row. /// Returns the tuple (original_matrix, encoded_matrix). - fn compute_matrices(polynomial: &P, param: &Self::LinCodePCParams) -> (Matrix, Matrix) { + fn compute_matrices( + polynomial: &P, + param: &Self::LinCodePCParams, + ) -> (RowMajorMatrix, ColumnMajorMatrix) { let mut coeffs = Self::poly_to_vec(polynomial); // 1. Computing the matrix dimensions. @@ -120,11 +123,11 @@ where // padding the coefficient vector with zeroes coeffs.resize(n_rows * n_cols, F::zero()); - let mat = Matrix::new_from_flat(n_rows, n_cols, &coeffs); + let mat = RowMajorMatrix::new_from_flat(n_rows, n_cols, &coeffs); // 2. Apply encoding row-wise let rows = mat.rows(); - let ext_mat = Matrix::new_from_rows( + let ext_mat = ColumnMajorMatrix::new_from_rows( cfg_iter!(rows) .map(|r| Self::encode(r, param).unwrap()) .collect(), @@ -522,8 +525,8 @@ fn generate_proof( sec_param: usize, distance: (usize, usize), b: &[F], - mat: &Matrix, - ext_mat: &Matrix, + mat: &RowMajorMatrix, + ext_mat: &ColumnMajorMatrix, col_tree: &MerkleTree, sponge: &mut S, ) -> Result, Error> diff --git a/poly-commit/src/utils.rs b/poly-commit/src/utils.rs index 0656a0b7..c00f2658 100644 --- a/poly-commit/src/utils.rs +++ b/poly-commit/src/utils.rs @@ -47,15 +47,22 @@ pub(crate) fn ceil_div(x: usize, y: usize) -> usize { #[derive(Derivative, CanonicalSerialize, CanonicalDeserialize)] #[derivative(Default(bound = ""), Clone(bound = ""), Debug(bound = ""))] -pub struct Matrix { +pub struct RowMajorMatrix { pub(crate) n: usize, pub(crate) m: usize, - row_major: Vec>, - col_major: Vec>, + rows: Vec>, } -impl Matrix { - /// Returns a Matrix of dimensions n x m given a list of n * m field elements. +#[derive(Derivative, CanonicalSerialize, CanonicalDeserialize)] +#[derivative(Default(bound = ""), Clone(bound = ""), Debug(bound = ""))] +pub struct ColumnMajorMatrix { + pub(crate) n: usize, + pub(crate) m: usize, + cols: Vec>, +} + +impl RowMajorMatrix { + /// Returns a RowMajorMatrix of dimensions n x m given a list of n * m field elements. /// The list should be ordered row-first, i.e. [a11, ..., a1m, a21, ..., a2m, ...]. /// /// # Panics @@ -71,25 +78,33 @@ impl Matrix { ); // TODO more efficient to run linearly? - let row_major: Vec> = (0..n) + let rows: Vec> = (0..n) .map(|row| (0..m).map(|col| entry_list[m * row + col]).collect()) .collect(); - let col_major = (0..m) - .map(|col| (0..n).map(|row| row_major[row][col]).collect()) - .collect(); - Self { - n, - m, - row_major, - col_major, - } + Self { n, m, rows } } - /// Returns a Matrix given a list of its rows, each in turn represented as a list of field elements. + /// Returns self as a list of rows + pub(crate) fn rows(&self) -> &Vec> { + &self.rows + } + + /// Returns the entry in position (i, j). **Indexing starts at 0 in both coordinates**, + /// i.e. the first element is in position (0, 0) and the last one in (n - 1, j - 1), + /// where n and m are the number of rows and columns, respectively. + /// + /// Index bound checks are waived for efficiency and behaviour under invalid indexing is undefined + #[cfg(test)] + pub(crate) fn entry(&self, i: usize, j: usize) -> F { + self.rows[i][j] + } + + /// Returns a RowMajorMatrix given a list of its rows, each in turn represented as a list of field elements. /// /// # Panics /// Panics if the sub-lists do not all have the same length. + #[cfg(test)] pub(crate) fn new_from_rows(row_major: Vec>) -> Self { let m = row_major[0].len(); @@ -100,42 +115,14 @@ impl Matrix { "Invalid matrix construction: not all rows have the same length" ); } - let col_major = (0..m) - .map(|col| { - (0..row_major.len()) - .map(|row| row_major[row][col]) - .collect() - }) - .collect(); Self { n: row_major.len(), m, - row_major, - col_major, + rows: row_major, } } - /// Returns the entry in position (i, j). **Indexing starts at 0 in both coordinates**, - /// i.e. the first element is in position (0, 0) and the last one in (n - 1, j - 1), - /// where n and m are the number of rows and columns, respectively. - /// - /// Index bound checks are waived for efficiency and behaviour under invalid indexing is undefined - #[cfg(test)] - pub(crate) fn entry(&self, i: usize, j: usize) -> F { - self.row_major[i][j] - } - - /// Returns self as a list of rows - pub(crate) fn rows(&self) -> &Vec> { - &self.row_major - } - - /// Returns self as a list of columns - pub(crate) fn cols(&self) -> &Vec> { - &self.col_major - } - /// Returns the product v * self, where v is interpreted as a row vector. In other words, /// it returns a linear combination of the rows of self with coefficients given by v. /// @@ -150,11 +137,54 @@ impl Matrix { ); (0..self.m) - .map(|col| inner_product(v, &self.col_major[col])) + .map(|col| { + inner_product( + v, + &(0..self.n) + .map(|row| self.rows[row][col]) + .collect::>(), + ) + }) .collect() } } +impl ColumnMajorMatrix { + /// Returns a ColumnMajorMatrix given a list of its rows, each in turn represented as a list of field elements. + /// + /// # Panics + /// Panics if the sub-lists do not all have the same length. + pub(crate) fn new_from_rows(row_major: Vec>) -> Self { + let m = row_major[0].len(); + + for row in row_major.iter().skip(1) { + assert_eq!( + row.len(), + m, + "Invalid matrix construction: not all rows have the same length" + ); + } + let cols = (0..m) + .map(|col| { + (0..row_major.len()) + .map(|row| row_major[row][col]) + .collect() + }) + .collect(); + + Self { + n: row_major.len(), + m, + cols, + } + } + + /// Returns self as a list of columns + pub(crate) fn cols(&self) -> &Vec> { + &self.cols + } +} + #[inline] pub(crate) fn inner_product(v1: &[F], v2: &[F]) -> F { ark_std::cfg_iter!(v1) @@ -215,14 +245,14 @@ pub(crate) mod tests { #[test] fn test_matrix_constructor_flat() { let entries: Vec = to_field(vec![10, 100, 4, 67, 44, 50]); - let mat = Matrix::new_from_flat(2, 3, &entries); + let mat = RowMajorMatrix::new_from_flat(2, 3, &entries); assert_eq!(mat.entry(1, 2), Fr::from(50)); } #[test] fn test_matrix_constructor_flat_square() { let entries: Vec = to_field(vec![10, 100, 4, 67]); - let mat = Matrix::new_from_flat(2, 2, &entries); + let mat = RowMajorMatrix::new_from_flat(2, 2, &entries); assert_eq!(mat.entry(1, 1), Fr::from(67)); } @@ -230,7 +260,7 @@ pub(crate) mod tests { #[should_panic(expected = "dimensions are 2 x 3 but entry vector has 5 entries")] fn test_matrix_constructor_flat_panic() { let entries: Vec = to_field(vec![10, 100, 4, 67, 44]); - Matrix::new_from_flat(2, 3, &entries); + RowMajorMatrix::new_from_flat(2, 3, &entries); } #[test] @@ -240,7 +270,7 @@ pub(crate) mod tests { to_field(vec![23, 1, 0]), to_field(vec![55, 58, 9]), ]; - let mat = Matrix::new_from_rows(rows); + let mat = RowMajorMatrix::new_from_rows(rows); assert_eq!(mat.entry(2, 0), Fr::from(55)); } @@ -252,7 +282,7 @@ pub(crate) mod tests { to_field(vec![23, 1, 0]), to_field(vec![55, 58]), ]; - Matrix::new_from_rows(rows); + ColumnMajorMatrix::new_from_rows(rows); } #[test] @@ -263,7 +293,7 @@ pub(crate) mod tests { to_field(vec![17, 89]), ]; - let mat = Matrix::new_from_rows(rows); + let mat = ColumnMajorMatrix::new_from_rows(rows); assert_eq!(mat.cols()[1], to_field(vec![76, 92, 89])); } @@ -276,7 +306,7 @@ pub(crate) mod tests { to_field(vec![55, 58, 9]), ]; - let mat = Matrix::new_from_rows(rows); + let mat = RowMajorMatrix::new_from_rows(rows); let v: Vec = to_field(vec![12, 41, 55]); // by giving the result in the integers and then converting to Fr // we ensure the test will still pass even if Fr changes