Skip to content

fix(metrics): return 1.0 for zero-entropy labels in HCVScore - #534

Merged
Mec-iS merged 2 commits into
smartcorelib:mainfrom
NotAFlightRisk:fix-hcv-zero-entropy
Nov 14, 2026
Merged

Mec-iS merged 2 commits into
smartcorelib:mainfrom
NotAFlightRisk:fix-hcv-zero-entropy

Conversation

@NotAFlightRisk

Copy link
Copy Markdown
Contributor

Checklist

  • My branch is up-to-date with main branch.
  • Everything works and tested on latest stable Rust.
  • Coverage and Linting have been applied (fmt and clippy, nothing new from either)

Current behaviour

When a clustering puts every point in one cluster (DBSCAN with a big enough eps will), HCVScore gives NaN for completeness and v-measure, where they should be 1.0 and 0.0. A ground truth with only one class goes the other way, with homogeneity coming out as inf and v-measure as NaN again. Both come down to dividing by an entropy of 0. The unwrap_or(0f64) looks like it's there for that, but entropy() always returns Some, so it never kicks in.

New expected behaviour

Zero entropy scores 1.0, which is what scikit-learn does (MI / entropy_C if entropy_C else 1.0), and what cluster_hcv_single_cluster_no_panic in tests_edge_cases.rs expects too, though that file isn't compiled at the moment. With a single class the mutual information comes out at about 1e-16 rather than exactly 0, so completeness ends up at 1e-16 and not 0.0. I've left that alone, and the test allows for it.

Change logs

Changed

  • HCVScore scores a zero entropy as 1.0 instead of NaN or inf

@codecov

codecov Bot commented Nov 13, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.06%. Comparing base (9eaae9e) to head (c9f105a).
⚠️ Report is 225 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #534       +/-   ##
===========================================
+ Coverage   43.97%   87.06%   +43.08%     
===========================================
  Files          85      100       +15     
  Lines        7281     9915     +2634     
===========================================
+ Hits         3202     8632     +5430     
+ Misses       4079     1283     -2796     

☔ 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

Mec-iS commented Nov 14, 2026 •

Copy link
Copy Markdown
Collaborator

Thanks for the fix. The root cause is correct: entropy() always returns Some, so the old unwrap_or(0f64) never fired, and a zero entropy produced NaN or inf. filter(|&e| e > 0f64).map_or(1f64, ...) matches scikit-learn's MI / entropy if entropy else 1.0, and the change is small and surgical.

Checked

  • Single cluster with 3 classes gives homogeneity 0, completeness 1 and v-measure 0, which matches sklearn.
  • Single class gives homogeneity 1. Completeness is about 0 because of float noise in MI, so v-measure is about 0.
  • Identical single-label inputs give a v-measure of 1.
  • The change adds no unsafe, no FFI and no allocations, and the new test carries the wasm_bindgen_test attribute. CI is green on wasm32, i686, Windows, macOS, MSRV 1.85 and all feature sets.

Requested changes / questions
2. Lint failure. The lint job failed on the latest run, but the checklist says fmt and clippy are clean. Please check the job log. If the failure is pre-existing on the base branch, say so in the PR. Otherwise, fix it.
3. Exact float equality in the test. assert_eq!(Some(0.0), scores.homogeneity()) in the first case depends on MI being exactly 0.0. The PR description notes that MI comes out around 1e-16 in the single-class case, so the same noise could appear here on other platforms or after future refactors. Use an epsilon check like the other assertions (.abs() < 1e-10).
4. Optional: clamp MI. mi could be clamped to 0.0 when mi.abs() < f64::EPSILON. That would make single-class completeness exactly 0.0 and match sklearn, and it would let the test assert exact values. It is fine as a follow-up if you prefer to leave it out of this PR.

Follow-ups, not blocking

  • tests_edge_cases.rs isn't compiled, so cluster_hcv_single_cluster_no_panic can't verify this fix. Please open an issue to wire that file in. It should not become a TODO in the code.
  • Check whether other metrics that divide by entropy (e.g. mutual_info_score normalisation, if present) have the same dead unwrap_or pattern.

Once 1–3 are addressed, this looks good to merge from my side.

@Mec-iS Mec-iS mentioned this pull request Nov 14, 2026
3 tasks done
@Mec-iS
Mec-iS merged commit 2bad22d into smartcorelib:main Nov 14, 2026
19 of 20 checks passed
@NotAFlightRisk

Copy link
Copy Markdown
Contributor Author

Cheers for merging @Mec-iS. The lint failure isn't from this PR - it's clippy's needless_range_loop in pca, lu, svd, naive bayes and svc, and main has failed it the same way since the 13th. The two files here come out clean.

I'd only put a tolerance on the single-class case because that's where I'd seen the noise, which isn't much of a reason. I've done your clamp, but the way sklearn does it (MI comes back as 0 straight away if either side only has one label) rather than with an epsilon. That makes those zeros exact by construction, so the test checks them with assert_eq in both cases now.

Opened #535 for tests_edge_cases.rs. And HCVScore is the only place anything divides by an entropy, there's no NMI or AMI yet, so there's no other dead unwrap_or to chase.

@Mec-iS

Mec-iS commented Nov 14, 2026

Copy link
Copy Markdown
Collaborator

ok thanks.

no problem, this will go all in v0.8.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants