Skip to content

Fix .balanced_accuracy() returning NaN for single-class data#337

Closed
ishaan-arora-1 wants to merge 1 commit into
stan-dev:masterfrom
ishaan-arora-1:fix/balanced-accuracy-nan-single-class
Closed

Fix .balanced_accuracy() returning NaN for single-class data#337
ishaan-arora-1 wants to merge 1 commit into
stan-dev:masterfrom
ishaan-arora-1:fix/balanced-accuracy-nan-single-class

Conversation

@ishaan-arora-1

Copy link
Copy Markdown
Contributor

Hey! I noticed that .balanced_accuracy() in loo_predictive_metric.R silently returns NaN when all observations belong to a single class (e.g., all 1s or all 0s). This happens because mean() gets called on an empty subset — yhat[mask] is length 0 when there are no negatives (or no positives), and mean(integer(0)) gives NaN.

Quick repro:

loo:::.balanced_accuracy(c(1, 1, 1, 1), c(0.8, 0.6, 0.7, 0.9))
# $estimate: NaN
# $se: NaN

Since balanced accuracy is inherently undefined with only one class present, this PR adds a guard clause that throws a clear error message instead of silently returning garbage values. Also added tests for both single-class cases (all 0s and all 1s).

All existing tests still pass.

When all observations belong to a single class (all 0s or all 1s),
.balanced_accuracy() silently returns NaN for both estimate and se
because mean() is called on an empty subset. This adds a guard clause
that throws an informative error instead, since balanced accuracy is
undefined when only one class is present.
@VisruthSK VisruthSK marked this pull request as draft March 3, 2026 22:38
@VisruthSK

Copy link
Copy Markdown
Member

Hi, sorry I'll have to make the same comment as on #327--this is part of the major refactor so we'll probably hold off on this PR. Thanks for pointing this out though, I'll add this as a test for the new code, which you can find here if you're curious.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.78%. Comparing base (d5e23dd) to head (ec67213).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #337   +/-   ##
=======================================
  Coverage   92.78%   92.78%           
=======================================
  Files          31       31           
  Lines        2992     2995    +3     
=======================================
+ Hits         2776     2779    +3     
  Misses        216      216           

☔ View full report in Codecov by Sentry.
📢 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.
@ishaan-arora-1

Copy link
Copy Markdown
Contributor Author

Hi, sorry I'll have to make the same comment as on #327--this is part of the major refactor so we'll probably hold off on this PR. Thanks for pointing this out though, I'll add this as a test for the new code, which you can find here if you're curious.

@VisruthSK Sure, lemme know if you'll need a hand in the process.

@florence-bockting

Copy link
Copy Markdown
Contributor

Thank you for for opening this PR. As @VisruthSK pointed out earlier in the discussion, we will tackle this aspect as part of the larger refactoring PR (#363), therefore I will close this PR now.

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

Labels

None yet

4 participants