fix(loo): derive the refit path's observed-variable name instead of hard-coding y_post - #441
Merged
Merged
Conversation
…ard-coding y_post The exact-refit repair read `idata.log_likelihood["y_post"]` in its alignment guards and in the held-out density evaluation. That name is a property of the likelihood, not a constant: the Beta-Binomial mechanism models register `y_post`, but the floor-rule likelihood registers `y_offfloor`. Registering an off-floor mechanism model — the primary estimand proposed in #433 — would have turned every guard into a KeyError raised at comparison time, after the fits had already run. Derive the name from the built model's single observed RV and verify it against the stored trace, so the failure mode is "this model is not supported", stated before any sampling, rather than a crash an hour in. Multi-outcome models are now refused explicitly: they carry one obs_id axis per outcome, so there is no well-defined single row for reloo to hold out. No behaviour change for the models that exist today — both live traces still resolve to `y_post` and pass every guard unchanged. Refs #433, #438 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Note
Drafted by an LLM-based AI tool (Claude Code/Opus 5).
What and why
The exact-refit (
reloo) repair added in #439 readidata.log_likelihood["y_post"]in its alignment guards and in the held-out density evaluation. That name is a property of the likelihood, not a constant. The Beta-Binomial mechanism models register their observed node asy_post, but the floor-rule likelihood registersy_offfloor(factories.py). Registering an off-floor mechanism model — which is the primary estimand proposed in #433 — would have turned all three sites into aKeyErrorraised at comparison time, after the reporting fits had already run.The point of the change is when the failure happens, not whether one can happen. Before: sample for an hour, then crash with
KeyError: 'y_post'in the comparison step. After: either it works, or it says "this model is not supported" before any sampling starts.The change
_observed_variable_name(model, idata_orig, model_id)derives the name from the built model's single observed RV and verifies it against the stored trace, replacing the hard-coded string at all three call sites. It refuses two cases explicitly rather than guessing:obs_idaxis per outcome, so there is no well-defined single row forrelooto hold out. Better to refuse than to splice against whichever outcome happens to sort first.log_likelihoodgroup that does not match the rebuilt model — either naming something else (construction drift) or holding more than one variable (no way to tell which the Pareto-k indices refer to).This follows the module's existing posture: every guard in
build_mechanism_wrapperfails closed with a reason, because a spliced exact elpd is only meaningful if the refit is provably the same model as the fit.Verification
tests/statistical_models/test_loo_refit.py. One of them closes the loop by assertinglog_likelihood__iactually reads the derived name rather than merely recording it — the guard and its consumer can drift apart, and a test on the guard alone would not have caught the original defect either.ruff check src/,npm run format:check,npm run spellcheckall clean.--config reportingtraces forlrp-rli-mech-063andlrp-rli-mech-104still resolvesobs_var='y_post'and passes every alignment guard unchanged.Note on scope
This is one of two prerequisites for the #433 off-floor mechanism model, and the smaller one.
build_mechanism_modelhas nolikelihoodparameter at all today, so the mechanism family is Beta-Binomial-only and off-floor support still needs plumbing into the factory before the refit path becomes reachable. That is left to #433 itself rather than built speculatively here.Branched off
mainafter #440 (d2b57c0) landed, which rewrites this same file's refit gate; the two changes are orthogonal and the full suite was re-run against the updated base.Refs #433, #438
🤖 Generated with Claude Code