Skip to content

fix: key bootstrap model selection by basis identity (#16) - #20

Merged
jkitchin merged 4 commits into
mainfrom
claude/jaxsr-issue-16-apu7ud
Aug 12, 2026
Merged

fix: key bootstrap model selection by basis identity (#16)#20
jkitchin merged 4 commits into
mainfrom
claude/jaxsr-issue-16-apu7ud

Conversation

@jkitchin

Copy link
Copy Markdown
Owner

Fixes #16.

The problem

bootstrap_model_selection keyed feature_frequencies by selected_features_, which for a parametric basis is the name with the fitted value substituted in. The nonlinear parameter is re-optimised in every replicate, so each replicate contributed a uniquely-named feature — nothing aggregated, and stability_score reported 0.0 even when every replicate had selected the same basis.

The fix

Key by basis identity. New BasisLibrary.canonical_name(index) and BasisLibrary.canonical_names return the name a basis was registered with ("exp(-a*x)") rather than the fitted rendering ("exp(-0.4913*x)"); non-parametric bases are unaffected. Both the frequency counts and the feature-set comparison behind stability_score now use it. Fitted values are reported the way a coefficient is — as a distribution over replicates — under a new "parameter_distributions" key, as the issue proposed.

On a reproducer of the WLF-style case from the issue (15 replicates):

feature_frequencies : {'q*y_x': 1.0, 'y_x/(c2+q)^2': 1.0, 'y_x': 0.33, '1': 0.067}
parameter_dists     : {'y_x/(c2+q)^2': {'c2': {'mean': 0.569, 'sd': 0.033, 'q05': 0.541, 'q95': 0.622, 'n': 15}}}
stability_score     : 0.6

Two related defects found in the same function

  • It mutated the caller's model. Every replicate was fitted on model.basis_library itself, and _resolve_parametric_params rebinds basis names and evaluation closures in place — so after the call the user's fitted model silently predicted with the last replicate's parameter values. Replicates now fit on a new BasisLibrary.copy(), with a regression test asserting the original model's predictions and names are unchanged.
  • The clone dropped constructor arguments. It enumerated eight of thirteen, losing param_optimizer, param_optimization_budget, prune_tol and the constraint settings — the first two govern exactly the parametric fits this issue is about. It now clones through get_params().

Also in this change

  • Input validation: unfitted template model, X/y length mismatch, n_bootstrap < 1.
  • New "n_successful" return key, so the denominator behind the reported frequencies is legible.
  • feature_frequencies is ordered most-selected first.
  • Tests: 10 for bootstrap_model_selection (aggregation, parameter distributions, non-mutation, reproducibility, validation) and 7 for the new BasisLibrary methods.
  • Docs: uncertainty guide section on parametric bases, re-synced into src/jaxsr/skill/; CHANGELOG under Unreleased.

Scope

The issue's cross-reference to #17 (what gets resampled) is untouched — that is a separate concern, and these frequencies still mean "fraction of pairs-bootstrap replicates", nothing stronger.

Checks

  • pytest tests/ — 638 passed, 10 skipped
  • black --check src/ tests/ and ruff check src/ tests/ — clean
  • scripts/test_under_numpy.py on the touched test files — 98 passed

🤖 Generated with Claude Code

https://claude.ai/code/session_01ARRqz2SjSoYH5N3t66HDjm


Generated by Claude Code

claude added 4 commits August 12, 2026 21:01
bootstrap_model_selection keyed feature_frequencies by the rendered basis
name. A parametric basis renders with its fitted parameter substituted in,
and that parameter is re-optimised in every replicate, so each replicate
produced a uniquely named feature: nothing aggregated, and stability_score
was 0.0 even when every replicate had selected the same basis.

Features are now keyed by basis identity — the name as registered with
add_parametric ("exp(-a*x)") rather than the fitted rendering
("exp(-0.4913*x)") — via the new BasisLibrary.canonical_name(). The fitted
values are reported the way a coefficient is, as a distribution over
replicates, under the new "parameter_distributions" key.

Two related defects in the same function:

- Every replicate was fitted on the caller's own basis library. Fitting a
  parametric library rebinds basis names and evaluation closures in place,
  so the call left the caller's model pinned to the last replicate's
  parameter values and silently changed its predictions. Replicates now fit
  on BasisLibrary.copy().
- Cloning the template model enumerated a subset of its constructor
  arguments, dropping param_optimizer, param_optimization_budget, prune_tol
  and the constraint settings. It now clones through get_params().

Also adds input validation, "n_successful" (the denominator of the reported
frequencies), and tests for all of the above.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARRqz2SjSoYH5N3t66HDjm
Resolves a conflict in tests/test_basis.py where both branches appended a
new test class at the end of the file; both are kept.

BasisLibrary.copy() now duplicates basis functions with dataclasses.replace
so it carries the `block` label that main's add_block() introduced, rather
than rebuilding each BasisFunction from an explicit field list that silently
drops fields added later.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARRqz2SjSoYH5N3t66HDjm
Main's #22 rewrote bootstrap_model_selection into a generator plus a public
summarize_selection_replicates, and added row/group/pipeline resampling.
This branch's fix is re-expressed inside that architecture rather than
alongside it:

- Canonical keying moves into _replicate_terms, so both the summariser and
  the bootstrap key model replicates by basis identity. Callers passing
  their own replicates (mappings, name sequences) are unaffected.
- Parameter distributions are accumulated in summarize_selection_replicates,
  so pipeline- and group-level replicates report them too, not just rows.
- The per-replicate library copy moves into _clone_estimator, which #22 made
  the single clone path. That also fixes MultiOutputSymbolicRegressor, whose
  per-output clones shared one parametric library.
- Dropped this branch's "n_successful" (main's n_replicates/n_failed say the
  same thing) and its unfitted-template check (main deliberately accepts an
  unfitted template and reports the modal structure instead).

test_clone_symbolic_regressor asserted the clone shares the template's
library; that identity is exactly what had to change, so it now asserts an
equivalent but independent library.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARRqz2SjSoYH5N3t66HDjm
Only CHANGELOG.md conflicted, where main's new derivatives entry and this
branch's entries both landed under Unreleased; both are kept.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARRqz2SjSoYH5N3t66HDjm
@jkitchin
jkitchin merged commit c45d7c2 into main Aug 12, 2026
7 checks passed
jkitchin pushed a commit that referenced this pull request Aug 12, 2026
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bootstrap_model_selection cannot aggregate parametric bases (stability score is meaningless)

2 participants