Fix multiclass macro F1 to average per-class F-scores

[gaoflow]

Checklist

• My branch is up-to-date with development branch.
• Everything works and tested on latest stable Rust.
• Coverage and Linting have been applied

Current behaviour

For multiclass inputs F1::get_score returns the F-measure of the macro-averaged
precision and recall, Fβ(mean_c Pc, mean_c Rc). Macro F is defined as the mean of the
per-class F-measures, mean_c Fβ(Pc, Rc); the two are equal only when every class has the
same precision and recall, so the result is wrong for most multiclass inputs.

Example (3 classes, chosen so precision and recall already match sklearn, which isolates the
aggregation):

y_true = [0, 0, 1, 1, 2, 2]
y_pred = [0, 1, 1, 1, 2, 2]

macro precision = 0.888889   (matches sklearn)
macro recall    = 0.833333   (matches sklearn)
F1              = 0.860215   (per-class F1 = [2/3, 0.8, 1.0])

sklearn.metrics.f1_score(average='macro') returns mean([2/3, 0.8, 1.0]) = 0.822222.

New expected behaviour

For the multiclass case, compute per-class precision/recall/F-beta and average over the
classes, matching sklearn's average='macro' (the same oracle used for the recent AUC fix
#374). F1 on the example above is now 0.822222, and the correction applies for beta != 1
as well (e.g. beta=2 goes from 0.843882 to sklearn's 0.821549).

The binary path is unchanged (still the positive-class F-measure), so the existing f1 test
passes byte-for-byte. Added f1_multiclass_macro, f1_multiclass_macro_beta and
f1_multiclass_imbalanced, all of which fail against the old aggregation.

Note: this changes only the F aggregation. Precision/Recall derive their class set from
y_true, so a class that is predicted but never true (or a third predicted label when
y_true has two) is handled exactly as before — that label-set question is separate and left
untouched here.

Change logs

Changed

• Multiclass F1/F-beta is now the mean of the per-class F-measures instead of the F-measure
  of the macro-averaged precision and recall (average='macro', per sklearn).

[codecov]

Codecov Report

❌ Patch coverage is 57.57576% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.47%. Comparing base (70d8a0f) to head (1e7ac28).
⚠️ Report is 22 commits behind head on development.

Files with missing lines	Patch %	Lines
src/metrics/f1.rs	57.57%	14 Missing ⚠️

Additional details and impacted files

@@               Coverage Diff               @@
##           development     #382      +/-   ##
===============================================
- Coverage        45.59%   44.47%   -1.13%     
===============================================
  Files               93       94       +1     
  Lines             8034     8086      +52     
===============================================
- Hits              3663     3596      -67     
- Misses            4371     4490     +119     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Mec-iS]

Thanks @gaoflow ! Here a brief review with some minor changes to apply.

Summary

The PR correctly identifies and fixes a real bug: the old code computed Fβ(mean_p, mean_r) instead of mean Fβ(p_c, r_c) for multiclass inputs . The fix aligns behaviour with sklearn's average='macro' and is well-motivated. However, there are several issues worth addressing before merging.

---

Correctness Issues

Dead code / unreachable branch:
Inside the else block (multiclass path), the final if classes == 0 { 0.0 } check is unreachable . classes was computed as classes_set.len(), and the code only enters the else branch when classes != 2. A zero-class input would have been caught by the earlier if n == 0 guard. This dead branch should be removed or replaced with a clearer comment.

classes == 1 (single-class input) goes to the multiclass path:
The binary check is classes == 2, so a single-class input (all labels identical) falls through to the multiclass loop. In that case the loop computes a perfectly valid Fβ = 1.0 if all predictions match, but the behaviour is arguably undefined — consider returning 1.0 or f64::NAN explicitly and documenting the decision.

---

Design / Architecture

Dual iteration over y_true:
The PR builds classes_set in one pass, then iterates over y_true/y_pred again to build tp_map, support, and predicted — two full O(n) passes . Both can easily be merged into a single pass. The classes_set is only used for classes_set.len() and iteration in the final loop, so it can be populated alongside the other maps.

Duplication with Precision and Recall:
The multiclass path re-implements per-class precision and recall from scratch using raw HashMap bookkeeping, while the binary path delegates to Precision::new() and Recall::new() . This creates two code paths that could diverge. Ideally, Precision and Recall should be refactored to expose per-class scores, which F1 then consumes — consistent with the single-responsibility principle.

to_f64_bits() as a HashMap key:
Using f64::to_f64_bits() as a key in a HashMap is a common trick, but f64 labels with different bit patterns for the same conceptual value (e.g., -0.0 vs 0.0) would be counted as separate classes . This is an existing pattern in the codebase, but worth flagging in a comment.

---

Minor / Style

• The #[allow(unused_imports)] may be needed for HashSet in some configurations — confirm HashSet is used (it is, for classes_set), so that's fine.
• Variable names like sup could be support_count for clarity.
• The comment // Binary case: F-measure of the positive class... is correct, but a note that "positive class is assumed to be the label with the higher bit representation (i.e., 1.0)" would help future readers, since Precision::new() has that assumption baked in.
• The three new tests are well-structured and cover the key cases: balanced multiclass, imbalanced, and varying beta . Consider adding a test for the classes == 1 edge case as well.

---

Suggested Fix for Dead Code

// Replace:
if classes == 0 {
    0.0
} else {
    fbeta_sum / classes as f64
}

// With:
fbeta_sum / classes as f64
// (classes >= 3 is guaranteed by the outer `else` branch; classes == 0 is caught by n == 0 guard)

---

Verdict

✅ Correct fix, good tests — the core change is sound and the existing f1 binary test still passes . The main concerns are the unreachable branch, dual iteration, and the silent re-implementation of per-class precision/recall. These are worth addressing for long-term maintainability, but none are blockers for correctness.

[Mec-iS]

Thanks, I will merge these changes and apply the fixes myself.