Skip to content

Refactor to allow partial views #245

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

Merged
merged 19 commits into from
Nov 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 15 additions & 11 deletions sprs-benches/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,11 @@ fn scipy_mat<'a>(
py: &Python,
mat: &sprs::CsMat<f64>,
) -> Result<&'a PyAny, String> {
let indptr = mat.indptr().to_proper().to_vec();
scipy_sparse
.call(
"csr_matrix",
((
mat.data().to_vec(),
mat.indices().to_vec(),
mat.indptr().to_vec(),
),),
((mat.data().to_vec(), mat.indices().to_vec(), indptr),),
Some([("shape", mat.shape())].into_py_dict(*py)),
)
.map_err(|e| {
Expand Down Expand Up @@ -51,14 +48,14 @@ fn eigen_prod(a: sprs::CsMatView<f64>, b: sprs::CsMatView<f64>) -> usize {
assert_eq!(a_cols, b_rows);
assert!(a.is_csr());
assert!(a.rows() <= isize::MAX as usize);
assert!(a.indptr()[a.rows()] <= isize::MAX as usize);
assert!(a.indptr().nnz() <= isize::MAX as usize);
assert!(b.is_csr());
assert!(b.rows() <= isize::MAX as usize);
assert!(b.indptr()[b.rows()] <= isize::MAX as usize);
let a_indptr = a.indptr().as_ptr() as *const isize;
assert!(b.indptr().nnz() <= isize::MAX as usize);
let a_indptr_proper = a.proper_indptr();
let a_indices = a.indices().as_ptr() as *const isize;
let a_data = a.data().as_ptr();
let b_indptr = b.indptr().as_ptr() as *const isize;
let b_indptr_proper = b.proper_indptr();
let b_indices = b.indices().as_ptr() as *const isize;
let b_data = b.data().as_ptr();
// Safety: sprs guarantees the validity of these pointers, and our wrapping
Expand All @@ -71,8 +68,15 @@ fn eigen_prod(a: sprs::CsMatView<f64>, b: sprs::CsMatView<f64>) -> usize {
// reinterpretation of the data will not produce negative values.
unsafe {
prod_nnz(
a_rows, a_cols, b_cols, a_indptr, a_indices, a_data, b_indptr,
b_indices, b_data,
a_rows,
a_cols,
b_cols,
a_indptr_proper.as_ptr() as *const isize,
a_indices,
a_data,
b_indptr_proper.as_ptr() as *const isize,
b_indices,
b_data,
)
}
}
Expand Down
4 changes: 3 additions & 1 deletion sprs-ldl/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,9 @@ impl Ldl {
}
FillInReduction::CAMDSuiteSparse => {
#[cfg(not(feature = "sprs_suitesparse_camd"))]
panic!("Unavailable without the `sprs_suitesparse_camd` feature");
panic!(
"Unavailable without the `sprs_suitesparse_camd` feature"
);
#[cfg(feature = "sprs_suitesparse_camd")]
sprs_suitesparse_camd::camd(mat.structure_view())
}
Expand Down
15 changes: 8 additions & 7 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,14 @@ pub type Ix2 = ndarray::Ix2;
pub use crate::indexing::SpIndex;

pub use crate::sparse::{
csmat::CsIter, csmat::OuterIterator, csmat::OuterIteratorMut,
csmat::OuterIteratorPerm, kronecker::kronecker_product, CsMat, CsMatBase,
CsMatI, CsMatVecView, CsMatView, CsMatViewI, CsMatViewMut, CsMatViewMutI,
CsStructure, CsStructureI, CsStructureView, CsStructureViewI, CsVec,
CsVecBase, CsVecI, CsVecView, CsVecViewI, CsVecViewMut, CsVecViewMutI,
SparseMat, TriMat, TriMatBase, TriMatI, TriMatIter, TriMatView,
TriMatViewI, TriMatViewMut, TriMatViewMutI,
csmat::CsIter,
indptr::{IndPtr, IndPtrBase, IndPtrView},
kronecker::kronecker_product,
CsMat, CsMatBase, CsMatI, CsMatVecView, CsMatView, CsMatViewI,
CsMatViewMut, CsMatViewMutI, CsStructure, CsStructureI, CsStructureView,
CsStructureViewI, CsVec, CsVecBase, CsVecI, CsVecView, CsVecViewI,
CsVecViewMut, CsVecViewMutI, SparseMat, TriMat, TriMatBase, TriMatI,
TriMatIter, TriMatView, TriMatViewI, TriMatViewMut, TriMatViewMutI,
};

pub use crate::sparse::symmetric::is_symmetric;
Expand Down
49 changes: 9 additions & 40 deletions src/sparse.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::array_backend::Array2;
use crate::errors::SprsError;
use crate::indexing::SpIndex;
use crate::IndPtrBase;
use std::ops::Deref;

#[cfg(feature = "serde")]
Expand Down Expand Up @@ -93,7 +94,8 @@ where
storage: CompressedStorage,
nrows: usize,
ncols: usize,
indptr: IptrStorage,
#[cfg_attr(feature = "serde", serde(flatten))]
indptr: IndPtrBase<Iptr, IptrStorage>,
indices: IndStorage,
data: DataStorage,
}
Expand Down Expand Up @@ -282,6 +284,7 @@ pub(crate) mod utils {
indptr: &[Iptr],
indices: &[I],
) -> Result<(), SprsError> {
let indptr = crate::IndPtrView::new(indptr)?;
if indptr.len() != outer + 1 {
return Err(SprsError::IllegalArguments(
"Indptr length does not match dimension",
Expand All @@ -298,15 +301,8 @@ pub(crate) mod utils {
"Iptr type not large enough for this matrix",
));
}
// Make sure both indptr and indices can be converted to usize
// Make sure indices can be converted to usize
// this could happen if index is negative for sized types
for i in indptr.iter() {
if i.try_index().is_none() {
return Err(SprsError::IllegalArguments(
"Indptr value out of range of usize",
));
}
}
for i in indices.iter() {
if i.try_index().is_none() {
return Err(SprsError::IllegalArguments(
Expand All @@ -315,43 +311,15 @@ pub(crate) mod utils {
}
}
let nnz = indices.len();
if nnz != indptr.last().unwrap().to_usize().unwrap() {
if nnz != indptr.nnz() {
return Err(SprsError::IllegalArguments(
"Indices length and inpdtr's nnz do not match",
));
}

// indptr should be non-monotonically increasing
if !indptr
.windows(2)
.all(|x| x[0].index_unchecked() <= x[1].index_unchecked())
{
return Err(SprsError::UnsortedIndptr);
}
// Guaranteed to have at least one element
let max_indptr = indptr.last().unwrap();
if max_indptr.index_unchecked() > nnz {
return Err(SprsError::IllegalArguments(
"An indptr value is out of bounds",
));
}
if max_indptr.index_unchecked() > usize::max_value() / 2 {
// We do not allow indptr values to be larger than half
// the maximum value of an usize, as that would clearly exhaust
// all available memory
// This means we could have an isize, but in practice it's
// easier to work with usize for indexing.
return Err(SprsError::IllegalArguments(
"An indptr value is larger than allowed",
));
}

// check that the indices are sorted for each row
for win in indptr.windows(2) {
let [i1, i2]: &[Iptr; 2] = win.try_into().unwrap();
let i1 = i1.to_usize().unwrap();
let i2 = i2.to_usize().unwrap();
let indices = &indices[i1..i2];
for range in indptr.iter_outer_sz() {
let indices = &indices[range];
// Indices must be monotonically increasing
if !sorted_indices(indices) {
return Err(SprsError::NonSortedIndices);
Expand Down Expand Up @@ -410,6 +378,7 @@ pub mod binop;
pub mod compressed;
pub mod construct;
pub mod csmat;
pub mod indptr;
pub mod kronecker;
pub mod linalg;
pub mod permutation;
Expand Down
3 changes: 2 additions & 1 deletion src/sparse/binop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::sparse::csmat::CompressedStorage;
use crate::sparse::prelude::*;
use crate::sparse::vec::NnzEither::{Both, Left, Right};
use crate::sparse::vec::SparseIterTools;
use crate::IndPtr;
use ndarray::{
self, Array, ArrayBase, ArrayView, ArrayViewMut, Axis, ShapeBuilder,
};
Expand Down Expand Up @@ -125,7 +126,7 @@ where
storage,
nrows,
ncols,
indptr: out_indptr,
indptr: IndPtr::new_trusted(out_indptr),
indices: out_indices,
data: out_data,
}
Expand Down
Loading