Implement sample_weight instead of silently ignoring it - #24
Merged
Conversation
SymbolicRegressor.fit() accepted sample_weight, documented it as "not yet implemented", and dropped it on the floor. A user passing measurement variances got a plausible answer computed from the assumption they believed they had overridden. Closes #19. Weighted least squares is now applied consistently, since a partially weighted pipeline is worse than an unweighted one -- it looks right: - all four selection strategies, so weights steer which terms are chosen, not just their coefficients - the reported MSE, R2, and the AIC/BIC/AICc computed from them - constraint refitting, negligible-term pruning, and the profile-likelihood optimisation of parametric basis parameters - sigma_, covariance_matrix_, coefficient_intervals(), predict_interval(), confidence_band(), anova(), the bootstrap functions, cross_validate(), jackknife+ conformal prediction - the active-learning posterior in acquisition.py, which was scoring candidates against the unweighted (Phi^T Phi)^-1 The mechanism is whitening: WLS on (Phi, y) is OLS on (sqrt(w)*Phi, sqrt(w)*y), so the existing solvers, Gram-matrix fast paths and interval machinery stay exact rather than growing a parallel weighted branch. Constraint evaluation deliberately keeps the raw Phi -- a shape constraint is a property of the fitted function, not of how much a row was trusted. Effective-sample-size policy: weights are normalised to average 1, so only their ratios matter and the fit is invariant to their overall scale; the n used by the information criteria stays the nominal sample count. Weighting describes trust, it does not create or destroy observations -- which is exactly why duplicating rows is not an equivalent trick. The new effective_sample_size_ property reports the Kish ESS as a diagnostic for when the nominal n overstates the evidence. Invalid weights (wrong length, negative, non-finite, all-zero) now raise ValueError rather than being ignored. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JigoMqmadVXVs7ALgnLnvG
Resolves conflicts with the group-aware resampling and parametric-identity work that landed on main (#16, #17, #18, #20, #23). Two of the four conflicts were substantive rewrites of functions this branch also touched, so weighting was re-layered onto the new versions rather than either side being taken wholesale: - cross_validate: gained groups/strategy upstream. Weights now follow their rows into every fold under all three splitting strategies, and into the per-group scores. A group whose rows all carry zero weight scores NaN rather than 0.0, which would read as a perfect prediction; a fold with no weight on either side raises, since there is nothing to fit or score. - bootstrap_model_selection: gained groups/resample_fn and basis-identity keying upstream. Weights follow their rows through both the row and group resamples. Combining sample_weight with resample_fn is rejected: those replicates regenerate their own rows, so a stored weight has no row to belong to. Weighting and the resampling level are orthogonal choices -- a weight says how precise a row is, a group says which rows are not independent -- so both can be passed together. Tests cover that seam. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JigoMqmadVXVs7ALgnLnvG
test_fit_ols_matches_normal_equations compared a JAX float32 solve against a float64 NumPy reference at rtol=1e-5 and failed CI at 1.06e-5 -- on the one coefficient that landed near zero (0.0065), where a 7e-8 absolute error is a large relative one. That is rounding, not a defect in the weighting: the NumPy-backend job checks the same identity in float64 and passed. Both solver tests now use rtol=1e-4 with an atol, so a near-zero coefficient is judged on absolute error instead, and a comment records why the tolerance is what it is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JigoMqmadVXVs7ALgnLnvG
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
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.
Closes #19.
SymbolicRegressor.fit()acceptedsample_weight, documented it as "not yet implemented", and dropped it on the floor. A user passing measurement variances got a plausible answer computed from the assumption they believed they had overridden.The reproducer from the issue now gives:
Mechanism
WLS on
(Phi, y)is OLS on(sqrt(w)·Phi, sqrt(w)·y), so the weights are applied by whitening at each entry point rather than by growing a parallel weighted code path. The Gram-matrix fast paths, the LASSO screening, the SVD interval machinery and the closed-form MSE all stay exact and untouched.Two places deliberately keep the raw matrix:
Phi_newin prediction intervals — so the interval is for a new observation of unit weight (one as precise as an average training point).Where weights now apply
Everything the issue listed, plus a few things that would otherwise have been inconsistent in a way that is hard to see:
fit_constrained_ols), negligible-term pruning, and the profile-likelihood optimisation of parametric basis parameterssigma_,covariance_matrix_,coefficient_intervals(),predict_interval(),confidence_band(),anova(), the bootstrap functions,cross_validate(), jackknife+ conformal predictionacquisition.py, which was scoring candidates against the unweighted(PhiᵀPhi)⁻¹— it would have proposed the next experiment as if the down-weighted rows had been fully informative. The kriging-believer batch path also grew_X_trainwithout the matching weights.The parametric case is a good illustration of why partial coverage is not enough: unweighted, the test fixture collapses to a constant; weighted, it recovers
exp(-2*x).Effective-sample-size policy
Weights are normalised to average 1, so only their ratios matter and the fit is invariant to their overall scale —
w,2*wandw/1000give the same model, MSE and criteria. Thenused by the information criteria stays the nominal sample count: weighting describes how much each measurement is trusted, it does not create or destroy measurements. That is precisely why duplicating rows is not an equivalent trick — it inflatesnand shifts every criterion.New
SymbolicRegressor.effective_sample_size_reports the Kish ESS as a diagnostic for when the nominalnoverstates the evidence (100.0 for the 200-point reproducer).sample_weight_exposes the normalised weights.Invalid weights (wrong length, negative, non-finite, all-zero) now raise
ValueErrorrather than being ignored.API surface
sample_weightwas also added tofit_symbolic(),MultiOutputSymbolicRegressor.fit(),SymbolicRegressor.score(),fit_ols(),fit_ridge(),select_features()and the four strategy functions,cross_validate(),compute_mse/rmse/mae/r2/adjusted_r2/mape/all_metrics(),compute_cv_score(),compute_loo_mse(),compute_press(), andbootstrap_model_selection().SymbolicRegressor.update()gainedsample_weight_new.Deliberately not weighted
conformal_predict_split()— its coverage comes from exchangeability of the user-supplied calibration residuals, not from the fit. Documented, with a pointer tomethod="jackknife+"for a weight-aware interval.SymbolicClassifier— it never accepted the argument, so there is no silent-ignore bug there.max_errorincompute_all_metrics()— a maximum has no weighted analogue; it is now taken over the rows with non-zero weight instead of over rows that carry none.Docs
New
docs/guides/sample-weights.mdand the matching skill guide (src/jaxsr/skill/re-synced from.claude/skills/jaxsr/), covering weight semantics, the effective-sample-size policy, recipes for variance-derived and replicate weights, and what is left unweighted. Includes the smooth-taper recipew = y_x² / (y_x² + eps²)for the superposition case in #14, in place of a hard|y_x|threshold.Verification
scripts/test_under_numpy.py)black --checkandruff checkcleantests/test_sample_weight.py, built around the invariants that make weights trustworthy: uniform weights change nothing, scale invariance, a zero weight equals dropping the row, and integer weights match row duplication in the coefficients whilencorrectly does not move.🤖 Generated with Claude Code
https://claude.ai/code/session_01JigoMqmadVXVs7ALgnLnvG
Generated by Claude Code