From 9e85c9e2b69fa443da8f12bb51e9d63c0d319517 Mon Sep 17 00:00:00 2001 From: Anton Date: Sun, 3 Oct 2021 00:56:13 +0200 Subject: [PATCH 1/9] CSR/CSC: Provide constructor for unsorted but otherwise valid data --- nalgebra-sparse/src/csr.rs | 27 +++++++++++++++++++++++++ nalgebra-sparse/tests/unit_tests/csr.rs | 23 +++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/nalgebra-sparse/src/csr.rs b/nalgebra-sparse/src/csr.rs index c64be915..fd39fc75 100644 --- a/nalgebra-sparse/src/csr.rs +++ b/nalgebra-sparse/src/csr.rs @@ -170,6 +170,24 @@ impl CsrMatrix { Self::try_from_pattern_and_values(pattern, values) } + /// Try to construct a CSR matrix from raw CSR data with unsorted columns. + pub fn try_from_unsorted_csr_data( + num_rows: usize, + num_cols: usize, + row_offsets: Vec, + col_indices: Vec, + values: Vec, + ) -> Result { + let sorted_num_cols: Vec = row_offsets[0..row_offsets.len() - 1] + .iter() + .enumerate() + .flat_map(|(index, &offset)| { + Self::sorted(col_indices[offset..row_offsets[index + 1]].to_vec()) + }) + .collect(); + return Self::try_from_csr_data(num_rows, num_cols, row_offsets, sorted_num_cols, values); + } + /// Try to construct a CSR matrix from a sparsity pattern and associated non-zero values. /// /// Returns an error if the number of values does not match the number of minor indices @@ -190,6 +208,15 @@ impl CsrMatrix { } } + /// Return sorted vector. + #[inline] + #[must_use] + pub fn sorted(row_offsets: Vec) -> Vec { + let mut sorted = row_offsets.clone(); + sorted.sort(); + return sorted; + } + /// The number of rows in the matrix. #[inline] #[must_use] diff --git a/nalgebra-sparse/tests/unit_tests/csr.rs b/nalgebra-sparse/tests/unit_tests/csr.rs index dee1ae1e..73d4dd27 100644 --- a/nalgebra-sparse/tests/unit_tests/csr.rs +++ b/nalgebra-sparse/tests/unit_tests/csr.rs @@ -171,6 +171,29 @@ fn csr_matrix_valid_data() { } } +#[test] +fn csr_matrix_valid_data_unsorted_column_indices() { + let csr = CsrMatrix::try_from_unsorted_csr_data( + 3, + 4, + vec![0, 1, 2, 5], + vec![1, 3, 2, 3, 0], + vec![5, 4, 1, 1, 4], + ) + .unwrap(); + + let expected_csr = CsrMatrix::try_from_csr_data( + 3, + 4, + vec![0, 1, 2, 5], + vec![1, 3, 0, 2, 3], + vec![5, 4, 1, 1, 4], + ) + .unwrap(); + + assert_eq!(csr, expected_csr); +} + #[test] fn csr_matrix_try_from_invalid_csr_data() { { From a2a55cddcaa2c2f0d2bb7a82fc394e1ebc5109c7 Mon Sep 17 00:00:00 2001 From: Anton Date: Mon, 4 Oct 2021 20:17:27 +0200 Subject: [PATCH 2/9] Check first and last offsets before sorting column indices --- nalgebra-sparse/src/csr.rs | 42 +++++++++++++++++++--------------- nalgebra-sparse/src/pattern.rs | 9 +++----- src/base/helper.rs | 12 ++++++++++ 3 files changed, 39 insertions(+), 24 deletions(-) diff --git a/nalgebra-sparse/src/csr.rs b/nalgebra-sparse/src/csr.rs index fd39fc75..101a5fb3 100644 --- a/nalgebra-sparse/src/csr.rs +++ b/nalgebra-sparse/src/csr.rs @@ -170,7 +170,14 @@ impl CsrMatrix { Self::try_from_pattern_and_values(pattern, values) } - /// Try to construct a CSR matrix from raw CSR data with unsorted columns. + /// Try to construct a CSR matrix from raw CSR data with unsorted column indices. + /// + /// It is assumed that each row contains unique column indices that are in + /// bounds with respect to the number of columns in the matrix. If this is not the case, + /// an error is returned to indicate the failure. + /// + /// An error is returned if the data given does not conform to the CSR storage format. + /// See the documentation for [CsrMatrix](struct.CsrMatrix.html) for more information. pub fn try_from_unsorted_csr_data( num_rows: usize, num_cols: usize, @@ -178,14 +185,22 @@ impl CsrMatrix { col_indices: Vec, values: Vec, ) -> Result { - let sorted_num_cols: Vec = row_offsets[0..row_offsets.len() - 1] - .iter() - .enumerate() - .flat_map(|(index, &offset)| { - Self::sorted(col_indices[offset..row_offsets[index + 1]].to_vec()) - }) - .collect(); - return Self::try_from_csr_data(num_rows, num_cols, row_offsets, sorted_num_cols, values); + use nalgebra::base::helper; + use SparsityPatternFormatError::*; + if helper::first_and_last_offsets_are_ok(&row_offsets, &col_indices) { + let mut sorted_col_indices = col_indices.clone(); + for (index, &offset) in row_offsets[0..row_offsets.len() - 1].iter().enumerate() { + sorted_col_indices[offset..row_offsets[index + 1]].sort_unstable(); + } + return Self::try_from_csr_data( + num_rows, + num_cols, + row_offsets, + sorted_col_indices, + values, + ); + } + return (Err(InvalidOffsetFirstLast)).map_err(pattern_format_error_to_csr_error); } /// Try to construct a CSR matrix from a sparsity pattern and associated non-zero values. @@ -208,15 +223,6 @@ impl CsrMatrix { } } - /// Return sorted vector. - #[inline] - #[must_use] - pub fn sorted(row_offsets: Vec) -> Vec { - let mut sorted = row_offsets.clone(); - sorted.sort(); - return sorted; - } - /// The number of rows in the matrix. #[inline] #[must_use] diff --git a/nalgebra-sparse/src/pattern.rs b/nalgebra-sparse/src/pattern.rs index 85f6bc1a..9fdcad38 100644 --- a/nalgebra-sparse/src/pattern.rs +++ b/nalgebra-sparse/src/pattern.rs @@ -125,6 +125,7 @@ impl SparsityPattern { major_offsets: Vec, minor_indices: Vec, ) -> Result { + use nalgebra::base::helper; use SparsityPatternFormatError::*; if major_offsets.len() != major_dim + 1 { @@ -132,12 +133,8 @@ impl SparsityPattern { } // Check that the first and last offsets conform to the specification - { - let first_offset_ok = *major_offsets.first().unwrap() == 0; - let last_offset_ok = *major_offsets.last().unwrap() == minor_indices.len(); - if !first_offset_ok || !last_offset_ok { - return Err(InvalidOffsetFirstLast); - } + if !helper::first_and_last_offsets_are_ok(&major_offsets, &minor_indices) { + return Err(InvalidOffsetFirstLast); } // Test that each lane has strictly monotonically increasing minor indices, i.e. diff --git a/src/base/helper.rs b/src/base/helper.rs index 00bd462c..f596c955 100644 --- a/src/base/helper.rs +++ b/src/base/helper.rs @@ -29,3 +29,15 @@ where use std::iter; iter::repeat(()).map(|_| g.gen()).find(f).unwrap() } + +/// Check that the first and last offsets conform to the specification of a CSR matrix +#[inline] +#[must_use] +pub fn first_and_last_offsets_are_ok( + major_offsets: &Vec, + minor_indices: &Vec, +) -> bool { + let first_offset_ok = *major_offsets.first().unwrap() == 0; + let last_offset_ok = *major_offsets.last().unwrap() == minor_indices.len(); + return first_offset_ok && last_offset_ok; +} From 469765a4e59915ac4db5695140177963dd6fb403 Mon Sep 17 00:00:00 2001 From: Anton Date: Fri, 8 Oct 2021 00:36:40 +0200 Subject: [PATCH 3/9] Apply permutation --- nalgebra-sparse/src/convert/serial.rs | 10 +------ nalgebra-sparse/src/csc.rs | 4 +++ nalgebra-sparse/src/csr.rs | 38 +++++++++++++++++++++---- nalgebra-sparse/src/lib.rs | 1 + nalgebra-sparse/src/pattern.rs | 12 ++++++-- nalgebra-sparse/src/utils.rs | 23 +++++++++++++++ nalgebra-sparse/tests/unit_tests/csr.rs | 2 +- src/base/helper.rs | 12 -------- 8 files changed, 71 insertions(+), 31 deletions(-) create mode 100644 nalgebra-sparse/src/utils.rs diff --git a/nalgebra-sparse/src/convert/serial.rs b/nalgebra-sparse/src/convert/serial.rs index ecbe1dab..219a6bf7 100644 --- a/nalgebra-sparse/src/convert/serial.rs +++ b/nalgebra-sparse/src/convert/serial.rs @@ -14,6 +14,7 @@ use crate::coo::CooMatrix; use crate::cs; use crate::csc::CscMatrix; use crate::csr::CsrMatrix; +use crate::utils::apply_permutation; /// Converts a dense matrix to [`CooMatrix`]. pub fn convert_dense_coo(dense: &Matrix) -> CooMatrix @@ -390,15 +391,6 @@ fn sort_lane( apply_permutation(values_result, values, permutation); } -// TODO: Move this into `utils` or something? -fn apply_permutation(out_slice: &mut [T], in_slice: &[T], permutation: &[usize]) { - assert_eq!(out_slice.len(), in_slice.len()); - assert_eq!(out_slice.len(), permutation.len()); - for (out_element, old_pos) in out_slice.iter_mut().zip(permutation) { - *out_element = in_slice[*old_pos].clone(); - } -} - /// Given *sorted* indices and corresponding scalar values, combines duplicates with the given /// associative combiner and calls the provided produce methods with combined indices and values. fn combine_duplicates( diff --git a/nalgebra-sparse/src/csc.rs b/nalgebra-sparse/src/csc.rs index 607cc0cf..b770bbf3 100644 --- a/nalgebra-sparse/src/csc.rs +++ b/nalgebra-sparse/src/csc.rs @@ -535,6 +535,10 @@ fn pattern_format_error_to_csc_error(err: SparsityPatternFormatError) -> SparseF use SparsityPatternFormatError::*; match err { + DifferentValuesIndicesLengths => E::from_kind_and_msg( + K::InvalidStructure, + "Lengths of values and column indices are not equal.", + ), InvalidOffsetArrayLength => E::from_kind_and_msg( K::InvalidStructure, "Length of col offset array is not equal to ncols + 1.", diff --git a/nalgebra-sparse/src/csr.rs b/nalgebra-sparse/src/csr.rs index 101a5fb3..c0bef4d3 100644 --- a/nalgebra-sparse/src/csr.rs +++ b/nalgebra-sparse/src/csr.rs @@ -5,11 +5,13 @@ use crate::cs::{CsLane, CsLaneIter, CsLaneIterMut, CsLaneMut, CsMatrix}; use crate::csc::CscMatrix; use crate::pattern::{SparsityPattern, SparsityPatternFormatError, SparsityPatternIter}; +use crate::utils::{apply_permutation, first_and_last_offsets_are_ok}; use crate::{SparseEntry, SparseEntryMut, SparseFormatError, SparseFormatErrorKind}; use nalgebra::Scalar; use num_traits::One; +use num_traits::Zero; use std::slice::{Iter, IterMut}; /// A CSR representation of a sparse matrix. @@ -184,20 +186,40 @@ impl CsrMatrix { row_offsets: Vec, col_indices: Vec, values: Vec, - ) -> Result { - use nalgebra::base::helper; + ) -> Result + where + T: Scalar + Zero, + { use SparsityPatternFormatError::*; - if helper::first_and_last_offsets_are_ok(&row_offsets, &col_indices) { - let mut sorted_col_indices = col_indices.clone(); + + let mut p: Vec = (0..col_indices.len()).collect(); + + if col_indices.len() != values.len() { + return (Err(DifferentValuesIndicesLengths)).map_err(pattern_format_error_to_csr_error); + } + + if first_and_last_offsets_are_ok(&row_offsets, &col_indices) { for (index, &offset) in row_offsets[0..row_offsets.len() - 1].iter().enumerate() { - sorted_col_indices[offset..row_offsets[index + 1]].sort_unstable(); + p[offset..row_offsets[index + 1]].sort_by(|a, b| { + let x = &col_indices[*a]; + let y = &col_indices[*b]; + x.partial_cmp(y).unwrap() + }); } + + // permute indices + let sorted_col_indices: Vec = p.iter().map(|i| col_indices[*i]).collect(); + + // permute values + let mut output: Vec = vec![T::zero(); p.len()]; + apply_permutation(&mut output[..p.len()], &values[..p.len()], &p[..p.len()]); + return Self::try_from_csr_data( num_rows, num_cols, row_offsets, sorted_col_indices, - values, + output, ); } return (Err(InvalidOffsetFirstLast)).map_err(pattern_format_error_to_csr_error); @@ -568,6 +590,10 @@ fn pattern_format_error_to_csr_error(err: SparsityPatternFormatError) -> SparseF use SparsityPatternFormatError::*; match err { + DifferentValuesIndicesLengths => E::from_kind_and_msg( + K::InvalidStructure, + "Lengths of values and column indices are not equal.", + ), InvalidOffsetArrayLength => E::from_kind_and_msg( K::InvalidStructure, "Length of row offset array is not equal to nrows + 1.", diff --git a/nalgebra-sparse/src/lib.rs b/nalgebra-sparse/src/lib.rs index bf845757..607a1abf 100644 --- a/nalgebra-sparse/src/lib.rs +++ b/nalgebra-sparse/src/lib.rs @@ -149,6 +149,7 @@ pub mod csr; pub mod factorization; pub mod ops; pub mod pattern; +pub mod utils; pub(crate) mod cs; diff --git a/nalgebra-sparse/src/pattern.rs b/nalgebra-sparse/src/pattern.rs index 9fdcad38..3e30ee9b 100644 --- a/nalgebra-sparse/src/pattern.rs +++ b/nalgebra-sparse/src/pattern.rs @@ -1,5 +1,6 @@ //! Sparsity patterns for CSR and CSC matrices. use crate::cs::transpose_cs; +use crate::utils::first_and_last_offsets_are_ok; use crate::SparseFormatError; use std::error::Error; use std::fmt; @@ -125,7 +126,6 @@ impl SparsityPattern { major_offsets: Vec, minor_indices: Vec, ) -> Result { - use nalgebra::base::helper; use SparsityPatternFormatError::*; if major_offsets.len() != major_dim + 1 { @@ -133,7 +133,7 @@ impl SparsityPattern { } // Check that the first and last offsets conform to the specification - if !helper::first_and_last_offsets_are_ok(&major_offsets, &minor_indices) { + if !first_and_last_offsets_are_ok(&major_offsets, &minor_indices) { return Err(InvalidOffsetFirstLast); } @@ -261,6 +261,8 @@ impl SparsityPattern { pub enum SparsityPatternFormatError { /// Indicates an invalid number of offsets. /// + /// Indicates that column indices and values have different lengths. + DifferentValuesIndicesLengths, /// The number of offsets must be equal to (major_dim + 1). InvalidOffsetArrayLength, /// Indicates that the first or last entry in the offset array did not conform to @@ -289,7 +291,8 @@ impl From for SparseFormatError { use SparsityPatternFormatError::DuplicateEntry as PatternDuplicateEntry; use SparsityPatternFormatError::*; match err { - InvalidOffsetArrayLength + DifferentValuesIndicesLengths + | InvalidOffsetArrayLength | InvalidOffsetFirstLast | NonmonotonicOffsets | NonmonotonicMinorIndices => { @@ -310,6 +313,9 @@ impl From for SparseFormatError { impl fmt::Display for SparsityPatternFormatError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { + SparsityPatternFormatError::DifferentValuesIndicesLengths => { + write!(f, "Lengths of values and column indices are not equal.") + } SparsityPatternFormatError::InvalidOffsetArrayLength => { write!(f, "Length of offset array is not equal to (major_dim + 1).") } diff --git a/nalgebra-sparse/src/utils.rs b/nalgebra-sparse/src/utils.rs new file mode 100644 index 00000000..411e6e0a --- /dev/null +++ b/nalgebra-sparse/src/utils.rs @@ -0,0 +1,23 @@ +//! Helper functions for sparse matrix computations + +/// Check that the first and last offsets conform to the specification of a CSR matrix +#[inline] +#[must_use] +pub fn first_and_last_offsets_are_ok( + major_offsets: &Vec, + minor_indices: &Vec, +) -> bool { + let first_offset_ok = *major_offsets.first().unwrap() == 0; + let last_offset_ok = *major_offsets.last().unwrap() == minor_indices.len(); + return first_offset_ok && last_offset_ok; +} + +/// permutes entries of in_slice according to permutation slice and puts them to out_slice +#[inline] +pub fn apply_permutation(out_slice: &mut [T], in_slice: &[T], permutation: &[usize]) { + assert_eq!(out_slice.len(), in_slice.len()); + assert_eq!(out_slice.len(), permutation.len()); + for (out_element, old_pos) in out_slice.iter_mut().zip(permutation) { + *out_element = in_slice[*old_pos].clone(); + } +} diff --git a/nalgebra-sparse/tests/unit_tests/csr.rs b/nalgebra-sparse/tests/unit_tests/csr.rs index 73d4dd27..38c2b344 100644 --- a/nalgebra-sparse/tests/unit_tests/csr.rs +++ b/nalgebra-sparse/tests/unit_tests/csr.rs @@ -178,7 +178,7 @@ fn csr_matrix_valid_data_unsorted_column_indices() { 4, vec![0, 1, 2, 5], vec![1, 3, 2, 3, 0], - vec![5, 4, 1, 1, 4], + vec![5, 4, 1, 4, 1], ) .unwrap(); diff --git a/src/base/helper.rs b/src/base/helper.rs index f596c955..00bd462c 100644 --- a/src/base/helper.rs +++ b/src/base/helper.rs @@ -29,15 +29,3 @@ where use std::iter; iter::repeat(()).map(|_| g.gen()).find(f).unwrap() } - -/// Check that the first and last offsets conform to the specification of a CSR matrix -#[inline] -#[must_use] -pub fn first_and_last_offsets_are_ok( - major_offsets: &Vec, - minor_indices: &Vec, -) -> bool { - let first_offset_ok = *major_offsets.first().unwrap() == 0; - let last_offset_ok = *major_offsets.last().unwrap() == minor_indices.len(); - return first_offset_ok && last_offset_ok; -} From 4a979897384d0e06beb640236194b7e5f3a3ecd9 Mon Sep 17 00:00:00 2001 From: Anton Date: Mon, 11 Oct 2021 22:11:50 +0200 Subject: [PATCH 4/9] Improve checking requirements for sorting column indices --- nalgebra-sparse/src/csc.rs | 4 -- nalgebra-sparse/src/csr.rs | 75 +++++++++++++++++++--------------- nalgebra-sparse/src/lib.rs | 2 +- nalgebra-sparse/src/pattern.rs | 17 ++++---- nalgebra-sparse/src/utils.rs | 12 ------ 5 files changed, 50 insertions(+), 60 deletions(-) diff --git a/nalgebra-sparse/src/csc.rs b/nalgebra-sparse/src/csc.rs index b770bbf3..607cc0cf 100644 --- a/nalgebra-sparse/src/csc.rs +++ b/nalgebra-sparse/src/csc.rs @@ -535,10 +535,6 @@ fn pattern_format_error_to_csc_error(err: SparsityPatternFormatError) -> SparseF use SparsityPatternFormatError::*; match err { - DifferentValuesIndicesLengths => E::from_kind_and_msg( - K::InvalidStructure, - "Lengths of values and column indices are not equal.", - ), InvalidOffsetArrayLength => E::from_kind_and_msg( K::InvalidStructure, "Length of col offset array is not equal to ncols + 1.", diff --git a/nalgebra-sparse/src/csr.rs b/nalgebra-sparse/src/csr.rs index c0bef4d3..beafba05 100644 --- a/nalgebra-sparse/src/csr.rs +++ b/nalgebra-sparse/src/csr.rs @@ -5,13 +5,13 @@ use crate::cs::{CsLane, CsLaneIter, CsLaneIterMut, CsLaneMut, CsMatrix}; use crate::csc::CscMatrix; use crate::pattern::{SparsityPattern, SparsityPatternFormatError, SparsityPatternIter}; -use crate::utils::{apply_permutation, first_and_last_offsets_are_ok}; +use crate::utils::apply_permutation; use crate::{SparseEntry, SparseEntryMut, SparseFormatError, SparseFormatErrorKind}; use nalgebra::Scalar; use num_traits::One; - use num_traits::Zero; + use std::slice::{Iter, IterMut}; /// A CSR representation of a sparse matrix. @@ -190,39 +190,52 @@ impl CsrMatrix { where T: Scalar + Zero, { - use SparsityPatternFormatError::*; - - let mut p: Vec = (0..col_indices.len()).collect(); + let count = col_indices.len(); + let mut p: Vec = (0..count).collect(); if col_indices.len() != values.len() { - return (Err(DifferentValuesIndicesLengths)).map_err(pattern_format_error_to_csr_error); + return Err(SparseFormatError::from_kind_and_msg( + SparseFormatErrorKind::InvalidStructure, + "Number of values and column indices must be the same", + )); } - if first_and_last_offsets_are_ok(&row_offsets, &col_indices) { - for (index, &offset) in row_offsets[0..row_offsets.len() - 1].iter().enumerate() { - p[offset..row_offsets[index + 1]].sort_by(|a, b| { - let x = &col_indices[*a]; - let y = &col_indices[*b]; - x.partial_cmp(y).unwrap() - }); + if row_offsets.len() == 0 { + return Err(SparseFormatError::from_kind_and_msg( + SparseFormatErrorKind::InvalidStructure, + "Number of offsets should be greater than 0", + )); + } + + for (index, &offset) in row_offsets[0..row_offsets.len() - 1].iter().enumerate() { + let next_offset = row_offsets[index + 1]; + if next_offset > count { + return Err(SparseFormatError::from_kind_and_msg( + SparseFormatErrorKind::InvalidStructure, + "No row offset should be greater than the number of column indices", + )); } - - // permute indices - let sorted_col_indices: Vec = p.iter().map(|i| col_indices[*i]).collect(); - - // permute values - let mut output: Vec = vec![T::zero(); p.len()]; - apply_permutation(&mut output[..p.len()], &values[..p.len()], &p[..p.len()]); - - return Self::try_from_csr_data( - num_rows, - num_cols, - row_offsets, - sorted_col_indices, - output, - ); + p[offset..next_offset].sort_by(|a, b| { + let x = &col_indices[*a]; + let y = &col_indices[*b]; + x.partial_cmp(y).unwrap() + }); } - return (Err(InvalidOffsetFirstLast)).map_err(pattern_format_error_to_csr_error); + + // permute indices + let sorted_col_indices: Vec = p.iter().map(|i| col_indices[*i]).collect(); + + // permute values + let mut sorted_vaues: Vec = vec![T::zero(); count]; + apply_permutation(&mut sorted_vaues[..count], &values[..count], &p[..count]); + + return Self::try_from_csr_data( + num_rows, + num_cols, + row_offsets, + sorted_col_indices, + sorted_vaues, + ); } /// Try to construct a CSR matrix from a sparsity pattern and associated non-zero values. @@ -590,10 +603,6 @@ fn pattern_format_error_to_csr_error(err: SparsityPatternFormatError) -> SparseF use SparsityPatternFormatError::*; match err { - DifferentValuesIndicesLengths => E::from_kind_and_msg( - K::InvalidStructure, - "Lengths of values and column indices are not equal.", - ), InvalidOffsetArrayLength => E::from_kind_and_msg( K::InvalidStructure, "Length of row offset array is not equal to nrows + 1.", diff --git a/nalgebra-sparse/src/lib.rs b/nalgebra-sparse/src/lib.rs index 607a1abf..64331817 100644 --- a/nalgebra-sparse/src/lib.rs +++ b/nalgebra-sparse/src/lib.rs @@ -149,9 +149,9 @@ pub mod csr; pub mod factorization; pub mod ops; pub mod pattern; -pub mod utils; pub(crate) mod cs; +pub(crate) mod utils; #[cfg(feature = "proptest-support")] pub mod proptest; diff --git a/nalgebra-sparse/src/pattern.rs b/nalgebra-sparse/src/pattern.rs index 3e30ee9b..85f6bc1a 100644 --- a/nalgebra-sparse/src/pattern.rs +++ b/nalgebra-sparse/src/pattern.rs @@ -1,6 +1,5 @@ //! Sparsity patterns for CSR and CSC matrices. use crate::cs::transpose_cs; -use crate::utils::first_and_last_offsets_are_ok; use crate::SparseFormatError; use std::error::Error; use std::fmt; @@ -133,8 +132,12 @@ impl SparsityPattern { } // Check that the first and last offsets conform to the specification - if !first_and_last_offsets_are_ok(&major_offsets, &minor_indices) { - return Err(InvalidOffsetFirstLast); + { + let first_offset_ok = *major_offsets.first().unwrap() == 0; + let last_offset_ok = *major_offsets.last().unwrap() == minor_indices.len(); + if !first_offset_ok || !last_offset_ok { + return Err(InvalidOffsetFirstLast); + } } // Test that each lane has strictly monotonically increasing minor indices, i.e. @@ -261,8 +264,6 @@ impl SparsityPattern { pub enum SparsityPatternFormatError { /// Indicates an invalid number of offsets. /// - /// Indicates that column indices and values have different lengths. - DifferentValuesIndicesLengths, /// The number of offsets must be equal to (major_dim + 1). InvalidOffsetArrayLength, /// Indicates that the first or last entry in the offset array did not conform to @@ -291,8 +292,7 @@ impl From for SparseFormatError { use SparsityPatternFormatError::DuplicateEntry as PatternDuplicateEntry; use SparsityPatternFormatError::*; match err { - DifferentValuesIndicesLengths - | InvalidOffsetArrayLength + InvalidOffsetArrayLength | InvalidOffsetFirstLast | NonmonotonicOffsets | NonmonotonicMinorIndices => { @@ -313,9 +313,6 @@ impl From for SparseFormatError { impl fmt::Display for SparsityPatternFormatError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - SparsityPatternFormatError::DifferentValuesIndicesLengths => { - write!(f, "Lengths of values and column indices are not equal.") - } SparsityPatternFormatError::InvalidOffsetArrayLength => { write!(f, "Length of offset array is not equal to (major_dim + 1).") } diff --git a/nalgebra-sparse/src/utils.rs b/nalgebra-sparse/src/utils.rs index 411e6e0a..a5da85c5 100644 --- a/nalgebra-sparse/src/utils.rs +++ b/nalgebra-sparse/src/utils.rs @@ -1,17 +1,5 @@ //! Helper functions for sparse matrix computations -/// Check that the first and last offsets conform to the specification of a CSR matrix -#[inline] -#[must_use] -pub fn first_and_last_offsets_are_ok( - major_offsets: &Vec, - minor_indices: &Vec, -) -> bool { - let first_offset_ok = *major_offsets.first().unwrap() == 0; - let last_offset_ok = *major_offsets.last().unwrap() == minor_indices.len(); - return first_offset_ok && last_offset_ok; -} - /// permutes entries of in_slice according to permutation slice and puts them to out_slice #[inline] pub fn apply_permutation(out_slice: &mut [T], in_slice: &[T], permutation: &[usize]) { From 4b41be75b0b833d7555e8185bab6456df96db884 Mon Sep 17 00:00:00 2001 From: Anton Date: Wed, 13 Oct 2021 21:18:17 +0200 Subject: [PATCH 5/9] Add tests for some csr matrix related failure cases --- nalgebra-sparse/src/csr.rs | 13 ++- nalgebra-sparse/tests/unit_tests/csr.rs | 102 +++++++++++++++++++++++- 2 files changed, 109 insertions(+), 6 deletions(-) diff --git a/nalgebra-sparse/src/csr.rs b/nalgebra-sparse/src/csr.rs index beafba05..ecb2ce65 100644 --- a/nalgebra-sparse/src/csr.rs +++ b/nalgebra-sparse/src/csr.rs @@ -178,7 +178,8 @@ impl CsrMatrix { /// bounds with respect to the number of columns in the matrix. If this is not the case, /// an error is returned to indicate the failure. /// - /// An error is returned if the data given does not conform to the CSR storage format. + /// An error is returned if the data given does not conform to the CSR storage format + /// with the exception of having unsorted column indices and values. /// See the documentation for [CsrMatrix](struct.CsrMatrix.html) for more information. pub fn try_from_unsorted_csr_data( num_rows: usize, @@ -190,6 +191,7 @@ impl CsrMatrix { where T: Scalar + Zero, { + use SparsityPatternFormatError::*; let count = col_indices.len(); let mut p: Vec = (0..count).collect(); @@ -215,6 +217,9 @@ impl CsrMatrix { "No row offset should be greater than the number of column indices", )); } + if offset > next_offset { + return Err(NonmonotonicOffsets).map_err(pattern_format_error_to_csr_error); + } p[offset..next_offset].sort_by(|a, b| { let x = &col_indices[*a]; let y = &col_indices[*b]; @@ -226,15 +231,15 @@ impl CsrMatrix { let sorted_col_indices: Vec = p.iter().map(|i| col_indices[*i]).collect(); // permute values - let mut sorted_vaues: Vec = vec![T::zero(); count]; - apply_permutation(&mut sorted_vaues[..count], &values[..count], &p[..count]); + let mut sorted_values: Vec = vec![T::zero(); count]; + apply_permutation(&mut sorted_values, &values, &p); return Self::try_from_csr_data( num_rows, num_cols, row_offsets, sorted_col_indices, - sorted_vaues, + sorted_values, ); } diff --git a/nalgebra-sparse/tests/unit_tests/csr.rs b/nalgebra-sparse/tests/unit_tests/csr.rs index 38c2b344..7cdde331 100644 --- a/nalgebra-sparse/tests/unit_tests/csr.rs +++ b/nalgebra-sparse/tests/unit_tests/csr.rs @@ -178,7 +178,7 @@ fn csr_matrix_valid_data_unsorted_column_indices() { 4, vec![0, 1, 2, 5], vec![1, 3, 2, 3, 0], - vec![5, 4, 1, 4, 1], + vec![5, 4, 2, 3, 1], ) .unwrap(); @@ -187,13 +187,111 @@ fn csr_matrix_valid_data_unsorted_column_indices() { 4, vec![0, 1, 2, 5], vec![1, 3, 0, 2, 3], - vec![5, 4, 1, 1, 4], + vec![5, 4, 1, 2, 3], ) .unwrap(); assert_eq!(csr, expected_csr); } +#[test] +fn csr_matrix_try_from_invalid_csr_data2() { + { + // Empty offset array (invalid length) + let matrix = + CsrMatrix::try_from_unsorted_csr_data(0, 0, Vec::new(), Vec::new(), Vec::::new()); + assert_eq!( + matrix.unwrap_err().kind(), + &SparseFormatErrorKind::InvalidStructure + ); + } + + { + // Offset array invalid length for arbitrary data + let offsets = vec![0, 3, 5]; + let indices = vec![0, 1, 2, 3, 5]; + let values = vec![0, 1, 2, 3, 4]; + + let matrix = CsrMatrix::try_from_unsorted_csr_data(3, 6, offsets, indices, values); + assert_eq!( + matrix.unwrap_err().kind(), + &SparseFormatErrorKind::InvalidStructure + ); + } + + { + // Invalid first entry in offsets array + let offsets = vec![1, 2, 2, 5]; + let indices = vec![0, 5, 1, 2, 3]; + let values = vec![0, 1, 2, 3, 4]; + let matrix = CsrMatrix::try_from_unsorted_csr_data(3, 6, offsets, indices, values); + assert_eq!( + matrix.unwrap_err().kind(), + &SparseFormatErrorKind::InvalidStructure + ); + } + + { + // Invalid last entry in offsets array + let offsets = vec![0, 2, 2, 4]; + let indices = vec![0, 5, 1, 2, 3]; + let values = vec![0, 1, 2, 3, 4]; + let matrix = CsrMatrix::try_from_unsorted_csr_data(3, 6, offsets, indices, values); + assert_eq!( + matrix.unwrap_err().kind(), + &SparseFormatErrorKind::InvalidStructure + ); + } + + { + // Invalid length of offsets array + let offsets = vec![0, 2, 2]; + let indices = vec![0, 5, 1, 2, 3]; + let values = vec![0, 1, 2, 3, 4]; + let matrix = CsrMatrix::try_from_unsorted_csr_data(3, 6, offsets, indices, values); + assert_eq!( + matrix.unwrap_err().kind(), + &SparseFormatErrorKind::InvalidStructure + ); + } + + { + // Nonmonotonic offsets + let offsets = vec![0, 3, 2, 5]; + let indices = vec![0, 1, 2, 3, 4]; + let values = vec![0, 1, 2, 3, 4]; + let matrix = CsrMatrix::try_from_unsorted_csr_data(3, 6, offsets, indices, values); + assert_eq!( + matrix.unwrap_err().kind(), + &SparseFormatErrorKind::InvalidStructure + ); + } + + { + // Minor index out of bounds + let offsets = vec![0, 2, 2, 5]; + let indices = vec![0, 6, 1, 2, 3]; + let values = vec![0, 1, 2, 3, 4]; + let matrix = CsrMatrix::try_from_unsorted_csr_data(3, 6, offsets, indices, values); + assert_eq!( + matrix.unwrap_err().kind(), + &SparseFormatErrorKind::IndexOutOfBounds + ); + } + + { + // Duplicate entry + let offsets = vec![0, 2, 2, 5]; + let indices = vec![0, 5, 2, 2, 3]; + let values = vec![0, 1, 2, 3, 4]; + let matrix = CsrMatrix::try_from_unsorted_csr_data(3, 6, offsets, indices, values); + assert_eq!( + matrix.unwrap_err().kind(), + &SparseFormatErrorKind::DuplicateEntry + ); + } +} + #[test] fn csr_matrix_try_from_invalid_csr_data() { { From 86eeb192dbcb4ee3a9dd5a5b4b02573578b5dff5 Mon Sep 17 00:00:00 2001 From: Anton Date: Sun, 17 Oct 2021 21:57:36 +0200 Subject: [PATCH 6/9] Add module for unit test data examples --- nalgebra-sparse/tests/unit_tests/csr.rs | 95 +++++++------------ nalgebra-sparse/tests/unit_tests/mod.rs | 1 + .../tests/unit_tests/test_data_examples.rs | 44 +++++++++ 3 files changed, 80 insertions(+), 60 deletions(-) create mode 100644 nalgebra-sparse/tests/unit_tests/test_data_examples.rs diff --git a/nalgebra-sparse/tests/unit_tests/csr.rs b/nalgebra-sparse/tests/unit_tests/csr.rs index 7cdde331..3c8207c8 100644 --- a/nalgebra-sparse/tests/unit_tests/csr.rs +++ b/nalgebra-sparse/tests/unit_tests/csr.rs @@ -5,6 +5,8 @@ use nalgebra_sparse::{SparseEntry, SparseEntryMut, SparseFormatErrorKind}; use proptest::prelude::*; use proptest::sample::subsequence; +use super::test_data_examples::InvalidCsrDataExamples; + use crate::assert_panics; use crate::common::csr_strategy; @@ -174,20 +176,20 @@ fn csr_matrix_valid_data() { #[test] fn csr_matrix_valid_data_unsorted_column_indices() { let csr = CsrMatrix::try_from_unsorted_csr_data( - 3, 4, - vec![0, 1, 2, 5], - vec![1, 3, 2, 3, 0], - vec![5, 4, 2, 3, 1], + 5, + vec![0, 3, 5, 8, 11], + vec![4, 1, 3, 3, 1, 2, 3, 0, 3, 4, 1], + vec![5, 1, 4, 7, 4, 2, 3, 1, 8, 9, 6], ) .unwrap(); let expected_csr = CsrMatrix::try_from_csr_data( - 3, 4, - vec![0, 1, 2, 5], - vec![1, 3, 0, 2, 3], - vec![5, 4, 1, 2, 3], + 5, + vec![0, 3, 5, 8, 11], + vec![1, 3, 4, 1, 3, 0, 2, 3, 1, 3, 4], + vec![1, 4, 5, 4, 7, 1, 2, 3, 6, 8, 9], ) .unwrap(); @@ -195,11 +197,12 @@ fn csr_matrix_valid_data_unsorted_column_indices() { } #[test] -fn csr_matrix_try_from_invalid_csr_data2() { +fn csr_matrix_try_from_invalid_csr_data_with_new_constructor() { + let invalid_data: InvalidCsrDataExamples = InvalidCsrDataExamples::new(); { // Empty offset array (invalid length) - let matrix = - CsrMatrix::try_from_unsorted_csr_data(0, 0, Vec::new(), Vec::new(), Vec::::new()); + let (offsets, indices, values) = invalid_data.empty_offset_array; + let matrix = CsrMatrix::try_from_unsorted_csr_data(0, 0, offsets, indices, values); assert_eq!( matrix.unwrap_err().kind(), &SparseFormatErrorKind::InvalidStructure @@ -208,10 +211,8 @@ fn csr_matrix_try_from_invalid_csr_data2() { { // Offset array invalid length for arbitrary data - let offsets = vec![0, 3, 5]; - let indices = vec![0, 1, 2, 3, 5]; - let values = vec![0, 1, 2, 3, 4]; - + let (offsets, indices, values) = + invalid_data.offset_array_invalid_length_for_arbitrary_data; let matrix = CsrMatrix::try_from_unsorted_csr_data(3, 6, offsets, indices, values); assert_eq!( matrix.unwrap_err().kind(), @@ -221,9 +222,7 @@ fn csr_matrix_try_from_invalid_csr_data2() { { // Invalid first entry in offsets array - let offsets = vec![1, 2, 2, 5]; - let indices = vec![0, 5, 1, 2, 3]; - let values = vec![0, 1, 2, 3, 4]; + let (offsets, indices, values) = invalid_data.invalid_first_entry_in_offsets_array; let matrix = CsrMatrix::try_from_unsorted_csr_data(3, 6, offsets, indices, values); assert_eq!( matrix.unwrap_err().kind(), @@ -233,9 +232,7 @@ fn csr_matrix_try_from_invalid_csr_data2() { { // Invalid last entry in offsets array - let offsets = vec![0, 2, 2, 4]; - let indices = vec![0, 5, 1, 2, 3]; - let values = vec![0, 1, 2, 3, 4]; + let (offsets, indices, values) = invalid_data.invalid_last_entry_in_offsets_array; let matrix = CsrMatrix::try_from_unsorted_csr_data(3, 6, offsets, indices, values); assert_eq!( matrix.unwrap_err().kind(), @@ -245,9 +242,7 @@ fn csr_matrix_try_from_invalid_csr_data2() { { // Invalid length of offsets array - let offsets = vec![0, 2, 2]; - let indices = vec![0, 5, 1, 2, 3]; - let values = vec![0, 1, 2, 3, 4]; + let (offsets, indices, values) = invalid_data.invalid_length_of_offsets_array; let matrix = CsrMatrix::try_from_unsorted_csr_data(3, 6, offsets, indices, values); assert_eq!( matrix.unwrap_err().kind(), @@ -257,9 +252,7 @@ fn csr_matrix_try_from_invalid_csr_data2() { { // Nonmonotonic offsets - let offsets = vec![0, 3, 2, 5]; - let indices = vec![0, 1, 2, 3, 4]; - let values = vec![0, 1, 2, 3, 4]; + let (offsets, indices, values) = invalid_data.nonmonotonic_offsets; let matrix = CsrMatrix::try_from_unsorted_csr_data(3, 6, offsets, indices, values); assert_eq!( matrix.unwrap_err().kind(), @@ -269,9 +262,7 @@ fn csr_matrix_try_from_invalid_csr_data2() { { // Minor index out of bounds - let offsets = vec![0, 2, 2, 5]; - let indices = vec![0, 6, 1, 2, 3]; - let values = vec![0, 1, 2, 3, 4]; + let (offsets, indices, values) = invalid_data.minor_index_out_of_bounds; let matrix = CsrMatrix::try_from_unsorted_csr_data(3, 6, offsets, indices, values); assert_eq!( matrix.unwrap_err().kind(), @@ -281,9 +272,7 @@ fn csr_matrix_try_from_invalid_csr_data2() { { // Duplicate entry - let offsets = vec![0, 2, 2, 5]; - let indices = vec![0, 5, 2, 2, 3]; - let values = vec![0, 1, 2, 3, 4]; + let (offsets, indices, values) = invalid_data.duplicate_entry; let matrix = CsrMatrix::try_from_unsorted_csr_data(3, 6, offsets, indices, values); assert_eq!( matrix.unwrap_err().kind(), @@ -294,9 +283,11 @@ fn csr_matrix_try_from_invalid_csr_data2() { #[test] fn csr_matrix_try_from_invalid_csr_data() { + let invalid_data: InvalidCsrDataExamples = InvalidCsrDataExamples::new(); { // Empty offset array (invalid length) - let matrix = CsrMatrix::try_from_csr_data(0, 0, Vec::new(), Vec::new(), Vec::::new()); + let (offsets, indices, values) = invalid_data.empty_offset_array; + let matrix = CsrMatrix::try_from_unsorted_csr_data(0, 0, offsets, indices, values); assert_eq!( matrix.unwrap_err().kind(), &SparseFormatErrorKind::InvalidStructure @@ -305,11 +296,9 @@ fn csr_matrix_try_from_invalid_csr_data() { { // Offset array invalid length for arbitrary data - let offsets = vec![0, 3, 5]; - let indices = vec![0, 1, 2, 3, 5]; - let values = vec![0, 1, 2, 3, 4]; - - let matrix = CsrMatrix::try_from_csr_data(3, 6, offsets, indices, values); + let (offsets, indices, values) = + invalid_data.offset_array_invalid_length_for_arbitrary_data; + let matrix = CsrMatrix::try_from_unsorted_csr_data(3, 6, offsets, indices, values); assert_eq!( matrix.unwrap_err().kind(), &SparseFormatErrorKind::InvalidStructure @@ -318,9 +307,7 @@ fn csr_matrix_try_from_invalid_csr_data() { { // Invalid first entry in offsets array - let offsets = vec![1, 2, 2, 5]; - let indices = vec![0, 5, 1, 2, 3]; - let values = vec![0, 1, 2, 3, 4]; + let (offsets, indices, values) = invalid_data.invalid_first_entry_in_offsets_array; let matrix = CsrMatrix::try_from_csr_data(3, 6, offsets, indices, values); assert_eq!( matrix.unwrap_err().kind(), @@ -330,9 +317,7 @@ fn csr_matrix_try_from_invalid_csr_data() { { // Invalid last entry in offsets array - let offsets = vec![0, 2, 2, 4]; - let indices = vec![0, 5, 1, 2, 3]; - let values = vec![0, 1, 2, 3, 4]; + let (offsets, indices, values) = invalid_data.invalid_last_entry_in_offsets_array; let matrix = CsrMatrix::try_from_csr_data(3, 6, offsets, indices, values); assert_eq!( matrix.unwrap_err().kind(), @@ -342,9 +327,7 @@ fn csr_matrix_try_from_invalid_csr_data() { { // Invalid length of offsets array - let offsets = vec![0, 2, 2]; - let indices = vec![0, 5, 1, 2, 3]; - let values = vec![0, 1, 2, 3, 4]; + let (offsets, indices, values) = invalid_data.invalid_length_of_offsets_array; let matrix = CsrMatrix::try_from_csr_data(3, 6, offsets, indices, values); assert_eq!( matrix.unwrap_err().kind(), @@ -354,9 +337,7 @@ fn csr_matrix_try_from_invalid_csr_data() { { // Nonmonotonic offsets - let offsets = vec![0, 3, 2, 5]; - let indices = vec![0, 1, 2, 3, 4]; - let values = vec![0, 1, 2, 3, 4]; + let (offsets, indices, values) = invalid_data.nonmonotonic_offsets; let matrix = CsrMatrix::try_from_csr_data(3, 6, offsets, indices, values); assert_eq!( matrix.unwrap_err().kind(), @@ -366,9 +347,7 @@ fn csr_matrix_try_from_invalid_csr_data() { { // Nonmonotonic minor indices - let offsets = vec![0, 2, 2, 5]; - let indices = vec![0, 2, 3, 1, 4]; - let values = vec![0, 1, 2, 3, 4]; + let (offsets, indices, values) = invalid_data.nonmonotonic_minor_indices; let matrix = CsrMatrix::try_from_csr_data(3, 6, offsets, indices, values); assert_eq!( matrix.unwrap_err().kind(), @@ -378,9 +357,7 @@ fn csr_matrix_try_from_invalid_csr_data() { { // Minor index out of bounds - let offsets = vec![0, 2, 2, 5]; - let indices = vec![0, 6, 1, 2, 3]; - let values = vec![0, 1, 2, 3, 4]; + let (offsets, indices, values) = invalid_data.minor_index_out_of_bounds; let matrix = CsrMatrix::try_from_csr_data(3, 6, offsets, indices, values); assert_eq!( matrix.unwrap_err().kind(), @@ -390,9 +367,7 @@ fn csr_matrix_try_from_invalid_csr_data() { { // Duplicate entry - let offsets = vec![0, 2, 2, 5]; - let indices = vec![0, 5, 2, 2, 3]; - let values = vec![0, 1, 2, 3, 4]; + let (offsets, indices, values) = invalid_data.duplicate_entry; let matrix = CsrMatrix::try_from_csr_data(3, 6, offsets, indices, values); assert_eq!( matrix.unwrap_err().kind(), diff --git a/nalgebra-sparse/tests/unit_tests/mod.rs b/nalgebra-sparse/tests/unit_tests/mod.rs index ee2166dc..0099a246 100644 --- a/nalgebra-sparse/tests/unit_tests/mod.rs +++ b/nalgebra-sparse/tests/unit_tests/mod.rs @@ -6,3 +6,4 @@ mod csr; mod ops; mod pattern; mod proptest; +mod test_data_examples; diff --git a/nalgebra-sparse/tests/unit_tests/test_data_examples.rs b/nalgebra-sparse/tests/unit_tests/test_data_examples.rs new file mode 100644 index 00000000..20721087 --- /dev/null +++ b/nalgebra-sparse/tests/unit_tests/test_data_examples.rs @@ -0,0 +1,44 @@ +/// Examples of *invalid* raw CSR data `(offsets, indices, values)`. +pub struct InvalidCsrDataExamples { + pub empty_offset_array: (Vec, Vec, Vec), + pub offset_array_invalid_length_for_arbitrary_data: (Vec, Vec, Vec), + pub invalid_first_entry_in_offsets_array: (Vec, Vec, Vec), + pub invalid_last_entry_in_offsets_array: (Vec, Vec, Vec), + pub invalid_length_of_offsets_array: (Vec, Vec, Vec), + pub nonmonotonic_offsets: (Vec, Vec, Vec), + pub nonmonotonic_minor_indices: (Vec, Vec, Vec), + pub minor_index_out_of_bounds: (Vec, Vec, Vec), + pub duplicate_entry: (Vec, Vec, Vec), +} + +impl InvalidCsrDataExamples { + pub fn new() -> Self { + let empty_offset_array = (Vec::::new(), Vec::::new(), Vec::::new()); + let offset_array_invalid_length_for_arbitrary_data = + (vec![0, 3, 5], vec![0, 1, 2, 3, 5], vec![0, 1, 2, 3, 4]); + let invalid_first_entry_in_offsets_array = + (vec![1, 2, 2, 5], vec![0, 5, 1, 2, 3], vec![0, 1, 2, 3, 4]); + let invalid_last_entry_in_offsets_array = + (vec![0, 2, 2, 4], vec![0, 5, 1, 2, 3], vec![0, 1, 2, 3, 4]); + let invalid_length_of_offsets_array = + (vec![0, 2, 2], vec![0, 5, 1, 2, 3], vec![0, 1, 2, 3, 4]); + let nonmonotonic_offsets = (vec![0, 3, 2, 5], vec![0, 1, 2, 3, 4], vec![0, 1, 2, 3, 4]); + let nonmonotonic_minor_indices = + (vec![0, 2, 2, 5], vec![0, 2, 3, 1, 4], vec![0, 1, 2, 3, 4]); + let minor_index_out_of_bounds = + (vec![0, 2, 2, 5], vec![0, 6, 1, 2, 3], vec![0, 1, 2, 3, 4]); + let duplicate_entry = (vec![0, 2, 2, 5], vec![0, 5, 2, 2, 3], vec![0, 1, 2, 3, 4]); + + return Self { + empty_offset_array, + offset_array_invalid_length_for_arbitrary_data, + invalid_first_entry_in_offsets_array, + invalid_last_entry_in_offsets_array, + invalid_length_of_offsets_array, + nonmonotonic_minor_indices, + nonmonotonic_offsets, + minor_index_out_of_bounds, + duplicate_entry, + }; + } +} From f90bb8d64a343ac2c349470d4f885bce22025c6d Mon Sep 17 00:00:00 2001 From: Anton Date: Mon, 18 Oct 2021 10:59:51 +0200 Subject: [PATCH 7/9] Fix wrong csr-constructor call --- nalgebra-sparse/tests/unit_tests/csr.rs | 190 ++++++++++++------------ 1 file changed, 95 insertions(+), 95 deletions(-) diff --git a/nalgebra-sparse/tests/unit_tests/csr.rs b/nalgebra-sparse/tests/unit_tests/csr.rs index 3c8207c8..6d884b75 100644 --- a/nalgebra-sparse/tests/unit_tests/csr.rs +++ b/nalgebra-sparse/tests/unit_tests/csr.rs @@ -196,6 +196,101 @@ fn csr_matrix_valid_data_unsorted_column_indices() { assert_eq!(csr, expected_csr); } +#[test] +fn csr_matrix_try_from_invalid_csr_data() { + let invalid_data: InvalidCsrDataExamples = InvalidCsrDataExamples::new(); + { + // Empty offset array (invalid length) + let (offsets, indices, values) = invalid_data.empty_offset_array; + let matrix = CsrMatrix::try_from_csr_data(0, 0, offsets, indices, values); + assert_eq!( + matrix.unwrap_err().kind(), + &SparseFormatErrorKind::InvalidStructure + ); + } + + { + // Offset array invalid length for arbitrary data + let (offsets, indices, values) = + invalid_data.offset_array_invalid_length_for_arbitrary_data; + let matrix = CsrMatrix::try_from_csr_data(3, 6, offsets, indices, values); + assert_eq!( + matrix.unwrap_err().kind(), + &SparseFormatErrorKind::InvalidStructure + ); + } + + { + // Invalid first entry in offsets array + let (offsets, indices, values) = invalid_data.invalid_first_entry_in_offsets_array; + let matrix = CsrMatrix::try_from_csr_data(3, 6, offsets, indices, values); + assert_eq!( + matrix.unwrap_err().kind(), + &SparseFormatErrorKind::InvalidStructure + ); + } + + { + // Invalid last entry in offsets array + let (offsets, indices, values) = invalid_data.invalid_last_entry_in_offsets_array; + let matrix = CsrMatrix::try_from_csr_data(3, 6, offsets, indices, values); + assert_eq!( + matrix.unwrap_err().kind(), + &SparseFormatErrorKind::InvalidStructure + ); + } + + { + // Invalid length of offsets array + let (offsets, indices, values) = invalid_data.invalid_length_of_offsets_array; + let matrix = CsrMatrix::try_from_csr_data(3, 6, offsets, indices, values); + assert_eq!( + matrix.unwrap_err().kind(), + &SparseFormatErrorKind::InvalidStructure + ); + } + + { + // Nonmonotonic offsets + let (offsets, indices, values) = invalid_data.nonmonotonic_offsets; + let matrix = CsrMatrix::try_from_csr_data(3, 6, offsets, indices, values); + assert_eq!( + matrix.unwrap_err().kind(), + &SparseFormatErrorKind::InvalidStructure + ); + } + + { + // Nonmonotonic minor indices + let (offsets, indices, values) = invalid_data.nonmonotonic_minor_indices; + let matrix = CsrMatrix::try_from_csr_data(3, 6, offsets, indices, values); + assert_eq!( + matrix.unwrap_err().kind(), + &SparseFormatErrorKind::InvalidStructure + ); + } + + { + // Minor index out of bounds + let (offsets, indices, values) = invalid_data.minor_index_out_of_bounds; + let matrix = CsrMatrix::try_from_csr_data(3, 6, offsets, indices, values); + assert_eq!( + matrix.unwrap_err().kind(), + &SparseFormatErrorKind::IndexOutOfBounds + ); + } + + { + // Duplicate entry + let (offsets, indices, values) = invalid_data.duplicate_entry; + let matrix = CsrMatrix::try_from_csr_data(3, 6, offsets, indices, values); + assert_eq!( + matrix.unwrap_err().kind(), + &SparseFormatErrorKind::DuplicateEntry + ); + } +} + #[test] fn csr_matrix_try_from_invalid_csr_data_with_new_constructor() { let invalid_data: InvalidCsrDataExamples = InvalidCsrDataExamples::new(); @@ -281,101 +376,6 @@ fn csr_matrix_try_from_invalid_csr_data_with_new_constructor() { } } -#[test] -fn csr_matrix_try_from_invalid_csr_data() { - let invalid_data: InvalidCsrDataExamples = InvalidCsrDataExamples::new(); - { - // Empty offset array (invalid length) - let (offsets, indices, values) = invalid_data.empty_offset_array; - let matrix = CsrMatrix::try_from_unsorted_csr_data(0, 0, offsets, indices, values); - assert_eq!( - matrix.unwrap_err().kind(), - &SparseFormatErrorKind::InvalidStructure - ); - } - - { - // Offset array invalid length for arbitrary data - let (offsets, indices, values) = - invalid_data.offset_array_invalid_length_for_arbitrary_data; - let matrix = CsrMatrix::try_from_unsorted_csr_data(3, 6, offsets, indices, values); - assert_eq!( - matrix.unwrap_err().kind(), - &SparseFormatErrorKind::InvalidStructure - ); - } - - { - // Invalid first entry in offsets array - let (offsets, indices, values) = invalid_data.invalid_first_entry_in_offsets_array; - let matrix = CsrMatrix::try_from_csr_data(3, 6, offsets, indices, values); - assert_eq!( - matrix.unwrap_err().kind(), - &SparseFormatErrorKind::InvalidStructure - ); - } - - { - // Invalid last entry in offsets array - let (offsets, indices, values) = invalid_data.invalid_last_entry_in_offsets_array; - let matrix = CsrMatrix::try_from_csr_data(3, 6, offsets, indices, values); - assert_eq!( - matrix.unwrap_err().kind(), - &SparseFormatErrorKind::InvalidStructure - ); - } - - { - // Invalid length of offsets array - let (offsets, indices, values) = invalid_data.invalid_length_of_offsets_array; - let matrix = CsrMatrix::try_from_csr_data(3, 6, offsets, indices, values); - assert_eq!( - matrix.unwrap_err().kind(), - &SparseFormatErrorKind::InvalidStructure - ); - } - - { - // Nonmonotonic offsets - let (offsets, indices, values) = invalid_data.nonmonotonic_offsets; - let matrix = CsrMatrix::try_from_csr_data(3, 6, offsets, indices, values); - assert_eq!( - matrix.unwrap_err().kind(), - &SparseFormatErrorKind::InvalidStructure - ); - } - - { - // Nonmonotonic minor indices - let (offsets, indices, values) = invalid_data.nonmonotonic_minor_indices; - let matrix = CsrMatrix::try_from_csr_data(3, 6, offsets, indices, values); - assert_eq!( - matrix.unwrap_err().kind(), - &SparseFormatErrorKind::InvalidStructure - ); - } - - { - // Minor index out of bounds - let (offsets, indices, values) = invalid_data.minor_index_out_of_bounds; - let matrix = CsrMatrix::try_from_csr_data(3, 6, offsets, indices, values); - assert_eq!( - matrix.unwrap_err().kind(), - &SparseFormatErrorKind::IndexOutOfBounds - ); - } - - { - // Duplicate entry - let (offsets, indices, values) = invalid_data.duplicate_entry; - let matrix = CsrMatrix::try_from_csr_data(3, 6, offsets, indices, values); - assert_eq!( - matrix.unwrap_err().kind(), - &SparseFormatErrorKind::DuplicateEntry - ); - } -} - #[test] fn csr_disassemble_avoids_clone_when_owned() { // Test that disassemble avoids cloning the sparsity pattern when it holds the sole reference From 752d1f300d9087d2493dc752957ad0237329a75f Mon Sep 17 00:00:00 2001 From: Anton Date: Wed, 20 Oct 2021 01:50:42 +0200 Subject: [PATCH 8/9] Permute values without unnecessary allocation --- nalgebra-sparse/src/csr.rs | 10 ++++------ nalgebra-sparse/tests/unit_tests/csr.rs | 2 +- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/nalgebra-sparse/src/csr.rs b/nalgebra-sparse/src/csr.rs index ecb2ce65..bbeec1da 100644 --- a/nalgebra-sparse/src/csr.rs +++ b/nalgebra-sparse/src/csr.rs @@ -5,13 +5,12 @@ use crate::cs::{CsLane, CsLaneIter, CsLaneIterMut, CsLaneMut, CsMatrix}; use crate::csc::CscMatrix; use crate::pattern::{SparsityPattern, SparsityPatternFormatError, SparsityPatternIter}; -use crate::utils::apply_permutation; use crate::{SparseEntry, SparseEntryMut, SparseFormatError, SparseFormatErrorKind}; use nalgebra::Scalar; use num_traits::One; -use num_traits::Zero; +use std::iter::FromIterator; use std::slice::{Iter, IterMut}; /// A CSR representation of a sparse matrix. @@ -189,7 +188,7 @@ impl CsrMatrix { values: Vec, ) -> Result where - T: Scalar + Zero, + T: Scalar, { use SparsityPatternFormatError::*; let count = col_indices.len(); @@ -228,11 +227,10 @@ impl CsrMatrix { } // permute indices - let sorted_col_indices: Vec = p.iter().map(|i| col_indices[*i]).collect(); + let sorted_col_indices: Vec = Vec::from_iter((p.iter().map(|i| &col_indices[*i])).cloned()); // permute values - let mut sorted_values: Vec = vec![T::zero(); count]; - apply_permutation(&mut sorted_values, &values, &p); + let sorted_values: Vec = Vec::from_iter((p.iter().map(|i| &values[*i])).cloned()); return Self::try_from_csr_data( num_rows, diff --git a/nalgebra-sparse/tests/unit_tests/csr.rs b/nalgebra-sparse/tests/unit_tests/csr.rs index 6d884b75..3ca2f0dc 100644 --- a/nalgebra-sparse/tests/unit_tests/csr.rs +++ b/nalgebra-sparse/tests/unit_tests/csr.rs @@ -292,7 +292,7 @@ fn csr_matrix_try_from_invalid_csr_data() { } #[test] -fn csr_matrix_try_from_invalid_csr_data_with_new_constructor() { +fn csr_matrix_try_from_unsorted_invalid_csr_data() { let invalid_data: InvalidCsrDataExamples = InvalidCsrDataExamples::new(); { // Empty offset array (invalid length) From 89416baacea24b4da17de7223646303ade219623 Mon Sep 17 00:00:00 2001 From: Anton Date: Wed, 20 Oct 2021 20:28:38 +0200 Subject: [PATCH 9/9] Bring apply permutation function back to serial.rs --- nalgebra-sparse/src/convert/serial.rs | 10 +++++++++- nalgebra-sparse/src/csr.rs | 3 ++- nalgebra-sparse/src/lib.rs | 1 - nalgebra-sparse/src/utils.rs | 11 ----------- 4 files changed, 11 insertions(+), 14 deletions(-) delete mode 100644 nalgebra-sparse/src/utils.rs diff --git a/nalgebra-sparse/src/convert/serial.rs b/nalgebra-sparse/src/convert/serial.rs index 219a6bf7..ecbe1dab 100644 --- a/nalgebra-sparse/src/convert/serial.rs +++ b/nalgebra-sparse/src/convert/serial.rs @@ -14,7 +14,6 @@ use crate::coo::CooMatrix; use crate::cs; use crate::csc::CscMatrix; use crate::csr::CsrMatrix; -use crate::utils::apply_permutation; /// Converts a dense matrix to [`CooMatrix`]. pub fn convert_dense_coo(dense: &Matrix) -> CooMatrix @@ -391,6 +390,15 @@ fn sort_lane( apply_permutation(values_result, values, permutation); } +// TODO: Move this into `utils` or something? +fn apply_permutation(out_slice: &mut [T], in_slice: &[T], permutation: &[usize]) { + assert_eq!(out_slice.len(), in_slice.len()); + assert_eq!(out_slice.len(), permutation.len()); + for (out_element, old_pos) in out_slice.iter_mut().zip(permutation) { + *out_element = in_slice[*old_pos].clone(); + } +} + /// Given *sorted* indices and corresponding scalar values, combines duplicates with the given /// associative combiner and calls the provided produce methods with combined indices and values. fn combine_duplicates( diff --git a/nalgebra-sparse/src/csr.rs b/nalgebra-sparse/src/csr.rs index bbeec1da..4324d18d 100644 --- a/nalgebra-sparse/src/csr.rs +++ b/nalgebra-sparse/src/csr.rs @@ -227,7 +227,8 @@ impl CsrMatrix { } // permute indices - let sorted_col_indices: Vec = Vec::from_iter((p.iter().map(|i| &col_indices[*i])).cloned()); + let sorted_col_indices: Vec = + Vec::from_iter((p.iter().map(|i| &col_indices[*i])).cloned()); // permute values let sorted_values: Vec = Vec::from_iter((p.iter().map(|i| &values[*i])).cloned()); diff --git a/nalgebra-sparse/src/lib.rs b/nalgebra-sparse/src/lib.rs index 64331817..bf845757 100644 --- a/nalgebra-sparse/src/lib.rs +++ b/nalgebra-sparse/src/lib.rs @@ -151,7 +151,6 @@ pub mod ops; pub mod pattern; pub(crate) mod cs; -pub(crate) mod utils; #[cfg(feature = "proptest-support")] pub mod proptest; diff --git a/nalgebra-sparse/src/utils.rs b/nalgebra-sparse/src/utils.rs deleted file mode 100644 index a5da85c5..00000000 --- a/nalgebra-sparse/src/utils.rs +++ /dev/null @@ -1,11 +0,0 @@ -//! Helper functions for sparse matrix computations - -/// permutes entries of in_slice according to permutation slice and puts them to out_slice -#[inline] -pub fn apply_permutation(out_slice: &mut [T], in_slice: &[T], permutation: &[usize]) { - assert_eq!(out_slice.len(), in_slice.len()); - assert_eq!(out_slice.len(), permutation.len()); - for (out_element, old_pos) in out_slice.iter_mut().zip(permutation) { - *out_element = in_slice[*old_pos].clone(); - } -}