Repository navigation
Use full training labels for holdout classification scoring - #1642
Aryan R. Ganjoo (AryanRG) wants to merge 10 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Sampled classifiers can still fail log-loss scoring, and each trial unnecessarily recomputes the full training class set.
2 open issues
What changed in this PR
Updates AutoML holdout classification scoring to use the full training class set when validation or sampled training data omits a class.
Changes:
- Passes full training targets into trial scoring.
- Infers holdout labels while preserving explicit overrides and token-classification behavior.
- Adds regression tests for missing classes, time splits, custom metrics, and sampling.
| File | Description |
|---|---|
| test/automl/test_holdout_labels.py | Adds holdout label regression tests. |
| flaml/automl/state.py | Passes full training targets to scoring. |
| flaml/automl/ml.py | Infers classification labels for holdout metrics. |
Review effort: Balanced
Li Jiang (thinkall)
left a comment
There was a problem hiding this comment.
Overall review at efb0e90208c145cd4780bcd2b329b2dfbd7f3106: changes requested. The ordinary missing-validation-class fix works, including independently checked string labels and binary single-class validation. One sampled-scoring P2 remains.
P2 - Align sampled probability columns with the full label set. Using the PR's 3,000-row rare-class fixture, change validation to classes [0, 1], use built-in metric="log_loss", and enable log_training_metric=True. A sampled model has two probability columns, but this change supplies labels [0, 1, 2]. Validation scoring becomes infinite; the training-score call then raises an uncaught ValueError and aborts the public AutoML.fit call. The exact pre-change scoring function, exercised through the same common framework with only the new keyword discarded, completes with loss 0.43185208871658 and model classes [0, 1]; the current call raises the class-count mismatch. This comparison does not claim an entirely old package installation.
Please preserve full-class semantics while aligning probability columns for both validation and training scoring, and add a sampled built-in log-loss test with training metrics enabled. The existing custom-F1 test avoids probability columns. Balanced Copilot independently reported this in the existing discussion; no duplicate inline is being added.
P3 - Preserve the existing string-task helper path. compute_estimator still accepts Union[str, Task]. With task="classification" and an explicit built-in estimator_class, the old helper succeeds but the new guard raises AttributeError: 'str' object has no attribute 'is_classification'. Normalize the supported string form before calling Task methods. This is lower priority: the sole in-repository caller already supplies a Task object, so it is not a second claim that normal public AutoML construction fails.
Validation: all 5 new tests and 4 existing LGBM/LRL1 CV/underlying-model reproducibility cases passed. Independent real public scoring and six isolated label-routing checks also passed; the sampled public regression and string-helper incompatibility were separately reproduced. Approval is withheld for the verified public regression.
Posted by thinkall-agent-auto-reviewer
|
Thanks Li Jiang (@thinkall), I've fixed the sampled log-loss crash and the string-task helper. Labels are now cached per fit, with regression coverage for both validation and training scoring. |
|
@microsoft-github-policy-service agree |
Li Jiang (thinkall)
left a comment
There was a problem hiding this comment.
Overall review at dd0c080b4f40600441ec26f6ccc042f279cfc9d0: changes requested for one remaining P2 in callable probability scoring.
The original built-in log-loss/training-metric failure is fixed, the string-task helper no longer raises the reported attribute error, and the cache refreshes across class-set changes and clears for regression. All 17 targeted repository tests passed. The six independent Medium review-code lenses and driver checks are complete; their original findings/severities are retained separately from this adjudicated public result.
P2 - Preserve the documented custom log-loss workflow as well as the built-in metric. The new aligner runs only in the string-metric branch. The published custom metric still calls log_loss(y, estimator.predict_proba(X), labels=labels). With a sampled two-class estimator and full labels [0, 1, 2], that callback raises the same class/probability-count ValueError and aborts AutoML.fit.
Using the PR's seed-1 fixture (3,000 rows, 40 rare-class examples), validation classes [0, 1], min_sample_size=50, sample=True, auto_augment=False, estimator_list=["lgbm"], and two trials, the exact released-main scoring module in the common framework completes the published callback with finite loss 0.5884176913322716. Current code aborts on the first callback: 50 training rows, estimator classes [0, 1], supplied labels [0, 1, 2]. This reference-module comparison does not claim an entirely old package installation. The latest corrective commit leaves the defect in place even though it predates that small delta; it is introduced by the whole PR versus released main.
Please keep callable probability outputs and their supplied class metadata consistent, preserve this previously valid documented workflow, and add a sampled regression using that recipe. A positive Copilot recommendation and passing built-in-only tests do not resolve the independently reproduced failure.
A lower-priority defensive-copy concern is also confirmed: a callback that mutates the cached label array changes later sequential trials, whereas the prior implementation supplied fresh inferred labels per trial. Keep that P3 hardening concern separate from the public workflow P2 above.
Posted by thinkall-agent-auto-reviewer
Li Jiang (thinkall)
left a comment
There was a problem hiding this comment.
Reviewed the correction at 93b8931ae2682918cb68c9e74076540fa4b713e0. The earlier documented-callback P2 and label-mutation concern are fixed: the original public callback now matches released-main reference loss 0.5884176913322716, and repeated array/list/Series callbacks plus a real six-call CV fit preserve label isolation. All 19 targeted tests passed. The Low harness is CLEAN for this localized delta, not a whole-PR approval.
One remaining P2 - Handle a valid one-class LightGBM sample before the probability-width fallback. A fitted LightGBM classifier can have classes_ == [0] yet return two probability columns. _align_log_loss_probabilities returns those columns unchanged because their width differs from len(classes_), but scoring still receives the full three labels. Both sampled trials become infinite loss, and public AutoML.fit returns without a best model.
Reproduction uses 3,000 targets initially zero, with rows 1500 and 2500 set to classes 1 and 2; Gaussian features from RandomState(81); validation labels alternating 0/1; metric="log_loss", estimator_list=["lgbm"], two trials, min_sample_size=50, sample=True, auto_augment=False, retrain_full=False, and log_training_metric=False. The exact released-main scoring module in the common framework returns a trained model with finite loss 17.2693881956416. Current code returns best_loss=inf and model=None. This is a reference-module comparison, not an entire old-package installation.
An observation-only wrapper confirms both current trials have classes [0], labels [0,1,2], and unchanged (300,2) input/output probabilities. An independent zero-filled full-class oracle is finite (18.021826694558573), so preserving the full-class metric need not discard these trials. Handle this supported backend boundary, or ensure appropriate class coverage before evaluation, and add a one-class-sample regression alongside the two-class case.
This is a remaining whole-PR alignment defect, not a claim that the callback guard introduced it. No exception from AutoML.fit is claimed here: the failure is specifically infinite loss and no model. A positive Balanced Copilot recommendation does not resolve that independently reproduced P2, so approval remains withheld.
Posted by thinkall-agent-auto-reviewer
Li Jiang (thinkall)
left a comment
There was a problem hiding this comment.
Overall reassessment at e3046451d9201ac966a8708631e1a3df2b74b4f5: the reported code fixes are verified; approval is withheld for external gates.
The original one-class public counterexample now returns trained models with finite losses for LightGBM, XGBoost and xgb_limitdepth. Losses match independent dtype-correct full-class sklearn oracles. Raw/wrapped classifiers, numeric/string label placement and input immutability are covered; legacy binary and unknown-backend behavior remain intact. The documented callback again returns its expected finite loss and legacy label inference.
I freshly reread the complete main-to-head production/test changes, not only the last corrective hunk. The scoped Low review-code harness is CLEAN (actual gpt-6.1-sol), 43 targeted tests passed, and full all-file pre-commit passed before the local evidence commit. No confirmed blocking code issue remains in this PR. Both earlier signed P2 discussions are now resolved after reproducing and verifying their fixes; no other reviewer's thread was changed.
Approval is not submitted: applicable check contexts are not all successful; actual Build: in_progress/None.
Posted by thinkall-agent-auto-reviewer
Li Jiang (thinkall)
left a comment
There was a problem hiding this comment.
Overall review of the complete current PR at e3046451d9201ac966a8708631e1a3df2b74b4f5: approved.
I freshly reassessed the complete main-to-head production/test changes rather than treating the last small repair as whole-PR clearance. Full-label holdout scoring, per-fit cache reset, explicit overrides, legacy callable behavior and callback isolation remain consistent. Sampled one-class LightGBM and both XGBoost paths now return finite trained models with correct full-class losses, and regular sampled-class mapping preserves class order and raw input probabilities. The earlier independently reproduced P2s are resolved; no blocking code issue remains.
Validated exact-head review-code evidence was reused (Low, actual gpt-6.1-sol), including the prior 43 passing tests and native dtype-correct oracle/callback checks. These are unchanged-source results, not a claim of a new model or test run. My two earlier signed P2 discussions were resolved only after their exact counterexamples passed.
Fresh same-head gates now confirm all discussions resolved, an explicit Balanced Copilot approval recommendation, every applicable check successful, and the actual Build/docs/formatting workflows successfully completed. The conditional gh-release skip is specifically justified by the unchanged workflow's pull_request exclusion.
Posted by thinkall-agent-auto-reviewer
Li Jiang (thinkall)
left a comment
There was a problem hiding this comment.
Re-reviewed commits e3046451d9201ac966a8708631e1a3df2b74b4f5 -> 44755c28bb65d3a5192297cf94e3d1b7d001b01c. The only delta is main's website/yarn.lock update of follow-redirects 1.16.0 -> 1.16.1. One explicitly head-bound review-code Low execution (actual runtime gpt-6.1-sol) found no new issue in this identical inherited patch. Controller Git comparisons verify all non-lockfile tree entries are unchanged. This is a scoped merge-delta review, not a fresh clean signoff for unchanged feature code.
Version/URL/caret/SRI shape checks pass. No independent local registry/tarball verification, package smoke test or website rebuild is claimed. Previous feature findings and runtime validation limits remain preserved.
At the 2026-10-26T12:13:44.943946+00:00 gate snapshot, actual current-head workflows were: Code formatting completed/success; Build in_progress/no conclusion; docs completed/success. Copilot has Balanced review without an Approval recommended verdict; 1 review thread(s) remain unresolved. Gate approval is not successful CI completion, and no new PR approval is being submitted while required gates remain unmet.
Posted by thinkall-agent-auto-reviewer
There was a problem hiding this comment.
🟢 Approval recommended
Static review identified no unresolved issues, and the added regression tests cover the affected paths, though tests were not executed during this review.
No open issues 🎉
1 resolved since last review
Review effort: Balanced
Li Jiang (thinkall)
left a comment
There was a problem hiding this comment.
Changes requested after reviewing the new commit since 44755c28bb65d3a5192297cf94e3d1b7d001b01c at exact head 5b3678ea1e618b20376853e3436c44f94fa29c96.
The public sampled-class correction is useful: all 65 focused repository tests pass, and six new sampled-probability cases fail on exact prior source but pass current. I completed all six independent review-code Medium lenses with actual grok-4.7 / xhigh and independently verified two remaining P2s, detailed inline:
- Residual binary axis bug: actual LightGBM with sole sampled encoded class 1 and public labels
[10, 20]predicts label 20, but public probability argmax maps to 10 on all 300 rows. Exact prior source is equally wrong, so this is an incomplete correction, not a newly introduced binary regression. - New non-finite-output masking regression: actual sampled XGBoost 1.7.6 with
base_margin=np.full(300, np.nan)returns all-NaN backend output. Prior public inference preserves it; current inference returns finite[1, 0, 0]rows. Numerical failure must not become confidence 1.
These are actual AutoML/backend probes, not simulated output dtypes. Local versions were Python 3.10.19, NumPy 1.26.4, pandas 2.3.3, scikit-learn 1.7.2, LightGBM 4.6.0 and XGBoost 1.7.6; the full supported matrix is not claimed. Random-forest one-column and higher-encoded multiclass controls work correctly on current code.
Please also document classes_[j], missing-class zero-fill and raw automl.model.predict_proba divergence, and make the retraining test actually exercise final retraining without external X_val. These are lower-priority documentation/coverage recommendations, not additional native runtime failures.
A positive Copilot review or passing CI does not clear the two independently reproduced blockers. All original lens outcomes and driver adjudications are retained; no contributor code was edited.
Posted by thinkall-agent-auto-reviewer
Li Jiang (thinkall)
left a comment
There was a problem hiding this comment.
Reviewed the corrective commit at 57e6f84e8c91f53a297f21e424bc1a5bb239f6be with review-code Low (actual runtime gpt-6.1-sol); the scoped independent result is clean. 84 regression cases passed, including mixed NaN/Inf float32/float64 rows and instrumented actual final retraining.
I re-executed both exact native cases from my prior review: the LightGBM sampled encoded-class-1 example now has 0/300 argmax-versus-predict mismatches instead of 300; the XGBoost NaN-base-margin case retains non-finite output rather than finite one-hot confidence. Random-forest one-column and higher-encoded multiclass controls also pass. I resolved only my two signed owned threads after these checks; the original Medium review evidence remains preserved.
The updated public docstring now explains the class-axis and raw-estimator distinction. Local execution used existing Python 3.10.19, NumPy 1.26.4, pandas 2.3.3, sklearn 1.7.2, LightGBM 4.6.0 and XGBoost 1.7.6, not the full supported matrix.
Approval is still withheld. Current Balanced Copilot recommends changes, with its sparse/skip-transform class-mapping concern awaiting follow-up. I did not resolve that other reviewer's thread. The clean result here covers the corrective delta, not blanket whole-PR clearance.
Actual current-head workflow API results: Build in_progress/no conclusion; Code formatting completed/success; docs completed/success. Workflow permission approval is not successful completion, and a green rollup alone is not sufficient.
Posted by thinkall-agent-auto-reviewer
Li Jiang (thinkall)
left a comment
There was a problem hiding this comment.
Changes requested after reviewing the new native-axis commit at exact head 8a7619ff03aba79d8838da1d891256c3fb6e3555 since 57e6f84e8c91f53a297f21e424bc1a5bb239f6be.
The sparse/skip-transform correction is useful and independently verified: paired actual LightGBM fits now retain all three public labels and matching probability columns for CSR input, disabled transformation and native string labels. The raw sampled estimator output remains unchanged. The full regression file passed 120 cases on Python 3.10, and 22 selected cases also passed on the existing Python 3.13/NumPy 2 stack: 142 passing executions, not 142 unique tests. All six review-code Medium lenses completed with actual grok-4.7 / xhigh; original outcomes and driver dispositions are preserved.
Two P2 blockers remain:
- New failed-refit class-axis mismatch, detailed inline: after an expected binary-metric validation error, the old trained estimator still returns its unchanged 120x2 probabilities, but public
classes_has switched to the rejected three-label dataset. Exact prior code keeps the old two-label axis. - Class metadata is requested before the Spark-output bypass. The existing Copilot concern is consistent with a paired controlled public-facade check: actual AutoML/BaseEstimator metadata properties return the original Spark-typed object on prior code, but current inference raises
AttributeErrorbefore the helper can bypass that output. This was a controlled facade using repository fallback Spark types, not native Spark training or save/load; PySpark is unavailable locally. I am referencing the existing inline rather than duplicating it or resolving another reviewer's thread.
Please bind retained class metadata to the successfully installed estimator, or restore/invalidate that pair on setup failure, and skip unsupported Spark metadata lookup before resolving labels. Lower-priority recommendations are to scope the class-axis documentation accurately and lock native string/CV/refit cache cases into repository regressions. Speculative naming, fittedness and unsupported-XGBoost-fixture claims were not promoted to blockers.
No contributor code was edited or dependencies installed. Local primary versions were Python 3.10.19/NumPy 1.26.4/LightGBM 4.6.0/XGBoost 1.7.6; the selected modern run used Python 3.13.5/NumPy 2.3.3/LightGBM 4.7.0/XGBoost 3.4.1. The complete supported matrix is not claimed. Passing CI cannot clear these source blockers.
Posted by thinkall-agent-auto-reviewer
There was a problem hiding this comment.
🔵 Needs a closer look
Cross-backend probability semantics and retained-model behavior warrant final human compatibility review, with the full backend and platform matrices not yet validated.
No open issues 🎉
1 resolved since last review
Review effort: Balanced
Li Jiang (thinkall)
left a comment
There was a problem hiding this comment.
Changes requested after a complete current-feature review at 592085573a760c9d8ba83f170c5d6043f6633678, not just the latest corrective delta.
The earlier native failed-refit axis and Spark-facade metadata-order findings are verified fixed. The complete current repository suite passed 142 cases on Python 3.10/NumPy 1.26/pandas 2.3; 44 selected repeat cases passed on Python 3.13/NumPy 2.3/pandas 3.0. These are 186 current-head executions, not 186 unique tests. Exact-prior production code fails ten of fourteen selected current regression controls, with four replay controls passing; all fourteen pass current. Spark cases remain controlled facades, not native Spark training/save-load.
Six independent review-code Medium lenses, all actual grok-4.7/xhigh, reviewed the whole feature. A separate native compatibility control confirms one remaining P2: a legacy serialized AutoML with a valid custom estimator class order [20, 10] keeps that public classes_ axis, but this feature silently sorts its probability columns to [10, 20]. A real custom RandomForest AutoML pickle created on exact main has 0/120 public-argmax/predict mismatches; loading the identical trusted local pickle under this head has 120/120. Details and the required order-preserving behavior are inline. This issue is in the whole feature, not a claim that the latest corrective commit alone introduced it.
There is also a smaller P3 cleanup case: a zero-budget log replay explicitly returns zero and installs an empty estimator but still exposes the old native _classes cache. Clear the cache on that replacement; this is not being described as a successful replay or valid probabilities from the empty estimator.
The broader default-transform rejected-refit state problem raised by several lenses was independently reproduced on exact main as well as prior/current code; it is not presented as a new transformer-state regression. Other backend/coverage recommendations remain qualified rather than being promoted without evidence. No contributor source or dependencies were changed, and the complete supported platform/backend matrix is not claimed.
Current Balanced Copilot review says Needs a closer look, not explicit Approval recommended. Neither its No open issues wording nor CI permission approval clears the source finding or the remaining approval gates.
Posted by thinkall-agent-auto-reviewer
Li Jiang (thinkall)
left a comment
There was a problem hiding this comment.
Reviewed the corrective update at exact head a36076dba938db49b01765c3733a50ab427410e7. The bounded review-code Low execution with actual gpt-6.1-sol is clean; this is a corrective review, not six renewed whole-feature votes or a full backend/platform compatibility claim.
The retained native reproducers confirm both earlier findings are fixed. The identical trusted main-created custom-RandomForest AutoML pickle improves from 120/120 public-argmax/predict mismatches on prior 59208557 to 0/120 current, while preserving the declared/raw [20, 10] class order. That owned legacy-order finding is now resolved. The explicit zero-budget empty-replay path also clears the old native class cache; this is metadata cleanup, not a successful replay or valid predictions from an empty estimator.
The complete current probability-label suite passed 148 cases on Python 3.10/NumPy 1.26/pandas 2.3; 50 selected repeat cases passed on Python 3.13/NumPy 2.3/pandas 3.0. These are 198 executions of 148 unique cases. All six added regressions fail on exact prior production and pass current. Earlier complete-feature evidence and its original findings/qualifications are retained; the current production diff was also inspected. Spark coverage remains controlled facade coverage, not native Spark training/save-load. No dependencies or contributor source were changed.
Current Balanced Copilot review 5676808336 says Needs a closer look, referring to an unpublished XGBoost raw-margin diagnostic concern without an inline. Targeted actual sampled AutoML fits on XGBoost 1.7.6 and 3.4.1 correctly align full training classes [0, 1, 2] from retained classes [0, 1]. Both native and AutoML predict_proba(..., output_margin=True) reject that unsupported option with TypeError before alignment, while direct Booster raw margins remain unchanged. That concrete path is not reproduced as a bug; unspecified unpublished/custom variants are not being declared tested or resolved.
No approval yet: No open issues wording is not an explicit Approval recommended verdict, and workflow permission approval is not successful completion. The explicit current-head Balanced recommendation and successful applicable actual workflow runs remain mandatory. No blanket whole-PR clean-cache clearance is inferred from this corrective result.
Posted by thinkall-agent-auto-reviewer


Why are these changes needed?
Holdout classification can finish with infinite log loss and no model when validation data is missing a training class.
Cache the full training class set for built-in holdout metrics and align sampled models' probabilities with it. For local tabular classification, public
predict_proba()columns followclasses_, including sparse inputs, disabled preprocessing, and single-class binary and multiclass boosted samples. Non-finite backend output, diagnostic outputs and Spark outputs are preserved. Custom metric callbacks retain their existing label handling.Regression tests cover missing classes, sampled predictions, failed refits and retraining. The
predict_proba()documentation explains class ordering, missing-class columns and differences from raw model output.Related issue number
None.
Validation
Focused regression and surrounding classification tests, plus repository pre-commit hooks. Full backend and platform matrices have not been run locally.
Checks
predict_proba()class ordering and sampled-model behavior.