feat(compare): per-fit --exclude-chains (gh#417, gh#418)#420
Merged
Conversation
…et (gh#417, gh#418) `fit predict`/`fit summary --exclude-chains` (gh#409) can drop a known-stuck chain before banding, but `camdl compare` had no such flag: its plug-in θ̂ was always the posterior MEAN over ALL chains. So a fit with a stuck chain (R̂≈140, one chain at 0% acceptance) could not be scored on its good chains — `predict` could drop them, `compare` could not — letting the two disagree about which draws a report used. `compare` now takes `--exclude-chains`, resolved PER FIT and bound to each model's name: camdl compare @A @b --exclude-chains @A:4 # drop chain 4 from @A only camdl compare a.toml b.toml --exclude-chains 3 # bare ids: cohort-wide `@fit:ids` targets one fit and leaves every other whole; a bare `ids` token applies cohort-wide (retained as a shorthand for the case where the compared fits share a stuck-chain index). Mixing the two forms, an unknown/ambiguous fit name, or a fit listed twice is a hard error, so a token can never silently bind nowhere or to the wrong fit. The read-side chain-selection proposal (2026-07-09) deferred `compare` pending exactly this per-fit syntax — the cohort-wide form over-excludes when the compared fits have DIFFERENT stuck chains — so this lands the proposal's stated prerequisite rather than deviating from it. Chain exclusion is EXPLICIT-only by design: the user names the chains to drop, per fit. There is deliberately no auto-detect/auto-drop mode — post-hoc exclusion biases the posterior toward the retained mode, so that decision must stay visible and auditable, never buried behind a flag. θ̂ derivation routes through the shared draws authority (`PosteriorDrawsRef::with_selection(...).load_params_with_info()`), so the mean is taken over the RETAINED cloud via the same `apply_keyed` filter predict/summary use. This also folds out the second, independent raw `draws.tsv` reader that `posterior_mean_params_toml` carried. `warn_requested`'s wording is generalized ("from each fit's posterior cloud"); a new `warn_requested_for_fit` names the targeted fit for the per-fit form, sharing the one bias caveat. Optimizer fits and explicit `prequential.json` inputs have no cloud, so the selection is inert there. Tests (red → green): - Unit `posterior_mean_excludes_selected_chains`: red was a compile failure (`posterior_mean_params_toml` took `&Path`, no selection); green after routing through the draws ref. All-chain θ̂ pools both chains (beta=5.0), subset θ̂ drops the stuck one (beta=1.0). - Unit `parse_cohort_exclude_forms` / `_rejects_bad_forms`: per-fit vs cohort binding; unknown/ambiguous/duplicate/mixed → hard error. - E2e `compare_per_fit_exclude_chains_rescores_only_the_named_fit`: injects a stuck chain into @A's cloud; `--exclude-chains @A:2` moves @A's derived prequential elpd while @b's is byte-identical (per-fit binding, not cohort-wide), and the warning names fit @A. compare + chain_selection unit: 14 passed compare_auto_prequential (incl. new e2e): 4 passed exclude_chains_e2e: 11 passed; fit_table_quantities: 3 passed
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.
What
Adds
camdl compare --exclude-chains, resolved per fit and bound to each model's name, so a comparison can score a fit over the same chain subset apredict/summarywould band (gh#409).Why
fit predict/fit summary --exclude-chains(gh#409) can drop a known-stuck chain before banding, butcompare's plug-in θ̂ was always the posterior mean over all chains. A fit with a stuck chain (R̂≈140, one chain at 0% acceptance) couldn't be scored on its good chains —predictcould drop them,comparecouldn't — letting a report's bands and scores disagree about which draws were used.Design
@fit:idstargets one fit and leaves every other whole; the fit name is the string the table shows and--baselinematches. Bareidsstay as a cohort-wide shorthand (fits sharing a stuck-chain index).@fit:ids, an unknown/ambiguous fit name, or a fit listed twice is a hard error — a token can never silently bind nowhere or to the wrong fit.PosteriorDrawsRef::with_selection(...).load_params_with_info()(the sameapply_keyedfilter predict/summary use), which also folds outcompare's second, independent rawdraws.tsvreader.comparepending exactly this per-fit syntax.Tests (red → green)
posterior_mean_excludes_selected_chains— all-chain θ̂ pools both chains (β=5.0), subset θ̂ drops the stuck one (β=1.0).parse_cohort_exclude_forms/_rejects_bad_forms— per-fit vs cohort binding; unknown/ambiguous/duplicate/mixed → hard error.compare_per_fit_exclude_chains_rescores_only_the_named_fit— injects a stuck chain into @A;--exclude-chains @a:2moves @A's prequential elpd while @b is byte-identical (per-fit, not cohort-wide), warning names @A.Local: compare+chain_selection unit 14, compare_auto_prequential 4, exclude_chains_e2e 11, fit_table_quantities 3 — all green.
Closes #417, closes #418.