LISA ILE driver: MC-error replicas and replica pooling (pass 3) - #110
Open
oshaughnessy-junior wants to merge 4 commits into
Open
LISA ILE driver: MC-error replicas and replica pooling (pass 3)#110oshaughnessy-junior wants to merge 4 commits into
oshaughnessy-junior wants to merge 4 commits into
Conversation
…ide)
Pass 3 of the catch-up. Closes 7 of the 92 gap items (gap 92 -> 85):
--mc-error-{replicas,sigma-trigger,ess-trigger,khat-trigger}, _pool_replica_rvs
(+._block_resampled) and _extract_mc_diag.
All three constraints recorded when pass 2 landed are honoured, and each was a real trap:
(a) BOTH COLLAPSE-GATE CALL SITES. The main driver gates on the first run AND on the
POOLED verdict, because replication can turn a healthy first run into a collapsed
pool; gating only the first bypasses --reject-collapsed-live-volume for exactly the
case pooling introduces. The new helper OWNS both, so the standalone
_report_and_gate_collapse calls the previous pass added to each analyze_event are
removed -- keeping them would have double-gated the first run. A test now asserts
analyze_event does NOT gate directly and that the helper does it twice.
(b) _rvs_is_pooled RESET ON ENTRY, in both analyze_event variants. Only the READER was
ported previously. Nothing set the marker until now, so this had no live effect
before; with pooling it does. On entry rather than in a finally: the pooled gate
RAISES, the caller's except swallows it, and a marker cleared only on the happy path
survives into the next event (audit Finding 7).
(c) The PER-REPLICA already_resampled sequence, not a global boolean. Each pass decides
independently whether to fair-draw, so near the n_extr boundary a run produces a
MIXTURE of raw and resampled replicas, which one boolean cannot describe (Finding 6).
_rep_fairdraw is captured beside each record.
_pool_replica_rvs (164 lines) and the replica block (~206 lines) were extracted from the
main driver VERBATIM by line range rather than retyped, then the block was wrapped as
_maybe_replicate_for_mc_error with manual_avoid_overflow_logarithm -> lnL_offset. Module
level, not inline, for the usual reason: this driver has TWO analyze_event variants.
ORDERING. The helper needs log_res/sqrt_var_over_res, so it runs after they are computed.
_maybe_save_av_state stays BEFORE it: after the replica loop the sampler holds the LAST
replica's adapted grid, not the run being reported. The main driver saves at the same
point, and a test pins load < aniso < integrate < guard < save < replicate.
THREE EXISTING TESTS CHANGED, all deliberately. test_the_second_gate_call_site_is_recorded_as_missing
existed to fail exactly when someone ported --mc-error-replicas without adding the second
gate; that port has now happened, so it is replaced by its successor, which asserts both
gates exist. The hook-ordering and hook-presence tests move from the standalone gate to
the helper. And the L0 rescue's lnL_offset test now counts PER HELPER -- two helpers take
that argument now, and a global count would absorb a call site that dropped it.
lisa-check: 214 passed. Both audits green (85 gap items, 147 _rvs reads classified; the
new reads in the replica block classify automatically because the fair-draw generator's
L0/replica rule block already covers both drivers and matches on source text).
Tests specific to the replica path follow in the next commit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
test_lisa_mc_error_replicas.py (27), wired into lisa-check. Behavioural where it can be:
_pool_replica_rvs is a pure function of its arguments, so the pooling maths is driven
directly rather than pinned at source level.
What the behavioural tests establish:
* block k's weights sum to Z_k/K, so the pooled record matches the linear-mean lnZ it is
reported beside;
* a MIXTURE of resampled and raw replicas is handled per block -- the fair-drawn one is
flattened (it is already an equal-weight posterior draw; weighting it again gives w^2),
the raw one keeps its genuine importance weights;
* and, so that test has teeth, that the sequence form genuinely differs from BOTH global
booleans;
* an empty replica is dropped in lockstep with its lnZ and its flag -- the version that
filtered rep_rvs alone weights the surviving block with the dropped replica's evidence,
which the test now measures directly;
* every fallback returns an INPUT record by identity, which is what _did_pool keys on;
* stale cached log_weights do not survive pooling (consumers PREFER that column, so a
stale one silently undoes the rebalancing).
REVERT-CHECKED with 8 mutations, each caught by its named test, file restored
byte-identical: the CLI flag substituted for the per-replica sequence; the sequence
collapsed to a bool inside the pooler; the lockstep filter broken; the POOLED collapse gate
deleted; the entry reset of _rvs_is_pooled removed; the pooled marker set even on a
fallback; stale cached weights kept; and AV state saved AFTER replication (which would
persist the last replica's grid instead of the reported run's).
That last one is worth noting: the ordering it protects was not something I reasoned out in
advance -- the mutation list is what made me check where the main driver saves, which is
before the replica block, for exactly that reason.
lisa-check: 242 passed. Both audits green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
oshaughnessy-junior
deployed
to
private-review-dispatch-rift
August 16, 2026 14:49 — with
GitHub Actions
Active
PHYSICS 11 -> 0. Gap unchanged at 85; the split is now PORT 42 / NA 43.
COSMOLOGY (--d-prior-redshift, dLofz, dVdz). Planck15 via the framework helper,
RIFT.likelihood.priors_utils.get_astropy_cosmology('Planck15'). Recorded with a
divergence to raise at port time: the MAIN driver does not use that helper, it hardcodes
FlatLambdaCDM from H0_SI/OMEGA_M = 67.900 / 0.3065, while astropy Planck15 is 67.740 /
0.3075 -- dL(z=5) 47756 vs 47732 Mpc, 0.05%. Negligible physically, but this whole
exercise exists to stop silent divergences, so it is written down rather than absorbed.
DISTANCE/INCLINATION REPARAMETERIZATION. "should be good enough; it's a testable axis
though - don't guess, measure." So PORT, but not default-on until measured, and the
ledger states the measurement: n_eff / lnZ scatter with and without the reparameterization
at a fixed LISA MBHB intrinsic point. A wrong axis under TDI shows up as no improvement
or worse conditioning, not as a bias, which is why it is cheap to test and easy to assume.
SKY BOX. "LISA and LIGO are never overlapping use cases -- follow whatever convention
Aasim used, document in the help string, don't change the name." VERIFIED against the
source rather than assumed: the sampled right_ascension/declination columns flow to
P.phi/P.theta and thence to lisa_sky_lamda/lisa_sky_beta, so the convention IS ecliptic
lambda/beta under the historical key names. --limit-right-ascension therefore bounds
lambda and --limit-declination bounds beta, and the help text must say so.
WARM-START PILOT FRAME. RO asked whether the convention matters. It does not: the seed
is points in the sampler's own coordinate space, read positionally against params_ordered,
so any self-consistent convention works and the sky answer above determines it. What the
ledger now records instead is the hazard -- a mismatch is UNDETECTABLE, since ecliptic
lambda and RA share [0,2pi) and beta and dec share [-pi/2,pi/2], so no range check can
separate them and a wrong-frame seed silently contracts the live volume around the wrong
region (biased lnZ, healthy-looking n_eff). Port with a producer-written frame tag the
reader refuses or warns on.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
oshaughnessy-junior
deployed
to
private-review-dispatch-rift
August 16, 2026 15:24 — with
GitHub Actions
Active
…ver too RO, 2026-08-16: "move it to the helper, so it is consistent by default and changed in a consistent fashion between codes; not hardcoded. Agree minute effect, but the sort of random complaint people do make in refereeing reports." The ILE driver built its own FlatLambdaCDM, preferring lal.H0_SI/lal.OMEGA_M with a hardcoded pair as fallback. The installed lal gives H0=67.900, Om0=0.3065; astropy Planck15 is H0=67.740, Om0=0.3075. dL(z=5) moves 47756 -> 47732 Mpc, 0.05%: physically nothing. The point is answerability -- "which cosmology is this?" had no citable answer, only "whatever the linked lalsuite constant happened to be" -- and that a change should now happen in ONE place for every code that needs one. This touches the SHARED main driver, not just the LISA fork, which is the point: doing it only on the LISA side would have created exactly the silent divergence this work exists to remove. The review history behind the lal-constant route (cbc/action_items#37 and the rapidpe_rift_review_o4 Cosmo_sourceframe wiki, lalsuite 7.6.1 -> 7.25.1) is kept in a comment at the site, so it reads as superseded deliberately rather than lost. Also drops the now-unused FlatLambdaCDM import. TESTS. test_cosmology_single_source.py (6), wired into the integrator-gate CI job beside test_cip_priors.py. Deliberately GENERAL rather than a check on this one line: no driver may construct its own cosmology object, none may hardcode the lal H0 constant, and the helper must default to Planck15. That is the property that keeps CIP, both ILE drivers and anything else from quietly disagreeing about z. Revert-checked: restoring the hardcoded pair fails three of the six. test_cip_priors.py and test_nal_io.py (133) still pass. LEDGER. The cosmology item's recorded reason no longer says "a divergence to raise at port time" -- there is none now. And --sampler-warmstart-samples records RO's scoping: these files are used INTERNALLY within a homogeneous run, so a one-line frame stamp written by the producer, warned on when absent or disagreeing, is the whole requirement; explicitly NOT a validation framework. lisa-check 242 passed; integrator gate 116 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
oshaughnessy-junior
deployed
to
private-review-dispatch-rift
August 16, 2026 19:58 — with
GitHub Actions
Active
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.
Pass 3 of the LISA catch-up, on top of #106. Closes 7 of the 92 gap items (gap 92 → 85):
--mc-error-{replicas,sigma-trigger,ess-trigger,khat-trigger},_pool_replica_rvs(+._block_resampled),_extract_mc_diag.--mc-error-replicasre-runs the extrinsic integration as independent cold replicas when the reported error is untrustworthy, then pools every replica's samples rather than picking one — lnZ is the linear mean over K replicas, so the exported posterior must represent that same mixture. (n_eff is the wrong selector anyway: it measures weight concentration, not coverage, so a mode-collapsed replica scores highest.)The three constraints recorded when pass 2 landed — all real traps
(a) Both collapse-gate call sites. Main gates on the first run and on the pooled verdict, because replication can turn a healthy first run into a collapsed pool. The new helper owns both, so the standalone
_report_and_gate_collapsecalls that pass 5a added to eachanalyze_eventare removed — keeping them would have double-gated the first run. A test assertsanalyze_eventdoes not gate directly and the helper does it twice.(b)
_rvs_is_pooledreset on entry, in both variants. Only the reader was ported before; nothing set the marker, so this had no live effect until now. On entry rather than in afinally: the pooled gate raises, the caller'sexceptswallows it, and a marker cleared only on the happy path survives into the next event (audit Finding 7).(c) Per-replica
already_resampledsequence, not a global boolean. Each pass decides independently whether to fair-draw, so near then_extrboundary a run produces a mixture, which one boolean cannot describe (Finding 6).How it was ported
_pool_replica_rvs(164 lines) and the replica block (~206 lines) were extracted from the main driver verbatim by line range rather than retyped, then the block was wrapped as_maybe_replicate_for_mc_error(module level — this driver has twoanalyze_eventvariants).Ordering: the helper needs
log_res/sqrt_var_over_res, so it runs after they are computed._maybe_save_av_statestays before it — after the replica loop the sampler holds the last replica's adapted grid, not the run being reported. Main saves at the same point; a test pinsload < aniso < integrate < guard < save < replicate.Three existing tests changed, all deliberately
test_the_second_gate_call_site_is_recorded_as_missingexisted to fail exactly when someone ported--mc-error-replicaswithout adding the second gate. That port has happened, so it is replaced by its successor asserting both gates exist. The hook-ordering/presence tests move from the standalone gate to the helper. And the L0 rescue'slnL_offsettest now counts per helper — two helpers take that argument now, and a global count would absorb a call site that dropped it.Tests
test_lisa_mc_error_replicas.py(27), wired intolisa-check. Behavioural where possible:_pool_replica_rvsis a pure function, so the pooling maths is driven directly — block weights summing toZ_k/K, the mixed flattened/raw case, and (so that test has teeth) that the sequence form differs from both global booleans.Revert-checked with 8 mutations, each caught by its named test, file restored byte-identical. One is worth calling out: "save AV state after replication" — the ordering it protects wasn't something I reasoned out in advance; building the mutation list is what made me go check where main saves, and why.
lisa-check: 242 passed. Both audits green.Still open after this
31 PORT (the 7
--internal-gmm-*, extrinsic-proposal handoff, sequential warm start, interpolate-time stencil names, misc) and 11 PHYSICS questions listed inLISA_DRIVER_DRIFT.md— the latter now with Aasim.🤖 Generated with Claude Code