ZD-717: Share multiclass proba preamble; align classes on estimator.classes_ (follow-up to #535/#539)#540
ZD-717: Share multiclass proba preamble; align classes on estimator.classes_ (follow-up to #535/#539)#540hunner wants to merge 2 commits into
Conversation
|
Really nice refactor — pulling the shared "reach the estimator for the full per-class probability matrix" preamble into one helper cleans up four tests at once, and switching the column alignment to
|
…p tests (ZD-717) Review follow-ups on PR #540: - Note in the module docstring that micro-averages pool only the present classes' aligned columns, so an absent training class is deliberately excluded from the micro number. - Lock in the dtype-mismatch behavior with a test: a model trained on int labels evaluated against a string-typed target column skips loudly via the unknown-label guard (naming the labels) — it cannot quietly misalign label_binarize or mark classes absent. No normalization added. - Cover the PSI class-membership gap: when the initial slice has all training classes and the new slice is missing one, the present set is driven by the initial dataset and all per-class tables compute without skipping (the new dataset only needs matching columns, since PSI compares score distributions rather than positives). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Thanks for the careful trace — addressed in 59cbf8d:
Suite is now 24 passed / 5 xgboost-gated skips on the affected files. 🤖 Automated reply |
…(ZD-717) Follow-ups from the PR #539 review. ROCCurve, PrecisionRecallCurve, PopulationStabilityIndex (#535) and GINITable (#539) each carried an identical ~40-line preamble to fetch the underlying estimator's per-class probability matrix for their one-vs-rest multiclass paths. Extract it into sklearn/_multiclass_proba.py (same pattern as _diagnosis_metrics.py) and point all four tests at it. Fix the class/column alignment while there: the preamble assumed np.unique(dataset.y) matches predict_proba column order, but sklearn orders columns by estimator.classes_ (training classes). Align on classes_ with a np.unique fallback, binarize against the full training class list, and emit per-class output only for classes present in the evaluated y. Net behavior change: an evaluation slice missing a training class now computes metrics for the present classes plus micro-average instead of raising SkipTestError. Two guards the old width check provided implicitly are kept explicit: a binary-trained model evaluated on a >2-class dataset skips (previously the new alignment would have crashed with IndexError), and evaluation labels the model was never trained on skip rather than being silently absorbed as all-negative rows. Adds unit tests for the helper (10) and un-gated sklearn multiclass tests for all four call sites, including the missing-class scenario; the pre-existing multiclass tests for ROC/PR were entirely xgboost-gated, so the refactored paths now have coverage without the extra. PSI gains its first unit test file. 114 passed / 5 xgboost-gated skips across tests/unit_tests/model_validation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…p tests (ZD-717) Review follow-ups on PR #540: - Note in the module docstring that micro-averages pool only the present classes' aligned columns, so an absent training class is deliberately excluded from the micro number. - Lock in the dtype-mismatch behavior with a test: a model trained on int labels evaluated against a string-typed target column skips loudly via the unknown-label guard (naming the labels) — it cannot quietly misalign label_binarize or mark classes absent. No normalization added. - Cover the PSI class-membership gap: when the initial slice has all training classes and the new slice is missing one, the present set is driven by the initial dataset and all per-class tables compute without skipping (the new dataset only needs matching columns, since PSI compares score distributions rather than positives). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
59cbf8d to
6f17bd5
Compare
PR SummaryThis pull request introduces a set of enhancements and additions to the model evaluation framework for multiclass classification. The key changes include:
Overall, these changes strengthen the robustness and flexibility of the model validation process in handling multiclass scenarios, ensuring that metrics are computed accurately even when the evaluation dataset does not completely mirror the training data. Test Suggestions
|
Pull Request Description
What and why?
Implements the three follow-up items from the review of #539 (ZD-717 multiclass work, continuing #535):
Extract the shared preamble.
ROCCurve,PrecisionRecallCurve,PopulationStabilityIndex([SC-17128] ZD-717: Add multiclass (one-vs-rest) support to VM tests #535) andGINITable([SC-17187] ZD-717: GINITable fails on multiclass models (follow-up to PR #535) #539) each carried an identical ~40-line block: reach the underlying estimator'spredict_proba(theVMModelwrapper only exposes the positive-class column), guard unusable models withSkipTestError, andlabel_binarizethe true labels. That now lives invalidmind/tests/model_validation/sklearn/_multiclass_proba.py(same private-module pattern as_diagnosis_metrics.py), and all four tests call it.ConfusionMatrixnever had the block (it uses argmax predictions) and is untouched.Align classes on
estimator.classes_. The preamble assumednp.unique(dataset.y)matchespredict_probacolumn order, but sklearn orders columns by the classes seen in training. The helper aligns onclasses_(falling back tonp.unique), binarizes against the full training class list, and emits per-class output only for classes present in the evaluatedy. Net behavior change: an evaluation slice missing a training class — e.g. a test split without a rare class, exactly the ZD-717 customer'sinput_gridover[train, test]— now computes metrics for the present classes plus micro-average instead of raisingSkipTestError.Close the coverage gap on the shape/missing-class branch (see tests below).
Two guards the old width check provided implicitly are kept explicit, with clear skip messages: a binary-trained model evaluated on a >2-class dataset skips (the new alignment would otherwise IndexError), and evaluation labels the model was never trained on skip rather than being silently absorbed as all-negative rows.
Binary paths of all four tests are unchanged, as is each test's
len(np.unique(dataset.y)) > 2branch decision and output structure (RawData keys, table columns, trace layout).How to test
Expected: 22 passed, 5 skipped (the pre-existing xgboost-extra gates). Full
tests/unit_tests/model_validation: 114 passed, 5 skipped (21 collection errors from missing optional extras are identical onmain— environmental).New coverage:
test_multiclass_proba.py— 10 unit tests for the helper: happy path,classes_alignment, missing-class, no-predict_proba/ raising-predict_proba/ wrong-width skips, binary-trained-model skip, unseen-label skip.What needs special review?
np.unique(dataset.y)columns — identical when nothing is missing, which was the only case that previously computed).Dependencies, breaking changes, and deployment notes
Follow-up to #535 / #539. Library-only, no migrations, no env vars, no version bump (left to the release process). No breaking changes: every previously-computing case produces identical output; the only behavior change is previously-skipped missing-class runs now computing.
Release notes
ROC Curve, Precision-Recall Curve, Population Stability Index, and GINITable multiclass metrics now align class columns with the model's training classes. Evaluating on a dataset slice that is missing one of the model's training classes now produces metrics for the classes present (plus the micro-average) instead of skipping the test.
Checklist
enhancement🤖 Generated with Claude Code