fix: WeakspotsDiagnosis handles multiclass and non-{0,1} labels#533
fix: WeakspotsDiagnosis handles multiclass and non-{0,1} labels#533hunner wants to merge 2 commits into
Conversation
WeakspotsDiagnosis computed precision/recall/f1 per feature-bin slice with
sklearn's binary defaults (average='binary', pos_label=1), which raised for
multiclass models and for binary labels not containing 1 (e.g. {0, 4} ->
'pos_label=1 is not a valid label'). Decide the averaging once from the global
label set (union of target and prediction columns of both datasets) and bind
the default precision/recall/f1 metrics via functools.partial -- macro for
multiclass, pos_label for non-standard binary, untouched for conventional
{0, 1}. Mirrors the MinimumF1Score fix (PR #529). Adds regression tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
OverfitDiagnosis and RobustnessDiagnosis compute f1/precision/recall with sklearn's binary defaults when the user selects those metrics, crashing on multiclass models and on binary labels not containing 1. Resolve the averaging once from the global label set (union of y_true and predictions), the same guard applied to WeakspotsDiagnosis. Non-prf metrics (auc, accuracy, regression) are untouched. Adds regression tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PR SummaryThis pull request introduces improvements to how classification metrics (f1, precision, and recall) are handled in model diagnosis tools. The changes adjust the averaging strategy by binding the appropriate keyword arguments based on the global set of labels encountered in both training and testing datasets. This ensures that when using metrics from scikit-learn, scenarios such as multiclass tasks or non-standard binary cases (for example, labels such as {0, 4} where the default binary behavior of pos_label=1 would fail) are correctly managed. Key functional changes include:
The modifications improve the robustness and reliability of the diagnosis tools by ensuring that metric calculations do not fail due to mismatches between dataset labels and scikit-learn default behaviors. Test Suggestions
|
|
Closing in favor of #534, which subsumes this fix: same averaging convention (global label set, macro for multiclass, Tracking stays on sc-17091 / ZD-704. 🤖 Automated reply |
Pull Request Description
What and why?
WeakspotsDiagnosisbins each feature into slices and scores every slice with sklearn'sprecision_score/recall_score/f1_scorecalled bare, so sklearn's defaultsaverage='binary', pos_label=1apply to every slice. Those defaults are only valid when a slice's labels are binary and include1. For anything else the test crashes before producing a result:1(e.g.{0, 4}) fails label validation →ValueError: pos_label=1 is not a valid label. It should be one of [0, 4]— the exact error reported by the customer on ZD-704 (v2.13.6, multiclass logistic regression);ValueError: Target is multiclass but average='binary'.The test tags itself
multiclass_classification, so multiclass is in-scope by its own metadata — the averaging strategy just never adapted. This is the same class of bug fixed forMinimumF1Scorein #529 (shipped in v2.13.6, confirmed working by the same customer), here multiplied across every feature slice. Even a purely binary model crashes if its two labels aren't{0, 1}(e.g.{0, 4}or string labels).After this change, the averaging strategy is decided once from the global label set — the union of the target and prediction columns of both input datasets — and bound into the default metric registry via
functools.partial:average="macro", labels=<global labels>, zero_division=0— macro for consistency with fix: MinimumF1Score multiclass detection uses y_true and y_pred #529; passing the global label set keeps per-slice scores comparable when a bin lacks some classes, andzero_division=0suppresses the resulting undefined-metric warnings instead of emitting one per empty class per slice;1not a label (e.g.{0, 4}):pos_label=labels.max()— the larger value is treated as the positive class;{0, 1}binary: untouched — sklearn defaults, zero behavior change for the vast majority of existing usage.Deciding globally rather than per slice matters: per-slice detection would silently mix binary and macro semantics across bars of the same chart, and would still crash on
{0, 4}-style slices.The sibling diagnosis tests
OverfitDiagnosisandRobustnessDiagnosisshare the identical bare-call weakness in their metric registries. Their default classification metric isauc, so default runs don't hit it, but explicitly selectingmetric="f1"/"precision"/"recall"crashes under the same conditions. The second commit applies the same global-label-set guard there, closing out this bug class for the ticket (sibling hardening was also the pattern asked for in #529's review).User-supplied custom metrics are left alone in all three tests — custom callables own their own kwargs.
How to test
Reproduce the customer's failure on
main, then confirm it on this branch:On
mainthis raisespos_label=1 is not a valid label...(or the multiclass/binary-average variant depending on which slice is scored first); on this branch it completes. Swapping[0, 2, 4]for[0, 4]exercises the non-{0,1} binary path, and[0, 1]confirms the untouched conventional path. The same shapes withmetric="f1"coverOverfitDiagnosis/RobustnessDiagnosis.Unit tests:
python -m unittest discover -s tests/unit_tests/model_validation/sklearn→ 44 passed, 2 skipped (pre-existing skips). New coverage includes the three averaging branches plus end-to-end no-raise runs for{0, 2, 4},{0, 4}, and{0, 1}across all three tests.What needs special review?
pos_label=labels.max()for non-{0,1} binary is a judgment call (macro averaging would also unblock it). Treating the larger label as the positive class matches the common encoding, and it's documented in the docstring.df_1/df_2are now built before metric preparation so the global label set exists when defaults are bound. They're pure column selections, so no behavior change — but it's the one reordering in the diff.aucpath has a separate multiclass gap (roc_auc_scorewithoutmulti_class=), not touched here.Dependencies, breaking changes, and deployment notes
None. Every changed path currently raises
ValueError, so behavior only changes from "crash" to "scored"; conventional{0, 1}binary paths are byte-for-byte unaffected.Release notes
Fixed
WeakspotsDiagnosis(andOverfitDiagnosis/RobustnessDiagnosiswith explicit f1/precision/recall metrics) failing withValueError: pos_label=1 is not a valid labelorTarget is multiclass but average='binary'for multiclass models and for binary models whose labels are not{0, 1}.Checklist
Closes ZD #704 / sc-17091
🤖 Generated with Claude Code