From 3071fbab60bca140f29c8f51120a5e032a5f7df8 Mon Sep 17 00:00:00 2001 From: "Lorenzo (Mec-iS)" Date: Tue, 21 Jul 2026 10:53:01 +0100 Subject: [PATCH 1/3] refactor(metrics): share per-class confusion counts across Precision/Recall/F1 Addresses the architectural review feedback from #382: the multiclass F1 path re-implemented per-class precision/recall bookkeeping using raw HashMaps, duplicating logic that already lived (in slightly different form) inside Precision and Recall. Introduce a pub(crate) `ConfusionCounts` helper (new src/metrics/confusion.rs) that computes the per-class tp / predicted / support / class set in a single pass. Precision and Recall each expose a crate-private `per_class_scores_from_counts` method deriving their per-class scores from those shared counts, and their `get_score` implementations are unified onto the same single-pass counts (the binary and multiclass paths now share one code path that either picks the positive class or macro-averages). F1's multiclass path builds the counts once and consumes the per-class precision/recall scores from Precision and Recall, eliminating the duplicated bookkeeping while keeping a single source of truth. Binary behaviour is preserved exactly (verified against all existing tests, including the {0,1} positive-class convention): for {0,1} labels the per-class positive-class precision/recall match the previous direct-computation formulas term-for-term. Also: add an `n == 0` early return to Precision/Recall (matching F1 post-#383) so the multiclass path can assume `classes >= 1`, and drop the now-dead `classes == 0` / `support.is_empty()` branches. README: bump the install snippet from ^0.4.3 to ^0.5.2 (current Cargo.toml version) and note the metrics refactoring in the roadmap. No behaviour change; all 432 lib tests + 67 doctests + 40 metrics tests pass under `cargo test --all-features`. `cargo fmt --check` and `cargo clippy --all-features -Dwarnings` are clean. --- README.md | 6 ++- src/metrics/confusion.rs | 79 +++++++++++++++++++++++++++++++++ src/metrics/f1.rs | 72 +++++++++++------------------- src/metrics/mod.rs | 2 + src/metrics/precision.rs | 95 ++++++++++++++++++---------------------- src/metrics/recall.rs | 89 ++++++++++++++++++------------------- 6 files changed, 196 insertions(+), 147 deletions(-) create mode 100644 src/metrics/confusion.rs diff --git a/README.md b/README.md index f370abdb..127b41e5 100644 --- a/README.md +++ b/README.md @@ -37,7 +37,7 @@ Add to Cargo.toml: ```toml [dependencies] -smartcore = "^0.4.3" +smartcore = "^0.5.2" ``` For the latest development branch: @@ -124,6 +124,10 @@ A curated set of Jupyter notebooks is available via the companion repository to - Tree and forest components refactored for reuse; Extra Trees added. - SVM multiclass support; SVR kernel enum and related improvements. - XGBoost-style regression introduced; single-linkage clustering implemented. +- Classification metrics hardened: multiclass macro F1 now averages per-class + F-measures (matching sklearn), and Precision/Recall/F1 share a single + per-class confusion-counts helper so the per-class bookkeeping lives in one + place. See CHANGELOG.md for precise details, deprecations, and breaking changes. Some features like nalgebra-bindings have been dropped in favor of ndarray-only paths. Default features are tuned for WASM/WASI builds; enable serde/datasets as needed. diff --git a/src/metrics/confusion.rs b/src/metrics/confusion.rs new file mode 100644 index 00000000..664d56f0 --- /dev/null +++ b/src/metrics/confusion.rs @@ -0,0 +1,79 @@ +//! Shared per-class confusion-count helpers for classification metrics. +//! +//! [`ConfusionCounts`] computes, in a single pass over `(y_true, y_pred)`, +//! the per-class true-positive, predicted, and support counts used by +//! [`Precision`](crate::metrics::precision::Precision), +//! [`Recall`](crate::metrics::recall::Recall), and +//! [`F1`](crate::metrics::f1::F1). +//! +//! Labels are keyed by their `f64` bit pattern; note that `-0.0` and `+0.0` +//! have distinct bit patterns and would be counted as separate classes. This +//! convention is shared across the classification metrics. + +use std::collections::{HashMap, HashSet}; + +use crate::linalg::basic::arrays::ArrayView1; +use crate::numbers::realnum::RealNumber; + +/// Per-class confusion counts for a classification result. +/// +/// Built in a single pass over `(y_true, y_pred)`. Exposes per-class +/// true-positive, predicted, and support counts so that `Precision`, +/// `Recall`, and `F1` can derive their per-class scores from a single +/// source of truth instead of each re-implementing the bookkeeping. +pub(crate) struct ConfusionCounts { + classes_set: HashSet, + predicted: HashMap, + support: HashMap, + tp_map: HashMap, +} + +impl ConfusionCounts { + /// Compute per-class confusion counts in a single pass. + pub(crate) fn from( + y_true: &dyn ArrayView1, + y_pred: &dyn ArrayView1, + ) -> Self { + let n = y_true.shape(); + let mut classes_set: HashSet = HashSet::new(); + let mut predicted: HashMap = HashMap::new(); + let mut support: HashMap = HashMap::new(); + let mut tp_map: HashMap = HashMap::new(); + for i in 0..n { + let t_bits = y_true.get(i).to_f64_bits(); + classes_set.insert(t_bits); + *support.entry(t_bits).or_insert(0) += 1; + *predicted.entry(y_pred.get(i).to_f64_bits()).or_insert(0) += 1; + if *y_true.get(i) == *y_pred.get(i) { + *tp_map.entry(t_bits).or_insert(0) += 1; + } + } + Self { + classes_set, + predicted, + support, + tp_map, + } + } + + /// The set of label bit patterns observed in `y_true`. + pub(crate) fn classes_set(&self) -> &HashSet { + &self.classes_set + } + + /// Number of predictions equal to the label with the given bit pattern. + pub(crate) fn predicted(&self, bits: u64) -> usize { + *self.predicted.get(&bits).unwrap_or(&0) + } + + /// Number of `y_true` entries equal to the label with the given bit + /// pattern (the class support). + pub(crate) fn support(&self, bits: u64) -> usize { + *self.support.get(&bits).unwrap_or(&0) + } + + /// Number of true positives for the label with the given bit pattern. + pub(crate) fn tp(&self, bits: u64) -> usize { + *self.tp_map.get(&bits).unwrap_or(&0) + } +} diff --git a/src/metrics/f1.rs b/src/metrics/f1.rs index 4f877fd2..fdfaaafa 100644 --- a/src/metrics/f1.rs +++ b/src/metrics/f1.rs @@ -23,13 +23,13 @@ //! //! //! -use std::collections::{HashMap, HashSet}; use std::marker::PhantomData; #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; use crate::linalg::basic::arrays::ArrayView1; +use crate::metrics::confusion::ConfusionCounts; use crate::metrics::precision::Precision; use crate::metrics::recall::Recall; use crate::numbers::basenum::Number; @@ -81,63 +81,43 @@ impl Metrics for F1 { } let beta2 = self.beta * self.beta; - // Single pass over y_true/y_pred: collect the class set (needed to pick - // the binary vs. multiclass path) and the per-class tp / support / - // predicted counts used by the multiclass path. Labels are keyed by - // their f64 bit pattern; note that -0.0 and +0.0 have distinct bit - // patterns and would be counted as separate classes — an existing - // convention shared with Precision and Recall. - let mut classes_set: HashSet = HashSet::new(); - let mut predicted: HashMap = HashMap::new(); - let mut support: HashMap = HashMap::new(); - let mut tp_map: HashMap = HashMap::new(); - for i in 0..n { - let t_bits = y_true.get(i).to_f64_bits(); - classes_set.insert(t_bits); - *support.entry(t_bits).or_insert(0) += 1; - *predicted.entry(y_pred.get(i).to_f64_bits()).or_insert(0) += 1; - if *y_true.get(i) == *y_pred.get(i) { - *tp_map.entry(t_bits).or_insert(0) += 1; - } - } - let classes = classes_set.len(); + // Build the per-class confusion counts once and delegate per-class + // precision/recall to `Precision` and `Recall`, so this metric no + // longer re-implements the tp/predicted/support bookkeeping. Labels + // are keyed by their f64 bit pattern; note that -0.0 and +0.0 have + // distinct bit patterns and would be counted as separate classes — + // an existing convention shared with Precision and Recall. + let counts = ConfusionCounts::from(y_true, y_pred); + let classes = counts.classes_set().len(); if classes == 2 { - // Binary case: F-measure of the positive class. The positive class - // is assumed to be the label with the higher bit representation - // (i.e. 1.0 when labels are 0.0/1.0) — the convention baked into - // Precision and Recall, which already return the positive-class - // scores. + // Binary case: F-measure of the positive class. The positive + // class is assumed to be T::one() (i.e. 1.0 when labels are + // 0.0/1.0) — the convention baked into Precision and Recall, + // which already return the positive-class scores. let p = Precision::new().get_score(y_true, y_pred); let r = Recall::new().get_score(y_true, y_pred); (1f64 + beta2) * (p * r) / ((beta2 * p) + r) } else { - // Multiclass case (including classes == 1, where the macro F-beta - // is just the single class's F-beta): macro F-measure is the mean - // of the per-class F-measures, not the F-measure of the - // macro-averaged precision and recall. + // Multiclass case (including classes == 1, where the macro + // F-beta is just the single class's F-beta): macro F-measure is + // the mean of the per-class F-measures, not the F-measure of the + // macro-averaged precision and recall. Per-class precision and + // recall are sourced from `Precision` and `Recall` to keep a + // single source of truth for the per-class scores. + let p_scores = Precision::::new().per_class_scores_from_counts(&counts); + let r_scores = Recall::::new().per_class_scores_from_counts(&counts); let mut fbeta_sum = 0.0; - for &bits in &classes_set { - let tp = *tp_map.get(&bits).unwrap_or(&0); - let pred_count = *predicted.get(&bits).unwrap_or(&0); - let support_count = *support.get(&bits).unwrap_or(&0); - let p_c = if pred_count > 0 { - tp as f64 / pred_count as f64 - } else { - 0.0 - }; - let r_c = if support_count > 0 { - tp as f64 / support_count as f64 - } else { - 0.0 - }; + for &bits in counts.classes_set() { + let p_c = *p_scores.get(&bits).unwrap_or(&0.0); + let r_c = *r_scores.get(&bits).unwrap_or(&0.0); let denom = beta2 * p_c + r_c; if denom > 0.0 { fbeta_sum += (1f64 + beta2) * p_c * r_c / denom; } } - // classes >= 1 is guaranteed here: n > 0 (early return above) means - // classes_set is non-empty. + // classes >= 1 is guaranteed here: n > 0 (early return above) + // means classes_set is non-empty. fbeta_sum / classes as f64 } } diff --git a/src/metrics/mod.rs b/src/metrics/mod.rs index a7184293..12a32357 100644 --- a/src/metrics/mod.rs +++ b/src/metrics/mod.rs @@ -59,6 +59,8 @@ pub mod auc; /// Compute the homogeneity, completeness and V-Measure scores. pub mod cluster_hcv; pub(crate) mod cluster_helpers; +/// Per-class confusion-count helpers shared by classification metrics. +pub(crate) mod confusion; /// Multitude of distance metrics are defined here pub mod distance; /// F1 score, also known as balanced F-score or F-measure. diff --git a/src/metrics/precision.rs b/src/metrics/precision.rs index 84444b6b..fee26bff 100644 --- a/src/metrics/precision.rs +++ b/src/metrics/precision.rs @@ -22,13 +22,14 @@ //! //! -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; use std::marker::PhantomData; #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; use crate::linalg::basic::arrays::ArrayView1; +use crate::metrics::confusion::ConfusionCounts; use crate::numbers::realnum::RealNumber; use crate::metrics::Metrics; @@ -40,6 +41,30 @@ pub struct Precision { _phantom: PhantomData, } +impl Precision { + /// Per-class precision scores derived from shared confusion counts. + /// + /// Returns a map from label bit pattern to that class's precision + /// (`tp / predicted`, or `0.0` when the class is never predicted). + pub(crate) fn per_class_scores_from_counts( + &self, + counts: &ConfusionCounts, + ) -> HashMap { + let mut scores: HashMap = HashMap::new(); + for &bits in counts.classes_set() { + let pred_count = counts.predicted(bits); + let tp = counts.tp(bits); + let prec = if pred_count > 0 { + tp as f64 / pred_count as f64 + } else { + 0.0 + }; + scores.insert(bits, prec); + } + scores + } +} + impl Metrics for Precision { /// create a typed object to call Precision functions fn new() -> Self { @@ -63,63 +88,27 @@ impl Metrics for Precision { y_pred.shape() ); } - let n = y_true.shape(); - - let mut classes_set: HashSet = HashSet::new(); - for i in 0..n { - classes_set.insert(y_true.get(i).to_f64_bits()); + // Empty input has no classes; return 0.0 (the multiclass path below + // relies on classes >= 1 to divide by `classes`). + if n == 0 { + return 0.0; } - let classes: usize = classes_set.len(); + + let counts = ConfusionCounts::from(y_true, y_pred); + let classes = counts.classes_set().len(); + let scores = self.per_class_scores_from_counts(&counts); if classes == 2 { - // Binary case: precision for positive class (assumed T::one()) - let positive = T::one(); - let mut tp: usize = 0; - let mut fp_count: usize = 0; - for i in 0..n { - let t = *y_true.get(i); - let p = *y_pred.get(i); - if p == t { - if t == positive { - tp += 1; - } - } else if t != positive { - fp_count += 1; - } - } - if tp + fp_count == 0 { - 0.0 - } else { - tp as f64 / (tp + fp_count) as f64 - } + // Binary case: precision for the positive class, assumed to be + // T::one() (i.e. 1.0 when labels are 0.0/1.0). If the positive + // label is not present in y_true the score is 0.0. + let positive_bits = T::one().to_f64_bits(); + *scores.get(&positive_bits).unwrap_or(&0.0) } else { - // Multiclass case: macro-averaged precision - let mut predicted: HashMap = HashMap::new(); - let mut tp_map: HashMap = HashMap::new(); - for i in 0..n { - let p_bits = y_pred.get(i).to_f64_bits(); - *predicted.entry(p_bits).or_insert(0) += 1; - if *y_true.get(i) == *y_pred.get(i) { - *tp_map.entry(p_bits).or_insert(0) += 1; - } - } - let mut precision_sum = 0.0; - for &bits in &classes_set { - let pred_count = *predicted.get(&bits).unwrap_or(&0); - let tp = *tp_map.get(&bits).unwrap_or(&0); - let prec = if pred_count > 0 { - tp as f64 / pred_count as f64 - } else { - 0.0 - }; - precision_sum += prec; - } - if classes == 0 { - 0.0 - } else { - precision_sum / classes as f64 - } + // Multiclass case: macro-averaged precision. classes >= 1 is + // guaranteed here because of the `n == 0` guard above. + scores.values().sum::() / classes as f64 } } } diff --git a/src/metrics/recall.rs b/src/metrics/recall.rs index e7418511..d2580843 100644 --- a/src/metrics/recall.rs +++ b/src/metrics/recall.rs @@ -22,13 +22,14 @@ //! //! -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; use std::marker::PhantomData; #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; use crate::linalg::basic::arrays::ArrayView1; +use crate::metrics::confusion::ConfusionCounts; use crate::numbers::realnum::RealNumber; use crate::metrics::Metrics; @@ -40,6 +41,30 @@ pub struct Recall { _phantom: PhantomData, } +impl Recall { + /// Per-class recall scores derived from shared confusion counts. + /// + /// Returns a map from label bit pattern to that class's recall + /// (`tp / support`, or `0.0` when the class has no support). + pub(crate) fn per_class_scores_from_counts( + &self, + counts: &ConfusionCounts, + ) -> HashMap { + let mut scores: HashMap = HashMap::new(); + for &bits in counts.classes_set() { + let support_count = counts.support(bits); + let tp = counts.tp(bits); + let rec = if support_count > 0 { + tp as f64 / support_count as f64 + } else { + 0.0 + }; + scores.insert(bits, rec); + } + scores + } +} + impl Metrics for Recall { /// create a typed object to call Recall functions fn new() -> Self { @@ -63,57 +88,27 @@ impl Metrics for Recall { y_pred.shape() ); } - let n = y_true.shape(); - - let mut classes_set = HashSet::new(); - for i in 0..n { - classes_set.insert(y_true.get(i).to_f64_bits()); + // Empty input has no classes; return 0.0 (the multiclass path below + // relies on classes >= 1 to divide by `classes`). + if n == 0 { + return 0.0; } - let classes: usize = classes_set.len(); + + let counts = ConfusionCounts::from(y_true, y_pred); + let classes = counts.classes_set().len(); + let scores = self.per_class_scores_from_counts(&counts); if classes == 2 { - // Binary case: recall for positive class (assumed T::one()) - let positive = T::one(); - let mut tp: usize = 0; - let mut fn_count: usize = 0; - for i in 0..n { - let t = *y_true.get(i); - let p = *y_pred.get(i); - if p == t { - if t == positive { - tp += 1; - } - } else if t == positive { - fn_count += 1; - } - } - if tp + fn_count == 0 { - 0.0 - } else { - tp as f64 / (tp + fn_count) as f64 - } + // Binary case: recall for the positive class, assumed to be + // T::one() (i.e. 1.0 when labels are 0.0/1.0). If the positive + // label is not present in y_true the score is 0.0. + let positive_bits = T::one().to_f64_bits(); + *scores.get(&positive_bits).unwrap_or(&0.0) } else { - // Multiclass case: macro-averaged recall - let mut support: HashMap = HashMap::new(); - let mut tp_map: HashMap = HashMap::new(); - for i in 0..n { - let t_bits = y_true.get(i).to_f64_bits(); - *support.entry(t_bits).or_insert(0) += 1; - if *y_true.get(i) == *y_pred.get(i) { - *tp_map.entry(t_bits).or_insert(0) += 1; - } - } - let mut recall_sum = 0.0; - for (&bits, &sup) in &support { - let tp = *tp_map.get(&bits).unwrap_or(&0); - recall_sum += tp as f64 / sup as f64; - } - if support.is_empty() { - 0.0 - } else { - recall_sum / support.len() as f64 - } + // Multiclass case: macro-averaged recall. classes >= 1 is + // guaranteed here because of the `n == 0` guard above. + scores.values().sum::() / classes as f64 } } } From 0b400abae3e0ec5f776c964cc4b25125314df754 Mon Sep 17 00:00:00 2001 From: "Lorenzo (Mec-iS)" Date: Tue, 21 Jul 2026 10:55:37 +0100 Subject: [PATCH 2/3] chore: bump to v0.5.3 - Cargo.toml: 0.5.2 -> 0.5.3 - README install snippet: ^0.5.2 -> ^0.5.3 - CHANGELOG: add [0.5.3] entry covering the metrics refactoring --- CHANGELOG.md | 6 ++++++ Cargo.toml | 2 +- README.md | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 29a5ed04..e952b830 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.5.3] +### Changed +- Classification metrics refactored: `Precision`, `Recall`, and `F1` now derive per-class scores from a single shared `ConfusionCounts` helper (`src/metrics/confusion.rs`) instead of each re-implementing the per-class tp/predicted/support bookkeeping. `Precision` and `Recall` expose a crate-private `per_class_scores_from_counts` used by `F1`'s multiclass path. +- `Precision` and `Recall` now early-return `0.0` on empty input and drop the unreachable `classes == 0` / `support.is_empty()` branches. +- Multiclass macro `F1` (landed in #382, cleaned up in #383) is unchanged behaviourally; it now consumes `Precision`/`Recall::per_class_scores_from_counts` instead of its own `HashMap` bookkeeping. + ## [0.4.8] - 2025-11-29 - WARNING: Breaking changes! - `LassoParameters` and `LassoSearchParameters` have a new field `fit_intercept`. When it is set to false, the `beta_0` term in the formula will be forced to zero, and `intercept` field in `Lasso` will be set to `None`. diff --git a/Cargo.toml b/Cargo.toml index 012e3bc6..78f45d3d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ name = "smartcore" description = "Machine Learning in Rust." homepage = "https://smartcorelib.org" -version = "0.5.2" +version = "0.5.3" authors = ["smartcore Developers"] edition = "2021" license = "Apache-2.0" diff --git a/README.md b/README.md index 127b41e5..862ef813 100644 --- a/README.md +++ b/README.md @@ -37,7 +37,7 @@ Add to Cargo.toml: ```toml [dependencies] -smartcore = "^0.5.2" +smartcore = "^0.5.3" ``` For the latest development branch: From e5bddb25b67edb1af14bea933aadbc83243fdd17 Mon Sep 17 00:00:00 2001 From: "Lorenzo (Mec-iS)" Date: Tue, 21 Jul 2026 11:07:33 +0100 Subject: [PATCH 3/3] refactor(metrics): address #384 review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rename ConfusionCounts::from -> new to avoid shadowing the std::convert::From trait convention (review blocker). - Add direct unit tests for ConfusionCounts (binary basic, multiclass, spurious predicted label, empty input, perfect predictions) covering tp/predicted/support/classes_set accessors — the helper was previously only tested indirectly through Precision/Recall/F1. - Add precision_binary_spurious_predicted_label test documenting the binary edge case where y_pred contains a label not in y_true: the new path uses predicted(positive) as the denominator (matching sklearn), so a spurious predicted label does not inflate it. The pre-refactor binary path would have counted it as a false positive (2/3 instead of 1.0). - Document that per_class_scores_from_counts iterates only over classes_set (y_true labels), so labels appearing only in y_pred are silently ignored — matching sklearn's behaviour. - Add order-independence comments for the HashMap::values().sum() calls in the Precision/Recall multiclass paths. - Clarify the binary Precision comment to note that the denominator is predicted(positive), not tp + fp_count, so spurious predicted labels do not affect the score. All 438 lib tests + 67 doctests + 46 metrics tests pass; fmt and clippy --all-features -Dwarnings clean. --- src/metrics/confusion.rs | 109 ++++++++++++++++++++++++++++++++++++++- src/metrics/f1.rs | 2 +- src/metrics/precision.rs | 44 ++++++++++++++-- src/metrics/recall.rs | 13 ++++- 4 files changed, 159 insertions(+), 9 deletions(-) diff --git a/src/metrics/confusion.rs b/src/metrics/confusion.rs index 664d56f0..cd0fb977 100644 --- a/src/metrics/confusion.rs +++ b/src/metrics/confusion.rs @@ -29,8 +29,14 @@ pub(crate) struct ConfusionCounts { } impl ConfusionCounts { - /// Compute per-class confusion counts in a single pass. - pub(crate) fn from( + /// Compute per-class confusion counts in a single pass over + /// `(y_true, y_pred)`. + /// + /// Named `new` rather than `from` to avoid shadowing the conventional + /// `std::convert::From` trait method (the two-argument signature does + /// not collide with `From::from`'s single-argument form, but the + /// shadowing is still confusing for readers). + pub(crate) fn new( y_true: &dyn ArrayView1, y_pred: &dyn ArrayView1, ) -> Self { @@ -77,3 +83,102 @@ impl ConfusionCounts { *self.tp_map.get(&bits).unwrap_or(&0) } } + +#[cfg(test)] +mod tests { + use super::*; + + fn bits_of(v: f64) -> u64 { + v.to_f64_bits() + } + + #[test] + fn confusion_counts_binary_basic() { + // y_true = [0, 1, 1, 0], y_pred = [0, 0, 1, 1] + let y_true: Vec = vec![0., 1., 1., 0.]; + let y_pred: Vec = vec![0., 0., 1., 1.]; + let counts = ConfusionCounts::new(&y_true, &y_pred); + + assert_eq!(counts.classes_set().len(), 2); + assert!(counts.classes_set().contains(&bits_of(0.0))); + assert!(counts.classes_set().contains(&bits_of(1.0))); + + // Class 0: support=2, predicted=2 (y_pred has two 0s), tp=1 + assert_eq!(counts.support(bits_of(0.0)), 2); + assert_eq!(counts.predicted(bits_of(0.0)), 2); + assert_eq!(counts.tp(bits_of(0.0)), 1); + + // Class 1: support=2, predicted=2 (y_pred has two 1s), tp=1 + assert_eq!(counts.support(bits_of(1.0)), 2); + assert_eq!(counts.predicted(bits_of(1.0)), 2); + assert_eq!(counts.tp(bits_of(1.0)), 1); + } + + #[test] + fn confusion_counts_multiclass() { + // y_true = [0, 0, 1, 2, 2, 2], y_pred = [0, 1, 1, 2, 0, 2] + let y_true: Vec = vec![0., 0., 1., 2., 2., 2.]; + let y_pred: Vec = vec![0., 1., 1., 2., 0., 2.]; + let counts = ConfusionCounts::new(&y_true, &y_pred); + + assert_eq!(counts.classes_set().len(), 3); + + // Class 0: support=2, predicted=2, tp=1 + assert_eq!(counts.support(bits_of(0.0)), 2); + assert_eq!(counts.predicted(bits_of(0.0)), 2); + assert_eq!(counts.tp(bits_of(0.0)), 1); + + // Class 1: support=1, predicted=2, tp=1 + assert_eq!(counts.support(bits_of(1.0)), 1); + assert_eq!(counts.predicted(bits_of(1.0)), 2); + assert_eq!(counts.tp(bits_of(1.0)), 1); + + // Class 2: support=3, predicted=2, tp=2 + assert_eq!(counts.support(bits_of(2.0)), 3); + assert_eq!(counts.predicted(bits_of(2.0)), 2); + assert_eq!(counts.tp(bits_of(2.0)), 2); + } + + #[test] + fn confusion_counts_spurious_predicted_label() { + // y_pred contains label 2 which never appears in y_true. The + // `predicted` map records it, but `classes_set` (sourced from + // y_true) does not include it, so it is silently ignored by + // per-class metrics that iterate `classes_set`. + let y_true: Vec = vec![0., 0., 1., 1.]; + let y_pred: Vec = vec![0., 2., 1., 1.]; + let counts = ConfusionCounts::new(&y_true, &y_pred); + + assert_eq!(counts.classes_set().len(), 2); + assert!(!counts.classes_set().contains(&bits_of(2.0))); + + // The spurious label is tracked in `predicted`... + assert_eq!(counts.predicted(bits_of(2.0)), 1); + // ...but has no support or tp entry. + assert_eq!(counts.support(bits_of(2.0)), 0); + assert_eq!(counts.tp(bits_of(2.0)), 0); + } + + #[test] + fn confusion_counts_empty_input() { + let y_true: Vec = vec![]; + let y_pred: Vec = vec![]; + let counts = ConfusionCounts::new(&y_true, &y_pred); + + assert!(counts.classes_set().is_empty()); + assert_eq!(counts.predicted(bits_of(0.0)), 0); + assert_eq!(counts.support(bits_of(0.0)), 0); + assert_eq!(counts.tp(bits_of(0.0)), 0); + } + + #[test] + fn confusion_counts_perfect_predictions() { + let y_true: Vec = vec![0., 1., 2., 0., 1.]; + let counts = ConfusionCounts::new(&y_true, &y_true); + + for &bits in counts.classes_set() { + assert_eq!(counts.tp(bits), counts.support(bits)); + assert_eq!(counts.tp(bits), counts.predicted(bits)); + } + } +} diff --git a/src/metrics/f1.rs b/src/metrics/f1.rs index fdfaaafa..63bc3ecd 100644 --- a/src/metrics/f1.rs +++ b/src/metrics/f1.rs @@ -87,7 +87,7 @@ impl Metrics for F1 { // are keyed by their f64 bit pattern; note that -0.0 and +0.0 have // distinct bit patterns and would be counted as separate classes — // an existing convention shared with Precision and Recall. - let counts = ConfusionCounts::from(y_true, y_pred); + let counts = ConfusionCounts::new(y_true, y_pred); let classes = counts.classes_set().len(); if classes == 2 { diff --git a/src/metrics/precision.rs b/src/metrics/precision.rs index fee26bff..d3ed9d3c 100644 --- a/src/metrics/precision.rs +++ b/src/metrics/precision.rs @@ -46,6 +46,13 @@ impl Precision { /// /// Returns a map from label bit pattern to that class's precision /// (`tp / predicted`, or `0.0` when the class is never predicted). + /// + /// Iterates only over `counts.classes_set()` (labels seen in `y_true`). + /// A label that appears in `y_pred` but never in `y_true` contributes to + /// the `predicted` counts in `ConfusionCounts` but is silently ignored + /// here — it does not inflate or deflate any class's precision. This + /// matches sklearn's behaviour, where the label set is derived from + /// `y_true`. pub(crate) fn per_class_scores_from_counts( &self, counts: &ConfusionCounts, @@ -95,19 +102,25 @@ impl Metrics for Precision { return 0.0; } - let counts = ConfusionCounts::from(y_true, y_pred); + let counts = ConfusionCounts::new(y_true, y_pred); let classes = counts.classes_set().len(); let scores = self.per_class_scores_from_counts(&counts); if classes == 2 { // Binary case: precision for the positive class, assumed to be - // T::one() (i.e. 1.0 when labels are 0.0/1.0). If the positive - // label is not present in y_true the score is 0.0. + // T::one() (i.e. 1.0 when labels are 0.0/1.0). The denominator + // is `predicted(positive)` — the number of predictions equal to + // the positive label — so a spurious predicted label not present + // in y_true does not affect the score. If the positive label is + // not present in y_true the score is 0.0. let positive_bits = T::one().to_f64_bits(); *scores.get(&positive_bits).unwrap_or(&0.0) } else { // Multiclass case: macro-averaged precision. classes >= 1 is - // guaranteed here because of the `n == 0` guard above. + // guaranteed here because of the `n == 0` guard above. The sum + // over `HashMap::values()` is order-independent (floating-point + // addition of non-negative finite values is commutative and + // associative for the magnitudes involved here). scores.values().sum::() / classes as f64 } } @@ -186,4 +199,27 @@ mod tests { let expected = (1.0 / 3.0 + 0.5 + 1.0 + 0.0) / 4.0; assert!((score - expected).abs() < 1e-8); } + + #[cfg_attr( + all(target_arch = "wasm32", not(target_os = "wasi")), + wasm_bindgen_test::wasm_bindgen_test + )] + #[test] + fn precision_binary_spurious_predicted_label() { + // y_true is binary {0, 1} but y_pred contains a spurious label 2 + // that never appears in y_true. The binary precision denominator is + // `predicted(positive=1)`, which counts only predictions of 1, so + // the spurious prediction of 2 does not inflate the denominator. + // tp(1)=2, predicted(1)=2 -> precision = 1.0. + // + // (The pre-refactor binary path counted any wrong prediction when + // y_true was negative as a false positive, which would have given + // 2/3 here; the new path matches sklearn's binary precision, which + // only counts predictions of the positive class in the denominator.) + let y_true: Vec = vec![0., 0., 1., 1.]; + let y_pred: Vec = vec![0., 2., 1., 1.]; + + let score: f64 = Precision::new().get_score(&y_true, &y_pred); + assert!((score - 1.0).abs() < 1e-8); + } } diff --git a/src/metrics/recall.rs b/src/metrics/recall.rs index d2580843..6031c6ad 100644 --- a/src/metrics/recall.rs +++ b/src/metrics/recall.rs @@ -46,6 +46,12 @@ impl Recall { /// /// Returns a map from label bit pattern to that class's recall /// (`tp / support`, or `0.0` when the class has no support). + /// + /// Iterates only over `counts.classes_set()` (labels seen in `y_true`). + /// A label that appears in `y_pred` but never in `y_true` has no support + /// and no true positives, so it is silently ignored — it does not + /// inflate or deflate any class's recall. This matches sklearn's + /// behaviour, where the label set is derived from `y_true`. pub(crate) fn per_class_scores_from_counts( &self, counts: &ConfusionCounts, @@ -95,7 +101,7 @@ impl Metrics for Recall { return 0.0; } - let counts = ConfusionCounts::from(y_true, y_pred); + let counts = ConfusionCounts::new(y_true, y_pred); let classes = counts.classes_set().len(); let scores = self.per_class_scores_from_counts(&counts); @@ -107,7 +113,10 @@ impl Metrics for Recall { *scores.get(&positive_bits).unwrap_or(&0.0) } else { // Multiclass case: macro-averaged recall. classes >= 1 is - // guaranteed here because of the `n == 0` guard above. + // guaranteed here because of the `n == 0` guard above. The sum + // over `HashMap::values()` is order-independent (floating-point + // addition of non-negative finite values is commutative and + // associative for the magnitudes involved here). scores.values().sum::() / classes as f64 } }