Warm-start coverage honesty + genuinely cold MC-error replicas (PR 161 blockers 1 and 2) - #47
Conversation
…as actually cold
Addresses the two open blockers on upstream PR 161.
[1] cover_frac is NOT a coverage guarantee, and the docstring said it was. A
FINITE set of uniform points occupies only the bins it lands in, so a seeded
grid is not a superset of a cold start. Measured fraction of the [-5,5]^d prior
box covered (cold = 1.0):
cover_frac: 0.0 0.2 0.5 0.9
d=2 0.027 0.634 0.982 1.000
d=4 0.0015 0.028 0.104 0.620
d=6 6.3e-05 0.00087 0.0033 0.0287
At d=6 even cover_frac=0.9 leaves 97% of the box unsampled, so "a warm-started
integral can never be MORE biased than a cold one" was false. Docstring
corrected with the measurements.
A portfolio warm start also seeded EVERY member, narrowing the backstop to
V=0.0033 along with the rest. bootstrap_from_samples now keeps member 0 cold
(portfolio_warmstart_backstop_cold, default True; keep_backstop_cold=False
restores the old behaviour).
Be precise about what that buys -- the measurements are not what one expects:
* With a CORRECT seed it is nearly free (d=4 n_eff 3630/3466/5695 cold-backstop
vs 3876/3461/5621 seeded; d=6 5078/5689/5163 vs 5857/5387/5266).
* It is NOT what protects the default AV+GMM portfolio. The GMM member has
nonzero density over the whole box, so q_mix never vanishes there: with a
deliberately displaced seed, |lnZ bias| <= 0.05 in every d=4/d=6 run in BOTH
arms. That unbounded support is the real, previously undocumented reason
production warm starts have not biased.
* It does NOT rescue a badly mismatched seed. All-AV portfolio, displaced seed,
d=6: cold backstop -1.1 to -4.2 nats (n_eff 3-9) vs -1.0 to -6.8 seeded. A
uniform member finds a sharp 6-D peak too rarely to carry the integral within
budget. Coverage in principle is necessary, not sufficient.
[2] MC-error replicas were not adaptation-independent for a portfolio. Neither
mcsamplerPortfolio, AV nor Ensemble defines reset_sampling (only the AC/GPU
sampler does), so the replica loop's reset was a no-op for them, and because
portfolio.integrate_log does not call setup() each replica reran on the previous
replica's contracted grid and fitted GMM. Replicas that share the adaptation
whose failure they exist to detect understate the MC error. The loop now calls
clear_warm_state() first. Measured AV live volume across 3 replicas:
before: 2.16e-07 -> 4.08e-08 -> 1.42e-08 (monotone contraction; not independent)
after: 2.16e-07 -> 2.17e-07 -> 2.04e-07 (each contracts independently)
Standalone AV was already cold -- its integrate_log calls setup() itself.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…port The merge gate rejected the previous commit, correctly. Holding member 0 cold unconditionally disables the AV warm start in the default [AV, GMM] portfolio -- member 0 IS the AV member -- and the gate's portfolio_warm case reported exactly that: "warm seed NOT installed on the draw path: AV member V=1.000, live bins=1". Three blocking regressions, all self-inflicted. The invariant was stated wrongly. It is not "member 0 must be cold" but "SOME member must have support everywhere". A GMM/ensemble member satisfies that inherently: it carries an explicit uniform defensive component (gmm_defensive_frac, default 0.05) plus Gaussian tails, so q_mix never vanishes however it is seeded. That matches the measurement -- in [AV, GMM] a displaced seed left |lnZ bias| <= 0.05 whether or not a member was held cold, while an ALL-AV portfolio gave -1.0 to -6.8 nats. So AV is now marked has_unbounded_support = False, and bootstrap_from_samples holds a member cold only when no member reports unbounded support. In [AV, GMM] nothing changes; in an all-AV portfolio member 0 stays cold. Two tests pin both directions, including the regression the gate caught. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ptation for replicas Both PR #47 findings. [P1] Full-support detection could discard the only real backstop. The check defaulted un-annotated samplers to full-support and counted a nominally broad member even after it had been RANGE-RESTRICTED. Reproduced: [unrestricted AV, restricted GMM] reports _full_support_members == [0], yet the restricted GMM set _has_broad, so member 0 was warm-started and contracted to V=0.095 -- nothing covering the prior box, which is the silent low bias this mechanism exists to prevent. Now: has_unbounded_support defaults FALSE and must be declared; only members that are BOTH declared broad AND still full-range (per _full_support_members) count. mcsamplerEnsemble declares it as a property keyed on gmm_defensive_frac > 0 -- the uniform defensive component is the actual guarantee, since Gaussian tails underflow to exactly zero far from the mode, so gmm_defensive_frac=0 correctly reports False. AV declares False. [P1] Replicas retained portfolio-level adaptation. clear_warm_state() rebuilds the members but not the portfolio's own learned state: portfolio_weights, portfolio_quality and its observation counts, portfolio_probe_ptr, portfolio_draw_iteration, breakpoint progression and per-member n_ess histories. Replicas therefore scheduled themselves from what earlier replicas learned, so the between-replica scatter -- the entire quantity being measured -- still understated the error. Adds reset_adaptation(), which does clear_warm_state() plus a restore of every field to its POST-SETUP value (snapshotted in setup, so an explicit initial weighting or breakpoint schedule is preserved rather than replaced by a hard-coded guess). The replica loop calls it. Four tests, each verified to fail when its defect is reintroduced, including the requested assertion that all portfolio-level state is identical across replicas. Noted while testing, NOT fixed here: mcsamplerEnsemble.bootstrap_from_samples rebuilds its integrator without the caller's gmm_defensive_frac, so a warm start silently restores the 0.05 default. Same class as the gmm_dict config loss fixed in #45; filed separately rather than widened into this PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Both findings fixed in [P1] Full-support detection could discard the only backstop — fixedReproduced your exact case. Both of your requirements implemented:
[P1] Replicas retained portfolio-level adaptation — fixedYou listed the fields precisely. Adds Includes the test you asked for: all portfolio-level state identical across replicas. Verification19 tests; the four added this round each verified to fail when its defect is reintroduced: GateAll portfolio rows PASS; flag-ON probe 0 regressions. One blocking row was reported — Worth noting for its own sake: that cell starves in 4 of 5 seeds, so its PASS at the default seed is the lucky draw and it has been a coin-flip gate on every branch. Tooling and the case for re-budgeting it are in #49; I have not changed the strict set or budgets here. Flagged, not fixed here
|
…ore claiming coverage PR #47 follow-up. has_unbounded_support read gmm_defensive_frac > 0 as a guarantee, but add_defensive_component() was only called by fit_gmm_adaptive. The fixed-component fit paths (mcsamplerEnsemble and MonteCarloEnsemble) called GMM.gmm(...).fit(...) directly, and gmm_adaptive defaults to None -- so the DEFAULT configuration requested a defensive component and never installed one, and the portfolio then contracted its AV member believing it was covered. Worse than a missing component: gmm.score() FLOORS its return at 1e-300, so the member always LOOKS like it has density everywhere. That floor is a guard against log(0), not coverage -- a sample landing there carries weight L*p/q ~ 1e300 and would wreck the estimate rather than support it. Measured, a fixed-component fit to a tight cloud returns exactly the floor at the far corner of the prior box for every d >= 4: d=2 2.9e-273 -> 1.3e-04 (with the defensive component) d=6 1.0e-300 -> 7.6e-10 This also corrects a claim I made in PR #47: "the GMM member's unbounded support is what has protected production". It is not unbounded support, it is a numerical floor. The measured |lnZ bias| <= 0.05 with a displaced seed means the far region was never sampled in those runs, not that it was covered. Fixes: add_defensive_component sets model.defensive_frac as a verifiable marker; both fixed-component fit paths now install it; has_unbounded_support inspects the installed models and reports False if ANY trained group lacks it, or if gmm_defensive_frac <= 0. While untrained it trusts the config -- sound only because every fit path now installs the component, which test_every_fit_path_installs_the_defensive_component pins. Note this changes the DEFAULT GMM proposal: fixed-component fits now carry a 5% defensive component (the documented gmm_defensive_frac default, previously inert). That is the intended behaviour per its own docstring and it bounds the importance weights, but it is a shared-sampler change -- gate results reported separately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Status + handoff — a decision is needed before this mergesDo not merge this PR until the question at the bottom is answered. The fixes are done and tested, but the last commit changes a shared sampler's default behaviour, and that is not mine to decide unilaterally. The remaining P1 is fixed, and it was worse than reportedConfirmed exactly as described: Additionally:
This retracts a claim I made earlier in this PR. I wrote that "the GMM member's unbounded support is what has protected production." It is not unbounded support, it is a numerical floor. The What
|
…pay for it
The previous commit installed it on every fixed-component fit, changing the
DEFAULT proposal for all users. The shape gate priced that and it is too
expensive: a 5% broad component spends 5% of draws where the likelihood is
negligible, and n_eff falls systematically at higher dimension --
GMM d6_n1_s101 2036 -> 1565 d6_n3_s202 39 -> 15
GMM d6_n3_s303 119 -> 75 * d8_n1_s303 448 -> 210 *
(* blocking regressions; d<=4 rows were unchanged within scatter)
So the install is now behind `gmm_defensive_all_paths`, default False.
mcsamplerPortfolio sets it on its own members in setup(), because a portfolio
member may be the mixture's ONLY full-support component and the guarantee has to
be real there. A standalone GMM user gets exactly the previous behaviour and
none of the cost.
has_unbounded_support now also returns False when neither install path is active
(no opt-in and gmm_adaptive off), so it cannot promise coverage that the
configuration will not build.
Verified: standalone GMM reports all_paths=False and declares no coverage;
a portfolio member reports True and declares coverage. 20 unit tests pass.
Gate re-run for this shape is pending -- the numbers above are from the
default-changing variant, which this commit supersedes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The gate answered the question I asked — the global default change is too expensive
A 5% broad component spends 5% of draws where the likelihood is negligible; at d>=6 that is a real efficiency loss. So the answer to the decision I posted above is no — do not change the shared default. Superseded by
|
| commit | contents | gate |
|---|---|---|
e459ec03 |
hold a member cold only when every member has compact support | green |
26dfcf98 |
declare full-support explicitly; reset portfolio-level adaptation | green |
34d3e770 |
defensive component on all paths (global default change) | 2 blocking |
e17a9c4 |
same, made opt-in and portfolio-scoped | pending |
Also still open, unchanged: mcsamplerEnsemble.bootstrap_from_samples rebuilds its integrator without the caller's gmm_defensive_frac (and now without gmm_defensive_all_paths), so a warm start can silently drop both. Same class as the gmm_dict loss fixed in #45. It interacts directly with the capability check, so it should be the next thing done.
Gate result for
|
| row | base | candidate | previous (global) variant |
|---|---|---|---|
| d6_n1_s101 | 2036 | 2036 | 1565 |
| d6_n3_s202 | 39 | 39 | 15 |
| d6_n3_s303 | 119 | 119 | 75 (blocked) |
| d8_n1_s303 | 448 | 448 | 210 (blocked) |
The single exception is d8_n3_s303 (26 vs 21) — both far below the n_eff=100 floor, BOTH-STARVED, non-blocking. I am not claiming perfect identity: that row is consistent with the worker-pool context sensitivity documented in #49, not with a code path difference, but I have not proven that and it does not affect the verdict.
All warm/sequential rows PASS, so the portfolio member still gets its coverage guarantee:
portfolio_warm mix_d6_n1_s101/202/303 n_eff 3159 / 3368 / 5707 PASS
portfolio_seq_nobs mix_d2_n1_s101/202/303 n_eff 2032 / 1603 / 811 PASS
portfolio_seq mix_d2_n1_s101_o+2.00 n_eff 6326 PASS
AV_seq mix_d2_n1_s101_o+2.00 n_eff 2432 PASS
Summary of the whole PR
| commit | contents | gate |
|---|---|---|
e459ec03 |
hold a member cold only when every member has compact support | green |
26dfcf98 |
declare full-support explicitly; reset portfolio-level adaptation | green |
34d3e770 |
defensive component on all paths (global default change) | 2 blocking — superseded |
e17a9c4 |
same, opt-in and portfolio-scoped | green |
All four review findings across three rounds are addressed, each with a regression test verified to fail when its defect is reintroduced. 20 portfolio unit tests + 6 gate tests pass.
34d3e770 is left in history rather than squashed away: it is the commit whose gate numbers justify the opt-in design, and losing that evidence would make the current shape look arbitrary.
Still open (not started)
mcsamplerEnsemble.bootstrap_from_samples rebuilds its integrator without the caller's gmm_defensive_frac or the new gmm_defensive_all_paths, so a warm start can silently drop both — which would quietly revoke the coverage guarantee this PR establishes. Same class as the gmm_dict configuration loss fixed in #45. This is the natural next task.
…led reruns, and is wired in Three P1s from review of #49. [P1] The confirm step recognised only PASS -> non-PASS. The comparator also blocks REGRESSION(metrics) -- JS, pull, width, correlation, evidence bias or n_eff worsening beyond tolerance -- and those rows produced "no blocking regressions to confirm" and exit 0. Our own gate v11 had exactly such a row (GMM d8_n1_s303, n_eff 448->210), which would have been waved through. Fixed structurally rather than by adding a second branch: compare_shape_results now exposes classify() / is_blocking() / blocking_keys() as the SINGLE definition of a regression, and confirm_regressions imports them. Two copies of that logic will always drift apart; there is now one. Verified the refactor reproduces the v11 verdict exactly (2 blocking). The confirm step now sees both rows where it previously saw one. [P1] Failed reruns were silently skipped, so worse == same == 0 read as "not confirmed". Now: a candidate that produces no record where the base did counts AGAINST the candidate (crashing is worse than passing, not missing evidence); a verdict requires --min-valid usable pairs, defaulting to all seeds; and too few valid pairs is INCONCLUSIVE with a nonzero exit, never a silent clear. [P1] Confirmation was documented but never invoked. compare_shape_results gains --confirm-base-checkout / --confirm-cand-checkout / --confirm-repeats and returns the confirmed verdict as its exit code, so the comparison workflow enforces it. Without those flags it still exits 1 on a blocking row, and now says explicitly that the row was NOT confirmed rather than implying it was. Adds test_confirm_regressions.py (5 checks, all on the dangerous direction -- the ways a confirmation can wrongly CLEAR a real regression). CI on this PR is red at "Set up job" on 4 jobs; the same runner-provisioning failure hits #47, which shares no files with this change, and no failing job reaches a step that executes repository code. Infrastructural, not from here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ust at setup Two PR #47 findings: the defensive component was installed correctly and then lost again during normal operation. [P1] Warm bootstrap disabled the portfolio's opt-in. bootstrap_from_samples() calls setup() to rebuild the integrator as one full-dim group, and did so bare -- resetting gmm_defensive_all_paths to False and refitting the warm GMM with no defensive component, AFTER the portfolio had already decided on the strength of that flag that it was safe to contract its AV member. It now carries the defensive config forward from the existing integrator. [P1] Updates absorbed the defensive component. _merge() blends component i with the fitted component order[i] for every i in range(self.k) and never consults self.adapt, so the broad component -- marked adapt=False precisely so it would be left alone -- drifted toward the fitted cloud on every update while defensive_frac stayed set and has_unbounded_support kept reporting coverage. Measured, far-field density fell 3.09e-07 -> 2.04e-08 after a single update cycle. update() now detaches the defensive component, updates the real ones and reinstates it; density is constant at 3.09e-07 across 4 updates with the component's weight, mean and covariance unchanged. The tests previously only checked initial installation. The two added here exercise the warm-bootstrap path and repeated updates, and assert on the actual far-field density and on the defensive component's own parameters rather than on the marker attribute -- the marker is exactly what stayed true while the guarantee evaporated. Both verified to fail when their defect is reintroduced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Conflict was structural, not semantic: both sides added a method immediately before bootstrap_from_samples, and the two bootstrap signatures differed. Resolution keeps BOTH bodies -- the merged diagnostic and reset_adaptation -- with the single signature that carries keep_backstop_cold. Verified after resolution: 22 portfolio tests and all 5 of the diagnostic's own tests pass.
|
Both lifecycle findings fixed, base conflict resolved, gate clean. One probe result needs explaining, and it is not from this branch. [P1] Warm bootstrap disabled the portfolio's defensive opt-in — fixed
[P1] Updates absorbed the defensive component — fixedConfirmed with the drift you predicted. Measured far-field density at the box corner, d=4:
TestsYour point that the tests only covered installation was right, and it is exactly why this survived. The two added here exercise the warm-bootstrap path and repeated updates, and assert on the actual far-field density and on the defensive component's own parameters -- deliberately not on the Base conflict — resolvedConflicted with #48 (escaped-mass diagnostic), which landed on Gate
The flag-ON probe reported 1 opt-in regression ( I also tested and discarded my own first explanation: I suspected the now-persistent defensive component was costing efficiency, but forcing the opt-in on gave n_eff 116 versus 36 with it off -- it helps there, not hurts. So this is the same near-threshold realization sensitivity documented in #49, now showing up in the probe. The probe has no confirm-on-fail step; on this evidence it needs one, and I would rather add that than let a coin-flip row sit against this PR. Filed as follow-up rather than widened into this branch. |
…ropped Review of #70 (P3): the previous commit narrowed "AV passes at every budget" to "all 8 seeds at each of the three budgets measured" in FOLLOWUPS item 7, but left the duplicate of that same claim in the CELL_BUDGET_MULT comment reading "at every budget". Now "at each measured budget". Which is, precisely, the defect this PR's item 2 fix and the whole #47/#51/#55 series keep finding: one fact with two representations, the secondary copy goes stale, both plausible read in isolation. Committing it while editing the file whose purpose is to warn about it is the joke telling itself. Swept the rest of the suite for the same shape rather than fixing only the reported line. One other latent instance: item 5's "PASS is 1/8 at every budget from x2 up" could be read as covering untested budgets above x32; now "every measured budget from x2 up". The remaining hits are correctly scoped already -- "no budget tested up to x32 ... at every seed" and "passes 8/8 at every budget measured". Reflowed the comment to the file's ~100 col convention. No numbers, decision or behaviour changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Addresses the two remaining PR 161 blockers (
cover_fraccoverage, MC-error replica independence). Stacked on the merged #45.[1]
cover_fracdoes not provide full-prior support — confirmedA FINITE set of uniform points occupies only the bins it lands in, so a seeded grid is not a superset of a cold start. Fraction of the
[-5,5]^dprior box actually covered (cold = 1.0):At d=6 even
cover_frac=0.9leaves 97% of the box unsampled, so the docstring's claim that a warm-started integral "can never be MORE biased than a cold one" was false. Corrected in place with these numbers.What actually protects production, which was undocumented. A portfolio warm start also narrowed the backstop member (V=1 -> 0.0033). I fixed that and then measured — and could NOT reproduce a bias in the default
[AV, GMM]portfolio either way: with a deliberately displaced seed,|lnZ bias| <= 0.05in every d=4/d=6 run in both arms. The reason is the GMM member's explicit uniform defensive component (gmm_defensive_frac, default 0.05) plus Gaussian tails, soq_mixnever vanishes. In an ALL-AV portfolio, where every component is a hard-edged box, the same seed gives -1.0 to -6.8 nats.So the invariant is "some member has support everywhere", not "member 0 is cold". AV is marked
has_unbounded_support = False; a member is held cold only when every member has compact support.[AV, GMM]is unchanged.Be clear about the limits. The cold backstop is cheap insurance, not a rescue: in the all-AV test it still gave -1.1 to -4.2 nats (n_eff 3-9), because a uniform member finds a sharp 6-D peak too rarely to carry the integral within budget. Coverage in principle is necessary, not sufficient. Detection of a mismatched seed is separate work, in progress.
[2] MC-error replicas were not adaptation-independent — fixed
Neither
mcsamplerPortfolio,mcsamplerAdaptiveVolumenormcsamplerEnsembledefinesreset_sampling(only the AC/GPU sampler does), so the replica loop's reset was a no-op for them; and sinceportfolio.integrate_logdoes not callsetup(), each replica reran on the previous replica's contracted grid and fitted GMM. Replicas that share the adaptation whose failure they exist to detect understate the MC error. The loop now callsclear_warm_state()first. AV live volume across 3 replicas:Standalone AV was already cold, as its comment claimed — its
integrate_logcallssetup().Gate
Shape gate vs
rift_O4d@87d66b75:COMPARE_EXIT=0, 0 blocking regressions; flag-ON probe 0 regressions. 15 portfolio unit tests + 6 gate tests pass.An earlier revision of this branch was rejected by the gate for disabling the AV warm start in
[AV, GMM](3 blockingportfolio_warmregressions, readingAV member V=1.000, live bins=1). That is what drove the corrected invariant above, and both directions are now pinned by tests.Unrelated gate defect noticed
GMM mix_d6_n3_s303is order-dependent: 66 / 119 / 104 across runs of the same unchanged base checkout (full matrix vs isolated), sitting right on then_eff = 100starvation floor. Not caused by this branch (nothing here touchesmcsamplerEnsemble), but it can flip a blocking verdict at random and should be re-budgeted or dropped from the strict set. Filed separately rather than fixed here.