feat: cross-validated accuracy for surrogates (fit_surrogate/RegimeSurrogate) - #117
Merged
Conversation
Neither backend reports how well it actually fits: RF has a free OOB score but GP doesn't have an equivalent, so fit_surrogate() now computes a uniform k-fold cross-validated R^2/RMSE per observable for both backends (RF's OOB is skipped in favor of consistency -- a directly comparable metric across backends matters more here than the extra fit cost). cv_folds is clamped to the observable's available row count; a UserWarning fires (opt-out via warn_below_r2=None) when any observable's CV R^2 falls below a threshold (default 0.0 -- no better than predicting the training mean). Motivating case: VBPCApy's Option A pipeline fits a RegimeSurrogate on ~1200 single-evaluation rows and queries it for point recommendations across dozens of regimes with no way to tell a genuinely-learned response surface from one that's overfit to noise.
) RegimeSurrogate wraps a SurrogateModel (previous commit) but callers shouldn't need to reach into .inner for the accuracy check -- cv_r2/ cv_rmse are now passthrough properties, and fit_regime_surrogate() forwards cv_folds/warn_below_r2 to the underlying fit_surrogate() call. recommend() also checks the specific objective's cv_r2 at call time and warns there too (opt-out via warn_below_r2=None), so a caller optimizing against a poorly-fit objective gets a signal right at the point of use rather than only in fit-time logs that are easy to miss in a pipeline. Closes #114
6 tasks
…ores CI's freshly-resolved numpy (2.5.2, vs the stale 2.4.4 my local env had pinned) caught a real strict-mypy mismatch local mypy missed: ResultsTable.scores is NDArray[np.floating[Any]], not float64 specifically, so y[mask] doesn't satisfy a float64-typed parameter. _cross_val_accuracy never actually needed float64 -- any floating dtype works for KFold/estimator fit/predict/arithmetic -- so widen the hint to match what's really passed instead of narrowing the caller.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Neither
SurrogateModelbackend reports how well it actually fits: RF has a free OOB score available but GP doesn't have an equivalent, so consumers currently have no way to distinguish "the surrogate genuinely learned the response surface" from "the surrogate is overfit to noise and itsrecommend()output is arbitrary" -- exactly the problem #114 describes for VBPCApy's Option A pipeline (fits aRegimeSurrogateon ~1200 single-evaluation rows, queries it for point recommendations across dozens of regimes, with no accuracy check anywhere).Design
fit_surrogate()now computes a uniform k-fold cross-validated R^2/RMSE per observable for both thegpandrfbackends (rather than RF-only OOB), so the metric is directly comparable across backends.cv_folds(default 5) is clamped to the observable's available row count; CV is skipped (nan) below 2 folds.UserWarningfires at fit time (warn_below_r2, default0.0-- "no better than predicting the training mean") naming any observable that falls below threshold. PassNoneto disable.SurrogateModel.cv_r2/.cv_rmse: new per-observable dict fields.RegimeSurrogate.cv_r2/.cv_rmse: passthrough properties (no need to reach into.inner).fit_regime_surrogate()forwardscv_folds/warn_below_r2.RegimeSurrogate.recommend()additionally checks the specificobjectivebeing optimized at call time and warns there too (warn_below_r2param, same default) -- per the issue's ask that this be "surfaced prominently enough that callers can't easily ignore it," a fit-time warning buried in pipeline logs isn't enough on its own.Test plan
just cigreen locally (ruff format/lint, mypy --strict, coverage 99.49%, 332 passed -- 25 new)cv_r2/cv_rmsepresent and finite for both backends on a well-fit deterministic relationship (R^2 > 0.8)cv_foldsclamped correctly for small data;cv_folds=1disables CV (nan, no spurious warning)warn_below_r2=Nonedisables itRegimeSurrogate.cv_r2/.cv_rmsematch.inner.cv_r2/.inner.cv_rmseexactlyrecommend()warns on a poorly-fit objective at call time, independent of the fit-time warning;warn_below_r2=Nonedisables it