Chore CI

.github/workflows/msrv.yml

@@ -0,0 +1,74 @@
+# Based on https://github.com/actions-rs/meta/blob/master/recipes/msrv.md
+name: MSRV
+
+on:
+  push:
+    # trigger on default branch but only when source code was changed
+    branches: [ master ]
+    paths-ignore:
+      - "COPYRIGHT"
+      - "LICENSE-*"
+      - "**.md"
+      - "**.txt"
+  pull_request:
+    branches: [ master ]
+    paths-ignore:
+      - "COPYRIGHT"
+      - "LICENSE-*"
+      - "**.md"
+      - "**.txt"
+
+jobs:
+  rustfmt:
+    name: Code format and lints
+    runs-on: ubuntu-latest
+    # nightly build may break clippy so change with caution
+    strategy:
+      matrix:
+        rust:
+          - stable
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions-rs/toolchain@v1
+        with:
+          profile: minimal
+          toolchain: ${{ matrix.rust }}
+          override: true
+          components: rustfmt, clippy
+      - uses: actions-rs/cargo@v1
+        with:
+          command: fmt
+          args: --all -- --check
+      - uses: actions-rs/cargo@v1
+        with:
+          command: clippy
+          args: -- -D warnings
+
+  test:
+    name: Test Suite
+    runs-on: ubuntu-latest
+    # convenient if other versions of rust were to be tested in the future
+    # just stable for now
+    strategy:
+      matrix:
+        rust:
+          - stable
+    steps:
+      - name: Checkout sources
+        uses: actions/checkout@v2
+
+      - name: Install toolchain
+        uses: actions-rs/toolchain@v1
+        with:
+          toolchain: ${{ matrix.rust }}
+          override: true
+
+      - name: Run cargo build
+        uses: actions-rs/cargo@v1
+        with:
+          command: build
+
+      - name: Run cargo test
+        uses: actions-rs/cargo@v1
+        with:
+          command: test

Cargo.toml

@@ -19,3 +19,4 @@ ndarray = "0.13.0"
 rand = "0.7"
 rand_pcg = "0.2.1"
 env_logger = "0.8.1"
+assert_float_eq = "1.1.3"

examples/tsp.rs

@@ -65,7 +65,7 @@ impl Problem {
             }
 
             let mut keyword = line[0].clone();
-            if keyword.ends_with(":") {
+            if keyword.ends_with(':') {
                 keyword.pop();
             }
 
@@ -533,7 +533,7 @@ fn find_min_cut(size: usize, weights: &mut [f64]) -> (f64, Vec<bool>) {
     }
 
     assert!(best_cut_weight.is_finite());
-    return (best_cut_weight, best_cut);
+    (best_cut_weight, best_cut)
 }
 
 #[cfg(test)]

src/helpers.rs

@@ -1,6 +1,8 @@
 use sprs::{CsVecBase, CsVecView};
 use std::ops::Deref;
 
+pub const EPS: f64 = 1e-8;
+
 pub(crate) fn resized_view<IStorage, DStorage>(
     vec: &CsVecBase<IStorage, DStorage>,
     len: usize,
@@ -58,3 +60,19 @@ pub(crate) fn assert_matrix_eq(mat: &CsMat<f64>, reference: &[Vec<f64>]) {
         assert_eq!(to_dense(&row), reference[r], "matrices differ in row {}", r);
     }
 }
+
+#[cfg(test)]
+pub(crate) fn assert_slice_eq(left: &[f64], right: &[f64]) {
+    assert!(left.iter().zip(right).all(|(&l, &r)| float_eq(l, r)));
+}
+
+#[cfg(test)]
+pub(crate) fn float_eq(left: f64, right: f64) -> bool {
+    if (left, right) == (f64::NEG_INFINITY, f64::NEG_INFINITY)
+        || (left, right) == (f64::INFINITY, f64::INFINITY)
+    {
+        true
+    } else {
+        expect_float_absolute_eq!(left, right, EPS).is_ok()
+    }
+}

src/lib.rs

@@ -49,6 +49,11 @@ assert_eq!(solution[y], 3.0);
 #[macro_use]
 extern crate log;
 
+#[cfg(test)]
+#[macro_use]
+#[cfg(test)]
+extern crate assert_float_eq;
+
 mod helpers;
 mod lu;
 mod mps;
@@ -563,7 +568,7 @@ mod tests {
         }
 
         {
-            let mut sol = orig_sol.clone().fix_var(v2, 2.5).unwrap();
+            let mut sol = orig_sol.fix_var(v2, 2.5).unwrap();
             assert_eq!(sol[v1], 1.5);
             assert_eq!(sol[v2], 2.5);
             assert_eq!(sol.objective(), 6.5);
@@ -610,7 +615,6 @@ mod tests {
 
         {
             let sol = orig_sol
-                .clone()
                 .add_constraint(&[(v1, -1.0), (v2, 1.0)], ComparisonOp::Ge, 3.0)
                 .unwrap();
 

src/lu.rs

@@ -57,8 +57,8 @@ impl LUFactors {
         scratch.dense_rhs.resize(rhs.len(), 0.0);
 
         if let Some(row_perm) = &self.row_perm {
-            for i in 0..rhs.len() {
-                scratch.dense_rhs[row_perm.orig2new[i]] = rhs[i];
+            for (i, &each_right) in rhs.iter().enumerate() {
+                scratch.dense_rhs[row_perm.orig2new[i]] = each_right;
             }
         } else {
             scratch.dense_rhs.copy_from_slice(rhs);
@@ -72,7 +72,7 @@ impl LUFactors {
                 rhs[col_perm.new2orig[i]] = scratch.dense_rhs[i];
             }
         } else {
-            rhs.copy_from_slice(&mut scratch.dense_rhs);
+            rhs.copy_from_slice(&scratch.dense_rhs);
         }
     }
 
@@ -253,12 +253,10 @@ pub fn lu_factorize<'a>(
             }
 
             let new_r = orig2new_row[orig_r];
-            if new_r < i_col {
-                upper.push(new_r, val);
-            } else if new_r == i_col {
-                upper_diag.push(pivot_val);
-            } else {
-                lower.push(orig_r, val / pivot_val);
+            match new_r.cmp(&i_col) {
+                std::cmp::Ordering::Less => upper.push(new_r, val),
+                std::cmp::Ordering::Equal => upper_diag.push(pivot_val),
+                std::cmp::Ordering::Greater => lower.push(orig_r, val / pivot_val),
             }
         }
 
@@ -465,7 +463,7 @@ fn tri_solve_process_col(tri_mat: &TriangleMat, col: usize, rhs: &mut [f64]) {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::helpers::{assert_matrix_eq, to_dense, to_sparse};
+    use crate::helpers::{assert_matrix_eq, assert_slice_eq, to_dense, to_sparse};
     use sprs::{CsMat, CsVec, TriMat};
 
     fn mat_from_triplets(rows: usize, cols: usize, triplets: &[(usize, usize, f64)]) -> CsMat<f64> {
@@ -519,21 +517,21 @@ mod tests {
         ];
         let u_diag_ref = [4.0, 0.5, 1.0];
         assert_matrix_eq(&lu.upper.nondiag.to_csmat(), &u_nondiag_ref);
-        assert_eq!(lu.upper.diag.as_ref().unwrap(), &u_diag_ref);
+        assert_slice_eq(lu.upper.diag.as_ref().unwrap(), &u_diag_ref);
 
         assert_eq!(lu.row_perm.as_ref().unwrap().new2orig, &[2, 0, 1]);
         assert_eq!(lu.col_perm.as_ref().unwrap().new2orig, &[0, 1, 2]);
 
         {
             let mut rhs_dense = [6.0, 3.0, 13.0];
             lu.solve_dense(&mut rhs_dense, &mut scratch);
-            assert_eq!(&rhs_dense, &[1.0, 2.0, 3.0]);
+            assert_slice_eq(&rhs_dense, &[1.0, 2.0, 3.0]);
         }
 
         {
             let mut rhs_dense_t = [14.0, 11.0, 5.0];
             lu_transp.solve_dense(&mut rhs_dense_t, &mut scratch);
-            assert_eq!(&rhs_dense_t, &[1.0, 2.0, 3.0]);
+            assert_slice_eq(&rhs_dense_t, &[1.0, 2.0, 3.0]);
         }
 
         {

src/mps.rs

@@ -48,7 +48,7 @@ impl MpsFile {
         };
 
         let problem_name = {
-            lines.to_next()?;
+            lines.next_mut()?;
             let mut tokens = Tokens::new(&lines);
             if tokens.next()? != "NAME" {
                 return Err(lines.err("expected NAME section"));
@@ -68,14 +68,14 @@ impl MpsFile {
         let mut constraints = vec![];
         let mut constr_name2idx = HashMap::new();
         {
-            lines.to_next()?;
+            lines.next_mut()?;
             if lines.cur != "ROWS" {
                 return Err(lines.err("expected ROWS section"));
             }
 
             loop {
-                lines.to_next()?;
-                if !lines.cur.starts_with(" ") {
+                lines.next_mut()?;
+                if !lines.cur.starts_with(' ') {
                     break;
                 }
 
@@ -137,8 +137,8 @@ impl MpsFile {
             let mut cur_name = String::new();
             let mut cur_def = VariableDef::default();
             loop {
-                lines.to_next()?;
-                if !lines.cur.starts_with(" ") {
+                lines.next_mut()?;
+                if !lines.cur.starts_with(' ') {
                     break;
                 }
 
@@ -182,8 +182,8 @@ impl MpsFile {
 
             let mut cur_vec_name = None;
             loop {
-                lines.to_next()?;
-                if !lines.cur.starts_with(" ") {
+                lines.next_mut()?;
+                if !lines.cur.starts_with(' ') {
                     break;
                 }
 
@@ -212,8 +212,8 @@ impl MpsFile {
         if lines.cur == "RANGES" {
             let mut cur_vec_name = None;
             loop {
-                lines.to_next()?;
-                if !lines.cur.starts_with(" ") {
+                lines.next_mut()?;
+                if !lines.cur.starts_with(' ') {
                     break;
                 }
 
@@ -240,8 +240,8 @@ impl MpsFile {
         if lines.cur == "BOUNDS" {
             let mut cur_vec_name = None;
             loop {
-                lines.to_next()?;
-                if !lines.cur.starts_with(" ") {
+                lines.next_mut()?;
+                if !lines.cur.starts_with(' ') {
                     break;
                 }
 
@@ -279,7 +279,9 @@ impl MpsFile {
                             var_def.max = Some(val);
                         }
                         _ => {
-                            return Err(lines.err(&format!("bound type {} is not supported", bound_type)));
+                            return Err(
+                                lines.err(&format!("bound type {} is not supported", bound_type))
+                            );
                         }
                     }
                 }
@@ -335,7 +337,7 @@ struct Lines<R: io::BufRead> {
 }
 
 impl<R: io::BufRead> Lines<R> {
-    fn to_next(&mut self) -> io::Result<()> {
+    fn next_mut(&mut self) -> io::Result<()> {
         loop {
             self.idx += 1;
             self.cur.clear();
@@ -344,7 +346,7 @@ impl<R: io::BufRead> Lines<R> {
                 return Ok(());
             }
 
-            if self.cur.starts_with("*") {
+            if self.cur.starts_with('*') {
                 continue;
             }
 
@@ -388,14 +390,14 @@ impl<'a> Tokens<'a> {
 }
 
 fn parse_f64(input: &str, line_idx: usize) -> io::Result<f64> {
-    input.parse().or_else(|_| {
-        Err(io::Error::new(
+    input.parse().map_err(|_| {
+        io::Error::new(
             io::ErrorKind::InvalidData,
             format!(
                 "line {}: couldn't parse float from string: `{}`",
                 line_idx, input
             ),
-        ))
+        )
     })
 }
 

src/ordering.rs

@@ -103,10 +103,10 @@ pub fn order_colamd<'a>(
             rows[i].end += prev_end;
         }
 
-        for c in 0..size {
-            for &r in cols[c].elems(&row_storage) {
+        for (i, col) in cols.iter().enumerate().take(size) {
+            for &r in col.elems(&row_storage) {
                 let row = &mut rows[r];
-                col_storage[row.begin] = c;
+                col_storage[row.begin] = i;
                 row.begin += 1;
             }
         }