helper: SNR-scaled chunk + cubic time interpolation; portfolio interval-narrowing plumbing; truth-known SNR study - #45
Conversation
…/0.80, wrong direction) band3 sd went 0.85 (n=4) -> 3.11 (n=9) -> 5.02 (n=20) vs baseline 4.02: selection at n=4 followed by regression to the mean. F-test vs baseline is non-significant and favours baseline. Retracts any claim that the VARAHA share constraint or reduced GMM cap improves consistency; they stay opt-in, off by default, documented as unproven. Still stands: n_eff does not certify correctness; the allocation rule degenerates to peaked-member-only unless constrained; the cold-start crash and L0-rescue gaps were real and are fixed. Gate CLEARED: default-path compare exit 0 / 0 blocking regressions; flag-ON probe 0 opt-in regressions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…U-only)
Tests the reviewer hypothesis that part of the high-SNR collapse is the chunk being too small: at
high SNR the posterior is a vanishing fraction of the prior volume, so a small chunk carries few
informative samples per adaptation step.
Design choices that answer the criticism of the real-event work:
* TRUTH IS KNOWN. Reuses the merge gate's MixtureTarget (true_lnZ), so we measure real BIAS
(lnI - true_lnZ) and a PASS/FAIL/STARVED verdict, not scatter about an unknown answer. Scatter
about an unknown answer is what let a 4-copy result look decisive and then evaporate at n=20.
* CPU-ONLY, so copies are cheap (24/cell here vs 4-9 on GPU). Statistical power was the binding
constraint on every previous conclusion in this study.
* SNR ladder = peak width: sigma_1d = 0.7*(20/SNR), anchored at the gate's own default, since
posterior width ~ 1/SNR on the fixed [-5,5]^d box.
TWO designs, because they answer different questions and the task flagged the confound explicitly:
A) FIXED BUDGET -- same nmax for every chunk, so steps = nmax/n_chunk FALLS as the chunk grows.
The production question: at the cost I already pay, should the chunk be bigger?
B) FIXED STEPS -- nmax = n_chunk*steps, equal adaptation opportunities, cost grows with chunk.
The mechanism question: do richer per-step statistics help independent of step count?
measure_chunk_memory.sh records peak GPU memory and wall time per chunk on the REAL ILE likelihood,
so the statistics can be read against the resource cost that decides whether any of this is usable
(bigger chunks match fewer slots -> held jobs and hand-tuned RequestMemory).
Smoke: at SNR 80 with EQUAL steps, chunk 10k collapsed 100% while chunk 40k passed 0%.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Implements the design-report recommendation for multiple AV instances with different ranges: a PER-MEMBER draw floor (not the existing per-GROUP VARAHA band), so a designated full-support member is guaranteed >=1 draw every chunk and cannot be rounded out of q_mix by integer allocation. NOT YET VALIDATED -- no truth-known ladder, no gate, no probe. Committed only so the tree is clean for merging upstream. Do not rely on this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…dently-wrong runs? The MC-error-estimate work (PR #38) added statutils.pareto_khat_from_log with the reading k>0.7 => weight tail unresolved, naive sigma is a lower bound. That is aimed squarely at the failure this study measured: the copy with the HIGHEST n_eff in its arm was the MOST biased (n_eff 58 -> 11 nats low; n_eff 123.6 -> 10 nats low), because n_eff measures weight CONCENTRATION, not COVERAGE. This harness scores k-hat as a binary detector of 'materially wrong' against KNOWN truth (shape_recovery.MixtureTarget.true_lnZ), reporting sensitivity/false-alarm head-to-head with n_eff, and isolating the decisive subset: runs n_eff would have PASSED that are actually wrong. Validating a diagnostic on REAL failures rather than synthetic GPD draws is only possible because truth is known here -- a real event cannot score it. Captures the importance log-weights by wrapping shape_recovery.shape_metrics rather than editing the gate: run_one currently discards dict_return, so the sampler-emitted mc_diag['pareto_khat'] never reaches the record. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…the purest confidently-wrong case yet A_sub (correct sub-box) is SNR-INDEPENDENT (bias -0.029, n_eff 2495 at SNR 40/80/160) while the full-box baseline degrades (n_eff 2029->1629). A_sub_wrong (wrong sub-box, STANDALONE) is biased by -36/-267/-1949 nats at SNR 40/80/160 while reporting 0% collapse and n_eff 220-840 -- n_eff calls it healthy. The SAME wrong sub-box inside a portfolio costs ~1 nat, not ~1949: the full-box member keeps q_mix covering, so the multi-AV variant fails safe where single-AV range mutation is catastrophic. P_mix also beats P_full at equal budget (collapse 30%->10%, n_eff ~20->~185). Caveat recorded: the ~-0.4 nat bias in the portfolio rows is present in P_full too, so it is the known low-n_eff IS downward skew, not an artifact of restriction. Safety and efficiency are established; portfolio-path unbiasedness at these n_eff is not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d fires on the best config) Scored against known truth, SNR 160, n=20/case. The run biased -1949 nats has khat 0.435 (below the 0.7 threshold; only 10% of copies trip it), while the most ACCURATE portfolio config has the HIGHEST khat (0.766, 80% firing) -- ranking by khat is anti-correlated with error here. Mechanism: k-hat estimates the tail of the weights actually DRAWN. A sampler confined to a wrong sub-box never samples the true peak, so its observed tail is genuinely resolved; the failure is total support non-overlap, not a heavy tail. k-hat cannot detect mass never touched. The high khat on P_mix is CORRECT behaviour (its full-box member does occasionally land near the peak) but means a khat gate would reject the best configuration. Keep k-hat as a tail diagnostic; do not use it as a correctness gate for mode-collapse failures. For that class cross-copy disagreement (replicas/bootstrap quantiles) remains the working detector. Also retracts my earlier khat run (sensitivity 8%/false-alarm 21%): that test defined 'wrong' as |bias|>0.1, so its failures were 0.3-nat runs with genuinely resolved tails -- not a fair evaluation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…R 40, capped 160k)
The driver default --n-chunk 1e4 is too small for loud events: at high SNR the extrinsic posterior is
a vanishing fraction of the prior volume, so a small chunk carries few informative samples per
adaptation step and the sampler adapts on noise. Measured on a truth-known synthetic SNR ladder
(demos/integrator_snr_lottery; collapse = failure to recover the KNOWN lnZ), at equal adaptation steps:
SNR 40: 31% -> 19% -> 0% (chunk 1e4 -> 4e4 -> 1.6e5)
SNR 80: 69% -> 38% -> 25%
SNR 160: 88% -> 62% -> 50%
and the gain SURVIVES at fixed total budget (pooled Fisher p=0.014 for SNR>=80), despite the larger
chunk taking 16x fewer adaptation steps -- so it is per-step statistics, not just more samples.
40k baseline, scaled linearly with SNR above 40, capped at 160k (the largest size measured) because
GPU memory grows with the chunk and an over-large request matches fewer slots. Override with
--internal-ile-n-chunk. The skymap branch's deliberate --n-chunk 500 still wins (appended later;
optparse takes the last occurrence).
NOT YET PAIRED with a memory-request bump: tools/measure_chunk_memory.sh is running to price it on the
real ILE likelihood. Do not raise the cap further until that lands.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…_member_range) Narrows ONE member's sampling range while member 0 stays the full-support backstop. This is a PROPOSAL-only change and needs NO prior renormalization: the weight is L*p_prior/q_mix with p_prior the TRUE prior, so a member's range is just a mixture component -- proposals need not cover the prior, only the MIXTURE must cover the support of L*p. The invariant that makes it safe (and the trap avoided): the portfolio takes joint_p_prior from whichever member DREW each sample, so a narrowed member that renormalized its prior over its sub-box would silently bias the integral. We therefore overwrite only llim/rlim, AFTER add_parameter has installed the SHARED prior callables -- verified: m0.prior_pdf['x0'] is m1.prior_pdf['x0']. Also fixed: add_parameter previously did self.llim.update(member.llim) for EVERY member, so a narrowed member would have clobbered the PORTFOLIO's own limits -- which are the reference range used downstream (L0-rescue puff width, breadcrumb bounds, distance-marginalization bounds). Now taken from member 0 before any narrowing. Member 0 may not be narrowed (raises). Measured justification: a WRONG sub-box costs a STANDALONE sampler up to -1949 nats while still reporting n_eff 220-840 (confidently wrong), but only ~1 nat inside a portfolio whose full-box member keeps q_mix covering the complement. Functional test: member0 [-5,5], member1 [-0.5,0.5], portfolio [-5,5], priors shared, backstop guard raises. Narrowing is applied before setup(), so all derived AV state is built from the narrowed range. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ate-time) The helper had no way to enable --interpolate-time, so the cubic path could not be turned on from a pipeline/asimov configuration at all. Nearest-sample-bin evaluation injects a time-quantization non-smoothness into the extrinsic likelihood surface that is a discretization artifact rather than physics; removing it makes convergence more robust. Requires the NoLoop likelihood (the --vectorized --gpu --force-xpy combination), which production already uses. Default off for backward compatibility; enabled explicitly in the IR1 review-ready analysis spec. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…mory change needed; 3.3x faster Measured on the real ILE likelihood: host peak 2409/2369/2506 MiB for chunk 1e4/4e4/1.6e5 (~3% spread, i.e. noise), while wall time falls 109->33 s. Since RequestMemory governs the HOST, the motivated action for the SNR-scaled chunk is to change nothing -- it does not restrict slot matching. Consistent with RIFT extrinsic jobs already over-requesting host RAM by 35-105x. Records two honest gaps: GPU memory was NOT measured (per-PID nvidia-smi filter failed, column reads 0), so the GPU-side cost is unquantified; and the n_eff column is one cold run per chunk (lottery noise), not evidence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ogs from the repo khat_run.log alone was 14k lines of per-chunk sampler stdout; khat_decisive.txt trimmed to its summary table. The reproducible artifacts (tools/*, results/*.txt, khat_validation.json) are kept.
The JSONs are the records the summary tables are derived from, so they are worth keeping for reproducibility -- but not as 12k lines of uncompressed diff.
…pe options through the helper (1) AV SEEDED GRID WAS NEVER APPLIED OUTSIDE integrate_log. bootstrap_from_samples / bootstrap_from_oracle / load_state only STORE the grid in self._warm; it was installed only inside AV.integrate_log. A PORTFOLIO drives members via draw_simplified()/update_sampling_prior() and never calls member.integrate_log, so a warm-started AV member kept drawing from the COLD grid while the driver printed 'warm-started N/N members'. The driver's L0 auto-rescue is affected the same way: it re-seeds then calls portfolio.integrate(), which never installs the members' seed. Added _apply_warm_state() (idempotent, fail-safe) called from draw_simplified, plus a _warm_applied flag reset whenever a NEW seed is stored so a mid-run re-seed takes effect. Verified: a warm portfolio run now logs 'seeded grid APPLIED to the active draw path: live bins=39488'. The stale code comment claiming 'a WARM member is fine: bootstrap_from_* builds it' was wrong and is removed. (2) --sampler-sequential-warmstart WAS INERT. analyze_event wipes sampler._rvs immediately before returning (that wipe is required -- it fixes a fairdraw export bug that silently dropped every binary after the first), but the caller captured the next-point seed by reading sampler._rvs AFTER the return, so it always saw an empty dict and never seeded. Capture now happens inside analyze_event before the wipe, handed to the caller via _SEQ_WS_PENDING. (3) pseudo_pipe: --internal-ile-interpolate-time was appending the raw '--interpolate-time True' to args_ile.txt, bypassing the helper. Now a helper passthrough (matching --internal-ile-rotate-phase), so the helper -- which knows whether the NoLoop path it requires is active -- owns the argument. Also exposes --internal-ile-n-chunk as a helper passthrough; it had no pseudo_pipe route at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Correction to the L0 auto-rescue claim, and fixes for two warm-start defects raised on oshaughn#161The correctionThis PR's description credits the L0 auto-rescue ( The measured effect is real; the stated mechanism was not established. Prompted by oshaughn#161, So in the 4/9 -> 8/9 measurement the re-seed had no effect on the AV member's sampling, and the I am not withdrawing the recommendation -- a second attempt that converts ~half the collapses is Fixes included here (so oshaughn#161 resolves on merge)
Not addressed here (they belong to the MC-error work, and I have not verified either): Also in this push
Re-validation statusThe warm-start fix changes sampling behaviour on the portfolio path, so the L0-rescue ensemble |
…oken warm start With the seed now actually installed on the portfolio draw path, a single warm-started run lands 6/9 (vs the 4/9 baseline measured while the seed was silently discarded), and adding the L0 rescue gives 8/9 -- Fisher two-sided p=0.576, NOT significant at n=9. Fixing warm start is worth more than the rescue policy; the rescue stays recommended as cheap insurance but its earlier headline is withdrawn. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Re-validation after the warm-start fixes — both gates cleanRe-ran on the updated pairing (base
Every probe row PASSes where base PASSes and is STARVED where base is STARVED, across Scope limit, stated explicitly: the gate does NOT exercise the warm-start path — A gate case that actually bootstraps a portfolio member would be worth adding, since this class of |
oshaughnessy-junior
left a comment
There was a problem hiding this comment.
Review findings at head 7d07646: two blocking correctness issues and one API validation issue.
[P1] Public restriction API bypasses coverage safeguards
restrict_member_range() records an override and add_parameter() mutates the member limits, but this path never sets _has_restricted_member or _full_support_members. Those flags are only initialized by the separate portfolio_restrict_ranges setup option. Consequently the advertised API disables the full-support draw floor, restricted-only active-member guard, and q_mix fallback guard; member 0 can receive zero draws and silently lose support. Please centralize restriction registration so both entry points establish the same invariants.
[P1] Warm-state cleanup misses portfolio members
At integrate_likelihood_extrinsic_batchmode:3665, sampler._warm = None does not clear a portfolio warm start: _warm and the active AV grid live on each member, not on the portfolio object. If capture returns no seed, or a member rejects the next seed, portfolio integrate_log() can reuse the preceding point contracted grid because it does not rerun setup(), potentially excluding the new point support. Please add a portfolio reset/clear method that forwards to members and restores their cold active state. The exception cleanup paths at lines 3645 and 3654 have the same issue.
[P2] Invalid restrictions silently become no-ops
restrict_member_range() accepts negative member indices and arbitrary parameter names without later validation. Negative indices and misspelled parameters never match the positive enumerate()/params checks in add_parameter(), so the call succeeds without applying a restriction. Please require 1 <= member_index < member_count and verify every pending parameter override is consumed before setup.
All existing CI checks are green, but the suite does not exercise these entry-point and lifecycle failures.
All three produce a WRONG answer with no exception and a healthy n_eff. 1. restrict_member_range() recorded an override but never set _has_restricted_member / _full_support_members -- those were only established by the separate setup(portfolio_restrict_ranges=) path. So the public API silently disabled the full-support draw floor, the restricted-only active-member guard and the q_mix fallback guard: member 0 could get zero draws and the mixture could lose full support, biasing lnZ low. Both entry points now feed the same bookkeeping, and setup() unions the two restricted sets before applying the coverage invariant. 2. The driver cleared warm state with `sampler._warm = None`, which does not reach portfolio members -- _warm and the contracted AV grid live on each member, and portfolio.integrate_log() never reruns member setup(). The next point could therefore draw from the PREVIOUS point's contracted live volume. Latent before AV._apply_warm_state actually installed seeds; live after. Adds portfolio.clear_warm_state() (nulls _warm/_warm_applied and restores each member's cold grid via setup()) and a driver helper _clear_warm_state() used at all five clear sites. 3. restrict_member_range() accepted negative member indices and arbitrary parameter names; neither can ever match the positive enumerate()/params checks in add_parameter, so the call "succeeded" and applied nothing. Now requires 1 <= member_index < member_count, and setup() raises if any registered override was never consumed. Adds test/integrators/test_portfolio_restrict_and_warm.py (CPU-only, ~1s). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…between points The merge gate only ever exercised cold starts, so neither portfolio warm-start defect could be caught by it. Both produce a wrong answer with no exception. New cases in shape_recovery.py (8 rows, ~26 s wall at --jobs 4): portfolio_warm (3 seeds, d=6) -- verifies a warm seed reaches the AV member's ACTIVE draw state. Deliberately white-box: measured with the install disabled, the run still PASSes every statistical check and scores HIGHER n_eff in 2 of 3 seeds (4857/5718/5532 broken vs 3159/3368/5707 correct), so no black-box assertion can see this. A1 checks the live volume contracted (V 0.03-0.13 and 498-656 bins when working; exactly 1.000 and 1 bin when inert). A2 checks draws concentrate in the seed box, measured on the AV MEMBER's own draws -- on the portfolio mixture it is useless (28x even when broken, because the GMM member is warm-started separately); on the member it is 6.3-27.2x vs 0.9-1.0x. portfolio_seq_nobs (3 seeds, d=2) -- two displaced targets on one reused sampler. This is the discriminant for state leaking between points: with the reset no-op'd it gives n_eff 1.0/1.0/9.9 and lnZ bias -22.8/-59.7/-0.56 versus n_eff 822-2094 and |bias| <= 0.019 when correct. portfolio_seq (which re-bootstraps on point B) is kept as ONE row only -- measured PASS with the bug active, because a fresh seed overwrites the stale grid. AV_seq is warn-only. MixtureTarget gains `offset` (applied after the rng.uniform draw, so existing targets stay bit-identical) -- two seeds cannot guarantee displaced modes. Driver: mcsamplerPortfolio.integrate_log does not call self.setup(), so member state survives into the next point. Adds an unconditional per-point reset in integrate_likelihood_extrinsic_batchmode, independent of any warm-start option, because the leak bites plain --n-events-to-analyze > 1 runs. run_shape_recovery.sh: use python3, not python -- a bare `python` does not exist in several IGWN environments and exits the whole gate 127 before it starts. Full proposal, rejected cases and measured scatter in PROPOSAL_bootstrap_gate_cases.md (section 8 records the two proposed assertions that failed the reintroduce-the-bug check). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
All three review findings are fixed in [P1] Public restriction API bypassed coverage safeguards — fixedCorrect as reported, and the consequence was exactly as described: [P1] Warm-state cleanup missed portfolio members — fixed, and it was worse than reportedConfirmed. Added Worth stating plainly: this was latent until my own The finding also led to a fourth defect. [P2] Invalid restrictions silently became no-ops — fixed
Tests
The merge gate now also exercises the warm/bootstrap path it never touched (8 rows, ~26 s wall at
Rejected cases, with the seed scatter that justifies rejecting each, are in Gate statusShape-recovery gate against base Still open from the upstream PR 161 review and explicitly not addressed here: the |
…tions Two defects from the PR #45 follow-up review. [P1, blocking] clear_warm_state() called member.setup() bare, which restores the cold grid but DISCARDS the member's configuration. mcsamplerEnsemble.setup() rebuilds its dimension grouping and re-reads n_comp / gmm_adapt / correlate_all_dims from kwargs, so a configured (0,1) GMM with n_comp=3 and adaptation off came back as separate (0,), (1,) groups with the scalar n_comp default -- a quietly different sampler for every point after the first. Reproduced: grouping [(0,1)] -> [(0,), (1,)]. setup() now stores each member's exact arguments (_member_setup_args / _oracle_setup_args) and clear_warm_state replays them. AV cannot show this defect (its setup ignores kwargs), which is why the AV-only test and the default gate config both missed it. Reset failures now PROPAGATE, in the method and in the driver helper. A reset that quietly did not happen leaves the next point on the previous point's grid -- the exact silent bias this guards against -- so it must abort the point, not degrade to a log line. [P2] restrict_member_range() could WIDEN a member past its original limits: the override path overwrote llim/rlim directly while the portfolio_restrict_ranges path clipped. Widening makes the member sample where the SHARED prior callables (absolute densities normalized over the original range) are not normalized, so the reported prior is wrong and the integral is biased -- silently. Now raises rather than clipping: the API is named "restrict", so a caller who meant to widen should find out instead of receiving a no-op. Three regression tests added, each verified to FAIL when its defect is reintroduced (config loss, swallowed reset failure, widening accepted). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Both follow-up findings confirmed and fixed in [P1, blocking]
|
| case | base (rift_O4d) |
candidate |
|---|---|---|
portfolio_warm s101/202/303 |
WARN — AV member V=1.000, live bins=1 (cold state), box ratio 1.0x |
PASS — V 0.03–0.13, 498–656 bins, 6.3–27.2x |
portfolio_seq_nobs s101 |
WARN — n_eff 10, lnZ bias -0.559 | PASS — n_eff 2094, bias +0.019 |
portfolio_seq_nobs s202 |
WARN — n_eff 1, lnZ bias -22.811 | PASS — n_eff 1593, bias -0.000 |
portfolio_seq_nobs s303 |
WARN — n_eff 1, lnZ bias -59.667 | PASS — n_eff 822, bias +0.002 |
All six report IMPROVED(fail->pass), which is non-blocking on this merge by design; once this is the base, re-breaking either path blocks.
One number worth keeping in view: base portfolio_warm scores n_eff 4857 / 5718 / 5532 against the candidate's 3159 / 3368 / 5707. The branch with the inert warm start looks better on n_eff in two of three seeds. That is why the case is white-box.
Still not addressed, and still unverified by me: the upstream PR 161 items on cover_frac with sparse points, and whether the MC-error replicas are genuinely cold / adaptation-independent.
… trained proposal The reset fix in 9e054c7 stored each member's setup-argument dict by REFERENCE, which reintroduced the state leak through the reset itself. gmm_dict is not an inert spec. mcsamplerEnsemble.setup() hands the caller's dict straight to monte_carlo.integrator, which stores it without copying (MonteCarloEnsemble.py:110) and then writes trained models into it (self.gmm_dict[dim_group] = model, :403). So the stored args were the very object being trained into, and replaying them on reset handed the next point the PREVIOUS point's trained proposal. Reproduced with a production-shaped gmm_dict={(0,1): None}: after reset the integrator still held the point-1 model. setup() now stores a snapshot (dict/list/tuple/set/ndarray copied; callables, modules and deliberately supplied pre-trained models passed through -- a blanket deepcopy would try to clone those), and clear_warm_state() replays a FRESH copy each time. The second part matters independently: passing the stored dict would let the rebuilt integrator train into the snapshot, so the leak would simply return one point later. The previous configuration test could not see any of this -- it supplied no gmm_dict, never trained, and compared only grouping / n_comp / gmm_adapt, all three of which survive the aliasing defect intact. The new test supplies gmm_dict as production does, mutates the trained proposal, and asserts on proposal CONTENTS across two consecutive resets. Verified to fail on both the reference-alias defect and the subtler replay-the-stored-object variant; also verified that the old config-only test still passes on the aliasing defect, which is why a separate test was needed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ert the trained proposal is cleared Closes the second half of the review finding: the gate never supplied gmm_dict, so it only ever exercised the gmm_dict=None branch of mcsamplerEnsemble.setup() and could not see setup-argument aliasing at all. The sequential cases now configure the GMM member the way production does, and assert white-box that the reset actually cleared its trained components. The check has to be white-box, and the measurement says why. With the trained proposal leaking across the reset the statistical rows still PASS -- n_eff 1700 / 487 / 196 and |lnZ bias| <= 0.010 versus 2036 / 1583 / 823 and <= 0.008 when correct. A stale GMM proposal is merely a BAD proposal: the AV member still covers the support, so the mixture stays unbiased and only efficiency suffers. That is unlike the AV grid leak, which removes support and does bias (n_eff 1, lnZ bias -59). Worth recording, because it means this particular aliasing defect costs efficiency rather than correctness -- but it is invisible to every statistical assertion, so it needs the direct observable. A `seq_gmm_trained_before_reset` flag is recorded alongside, so "cleared" cannot pass vacuously on a run where the GMM never fitted anything. Measured: 3/3 FAIL with the aliasing defect reintroduced, 3/3 PASS without. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Confirmed, reproduced, and fixed in To be clear about ownership: this was my regression, introduced by the reset fix in The mechanism
The fix has two parts
Your three reasons for the miss were all correctI verified the third directly. With the aliasing defect reintroduced: Grouping, Gate: production shape, and a white-box clearing checkYour point that production always supplies One measurement that recalibrates severityThe check has to be white-box, and this is why. With the trained proposal leaking, the statistical rows still pass:
A stale GMM proposal is merely a bad proposal: the AV member still covers the support, so the mixture stays unbiased and only efficiency suffers. That is unlike the AV grid leak, which removes support and does bias (n_eff 1, lnZ bias −59.7). So this particular defect costs efficiency, not correctness. That is not an argument against fixing it — it is real degradation on every point after the first, and a latent hazard if the member mix ever changes such that the GMM is load-bearing for coverage. But it does mean no statistical assertion could have caught it, which is the justification for the direct observable. Status at
|
…n-vacuous [P2] Adaptive seeded models still leaked. _snapshot_setup_args copied the containing dict but passed model objects through by reference. Production seeds per-group GMMs from a breadcrumb, and with --extrinsic-proposal-adapt those groups keep re-fitting: gmm.update() mutates the model in place. The stored "baseline" therefore drifted during point 1 and was replayed into point 2. (With adapt OFF, the default, _train skips seeded groups, so this path was previously harmless -- it is specific to the opt-in adaptive configuration.) Objects nested inside a spec container are now cloned. deepcopy is NOT usable: a real gmm holds a module reference (xpy) and bound functions, so deepcopy raises "cannot pickle 'module' object" -- which would have taken the fallback path and left the model SHARED, i.e. the fix inert on exactly the configuration it exists for. Instead: shallow-copy the object (never pickles) and clone its mutable attributes, leaving module/function refs shared. Verified on the real gmm class: mutating the clone leaves the original untouched. Top-level non-container args stay by reference. A clone that fails is now REPORTED, not silently shared. [P2] The gate's clearing check could pass vacuously. Keying on `cleared is False` let three failures read as success: training never happened (so "cleared" is trivially true), inspection raised (cleared is None), or the fields were absent. Both observations must now be affirmatively True; anything else fails, and the underlying error is surfaced in the reason string. Adds test_seq_gmm_check.py (6 checks on evaluate()) and two portfolio tests, both verified to fail when nested objects are passed by reference again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Both fixed in [P2] Adaptive seeded GMMs — fixed by cloning, and the obvious clone did not workConfirmed: I chose clone over document-as-intentional. Different points have different intrinsic parameters and therefore different extrinsic posteriors; the breadcrumb seed is a handoff for this point, so carrying point 1's adaptation into point 2 is the same class of leak, not a feature. Worth recording, because it nearly made the fix inert: A clone that fails is now reported, not silently shared — that fallback is exactly how this class of bug survives. Top-level non-container arguments still pass by reference (callables, modules, sampler objects); only objects nested inside spec containers are cloned. [P2] Vacuous gate check — fixedCorrect, and there were three failure modes reading as success, not one: training never happened (so "cleared" is trivially true and proves nothing), the inspection raised ( New VerificationBoth new portfolio tests verified to fail when nested objects are passed by reference again: 12 portfolio tests + 6 gate tests, all passing. Gate at
|
…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>
Follow-on to #34 (merged). Two helper-level configuration improvements for high-SNR extrinsic
integration, the study that motivated them, and one portfolio API addition. No default sampler is
changed; the one behavioural change is opt-in.
1. SNR-scaled extrinsic chunk size (helper)
The driver default
--n-chunk 1e4is too small for loud events: at high SNR the extrinsic posterioris a vanishing fraction of the prior volume, so a small chunk carries few informative samples per
adaptation step and the sampler adapts on noise. The helper now emits 40k baseline, scaling
linearly with SNR above 40, capped at 160k (override
--internal-ile-n-chunk).Measured on a truth-known synthetic SNR ladder (collapse = failure to recover the KNOWN lnZ,
demos/integrator_snr_lottery), at equal adaptation steps:and the gain survives at fixed total budget (pooled Fisher p=0.014 for SNR>=80) despite the
larger chunk taking 16x fewer adaptation steps — so it is per-step statistics, not just more samples.
Resource cost, measured on the real ILE likelihood: host peak RSS is flat across a 16x chunk
range (2409 / 2369 / 2506 MiB — ~3% = noise) while wall time falls 3.3x (109 -> 33 s). Since
RequestMemorygoverns the HOST, this needs no memory-request change and does not restrict slotmatching. GPU memory was NOT successfully measured (the per-PID nvidia-smi filter failed); that gap
is recorded in the demo README.
The skymap branch's deliberate
--n-chunk 500still wins (appended later; optparse takes the last).2. Cubic Q_lm time interpolation (helper)
--internal-ile-interpolate-timepasses--interpolate-time True. The helper previously had noway to enable this at all, so the cubic path was unreachable from a pipeline/asimov configuration.
Nearest-sample-bin evaluation injects a time-quantization non-smoothness into the extrinsic
likelihood surface that is a discretization artifact rather than physics. Requires the NoLoop
likelihood (
--vectorized --gpu --force-xpy), which production already uses. Default off.3. Portfolio: per-member range plumbing for interval narrowing
mcsamplerPortfolio.restrict_member_range(member, param, lo, hi)narrows ONE member's sampling rangewhile member 0 remains a full-support backstop. This is a proposal-only change and needs no prior
renormalization: the weight is
L*p_prior/q_mixwithp_priorthe TRUE prior, so a member's range isjust a mixture component — proposals need not cover the prior, only the MIXTURE must cover the
support of
L*p.The invariant that makes it safe: the portfolio takes
joint_p_priorfrom whichever member DREW eachsample, so a narrowed member that renormalized its prior over the sub-box would silently bias the
integral. Only
llim/rlimare overwritten, AFTERadd_parameterinstalls the SHARED priorcallables (verified:
m0.prior_pdf['x0'] is m1.prior_pdf['x0']).Latent bug fixed on the way:
add_parameterdidself.llim.update(member.llim)for EVERY member,so a narrowed member would have clobbered the PORTFOLIO's own limits — the reference range used by the
L0-rescue puff width, breadcrumb bounds and distance-marginalization bounds.
Member 0 may not be narrowed (raises). Measured justification: a WRONG sub-box costs a STANDALONE
sampler up to -1949 nats while still reporting n_eff 220-840 (confidently wrong), but only ~1
nat inside a portfolio whose full-box member keeps
q_mixcovering.4. Study:
demos/integrator_snr_lottery(truth-known, CPU-only)Reuses the merge gate's
MixtureTarget(true_lnZknown), so it measures real bias, not scatterabout an unknown answer — and being CPU-only it affords many copies, which was the binding constraint
on every earlier conclusion here.
Headline finding, relevant to the MC-error work in #38: k-hat does not catch this failure class.
Scored against known truth at SNR 160 (n=20/case), the run biased -1949 nats has k-hat 0.435
(below the 0.7 threshold; only 10% of copies trip it), while the most ACCURATE portfolio config has
the HIGHEST k-hat (0.766, 80% firing). Mechanism: k-hat estimates the tail of the weights actually
DRAWN — a sampler confined to a wrong sub-box never samples the true peak, so its observed tail
genuinely IS resolved. k-hat cannot detect mass never touched. Keep it as a tail diagnostic; do
not use it as a correctness gate for mode-collapse/support-mismatch failures. For those, cross-copy
disagreement (replicas/bootstrap quantiles) remains the working detector.
What this PR does NOT claim
RETRACTED in-tree. It looked decisive at n=4 (sd 0.85 vs 5.96) and was gone by n=20 (5.02 vs
4.02; variance-ratio p=0.66-0.80, pointing the WRONG way). Classic selection + regression to the
mean.
--portfolio-varaha-min-frac/-max-fracremain opt-in, OFF, and documented as unproven.measured production curve. The direction is solid across two independent designs; the exact
coefficients are not.
Validation
rift_O4dvs this branch:COMPARE_EXIT=0, 0 blockingregressions, 96/96 rows both arms.
probe_portfolio_optin_flags.py, extended here to cover the VARAHA shareconstraints and the GMM component cap, scored by the gate's own
evaluate()): 0 opt-inregressions — every row PASSes where base PASSes, STARVED where base is STARVED.
test_mcsamplerEnsemble_extended.py --as-test, both lnL and non-lnL): GMMresults bit-identical to base.
An end-to-end asimov review-ready test (S240426s, SEOBNRv5PHM, 4096 Hz, both arms) is set up and
tracked separately; it is currently blocked on a CIT infrastructure issue (execute nodes do not mount
personal home) unrelated to this code.
Transferable lore: https://github.com/oshaughnessy-junior/rift-integrator-lore
🤖 Generated with Claude Code