Skip to content

Fix multiclass / non-{0,1}-label crashes across the sklearn classification tests (ZD-704)#534

Merged
johnwalz97 merged 3 commits into
mainfrom
john/zd-704/weakspots-multiclass-averaging
Jul 8, 2026
Merged

Fix multiclass / non-{0,1}-label crashes across the sklearn classification tests (ZD-704)#534
johnwalz97 merged 3 commits into
mainfrom
john/zd-704/weakspots-multiclass-averaging

Conversation

@johnwalz97

@johnwalz97 johnwalz97 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Pull Request Description

What and why?

A customer (ZD-704) running a multiclass model hit ValueErrors in the scikit-learn tests, one at a time. Rather than fix them reactively, I swept every classification sklearn test against a multiclass model and fixed all the genuine breakages.

Root cause (shared): these tests score classification metrics / build plots using scikit-learn defaults that only hold for a standard {0, 1} binary problem.

Tests fixed

Per-region diagnosis testsWeakspotsDiagnosis, OverfitDiagnosis, RobustnessDiagnosis. They score precision/recall/F1/AUC on subsets of the data (feature bins, noise-perturbed copies):

  • precision/recall/F1 default to average="binary", pos_label=1Target is multiclass but average='binary' (multiclass) and pos_label=1 is not a valid label (binary encoded outside {0,1}, e.g. {0,4}).
  • roc_auc_score (default auc metric) → multi_class must be in ('ovo', 'ovr') on multiclass. A true probability-based multiclass ROC AUC isn't possible here (the library retains only a single probability column), so multiclass AUC falls back to the label-binarize convention already used by ClassifierPerformance.

The customer's WeakspotsDiagnosis case was a genuinely multiclass target where one feature bin happened to hold only two classes (0 and 4) — incidental, not a deliberate encoding (customer confirmed: not binary, not intentionally 0/4).

CalibrationCurve — wraps sklearn's binary-only calibration_curve, which raised a cryptic pos_label is not specified on multiclass. Now skips cleanly with SkipTestError, matching ROCCurve/PrecisionRecallCurve.

SHAPGlobalImportanceselect_shap_values only handled the older SHAP list layout (one 2D array per class). Newer SHAP returns a single (samples, features, classes) array for multiclass, which fell through to the regression branch and was passed whole to shap.summary_plotIndexError: index N is out of bounds for axis 1. Now selects along the trailing class axis; multiclass works with class_of_interest, and raises a clear, actionable error without it.

What changed

New shared module _diagnosis_metrics.py (resolve_averaging/full_labels, bind_averaging/apply_averaging, multiclass_auc) used by all three diagnosis tests. WeakspotsDiagnosis refactored onto it; OverfitDiagnosis/RobustnessDiagnosis route auc (multiclass) and label metrics through it. Averaging is resolved once from the full label set (union of true + predicted labels) so every subset is scored consistently; standard {0,1} stays on pos_label=1 (unchanged). Plus the two standalone fixes above.

Full multiclass sweep (21 classification tests)

Status Tests
✅ Fixed here WeakspotsDiagnosis, OverfitDiagnosis, RobustnessDiagnosis, CalibrationCurve (skips), SHAPGlobalImportance
✅ Already worked ClassifierPerformance, ConfusionMatrix, MinimumAccuracy, MinimumF1Score, MinimumROCAUCScore, ModelParameters, ModelsPerformanceComparison, FeatureImportance, PermutationFeatureImportance, PopulationStabilityIndex, TrainingTestDegradation, HyperParametersTuning
✅ Binary-by-design, skip gracefully ROCCurve, PrecisionRecallCurve, ClassifierThresholdOptimization

ScoreProbabilityAlignment needs a scorecard "score" column (not a multiclass issue). Clustering/regression tests aren't applicable to a classifier.

How to test

uv run python -m pytest tests/unit_tests/model_validation/sklearn/ -q

New/updated: shared-helper units (incl. multiclass_auc and select_shap_values edge cases) and end-to-end runs across binary {0,1}, binary {0,4}, multiclass (default and per-metric), and regression. Full sklearn unit dir: 66 passed.

What needs special review?

  • Multiclass AUC semantics: no per-class probabilities are stored, so multiclass auc in Overfit/Robustness is a label-binarize AUC (matches ClassifierPerformance), not a probability ROC AUC.
  • Resolving averaging globally (from the full dataset) rather than per subset — deliberate, so metrics stay comparable and sparse subsets don't flip to binary.
  • pos_label = max(labels) for non-{0,1} binary targets.
  • SHAPGlobalImportance now requires class_of_interest for multiclass (clear error if omitted) — consistent with the pre-existing list-branch contract, but confirm that's the desired UX vs. defaulting to a class.

Dependencies, breaking changes, and deployment notes

No breaking changes; {0,1} binary and regression results unchanged. Needs a new library release tagged to PyPI before the customer can pip install the fix.

Release notes

Fixed ValueErrors when running scikit-learn tests on multiclass targets (or binary targets encoded outside {0, 1}): WeakspotsDiagnosis, OverfitDiagnosis, and RobustnessDiagnosis now handle multiclass; CalibrationCurve skips cleanly for multiclass; and SHAPGlobalImportance supports multiclass via class_of_interest. Previously these raised errors such as Target is multiclass but average='binary', pos_label=1 is not a valid label, multi_class must be in ('ovo', 'ovr'), or a SHAP IndexError.

Checklist

  • What and why
  • How to test
  • What needs special review
  • Dependencies, breaking changes, and deployment notes
  • Labels applied (bug)
  • PR linked to Shortcut
  • Unit tests added
  • Tested locally
  • Documentation updated (if required)

🤖 Generated with Claude Code

…ry labels

WeakspotsDiagnosis scores precision/recall/F1 per feature slice using
scikit-learn's defaults (average="binary", pos_label=1). This raises when a
slice does not look like a standard {0, 1} binary problem:

- Multiclass targets -> "Target is multiclass but average='binary'".
- Binary targets encoded outside {0, 1} -> "pos_label=1 is not a valid label".

The customer-reported case (ZD-704) is a genuinely multiclass target where one
feature bin happened to contain only two classes (e.g. {0, 4}); the old
per-slice scoring treated that slice as binary and crashed with
"pos_label=1 is not a valid label. It should be one of [0, 4]".

Resolve the averaging mode once from the full label set (union of true and
predicted labels across every dataset) and apply it to every slice:
multiclass -> average="macro"; binary -> average="binary" with the largest
class value as pos_label. For the standard {0, 1} case pos_label stays 1, so
existing results are unchanged. accuracy and any custom metric without an
`average` parameter are left untouched.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@johnwalz97 johnwalz97 added the bug Something isn't working label Jul 8, 2026
@johnwalz97 johnwalz97 requested a review from AnilSorathiya July 8, 2026 16:04
…e diagnosis metric helpers

OverfitDiagnosis and RobustnessDiagnosis have the same multiclass failures as
WeakspotsDiagnosis, plus an AUC-specific one:

- metric="f1"/"precision"/"recall" call scikit-learn with average="binary"
  (pos_label=1), raising "Target is multiclass but average='binary'" on
  multiclass targets and "pos_label=1 is not a valid label" on binary targets
  encoded outside {0, 1};
- the default metric "auc" calls roc_auc_score, which raises "multi_class must
  be in ('ovo', 'ovr')" for multiclass targets. A true multiclass ROC AUC is not
  possible here because the library retains only a single probability column, so
  fall back to the label-binarize convention already used by ClassifierPerformance
  (multiclass_roc_auc_score).

Extract the shared logic into _diagnosis_metrics.py:
- resolve_averaging/full_labels: decide (average, pos_label) once from the full
  label set (union of true and predicted labels) so every feature slice or
  perturbed copy is scored consistently;
- bind_averaging/apply_averaging: bind averaging onto metrics that accept it and
  pass accuracy/regression metrics through unchanged;
- multiclass_auc: robust per-subset label-binarize AUC (aligns columns on the
  full label set, scores only classes actually present, returns 0.0 otherwise).

WeakspotsDiagnosis now imports the shared helpers instead of its own copies. The
standard {0, 1} binary and regression paths are unchanged; the only behavior
change is that multiclass runs where it previously crashed. Adds unit tests for
the shared helpers and end-to-end tests for OverfitDiagnosis/RobustnessDiagnosis
across binary {0,1}, binary {0,4}, multiclass (default auc and f1), and regression.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@johnwalz97 johnwalz97 changed the title Fix WeakspotsDiagnosis crash on multiclass targets and non-{0,1} binary labels Fix multiclass / non-{0,1}-label crashes in the sklearn diagnosis tests (Weakspots, Overfit, Robustness) Jul 8, 2026
Two more sklearn tests fail on multiclass targets (found by sweeping every
classification test against a multiclass model):

- CalibrationCurve wraps sklearn's binary-only calibration_curve, which raised a
  cryptic "y_true takes value in {...} and pos_label is not specified" error on
  multiclass. Skip cleanly with SkipTestError instead, matching ROCCurve and
  PrecisionRecallCurve.
- SHAPGlobalImportance's select_shap_values only handled the older SHAP list
  layout (one 2D array per class). Newer SHAP returns a single
  (samples, features, classes) array for multiclass, which fell through to the
  regression branch and was passed whole to shap.summary_plot, raising
  "IndexError: index N is out of bounds for axis 1". Handle the 3D array by
  selecting along the trailing class axis; multiclass now works when
  class_of_interest is given and raises a clear, actionable error when it is not.

Binary and regression paths are unchanged. Adds unit tests for select_shap_values
(list, 3D-array, 2D-array, and error cases) and CalibrationCurve (multiclass skip,
binary runs).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

PR Summary

This pull request significantly enhances the model validation module by introducing a broad suite of unit tests and a new helper module for diagnosis metrics. The changes include:

  • New unit tests for various diagnosis functions in the sklearn model validation submodules such as CalibrationCurve, OverfitDiagnosis, RobustnessDiagnosis, SHAPGlobalImportance, WeakspotsDiagnosis, and diagnosis metrics. These tests cover both binary and multiclass scenarios, including edge cases like non-standard label encodings and sparse class slices.

  • Introduction of a new helper module (_diagnosis_metrics.py) that encapsulates shared metric logic. This module provides functions to:

    • Compute the full set of labels across datasets
    • Resolve averaging strategies (returning a tuple of average type and pos_label) depending on whether the target is binary or multiclass
    • Bind averaging parameters (average, pos_label, zero_division) to metric functions in a reusable way
    • Compute a multiclass ROC AUC using label binarization when only a single probability column is available
  • Modifications in the diagnosis implementations (OverfitDiagnosis, RobustnessDiagnosis, SHAPGlobalImportance, and WeakspotsDiagnosis) to consistently integrate the new averaging logic. This includes proper resolution of label spaces and binding of metric parameters to account for binary targets encoded outside the conventional {0, 1} or multiclass predictions.

These changes not only expand the test coverage to ensure robust functionality across a variety of scenarios but also improve the maintainability of the code by centralizing common diagnostic calculations and parameter bindings.

Test Suggestions

  • Run all unit tests to ensure that existing functionality has not regressed after integrating the new helper module.
  • Test additional edge cases with datasets having unusual label encodings (e.g., labels not in {0, 1}).
  • Validate error handling by intentionally providing invalid parameters to functions like select_shap_values and verifying that clear and informative errors are raised.
  • Perform integration tests to ensure that the diagnosis functions correctly handle both binary and multiclass datasets during real-world scenarios.
  • Add performance tests to ensure that the added averaging and label resolution logic does not introduce significant overhead for large datasets.

@johnwalz97 johnwalz97 changed the title Fix multiclass / non-{0,1}-label crashes in the sklearn diagnosis tests (Weakspots, Overfit, Robustness) Fix multiclass / non-{0,1}-label crashes across the sklearn classification tests (ZD-704) Jul 8, 2026

@hunner hunner left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving — this subsumes #529's follow-up work in #533 (which I'm closing in favor of this PR) and I verified the claims empirically rather than by read-through: a 15-scenario matrix (Weakspots / Overfit / Robustness × default auc / explicit f1 × labels {0,2,4} / {0,4} / {0,1}) run identically against main, #533, and this branch. main crashes on 9 of 15; this branch passes all 15, including the multiclass default-auc path that #533 left out of scope (and which would very likely have been this customer's next ticket). multiclass_auc does match ClassifierPerformance's LabelBinarizer convention as claimed, with sensible hardening on top (global-label fit for column alignment, scorable-class filtering; unseen perturbed labels binarize to all-zero rows rather than crashing). Local suite: 58 passed, 2 pre-existing skips (SHAP tests covered by the max-deps CI matrix, which is green).

Two findings worth fixing before merge, both demonstrated:

1. CalibrationCurve still crashes on binary targets encoded outside {0, 1} — the exact error this PR calls out as cryptic. The new guard only skips when len(np.unique(dataset.y)) > 2, so a two-class {0, 4} target sails past it and hits sklearn's ValueError: y_true takes value in {0, 4} and pos_label is not specified.... Reproduced on this branch with a {0, 4} logistic model. Since the PR's own convention (in resolve_averaging) is "largest label is the positive class", passing pos_label through to calibration_curve for the two-class case closes it consistently:

prob_true, prob_pred = calibration_curve(
    dataset.y, dataset.y_prob(model), n_bins=n_bins,
    pos_label=np.unique(dataset.y).max(),
)

2. apply_averaging in WeakspotsDiagnosis rebinds user-supplied custom metrics, silently overriding their explicit choices. bind_averaging rebinds any callable whose signature exposes average — and inspect.signature of functools.partial(f1_score, average="weighted") still exposes average, so the most natural way for a user to customize averaging gets clobbered. Demonstrated on this branch:

p = functools.partial(f1_score, average="weighted")
bind_averaging(p, "macro", None)(y_true, y_pred)
# returns the macro score (0.8413), not the user's weighted score (0.8492)

Since WeakspotsDiagnosis applies it to the whole prepared registry, this hits every custom metric, not just defaults. Suggest only applying averaging when the caller didn't pass metrics (i.e. rebind DEFAULT_METRICS only) — custom callables should own their kwargs.

Two non-blocking notes: the PR body's "every subset is scored consistently" slightly oversells — the strategy is resolved globally but macro without labels= still averages over each slice's present classes only (a {0,2} slice of a {0,2,4} problem: 0.80 here vs 0.53 if labels= were pinned). I actually think your behavior is the better choice for this test — a homogeneous, perfectly-predicted bin shouldn't be flagged as a weakspot — so just worth a precise sentence rather than a change. And the Shortcut checklist item: this is sc-17091.

🤖 Automated reply

@johnwalz97 johnwalz97 merged commit 7873fff into main Jul 8, 2026
22 checks passed
@johnwalz97 johnwalz97 deleted the john/zd-704/weakspots-multiclass-averaging branch July 8, 2026 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants