Fix confusion matrix (#374)

* Add serialization for LogisticRegression

* Confusion matrix should use labels from predictions and ground truth

* Clippy fixes

* This is the correct test

* clippy lints

* fix ownership

* fix ownership

* cleanup lints

* Remove blank lines

* Dedup labels (review)

* Improve combined_labels API (review)

* Make confusion_matrix layout reproducible, use sensible default when boolean classes

---------

Co-authored-by: Lev Kokotov <[email protected]>
Co-authored-by: Montana Low <[email protected]>

Author: 3 people

algorithms/linfa-ftrl/Cargo.toml

@@ -24,7 +24,7 @@ version = "1.0"
 features = ["derive"]
 
 [dependencies]
-ndarray = { version = "0.15.4", features = ["serde"] }
+ndarray = { version = "0.15", features = ["serde"] }
 ndarray-rand = "0.14.0"
 argmin = { version = "0.9.0", default-features = false }
 argmin-math = { version = "0.3", features = ["ndarray_v0_15-nolinalg"] }

algorithms/linfa-logistic/Cargo.toml

@@ -30,7 +30,6 @@ argmin = { version = "0.9.0", default-features = false }
 argmin-math = { version = "0.3", features = ["ndarray_v0_15-nolinalg"] }
 thiserror = "1.0"
 
-
 linfa = { version = "0.7.1", path = "../.." }
 
 [dev-dependencies]

algorithms/linfa-trees/src/decision_trees/hyperparams.rs

@@ -50,7 +50,7 @@ pub enum SplitQuality {
 /// let tree = params.fit(&train).unwrap();
 /// // Predict on validation and check accuracy
 /// let val_accuracy = tree.predict(&val).confusion_matrix(&val).unwrap().accuracy();
-/// assert!(val_accuracy > 0.99);
+/// assert!(val_accuracy > 0.9);
 /// ```
 ///
 #[cfg_attr(

src/dataset/mod.rs

@@ -324,7 +324,28 @@ pub trait Labels {
     }
 
     fn labels(&self) -> Vec<Self::Elem> {
-        self.label_set().into_iter().flatten().collect()
+        self.label_set()
+            .into_iter()
+            .flatten()
+            .collect::<HashSet<_>>()
+            .into_iter()
+            .collect()
+    }
+
+    fn combined_labels<T>(&self, other: &T) -> Vec<Self::Elem>
+    where
+        T: Labels<Elem = <Self as Labels>::Elem>,
+    {
+        let mut combined = self.label_set();
+        combined.extend(other.label_set());
+
+        combined
+            .iter()
+            .flatten()
+            .collect::<HashSet<_>>()
+            .into_iter()
+            .cloned()
+            .collect()
     }
 }
 

src/metrics_classification.rs

@@ -2,7 +2,7 @@
 //!
 //! Scoring is essential for classification and regression tasks. This module implements
 //! common scoring functions like precision, accuracy, recall, f1-score, ROC and ROC
-//! Aread-Under-Curve.
+//! Area-Under-Curve.
 use std::collections::HashMap;
 use std::fmt;
 
@@ -290,7 +290,23 @@ where
             return Err(Error::MismatchedShapes(targets.len(), ground_truth.len()));
         }
 
-        let classes = self.labels();
+        let mut classes = self.combined_labels(ground_truth);
+        // Sort classes to get reproducible confusion_matrix
+        classes.sort();
+        if classes.len() == 2 {
+            // In case of binary classes, we sort in reverse order to get a sensible default for
+            // boolean values and get a confusion matrix with the conventional layout by default:
+            //
+            //              | actual true  | actual false
+            //  pred true   |     TP       |      FP
+            //  -----------------------------------------
+            //  pred false  |     FN       |      TN
+            //
+            // So to get classes to be [true, false], as false < true or 0 < 1, we reverse the order.
+            // As precision and recall metrics are computed wrt the first label,
+            // it is less confusing if it corresponds to true.
+            classes.reverse();
+        }
 
         let indices = map_prediction_to_idx(
             targets.as_slice().unwrap(),
@@ -595,10 +611,11 @@ mod tests {
 
         let cm = predicted.confusion_matrix(ground_truth).unwrap();
 
-        let labels = array![0, 1];
-        let expected = array![[2., 1.], [0., 3.]];
+        let expected_labels = array![1, 0];
+        let expected = array![[3., 0.], [1., 2.]];
 
-        assert_cm_eq(&cm, &expected, &labels);
+        assert_eq!(expected_labels, cm.members);
+        assert_abs_diff_eq!(expected, cm.matrix);
     }
 
     #[test]
@@ -636,6 +653,17 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_division_by_zero_cm() {
+        let ground_truth = Array1::from(vec![1, 1, 0, 1, 0, 1]);
+        let predicted = Array1::from(vec![0, 0, 0, 0, 0, 0]);
+
+        let x = ground_truth.confusion_matrix(predicted).unwrap();
+        let f1 = x.f1_score();
+
+        assert!(f1.is_nan());
+    }
+
     #[test]
     fn test_roc_curve() {
         let predicted = ArrayView1::from(&[0.1, 0.3, 0.5, 0.7, 0.8, 0.9]).mapv(Pr::new);