Merge pull request #221 from mulimoen/documentation

Minor documentation and CI changes

Author: vbarrielle

.github/workflows/ci.yml

@@ -7,7 +7,59 @@ on:
     - master
   schedule:
   - cron: '0 0 15 * *'
+
+env:
+    CARGO_TERM_COLOR: always
+    RUSTFLAGS: -D warnings
+
 jobs:
+  rustfmt:
+    name: rustfmt
+    runs-on: ubuntu-18.04
+    steps:
+    - name: Checkout repository
+      uses: actions/checkout@v2
+    - name: Install rust
+      uses: actions-rs/toolchain@v1
+      with:
+        toolchain: stable
+        override: true
+        profile: minimal
+        components: rustfmt
+    - name: Check formatting
+      run: cargo fmt -- --check
+
+  documentation:
+    name: documentation
+    runs-on: ubuntu-18.04
+    steps:
+    - name: Checkout repository
+      uses: actions/checkout@v2
+    - name: Install rust
+      uses: actions-rs/toolchain@v1
+      with:
+        toolchain: stable
+        override: true
+        profile: minimal
+    - name: Documentation
+      run: cargo doc --all-features --workspace
+
+  clippy:
+    name: clippy
+    runs-on: ubuntu-18.04
+    steps:
+    - name: Checkout repository
+      uses: actions/checkout@v2
+    - name: Install rust
+      uses: actions-rs/toolchain@v1
+      with:
+        toolchain: stable
+        override: true
+        profile: minimal
+        components: clippy
+    - name: Run Clippy
+      run: cargo clippy --workspace --all-features
+
   test:
     name: test
     runs-on: ${{ matrix.os }}
@@ -57,10 +109,8 @@ jobs:
       if: matrix.os != 'ubuntu-18.04'
 
     - name: Build (all)
-      run: env && cargo build --verbose --workspace
+      run: cargo build --verbose --workspace
       if: matrix.os == 'ubuntu-18.04'
-      env:
-        RUSTFLAGS: -D warnings
 
     - name: Test (exclude suitesparse)
       run: cargo test --verbose --workspace --exclude suitesparse_ldl_sys --exclude sprs_suitesparse_ldl
@@ -74,25 +124,6 @@ jobs:
       run: cargo test -p sprs-ldl --features sprs_suitesparse_ldl -Zpackage-features
       if: matrix.os == 'ubuntu-18.04' && matrix.build == 'nightly'
 
-  rustfmt:
-    name: rustfmt
-    runs-on: ubuntu-18.04
-    steps:
-    - name: Checkout repository
-      uses: actions/checkout@v2
-    - name: Install rust
-      uses: actions-rs/toolchain@v1
-      with:
-        toolchain: stable
-        override: true
-        profile: minimal
-        components: rustfmt
-    - name: Check formatting
-      run: |
-        cargo fmt -- --check
-    - name: Documentation
-      run: cargo doc
-
   benches:
     name: benchmarks
     runs-on: ubuntu-18.04
@@ -112,8 +143,8 @@ jobs:
       run: |
           cargo bench --workspace
 
-  optional_features:
-    name: Optional features
+  optional_none:
+    name: Optional features (none selected)
     runs-on: ubuntu-18.04
     steps:
     - name: Checkout repository
@@ -129,7 +160,7 @@ jobs:
           cargo test --no-default-features
 
   optional_approx:
-    name: Optional features
+    name: Optional features (approx selected)
     runs-on: ubuntu-18.04
     steps:
     - name: Checkout repository

Cargo.toml

@@ -54,3 +54,6 @@ members = [
     "sprs-rand",
     "sprs-benches",
 ]
+
+[package.metadata.docs.rs]
+all-features = true

sprs-benches/Cargo.toml

@@ -4,8 +4,6 @@ version = "0.1.0"
 authors = ["Vincent Barrielle <[email protected]>"]
 edition = "2018"
 
-# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
-
 [dependencies.sprs]
 version = "0.8.0"
 path = ".."
@@ -18,7 +16,7 @@ path = "../sprs-rand"
 version = "0.2.12"
 
 [dependencies]
-pyo3 = { version="0.9.2", optional=true }
+pyo3 = { version="0.11.1" }
 
 [build-dependencies]
 cc = "1.0.52"
@@ -29,6 +27,5 @@ libflate = {version="1.0.0", optional=true }
 [features]
 
 default = []
-nightly = ["pyo3"]
 dl_eigen = ["reqwest", "tar", "libflate"]
 eigen = []

sprs-benches/src/main.rs

@@ -1,12 +1,10 @@
-#[cfg(feature = "nightly")]
 use pyo3::{
     prelude::*,
     types::{IntoPyDict, PyModule},
 };
 use sprs::smmp;
 use sprs_rand::rand_csr_std;
 
-#[cfg(feature = "nightly")]
 fn scipy_mat<'a>(
     scipy_sparse: &'a PyModule,
     py: &Python,
@@ -155,11 +153,8 @@ fn bench_densities() -> Result<(), Box<dyn std::error::Error>> {
         },
     ];
 
-    #[cfg(feature = "nightly")]
     let gil = Python::acquire_gil();
-    #[cfg(feature = "nightly")]
     let py = gil.python();
-    #[cfg(feature = "nightly")]
     let scipy_sparse = PyModule::import(py, "scipy.sparse").map_err(|e| {
         let res = format!("Python error: {:?}", e);
         e.print_and_set_sys_last_vars(py);
@@ -169,7 +164,7 @@ fn bench_densities() -> Result<(), Box<dyn std::error::Error>> {
     for spec in &bench_specs {
         let shape = spec.shape;
 
-        let is_shape_bench = spec.shapes.len() != 0;
+        let is_shape_bench = !spec.shapes.is_empty();
         let densities = if is_shape_bench {
             spec.shapes
                 .iter()
@@ -190,7 +185,6 @@ fn bench_densities() -> Result<(), Box<dyn std::error::Error>> {
         let mut times_autothread = Vec::with_capacity(densities.len());
         let mut times_2threads = Vec::with_capacity(densities.len());
         let mut times_4threads = Vec::with_capacity(densities.len());
-        #[cfg(feature = "nightly")]
         let mut times_py = Vec::with_capacity(densities.len());
         #[cfg(feature = "eigen")]
         let mut times_eigen = Vec::with_capacity(densities.len());
@@ -261,7 +255,6 @@ fn bench_densities() -> Result<(), Box<dyn std::error::Error>> {
             res_densities.push(prod.density());
 
             // bench scipy as well
-            #[cfg(feature = "nightly")]
             {
                 let m1_py = scipy_mat(scipy_sparse, &py, &m1)?;
                 let m2_py = scipy_mat(scipy_sparse, &py, &m2)?;
@@ -307,7 +300,6 @@ fn bench_densities() -> Result<(), Box<dyn std::error::Error>> {
         println!("Product times (sprs, 2 threads): {:?}", times_2threads);
         println!("Product times (sprs, 4 threads): {:?}", times_4threads);
         println!("Product times (sprs, auto threads): {:?}", times_autothread);
-        #[cfg(feature = "nightly")]
         println!("Product times (scipy): {:?}", times_py);
         #[cfg(feature = "eigen")]
         println!("Product times (eigen): {:?}", times_eigen);
@@ -352,7 +344,6 @@ fn bench_densities() -> Result<(), Box<dyn std::error::Error>> {
                 max_time,
                 *times_eigen.iter().max().unwrap_or(&1),
             );
-            #[cfg(feature = "nightly")]
             let max_time =
                 std::cmp::max(max_time, *times_py.iter().max().unwrap_or(&1));
             let max_time = max_time as f32;
@@ -423,7 +414,6 @@ fn bench_densities() -> Result<(), Box<dyn std::error::Error>> {
                     PathElement::new(vec![(x, y), (x + 20, y)], &BLUE)
                 });
 
-            #[cfg(feature = "nightly")]
             chart
                 .draw_series(LineSeries::new(
                     abscisses

sprs-ldl/src/lib.rs

@@ -78,6 +78,16 @@ pub struct Ldl {
     fill_red_method: FillInReduction,
 }
 
+impl Default for Ldl {
+    fn default() -> Self {
+        Self {
+            check_symmetry: SymmetryCheck::CheckSymmetry,
+            fill_red_method: FillInReduction::ReverseCuthillMcKee,
+            check_perm: PermutationCheck::CheckPerm,
+        }
+    }
+}
+
 /// Structure to compute and hold a symbolic LDLT decomposition
 #[derive(Debug, Clone)]
 pub struct LdlSymbolic<I> {
@@ -101,11 +111,7 @@ pub struct LdlNumeric<N, I> {
 
 impl Ldl {
     pub fn new() -> Self {
-        Self {
-            check_symmetry: SymmetryCheck::CheckSymmetry,
-            fill_red_method: FillInReduction::ReverseCuthillMcKee,
-            check_perm: PermutationCheck::CheckPerm,
-        }
+        Self::default()
     }
 
     pub fn check_symmetry(self, check: SymmetryCheck) -> Self {
@@ -257,10 +263,10 @@ impl<I: SpIndex> LdlSymbolic<I> {
 
         LdlSymbolic {
             colptr: l_colptr,
-            parents: parents,
+            parents,
             nz: l_nz,
-            flag_workspace: flag_workspace,
-            perm: perm,
+            flag_workspace,
+            perm,
         }
     }
 
@@ -294,11 +300,11 @@ impl<I: SpIndex> LdlSymbolic<I> {
         let pattern_workspace = DStack::with_capacity(n);
         let mut ldl_numeric = LdlNumeric {
             symbolic: self,
-            l_indices: l_indices,
-            l_data: l_data,
-            diag: diag,
-            y_workspace: y_workspace,
-            pattern_workspace: pattern_workspace,
+            l_indices,
+            l_data,
+            diag,
+            y_workspace,
+            pattern_workspace,
         };
         ldl_numeric.update(mat).map(|_| ldl_numeric)
     }
@@ -466,6 +472,7 @@ pub fn ldl_symbolic<N, I, PStorage>(
 /// Perform numeric LDLT decomposition
 ///
 /// pattern_workspace is a DStack of capacity n
+#[allow(clippy::too_many_arguments)]
 pub fn ldl_numeric<N, I, PStorage>(
     mat: CsMatViewI<N, I>,
     l_colptr: &[I],

src/lib.rs

@@ -162,6 +162,8 @@ pub enum FillInReduction {
 ///
 /// Comparisons of sparse matrices with different storages might be slow.
 /// It is advised to compare using the same storage order for efficiency
+///
+/// These traits requires the `approx` feature to be activated
 pub mod approx {
     pub use approx::{AbsDiffEq, RelativeEq, UlpsEq};
 }

src/sparse/binop.rs

@@ -87,7 +87,7 @@ pub fn csmat_binop<N, I, Iptr, F>(
     binop: F,
 ) -> CsMatI<N, I, Iptr>
 where
-    N: Num,
+    N: Num + Clone,
     I: SpIndex,
     Iptr: SpIndex,
     F: Fn(&N, &N) -> N,
@@ -109,10 +109,7 @@ where
     // Sadly the vec! macro requires Clone, but we don't want to force
     // Clone on our consumers, so we have to use this workaround.
     // This should compile to decent code however.
-    let mut out_data = Vec::with_capacity(max_nnz);
-    for _ in 0..max_nnz {
-        out_data.push(N::zero());
-    }
+    let mut out_data = vec![N::zero(); max_nnz];
 
     let nnz = csmat_binop_same_storage_raw(
         lhs,

src/sparse/csmat.rs

@@ -702,10 +702,7 @@ impl<N, I: SpIndex, Iptr: SpIndex>
         if outer_ind >= outer_dims {
             // we need to add a new outer dimension
             let last_nnz = *self.indptr.last().unwrap(); // indptr never empty
-            self.indptr.reserve(1 + outer_ind - outer_dims);
-            for _ in outer_dims..outer_ind {
-                self.indptr.push(last_nnz);
-            }
+            self.indptr.resize(outer_ind + 1, last_nnz);
             self.set_outer_dims(outer_ind + 1);
             self.indptr.push(last_nnz + Iptr::one());
             self.indices.push(inner_ind_idx);
@@ -2996,14 +2993,15 @@ mod approx_impls {
             } else {
                 // Checks if all elements in self has a matching element
                 // in other
-                if !self.iter().all(|(n, (i, j))| {
+                let all_matching = self.iter().all(|(n, (i, j))| {
                     n.abs_diff_eq(
                         other
                             .get(i.to_usize().unwrap(), j.to_usize().unwrap())
                             .unwrap_or(&N::zero()),
                         epsilon.clone(),
                     )
-                }) {
+                });
+                if !all_matching {
                     return false;
                 }
 
@@ -3055,15 +3053,16 @@ mod approx_impls {
             } else {
                 // Checks if all elements in self has a matching element
                 // in other
-                if !self.iter().all(|(n, (i, j))| {
+                let all_matches = self.iter().all(|(n, (i, j))| {
                     n.ulps_eq(
                         other
                             .get(i.to_usize().unwrap(), j.to_usize().unwrap())
                             .unwrap_or(&N::zero()),
                         epsilon.clone(),
                         max_ulps,
                     )
-                }) {
+                });
+                if !all_matches {
                     return false;
                 }
 
@@ -3122,15 +3121,16 @@ mod approx_impls {
             } else {
                 // Checks if all elements in self has a matching element
                 // in other
-                if !self.iter().all(|(n, (i, j))| {
+                let all_matches = self.iter().all(|(n, (i, j))| {
                     n.relative_eq(
                         other
                             .get(i.to_usize().unwrap(), j.to_usize().unwrap())
                             .unwrap_or(&N::zero()),
                         epsilon.clone(),
                         max_relative.clone(),
                     )
-                }) {
+                });
+                if !all_matches {
                     return false;
                 }