Merge pull request #237 from mulimoen/bugfix/serde

Fix serde deserialisation

Author: vbarrielle

Cargo.toml

@@ -61,6 +61,7 @@ members = [
     "suitesparse_bindings/suitesparse-src",
     "sprs-rand",
     "sprs-benches",
+    "sprs-tests",
 ]
 
 [package.metadata.docs.rs]

sprs-tests/Cargo.toml

@@ -0,0 +1,10 @@
+[package]
+name = "sprs-tests"
+version = "0.1.0"
+authors = ["Magnus Ulimoen <[email protected]>"]
+edition = "2018"
+publish = false
+
+[dev-dependencies]
+sprs = { path = "..", features = ["serde"], default-features = false }
+serde_json = "1.0.58"

sprs-tests/README.md

@@ -0,0 +1,3 @@
+# sprs-tests
+
+Ancillary crate to test `sprs` by pulling in extra dependencies

sprs-tests/src/main.rs

@@ -0,0 +1 @@
+fn main() {}

sprs-tests/tests/tests.rs

@@ -0,0 +1,87 @@
+mod serde_tests {
+    use sprs::*;
+    #[test]
+    fn valid_vectors() {
+        let json_vec =
+            r#"{ "dim": 100, "indices": [4, 6, 10], "data": [4, 1, 8] }"#;
+        let _vec: CsVecI<u8, i32> = serde_json::from_str(&json_vec).unwrap();
+
+        let json_vec = r#"{ "dim": 200, "indices": [4, 6, 10, 120], "data": [4, 1, 8, 1] }"#;
+        let _vec: CsVecI<i8, u16> = serde_json::from_str(&json_vec).unwrap();
+    }
+
+    #[test]
+    fn invalid_vectors() {
+        // non-sorted indices
+        let json_vec =
+            r#"{ "dim": 100, "indices": [4, 6, 5], "data": [4, 1, 8] }"#;
+        let e: Result<CsVecI<u8, i32>, _> = serde_json::from_str(&json_vec);
+        assert!(e.is_err());
+
+        // max(indices) > dim
+        let json_vec =
+            r#"{ "dim": 2, "indices": [4, 6, 8], "data": [4, 1, 8] }"#;
+        let e: Result<CsVecI<u8, i32>, _> = serde_json::from_str(&json_vec);
+        assert!(e.is_err());
+
+        // indices.len != data.len
+        let json_vec =
+            r#"{ "dim": 100, "indices": [4, 6, 8, 10], "data": [4, 1, 8] }"#;
+        let e: Result<CsVecI<u8, i32>, _> = serde_json::from_str(&json_vec);
+        assert!(e.is_err());
+
+        // indice does not fit in datatype
+        let json_vec =
+            r#"{ "dim": 100000, "indices": [4, 6, 32768], "data": [4, 1, 8] }"#;
+        let e: Result<CsVecI<u8, i16>, _> = serde_json::from_str(&json_vec);
+        assert!(e.is_err());
+    }
+
+    #[test]
+    fn valid_matrices() {
+        let json_mat = r#"{ "storage": "CSR", "ncols": 10, "nrows": 2, "indptr": [0, 2, 3], "indices": [4, 6, 9], "data": [4, 1, 8] }"#;
+        let _mat: CsMatI<u8, i32, u16> =
+            serde_json::from_str(&json_mat).unwrap();
+        let _mat: CsMat<u8> = serde_json::from_str(&json_mat).unwrap();
+    }
+
+    #[test]
+    fn invalid_matrices() {
+        // indices not sorted
+        let json_mat = r#"{ "storage": "CSR", "ncols": 10, "nrows": 2, "indptr": [0, 3, 3], "indices": [4, 9, 6], "data": [4, 1, 8] }"#;
+        let mat: Result<CsMatI<u8, i32, u16>, _> =
+            serde_json::from_str(&json_mat);
+        assert!(mat.is_err());
+
+        // data length != indices length
+        let json_mat = r#"{ "storage": "CSR", "ncols": 10, "nrows": 2, "indptr": [0, 2, 3], "indices": [4, 9, 6], "data": [4, 1, 8, 10] }"#;
+        let mat: Result<CsMatI<u8, i32, u16>, _> =
+            serde_json::from_str(&json_mat);
+        assert!(mat.is_err());
+    }
+
+    #[test]
+    fn valid_indptr() {
+        let indptr = r#"{ "storage": [0, 0, 1, 2, 2, 6] }"#;
+        let indptr: IndPtr<usize> = serde_json::from_str(&indptr).unwrap();
+        assert_eq!(indptr.raw_storage(), &[0, 0, 1, 2, 2, 6]);
+
+        let indptr = r#"{ "storage": [5, 5, 8, 9] }"#;
+        let indptr: IndPtr<usize> = serde_json::from_str(&indptr).unwrap();
+        assert_eq!(indptr.raw_storage(), &[5, 5, 8, 9]);
+    }
+
+    #[test]
+    fn invalid_indptr() {
+        let indptr = r#"{ "storage": [0, 0, 1, 2, 2, 1] }"#;
+        let indptr: Result<IndPtr<usize>, _> = serde_json::from_str(&indptr);
+        assert!(indptr.is_err());
+        let indptr = r#"{ "storage": [2, 1, 2, 2, 2, 7] }"#;
+        let indptr: Result<IndPtr<usize>, _> = serde_json::from_str(&indptr);
+        assert!(indptr.is_err());
+        // Larger than permitted by i16
+        let indptr = r#"{ "storage": [0, 32768] }"#;
+        let indptr: Result<IndPtr<i16>, _> = serde_json::from_str(&indptr);
+        assert!(indptr.is_err());
+    }
+}

src/sparse.rs

@@ -5,7 +5,9 @@ use crate::IndPtrBase;
 use std::ops::Deref;
 
 #[cfg(feature = "serde")]
-use serde::{Deserialize, Serialize};
+mod serde_traits;
+#[cfg(feature = "serde")]
+use serde_traits::{CsMatBaseShadow, CsVecBaseShadow, Deserialize, Serialize};
 
 pub use self::csmat::CompressedStorage;
 
@@ -83,6 +85,12 @@ pub use self::csmat::CompressedStorage;
 
 #[derive(Eq, PartialEq, Debug, Copy, Clone, Hash)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(
+    feature = "serde",
+    serde(
+        try_from = "CsMatBaseShadow<N, I, IptrStorage, IndStorage, DataStorage, Iptr>"
+    )
+)]
 pub struct CsMatBase<N, I, IptrStorage, IndStorage, DataStorage, Iptr = I>
 where
     I: SpIndex,
@@ -150,19 +158,28 @@ pub type CsStructure = CsStructureI<usize>;
 
 #[derive(Eq, PartialEq, Debug, Copy, Clone, Hash)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-pub struct CsVecBase<IStorage, DStorage> {
+#[cfg_attr(
+    feature = "serde",
+    serde(try_from = "CsVecBaseShadow<IStorage, DStorage, N, I>")
+)]
+pub struct CsVecBase<IStorage, DStorage, N, I: SpIndex = usize>
+where
+    IStorage: Deref<Target = [I]>,
+    DStorage: Deref<Target = [N]>,
+{
     dim: usize,
     indices: IStorage,
     data: DStorage,
 }
 
-pub type CsVecI<N, I> = CsVecBase<Vec<I>, Vec<N>>;
-pub type CsVecViewI<'a, N, I> = CsVecBase<&'a [I], &'a [N]>;
-pub type CsVecViewMutI<'a, N, I> = CsVecBase<&'a [I], &'a mut [N]>;
+pub type CsVecI<N, I = usize> = CsVecBase<Vec<I>, Vec<N>, N, I>;
+pub type CsVecViewI<'a, N, I = usize> = CsVecBase<&'a [I], &'a [N], N, I>;
+pub type CsVecViewMutI<'a, N, I = usize> =
+    CsVecBase<&'a [I], &'a mut [N], N, I>;
 
-pub type CsVecView<'a, N> = CsVecViewI<'a, N, usize>;
-pub type CsVecViewMut<'a, N> = CsVecViewMutI<'a, N, usize>;
-pub type CsVec<N> = CsVecI<N, usize>;
+pub type CsVecView<'a, N> = CsVecViewI<'a, N>;
+pub type CsVecViewMut<'a, N> = CsVecViewMutI<'a, N>;
+pub type CsVec<N> = CsVecI<N>;
 
 /// Sparse matrix in the triplet format.
 ///

src/sparse/compressed.rs

@@ -39,7 +39,7 @@ pub trait SpVecView<N, I: SpIndex> {
 }
 
 impl<N, I, IndStorage, DataStorage> SpVecView<N, I>
-    for CsVecBase<IndStorage, DataStorage>
+    for CsVecBase<IndStorage, DataStorage, N, I>
 where
     IndStorage: Deref<Target = [I]>,
     DataStorage: Deref<Target = [N]>,

src/sparse/csmat.rs

@@ -144,7 +144,7 @@ where
     IStorage: Deref<Target = [I]>,
     DStorage: Deref<Target = [N]>,
 {
-    fn new_checked(
+    pub(crate) fn new_checked(
         storage: CompressedStorage,
         shape: (usize, usize),
         indptr: IptrStorage,
@@ -530,7 +530,7 @@ impl<N, I: SpIndex, Iptr: SpIndex> CsMatI<N, I, Iptr> {
     }
 
     /// Append an outer dim to an existing matrix, provided by a sparse vector
-    pub fn append_outer_csvec(mut self, vec: CsVecBase<&[I], &[N]>) -> Self
+    pub fn append_outer_csvec(mut self, vec: CsVecViewI<N, I>) -> Self
     where
         N: Clone,
     {

src/sparse/indptr.rs

@@ -3,6 +3,8 @@
 //!
 //! [`CsMatBase`]: type.CsMatBase.html
 
+#[cfg(feature = "serde")]
+use super::serde_traits::IndPtrBaseShadow;
 use crate::errors::SprsError;
 use crate::indexing::SpIndex;
 #[cfg(feature = "serde")]
@@ -12,6 +14,10 @@ use std::ops::{Deref, DerefMut};
 
 #[derive(Eq, PartialEq, Debug, Copy, Clone, Hash)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(
+    feature = "serde",
+    serde(try_from = "IndPtrBaseShadow<Iptr, Storage>")
+)]
 pub struct IndPtrBase<Iptr, Storage>
 where
     Iptr: SpIndex,

src/sparse/serde_traits.rs

@@ -0,0 +1,96 @@
+use super::*;
+pub(crate) use serde::{Deserialize, Serialize};
+use std::convert::TryFrom;
+
+#[derive(Deserialize)]
+pub(crate) struct CsVecBaseShadow<IStorage, DStorage, N, I: SpIndex = usize>
+where
+    IStorage: Deref<Target = [I]>,
+    DStorage: Deref<Target = [N]>,
+{
+    dim: usize,
+    indices: IStorage,
+    data: DStorage,
+}
+
+impl<IStorage, DStorage, N, I: SpIndex>
+    TryFrom<CsVecBaseShadow<IStorage, DStorage, N, I>>
+    for CsVecBase<IStorage, DStorage, N, I>
+where
+    IStorage: Deref<Target = [I]>,
+    DStorage: Deref<Target = [N]>,
+{
+    type Error = SprsError;
+    fn try_from(
+        val: CsVecBaseShadow<IStorage, DStorage, N, I>,
+    ) -> Result<Self, Self::Error> {
+        let CsVecBaseShadow { dim, indices, data } = val;
+        Self::new_(dim, indices, data).map_err(|(_, _, e)| e)
+    }
+}
+
+#[derive(Deserialize)]
+pub struct CsMatBaseShadow<N, I, IptrStorage, IndStorage, DataStorage, Iptr = I>
+where
+    I: SpIndex,
+    Iptr: SpIndex,
+    IptrStorage: Deref<Target = [Iptr]>,
+    IndStorage: Deref<Target = [I]>,
+    DataStorage: Deref<Target = [N]>,
+{
+    storage: CompressedStorage,
+    nrows: usize,
+    ncols: usize,
+    indptr: IptrStorage,
+    indices: IndStorage,
+    data: DataStorage,
+}
+
+impl<IptrStorage, IndStorage, DStorage, N, I: SpIndex, Iptr: SpIndex>
+    TryFrom<CsMatBaseShadow<N, I, IptrStorage, IndStorage, DStorage, Iptr>>
+    for CsMatBase<N, I, IptrStorage, IndStorage, DStorage, Iptr>
+where
+    IndStorage: Deref<Target = [I]>,
+    IptrStorage: Deref<Target = [Iptr]>,
+    DStorage: Deref<Target = [N]>,
+{
+    type Error = SprsError;
+    fn try_from(
+        val: CsMatBaseShadow<N, I, IptrStorage, IndStorage, DStorage, Iptr>,
+    ) -> Result<Self, Self::Error> {
+        let CsMatBaseShadow {
+            storage,
+            nrows,
+            ncols,
+            indptr,
+            indices,
+            data,
+        } = val;
+        let shape = (nrows, ncols);
+        Self::new_checked(storage, shape, indptr, indices, data)
+            .map_err(|(_, _, _, e)| e)
+    }
+}
+
+#[derive(Deserialize)]
+pub struct IndPtrBaseShadow<Iptr, Storage>
+where
+    Iptr: SpIndex,
+    Storage: Deref<Target = [Iptr]>,
+{
+    storage: Storage,
+}
+
+impl<Iptr: SpIndex, Storage> TryFrom<IndPtrBaseShadow<Iptr, Storage>>
+    for IndPtrBase<Iptr, Storage>
+where
+    Storage: Deref<Target = [Iptr]>,
+{
+    type Error = SprsError;
+    fn try_from(
+        val: IndPtrBaseShadow<Iptr, Storage>,
+    ) -> Result<Self, Self::Error> {
+        let IndPtrBaseShadow { storage } = val;
+        Self::new(storage)
+    }
+}