O4d: JAX correctness, bounded multipeak, and GPU precompute - #193
Merged
Merged
Conversation
The multipeak scheme landed broken under the sampler. wrapper._batched is
jax.jit for every scheme, but multipeak_local_marginalize is a numpy/scipy
planner with a Python loop over rows: under a trace its np.asarray on the
coefficient tables raises TracerArrayConversionError. Every Table 3 multipeak
cell died in about 30 s.
My wiring tests missed it because they all called _fused, which is EAGER. The
sampler reaches the likelihood through _batched, and no test crossed that seam.
The new test does, and it fails on the previous commit.
Fixes:
- _batched is left un-jitted for multipeak only; every other scheme is
unchanged.
- gradients and the Hessian REFUSE for multipeak rather than returning
something. There is no AD through a numpy planner, and a silent wrong
gradient would reach --fisher-precondition, which swallows exceptions and
falls back to raw coordinates with the flag still recorded as supplied.
Also collapses the three live EXPECTED_TESTS= assignments the #312 merge left
behind, keeping the measured binding value. This is not hypothetical: bumping
the floor for the two new tests, I edited the FIRST of the three and the change
was inert. 762 measured + 2 = 764.
Tests: 9 passed in the multipeak suite, 40 passed across peaklocal / default /
gh-selection wiring, on /scratch/$USER/envs/jaxci-py311 (jax 0.10.2, cupy
absent, CPU backend), ldas-pcdev13.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…earch-projects-RIT into claude/multipeak-hostside # Conflicts: # .travis/test-jax.sh
Takes #315's single measured assignment wholesale and drops my own collapse, so there is one live EXPECTED_TESTS= line and one changelog. 765 is MEASURED by running this script on the merged tree ('collected 765 tests from 46 files'), not 763 + 2. It happens to equal the sum this time; the block above records four landings where it did not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…do-cosmo # Conflicts: # CHANGES.rst
…cosmo Match JAX AV to production ILE precompute settings
…-selection Add opt-in response-order checks and selection
…ostprocess Convert JAX fairdraw sidecars in terminal pipeline
…tside jax_ile: multipeak is host-side, so do not trace it (fixes every Table 3 cell)
…xtrinsics Fix JAX ILE double application of template extrinsic angles
…-bounds Fix JAX AV escaping requested sampling bounds
…bounded Add bounded JAX multipeak marginalization
* Preserve native waveform adapter and tested conditioning building blocks * Harden physical waveform parity and register CPU CI coverage * Enable JAX x64 in waveform CPU CI check --------- Co-authored-by: Richard O'Shaughnessy <richard.oshaughnessy@ligo.org>
* Add opt-in GPU compound precompute and short validity gates * Keep compound likelihood banks on device through ILE and JAX handoff * Preserve compound waveform controls and strengthen handoff diagnostics * Pin smoke waveform convention and add GWSignal and cubic GPU regressions * Fix JAX AV sampling outside requested bounds * Reject out-of-box smoke samples and record AV bounds diagnosis * Record completed GPU validity gates and bound short mode benchmark * Remove duplicated compound FFT and V weighting work; profile cold and warm calls * Record isolated GPU precompute and JAX consumer profiles * Bound JAX banded data-term graph with differentiable loops * Harden cache lifetimes and register precompute regression gates * Record completed readiness gate and remaining throughput blocker * Tile JAX banded gathers to cut compile cost and preserve warm speed * Preserve empty JAX band batches and record GPU validation --------- Co-authored-by: Richard O'Shaughnessy <richard.oshaughnessy@ligo.org>
oshaughnessy-junior
deployed
to
private-review-dispatch-rift-upstream
September 13, 2026 00:51 — with 
GitHub Actions
Active
GitHub Actions
Active
* Score GMM draws as component-truncated mixtures * Gate decoy portfolio estimate by measured Monte Carlo uncertainty --------- Co-authored-by: Richard O'Shaughnessy <richard.oshaughnessy@ligo.org>
oshaughnessy-junior
deployed
to
private-review-dispatch-rift-upstream
September 13, 2026 08:42 — with
GitHub Actions
Active
GitHub Actions
Active
Co-authored-by: Richard O'Shaughnessy <richard.oshaughnessy@ligo.org>
oshaughnessy-junior
had a problem deploying
to
private-review-dispatch-rift-upstream
September 13, 2026 08:58 — with
GitHub Actions
Failure
GitHub Actions
Failure
Co-authored-by: Richard O'Shaughnessy <richard.oshaughnessy@ligo.org>
oshaughnessy-junior
deployed
to
private-review-dispatch-rift-upstream
September 13, 2026 09:00 — with
GitHub Actions
Active
GitHub Actions
Active
oshaughn
deployed
to
private-review-dispatch-rift-upstream
September 13, 2026 11:46 — with
GitHub Actions
Active
GitHub Actions
Active
mebiri
pushed a commit
to mebiri/research-projects-RIT
that referenced
this pull request
Sep 15, 2026
…dule default Review finding on oshaughn#193, and correct. JAXDistPhiMargLikelihood.sample_phi_ref carried its own `interp=JAX_INTERP_DEFAULT`, independent of the constructor's, so an instance built with any non-default stencil drew phi_ref from a DIFFERENT likelihood than the one it reports lnL and evidence from -- silently, nothing raised. Latent while both defaults were the string 'linear'; c0c4cf4 made it bite the documented recovery path, so `JAXDistPhiMargLikelihood(..., interp="linear")` gave a linear evidence with sinc phase draws. Same shape as the estimate_distance_peak leak fixed in 9a39a02: a second place where interp reached one consumer and not another. * sample_phi_ref now takes interp=None and falls back to self.interp; an explicit value still overrides, which is the only legitimate use. * All five wrapper classes now store self.interp. Four did not -- they only closed over it in their jitted closures, which is why the likelihood itself was always right and only the sampler drifted, and which left nothing for a method to fall back to. Verified by spying on the argument that actually reaches phi_ref_conditional_lnL, not on the drawn phases: the draw picks a grid index from 32 bins under a fixed seed, so it is insensitive to small lnL changes and reports "identical" for linear vs sinc even when the wiring is broken. That probe would have passed against the bug. test_no_method_silently_overrides_the_instance_stencil checks the SHAPE rather than this one method -- any non-__init__ method taking interp must default it to None and fall back to self.interp, and any class taking interp must store it -- so a new sampler with the same defect fails. Mutation-tested three ways: restoring the module default, dropping the fallback line, and removing one class's self.interp each fail it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mebiri
pushed a commit
to mebiri/research-projects-RIT
that referenced
this pull request
Sep 15, 2026
…inc-stencil-parity jax: add the 'sinc' stencil and make it the default (result-changing)
mebiri
pushed a commit
to mebiri/research-projects-RIT
that referenced
this pull request
Sep 15, 2026
left unlisted CI failed with "test_jax_stencil_parity.py is neither gated nor explicitly excluded". That file is NOT from this PR: PR oshaughn#193 (the JAX sinc stencil) landed it on rift_O4d minutes ago WITHOUT a manifest entry, so rift_O4d currently fails its own jax-ile-check -- verified by walking every test_*.py on the base against its FILES and EXCLUDED lists, where it is the only unlisted one. This PR merely inherited it. Fixed here rather than waited on, since this branch already owns that manifest: added to FILES with a ledger entry recording what it covers and that it arrived unlisted. If oshaughn#193 lands the same fix the conflict is one line. WHY MY LOCAL GATE WAS GREEN AT 109 AND CI RED. The local run tested this branch alone; CI tests the branch MERGED WITH THE BASE, and the file only exists on the base. A green local gate is evidence about a combination that will never ship -- which is exactly the failure mode the manifest check exists to catch, working as intended, just not on the PR that caused it. Checked the automerge kept both sides rather than trusting it: this branch's export_ess_estimate, _ADAPT_ADAPT_MODES, the _match_long_opt canonicalisation and the EXPERIMENTAL labelling are all present, as is oshaughn#193's sinc gatherer. EXPECTED_TESTS 109 -> 133 and both ci.yml cost notes, by collection. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mebiri
pushed a commit
to mebiri/research-projects-RIT
that referenced
this pull request
Sep 15, 2026
Adding test_jax_stencil_parity.py to the manifest (previous commit) fixed
the manifest check and immediately tripped a different rule: the gate
fails on ANY skip, and that file's cupy leg,
test_gpu_gather_parity_against_numpy_window, self-skips on a CPU runner.
It is a real gate -- on a GPU host.
The advice the gate prints ("exclude the file in FILES") is too blunt
here: it would drop the 23 CPU tests that are the point of oshaughn#193 in order
to silence one leg. So this adds DESELECTED_TESTS, a per-test escape
hatch applied to BOTH the collection and the run, with the reason written
down next to it. Deselecting is not the same as tolerating a skip: the
skip left the gate green while asserting nothing, whereas a deselected
test is accounted for in the ledger.
The hazard in doing it this way is that pytest SILENTLY IGNORES a
--deselect whose nodeid does not resolve -- measured, not assumed: a
deliberately mistyped nodeid collected 24 tests, exactly as if no
--deselect had been passed. Rename the test and the deselect quietly
stops applying, the skip returns, and the count is off by one. So the
entry is self-verifying: the file must exist, it must still define a
function of that name, and the nodeid must actually be absent from the
collection. All three legs were mutation-tested (renamed nodeid, moved
file, deselect not applied); all three are lethal and the clean case
passes.
EXPECTED_TESTS 140 -> 139 is the deselected test, not a lowered bar; the
ledger's per-file counts, the floor, and a real collection all agree at
139. Also removes a duplicated EXPECTED_TESTS line the merge resolution
left behind.
mebiri
pushed a commit
to mebiri/research-projects-RIT
that referenced
this pull request
Sep 15, 2026
… refuse bin/integrate_likelihood_extrinsic_batchmode defaulted --interpolate-time to 'nearest', the stencil its own help text disqualifies (200-443 nats; 1 nat of error by SNR 2-6; error grows as SNR^2), while the jax driver has defaulted to 'sinc' since PR oshaughn#193. Two drivers, opposite defaults, same physical choice: a cross-implementation comparison run at defaults was measuring a flag. Issue #233. The default is now time_interp_choice.TIME_INTERP_DEFAULT ('sinc'), and jax_ile.core.JAX_INTERP_DEFAULT is an ALIAS of that constant rather than a second literal, so the two cannot drift apart again. THIS CHANGES RESULTS for any run that did not pass --interpolate-time; pass '--interpolate-time nearest' to reproduce a pre-2026-09-02 run. A bare default flip would not have been safe. --interpolate-time carried three behaviours keyed on != 'nearest', all written when 'nearest' WAS the default, and each would have fired on runs passing no flag at all: the honoured-path gate would have turned every configuration without --time-marginalization/ --vectorized/--gpu into a startup ValueError; `auto` time-posterior export would have flipped grid -> continuous (a denser re-evaluation of the whole likelihood, a different draw algorithm, two new output columns, a newly reachable MemoryError); and --calibration-fused-kernel would have been silently abandoned for the loop path. The driver therefore distinguishes an explicit request from an inherited default -- on `is None`, before any string coercion, because str(None) == 'none' is itself a legal explicit spelling meaning 'nearest' -- and downgrades the default to 'nearest' with a printed reason where an explicit request is still refused unchanged. In the pipeline, an explicit off-request had to stop being dropped: emitting nothing used to mean 'nearest', so '--internal-ile-interpolate-time False' would now have turned interpolation ON. Measured tables (cost on CPU and GPU across mass and fmin, accuracy against the exact reference on the same grid, and the reproduction check against sections 3-4) are in DESIGN_q_window_stencil.md 9.6, with the concerns that are NOT resolved: high-mass low-fmin BBH is the population this default is worse for, and the 'bandlimited' quadrature's advantage was measured against 'nearest' and has not been re-measured under the new default. 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
2 participants
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.
Summary
Follow-up to #191, bringing the current
oshaughnessy-junior/rift_O4dintooshaughn/rift_O4d.oshaughn/rift_O4d@a70501d63(rc4 release-note/version update)oshaughnessy-junior/rift_O4d@da91a09f5Payload
multipeak-jaxlocal marginalization, with explicit refusal/decline accounting and no JIT/AD path through its host-side planner.Current CI status — merge blocker
The upstream CI run for
da91a09f5is currently failingcore-unit-check:test_portfolio_balance_heuristic.py --as-testreportslnI-lnZ = -0.827,sigma/I = 0.219, or 5.87 sigma from the known integral, exceeding its new 3-sigma gate.Other completed checks in that run are passing; several longer checks remain in progress. This PR is deliberately not represented as green or ready to merge until the blocker is resolved and the aggregate suite has passed at the current head.
Validation scope
The constituent PRs supplied focused tests and the GPU precompute suite, but this roll-up requires fresh aggregate validation at
da91a09f5. The required posterior shape-recovery gate for the integrator changes has not been re-established by this PR description.