ILE: add accurate reflected Q time pregrid - #189
Closed
oshaughnessy-junior wants to merge 97 commits into
Closed
oshaughnessy-junior wants to merge 97 commits into
oshaughnessy-junior wants to merge 97 commits into
Conversation
… limit External review, correct and the counterpart to the numpy fix already landed: the whole-cell fallback integrates with the SAME fixed node count spread over the entire cell, so rejecting a stalled Newton centre makes the resolution WORSE, not safer. The numpy twin measured 1.7e-03 nats of inner-u error that way. JAX CANNOT ADAPT THE COUNT -- shapes may not depend on traced values -- so this cannot be the per-call derivation the numpy path uses. The sizing is exposed instead as `required_u_nodes(amplitude)`, derived from the exact bound |d2g/du2| <= M2u ~ 5A: nothing on this axis is narrower than 1/sqrt(M2u), so a spacing of sigma_min/3 resolves the sharpest feature the coefficients admit. Same caller-side pattern as `required_n_phi`, for the same reason. DELIBERATELY NOT WIRED INTO THE DEFAULT, and the number is why. It reaches 2048 nodes at amplitude 1e4 -- roughly 40x the windowed cost -- to recover an effect measured at 2.2e-04 nats in the numpy twin, against a rule whose acceptance tolerance is 23 nats, on a path that no production calculation reaches: ANGLE_MARG_DEFAULT is 'exact', choose_angle_marg_scheme returns 'peak-local' at no amplitude, and a test pins that. Paying 40x by default for that would be the wrong trade, so U_NODES_PER_CELL's docstring now states plainly that its amplitude-independence holds for WINDOWED cells and not for fallback ones, and points at the helper. The alternative worth recording for whoever needs it: full convergence on a boundary-peaked cell needs a spacing set by the decay rate 1/M1u rather than the curvature scale, which the numpy twin measured at a 25x slowdown for the last 2.2e-04 nats. Both knobs are honest and both cost what they cost. 2 tests: the helper is derived and follows the sqrt law with a cap, and a whole-cell integration sized by it agrees with a 4x finer one to better than 1e-4 nats. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Same false claim I already retracted on the numpy side, still live here. The comment asserted the whole-cell fallback 'can only add nodes'; it adds none, it spreads the same n_nodes over the whole cell, so the fallback is COARSER than the window it replaces. A comment contradicting the code it describes is worse than no comment -- it is what let the defect sit unexamined. Points at required_u_nodes() and why raising the default is the wrong trade. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…metic 308 I raised the floor by adding 2 to the previous 306 -- which is exactly what the comment directly above it tells you never to do. Running the gate's own collection reports 311: the pre-existing floor on this base is 309, not 306, because #239 merged in between. The gate would have PASSED at 308. A >= floor set by arithmetic fails in the safe-looking direction -- it under-promises silently and masks exactly the tests it exists to notice going missing. Accounting written into the comment, including this error, since the number is only trustworthy with the method that produced it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both earned their place this week rather than being general advice. 'Do not let a comment outlive the code it describes' -- a comment contradicting its code is not a documentation defect, it is a place a bug can hide, because it answers the reviewer's question before the reviewer reaches the code. Three files, three authors, one week. This module's instance is the one #246 fixes. 'Do not put a broad except around a certificate call, in shipped code OR in a harness' -- found while measuring this note's own acceptance table: a broad 'except Exception' caught a tuple-unpack error and scored it as a DECLINE, reporting a flat 0% acceptance at every amplitude. Uniform, plausible, entirely fabricated, and caught only because it contradicted a number already in hand. A decline must come from the ledger, never from an exception. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CI collects 310; my local collection said 311. The gap is my harness -- it sliced this script by line number to reuse FILES and stopped before the loop that fills DESELECT from DESELECTED_TESTS, so it counted the GPU stencil-parity leg that the gate deliberately deselects on a CPU runner. Both of my attempts were wrong in opposite directions: 308 by arithmetic (below the truth, passes, under-promises) and 311 by a mis-set-up local collection (above it, fails). Comment now points at the job's own 'collected N tests' line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…uction tables Found by measuring the rho=163.08 coefficient tables rather than synthetic draws, and it is the concrete instance of the rule this branch already states: OMITTED-MASS CONTROL IS NOT INTERNAL ACCURACY, and the certificate can only see the first. On the real (2,+-2) structure -- A only in the k=2 phi harmonic at q=+-1, B almost entirely the real (k=0,ks=0) term -- the enumerated cover COLLAPSES TO ONE REGION SPANNING THE WHOLE TORUS: n_regions=1, area_outside=0, margin=-inf. The certificate reports that nothing whatever is omitted, and that is true. Inside, _log_box_integral capped each axis at 256 while the local curvature at amplitude 2.7e4 asks for ~3000, so the value sat up to 0.36 nats from a torus reference self-converged to 2e-12 -- errors of BOTH signs, so not a normalization offset. 0.36 nats is over half the saddle-point prototype's total error, arriving with a certificate that reads as exact. No random-coefficient draw reaches this branch; they make a genuinely 2-D landscape with isolated peaks. Only the physical sparsity collapses the cover. Cap 256 -> 512: worst error 0.359 -> 0.0014 nats (258x) for 0.07s -> 0.23s (3.3x). 1024 buys ~nothing more for 12x, so 512 is where the trade turns. This does NOT widen the certificate's reach -- declines are omitted-mass declines, internal accuracy is independent, and both are needed. The cap still BINDS at 512, so rep['n_boxes_pts_capped'] now counts under-resolved boxes. A capped box is an estimate the certificate cannot describe; it must never be silent. Two regressions, on the ACTUAL coefficients (my first fixture built A and B by hand and rescaled C to a target amplitude -- it declined, because uniform rescaling destroys the linear/quadratic balance that makes g peak at all). Verified non-vacuous: the accuracy test fails at cap 256 (0.263 nats) and passes at 512 (3.1e-04). Integrate gate 26 -> 28, from running the gate's own collection command. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t by a test Instances of the anti-goal this branch just added to the design note, reported by the paper-1 sessions verifying the shipped defaults against the tree. 1. bin/integrate_likelihood_extrinsic_jax refused --distance-grid-scheme loguniform with "...which the DEFAULT 'grid' scheme does not compute". ANGLE_MARG_DEFAULT has been "exact" since #225. The refusal condition was always right -- it fires on an explicit --angle-marg-scheme grid -- so nothing was broken; the only wrong thing was the text a user reads at the exact moment they are reasoning about which scheme they are running, which actively taught the wrong default. The TEST for this refusal already carried the correct comment ("Since #225 the default is a dense scheme, so it must be named explicitly to be refused"), so the codebase knew and only the user-facing string did not. It asserts on the substring "requires --angle-marg-scheme", which is unchanged. 2. ANGLE_MARG_CROSSOVER_AMPLITUDE's note said the auto selector sees "~2x the true amplitude", concluding laplace engages from true A ~ 225 (SNR ~ 21). That ran TWO DIFFERENT QUANTITIES together: the 2.0 is the `margin` ARGUMENT of estimate_angle_amplitude -- a deliberate parameter -- while the realized ratio of the margined bound to the true amplitude was MEASURED on the injection ladder at rung 1 (bound 1109.17 against rho^2/2 = 831.1) as 1.335. The realized number is the one that decides where the switch happens: rho ~ 26.0, which is what the manuscript quotes, not 21. Comment now keeps the assumed margin and the measured ratio distinct and says why, so the code and the paper stop quoting different crossovers for the same switch. I did not re-derive 1.335 here: estimate_angle_amplitude takes a data object rather than coefficient tables, so it is cited to the ladder's amplitude table rather than claimed. 49 tests pass across the loguniform-refusal and peak-local wiring suites. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ed it
The comment I wrote two commits ago to fix a conflation introduced a smaller one. It
said the ratio was measured against "the true amplitude"; the denominator is the NOMINAL
rho^2/2, not a measured maximum of the (phi,psi) exponent. Everything from there to "the
raw estimator sits below true A" runs through the identification true A == rho^2/2 --
this file's own convention, but not a measurement, and the comment stated it as one.
Two further limits now recorded, the second of which would have read as support:
* the ratio is constant to 6e-5 across rho = 40.77 ... 652.31, and that is ARITHMETIC,
not evidence. The ladder is ONE injection replayed at scaled amplitudes, so the
exponent rescales uniformly and the ratio is forced. A later reader -- including a
later one of us -- would take four decades of agreement as validating the margin.
It validates nothing.
* 1.335 is one injection's SKY-SAMPLE realization. The shortfall is set by how sharp
the sky peak is relative to the sample, and the sample does not contain the
injection's sky position while the exponent is sharp enough that 1 of 9824
(sky, time) points sits within 23 nats of the peak. A shortfall is expected by
design there and is not bounded for another event.
What survives is the claim worth having: at this injection the margin is load-bearing
rather than decorative. And the general-case protection is _runtime_amp_failsafe
recomputing the amplitude at the point of use, which holds whether or not the margin was
well chosen -- so this is a caveat on what may be WRITTEN, not on whether the code is safe.
rho ~ 26.0 is unaffected: it follows from the bound-to-rho^2/2 ratio, which is the
quantity actually measured. Ratios measured by the paper-1 ladder session.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…what it means Ran the rho=40.77 production tables (the cell the manuscript quotes) through the same old-cap/new-cap comparison that exposed the rung-3 defect. Result: error 0.00000 nats at cap 256 AND at 512, on all eight mass-carrying points. So no accuracy figure taken at that rung before today needs re-taking -- the good outcome, and worth having measured rather than assumed from the rung-3 result. BUT IT CORRECTS MY OWN COMMIT. I introduced n_boxes_pts_capped two commits ago describing a capped box as "under-resolved and the value is an estimate". Rung 1 caps on EVERY mass-carrying point and is exact. So the flag means the sizing rule ASKED FOR MORE NODES THAN IT GOT -- a truncated request -- and not that the answer is wrong. The trapezoid on a periodic integrand converges fast enough that the derived count is conservative at amplitude ~2.5e3 and binding at ~2.8e4. Left as-is, a counter that fires on a provably exact result would have taught the next reader to distrust correct values, which is the same defect class as a comment that contradicts its code. Now documented as "look here", not "this is broken" -- still worth surfacing, because it is the ONLY signal available: the certificate cannot see inside a box at all. Also guarded ANGLE_MARG_CROSSOVER_AMPLITUDE against two ratios now circulating for this ladder, 0.1888 and 7.069, neither of which is the margin: both are the SNR-guess deficit squared (guess_amp == guess_snr^2/2 exactly, rho/guess_snr = 2.3014 constant). guess_snr is the ABANDONED sizing route. 7.069 recorded as "the margin" would inflate a ~1.5x effect to 7x and credit the live estimator with the dead route's deficit. They are easy to accept because they AGREE with the conclusion for an unrelated reason -- corroboration by coincidence. The reportable fact is kept: guess_snr sits 2.30x below true rho on this ladder, so the abandoned route would have sized the dense grids from an amplitude 7.07x too small -- the docstring's stated failure mode, measured rather than argued. Scope on all of it: l_max 2, one injection, one seed, one guess_snr. Ratios measured by the paper-1 ladder session. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… lines from its own retraction Prompted by a peer hitting the same thing in their own file: when you correct a claim, grep the NUMBER, not the paragraph. I had not, and there were two. 1. joint_angle_peak_local.py:692 still read "the conservative branch: it can only add nodes, never move the centre" -- the exact false sentence whose retraction sits FOURTEEN LINES BELOW IT in the same function. I wrote the correction into a new block and left the original standing, so the file simultaneously asserted and denied the claim, and the assertion came first. A reader scanning top-down gets the false one. Now says what is actually true: whole-cell fallback is conservative for the CENTRE (it never lands on a non-stationary point) and NOT for the resolution. 2. anglemarg.py:131 still described the crossover as "rho ~21-30". 21 is the superseded figure, from assuming the realized factor equalled the margin=2.0 argument; the measured ratio gives ~26. Two crossovers in one file, 40 lines apart, one of them the number the manuscript quotes. Both are the anti-goal this branch added to the design note, committed by me, hours after committing the rule. Comment-only: verified no non-comment line changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
'A comment that contradicts its code is a place a bug can hide' is diagnosis; this is the procedure. When you correct a claim, grep the NUMBER, not the paragraph -- a correction written into a new block leaves the old one standing, and the assertion usually comes first, so a top-down reader gets the false version. Evidence is mine, from hours after I committed the rule: 'can only add nodes' fourteen lines from its own retraction, and 'rho ~21-30' forty lines from the corrected ~26, the superseded figure being the one the manuscript quotes. Delete the stale copy rather than annotating it -- two copies of one claim at different scopes is worse than either alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d against I shipped 'sized the dense grids from an amplitude 7.07x too small' with no denominator, in the same comment block written to stop unlabelled ratios being taken for the margin. It is 7.07x against the LIVE data-derived bound (the operative figure, since that is what sizes grids today) and 5.30x against the nominal rho^2/2 -- the two differing by exactly the 1.335 named four lines above, so a reader dividing by the wrong one is off by 1.335 and has no way to notice. Both now stated with their denominators, and the fact that I introduced the defect one commit before fixing it is recorded next to them, because that is the part a later reader should weigh when deciding how much to trust the surrounding numbers. Arithmetic verified: 2.3014^2 = 5.2964, x 1.33465 = 7.0689. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…s the grep
Applied a peer's numeral-frequency sweep to my own files. A duplicated number is
invisible to rereading because every copy is LOCALLY CONSISTENT -- each reads correctly in
its own paragraph -- so the technique has to be counting:
grep -oE '[0-9]+\.[0-9]{2,}(e[-+]?[0-9]+)?' FILE | sort | uniq -c | sort -rn
Two findings in my files:
* 0.36 stated three times in separated blocks. All agreed today; separated copies are
exactly the ones that can stop agreeing. Now stated ONCE on _BOX_MAX_PTS, with the
two restatements replaced by references to it -- structural rather than editorial, so
a later editor cannot helpfully restore a superseded copy.
* the same number spelled BOTH 7.069 and 7.07 within one comment block, which DEFEATS
THE GREP ITSELF: correcting one spelling silently leaves the other, and the sweep
reports them as two unrelated values. Normalized to one spelling (and 5.30 -> 5.296
for the same reason). A number must have one spelling before frequency-counting it
means anything.
Technique and both findings recorded in the design note. Two sessions found their own
violations of this rule hours after committing it, which is the useful part: writing the
rule is what makes you look, and looking is what feels unnecessary right after correcting
the paragraph in front of you.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
My first sweep grouped numerals by STRING, which reports 7.069 and 7.07 as two unrelated
values -- the exact defect it was written to catch. A peer hit the same failure one level
subtler, grouping at three significant figures and putting 6.8966e-04 and 0.00069 in
different buckets, so their tool missed a third spelling of a value it was built to find
and reported clean. That is worse than not running it: you now believe you checked.
Re-swept my files grouping by numeric value at 4 s.f. and flagging groups whose SPELLINGS
differ. Two real same-quantity splits, both mine:
* anglemarg.py restated a table's -1.8e-04 as -1.8e-4 in prose, so correcting the table
would have left the prose copy standing. Spelled to match the table it restates.
* joint_anglemarg_peaklocal.py stated the 1.7e-03 fallback measurement in THREE
separated blocks. Now stated once on U_NODES_PER_CELL with the other two referring
to it -- structural, so a later editor cannot restore a superseded copy.
Left deliberately, checked rather than assumed: rho spelled 163.1 in an aligned table row
label and 163.08 in prose. That is the column-alignment carve-out, not a second spelling,
and a normalization rule that cannot tell the difference does damage.
Design note now carries the value-grouping requirement, both carve-outs, and the
convention that avoids the problem: quote ONE rounded form and let the committed records
carry the digits -- duplicated full precision is not an audit trail, it is a second
spelling that hides from the grep.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A verification that CANNOT FAIL is indistinguishable from one that passed. Stated by a
peer as the unification of three things I had been recording as separate lessons, and they
are right that it is one failure wearing three faces:
* a guard that cannot discriminate -- n_boxes_pts_capped fires on every mass-carrying
point at rung 1 (exact to 0.00000 nats) and identically at rung 3 (0.36 nats wrong);
* a check whose pass condition is empty output -- a missing binary plus 2>/dev/null is
indistinguishable from a clean result;
* a sweep that hides the defect from itself -- grouping numerals by string reports
7.069 and 7.07 as unrelated, so the tool written to find multi-spelling reports clean
on a file that has it.
The third is the worst, because running it converts 'unchecked' into 'checked and clean'
without touching the code. Operational form: before trusting a check, name the input that
would make it FAIL. If you cannot, it is decoration.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Complement to the rule directly above it, and stated by the peer whose overreach produced it. One is a check that CANNOT fail; this is a claim nobody checked BECAUSE its parts were checked. The instance: four per-axis scheme defaults, each independently verified from the code and each holding, compressed into one sentence asserting a pattern that one of the four axes is a counterexample to -- the default there having been deliberately moved to the ACCURATE scheme, with the superseded spelling kept under a separate name so older runs reproduce. Every input true, summary false. Verifying the parts is the step that makes checking the whole feel unnecessary, which is exactly when it is required. Placed beside the 'verification that cannot fail' rule rather than in its own section, because separating two halves of one lesson is the duplication defect this file already warns about. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The test docstring for the 0.36-nat finding called it 'half the saddle-point prototype's total error at rho=40.77'. That 0.654 nat figure has since been retracted by the session that produced it: the prototype's start-point search was unconverged, per-point values move up to 1.2 nats, and the rung-1 point CHANGES SIGN under refinement (+0.654 -> -0.547 -> -0.078 -> +0.663). So my sentence had a retracted denominator. A ratio against a retracted number is worse than no ratio -- it inherits the other figure's instability while looking like corroboration, which is the same shape as the 0.1888/7.069 ratios guarded against in anglemarg.py and as the coincidental agreement noted there. The finding needs no comparison to be a defect: the certificate reported nothing omitted while the value was wrong, stated against the converged torus reference in the same test and against nothing else. This breaks the freeze I put on this branch, deliberately: the branch was frozen for being too broad, and shipping a claim resting on a withdrawn measurement is a correctness issue rather than more scope. Both dumps re-fetched from origin/main and confirmed BYTE-IDENTICAL to the copies the rung-3 and rung-40 numbers were measured on, so neither measurement needs redoing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rd it exposed An automated agent (Session Router Gate, 28fcd5e) pushed to this branch and wired required_u_nodes into fused_log_likelihood_distphipsimarg_peaklocal with a refusal above 2048 nodes. Reverted, with numbers, because it does the risky half of the review finding and not the safe half: * IT BROKE CI. test_wrapper_peak_local_matches_exact[30.0] now raises -- 1 failed / 309 passed -- because amp_sizing=2690.91 needs 2188 nodes. * IT REFUSES FROM amp_sizing ~ 2359, i.e. rho ~ 69, where this rule's own certificate accepts to rho 141-200 (100% at 1e4, 85% quadrupole-dominated at 2e4). That halves the usable range to remove a 1.7e-03 nat inner-u error, against a 23 nat acceptance tolerance, on a path production cannot reach. * IT LEFT samplers.py UNTOUCHED, so it MADE THE REVIEWED DEFECT REAL: the guard still modelled 48 nodes while the kernel now requested 896 at the production floor amp_sizing=450 -- the documented live slab going 3.6 GiB -> 67.2 GiB at chunk one. P1 (batch-memory guard): fixed at the root instead. u_nodes_in_use() is now the single place both the kernel and the guard read, so they cannot diverge again whatever anyone wires later. Reading U_NODES_PER_CELL directly from outside the module is what made a one-line change in one file silently invalidate a guard in another. P1 (production data in a test): correct, and fixed. The fixture hard-coded coefficients and sky/time indices from an actual production evaluation; merging it would have published run-derived scientific data. Replaced by a seeded synthetic draw -- and it needed a SEED SEARCH, because the same sparsity PATTERN with round numbers does not reproduce the collapse at all (n_regions=4, area_outside=31.7, zero error). The relative PHASES decide whether the regions merge into one spanning the torus. Seed 113 of 200 reproduces it: n_regions=1, area_outside=0, 0.298 nats at cap 256 and 0.0016 at 512, so the regression still FAILS at the old cap and passes at the new one. No coefficient values or location metadata remain. 28 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… can be The roster landed in #243 with three entries whose status was BROKEN -- tests that collect and FAIL on rift_O4d, found only because that audit ran files no job had run in months. A roster that tolerates BROKEN indefinitely is the rot it was built to stop, so: all three fixed, the two that are real test suites registered with core-unit-check, and the roster's BROKEN section deleted because it is now empty. 1. test/integrators/test_replica_pooling.py -- 10 of 15 failed. It slices six helpers out of bin/integrate_likelihood_extrinsic_batchmode by regex and exec()s them. The driver was refactored so _lnZ_of_rvs and _kish_neff_of_rvs delegate to a seventh, _lw_of, which the list did not name; inside the exec'd module _lw_of was undefined, the driver's own `except Exception: return None` swallowed the NameError, and the tests died on `None - float` -- a symptom three steps from the cause. _lw_of added, but the name list is no longer the only defence: after exec, every global the sliced functions reference must resolve, and the assertion NAMES the missing helper. Mutation-checked twice -- dropping _lw_of again, and renaming the driver's helper to something novel -- and both now fail with "sliced helpers reference names that were not sliced out of the driver: [...]" instead of a TypeError elsewhere. 2. RIFT/hyperpipe/marg_list.py -- _stage_event_file wrote to the wrong directory. It staged event-<i>.net into base_dir while accepting run_dir and never using it. Under hydra those differ: base_dir is the ORIGINAL cwd the user launched from, run_dir the per-run output dir. So staged event files landed in the launch directory, and two runs started from one directory overwrote each other's event-<i>.net. The implementation was the outlier, not the test. assemble_marg_list's own docstring says run_dir is "where event-<i>.net files and copies of non-core exes are written"; the exe staging a few lines below already does that; and test_marg_list.py, test_hydra_integration.py and standalone_check.py all assert the run_dir location. Sources still resolve against base_dir. Full hyperpipe suite: 37 passed, 1 skipped. Two of the three tests that would have caught this could not: test_marg_list.py was gated by no job, and test_hydra_integration.py skips without hydra. 3. RIFT/interpolators/jax_gp/test_interpolators.py -- 10 errors, not 10 skips. It needs jax and optax, neither in requirements.txt, and let the ImportError escape at collection, so "not installed" reported as ten FAILING tests. Now skips at module level, guarded so a direct `python -m` run still raises the real ImportError. Stays OPTDEP: promoting it to jax-ile-check needs optax installed there and that job's pinned counts re-measured, which is a separate costed change. core-unit-check gains test_replica_pooling.py and test_marg_list.py: 278/266 -> 296/284, ~55 s. REPORTED, NOT FIXED: test_mcsampler_foridiots.py stays out (a demo with no test functions, HANDRUN not BROKEN), but the reason it dies is not the demo's. It hits mcsamplerGPU.py:1324, `weights_alt = int_vals**tempering_exp` in the `not save_intg` branch of integrate(), where int_vals exists nowhere in scope -- so that branch cannot ever have run, and it is reachable on CPU. The sibling branches use self._rvs["integrand"][-n_history:] and the local holding those values when nothing is saved is `fval`, so `fval**tempering_exp` is the near-certain intent. Guessing it is core sampler code and RO'S call, not a side effect of a test-hygiene PR; the diagnosis is recorded beside the roster entry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
u_nodes_in_use's docstring said "Both sides now call this". Only the guard did. joint_lnL_phi_dense still defaulted straight to U_NODES_PER_CELL, the fused caller passed no n_nodes at all, and the guard called the helper with no amp_sizing though it had one. So an amplitude-dependent change would have moved the guard and left the kernel behind -- the exact divergence the helper was added to prevent, one commit after adding it. This is the anti-goal this branch itself states, committed by me while stating it: a comment that contradicts its code is a place a bug can hide. Worse than the usual case, because the comment asserted the very property the reader would otherwise have checked. Fixed by making the claim TRUE rather than by weakening it to describe the constant: * joint_lnL_phi_dense n_nodes defaults to None and resolves through u_nodes_in_use() * the fused caller passes n_nodes=u_nodes_in_use(amp_sizing) * the guard passes the SAME amp_sizing, read from like.angle_marg_info BIT-IDENTICAL today: u_nodes_in_use returns U_NODES_PER_CELL at every amplitude including None (verified across None/1/450/2690.91/1e4/1e6), so threading amp_sizing changes no result. It is threaded so that a future amplitude-dependent sizing moves both sides. log_inner_u_integral still defaults to the constant, and that is fine: joint_lnL_phi_dense passes n_nodes to it positionally, so the default is unreachable on this path. Checked rather than assumed, since it would have been a second silent default. Regression pins the invariant that matters. NOT "both currently equal 48" -- that passes even if neither side reads the helper -- but that patching the HELPER moves BOTH, observed on the guard's cap and on the count the kernel actually hands the inner integral. Verified non-vacuous: reverting the kernel to its pre-fix default makes it FAIL, restoring makes it pass. The test shape is deliberately not the production one, and the first version was wrong for an instructive reason: at npts=614 with 256 distance nodes the cap is ALREADY pinned at its floor of 1 -- the measured "peak-local batches one sample" result -- so quadrupling the node count cannot move it and the assertion read "1 < 1" and failed while the wiring was correct. A saturated observable cannot test the thing it saturates on. npts=64 with 32 nodes gives 85, clear of the floor and of the 8000 ceiling. Gate 310 -> 311, measured. 25 tests pass across both jax suites. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review P2, and correct: the guard added to test_interpolators.py could never run. Pytest imports RIFT/interpolators/jax_gp/__init__.py before any test module in that directory, and that initializer did `import jax as _jax` unconditionally -- so on a machine without the stack collection died with ModuleNotFoundError before reaching the guard. I did not catch it because this environment HAS jax and lacks only optax, so the guard fired on optax and looked like it worked. Reproduced under a meta_path blocker that makes jax, jaxlib, optax, equinox and tinygp raise ModuleNotFoundError exactly as absence does; the failure is the reviewer's, verbatim. The package docstring already called this subpackage OPTIONAL and said the jax stack "is not required for normal operation". The initializer contradicted it. WHAT IS *NOT* CHANGED, deliberately. Only the ABSENCE of jax is tolerated. When jax is present the sequence is unchanged and stays EAGER, because the x64 enable is load-bearing by side effect: applications/compare.py, applications/jax_cip.py and applications/export_at_scale.py all import this package for nothing else, each saying "enables float64" at the import site. Deferring it into get_interpolator() would leave those three silently in float32 -- a wrong-gradient bug that raises nothing. Verified: jax_enable_x64 still goes False -> True across `import RIFT.interpolators.jax_gp`. A module __getattr__ keeps the jax-absent case honest: BaseInterpolator re-raises the real ModuleNotFoundError naming jax rather than a bare AttributeError that reads like a typo, while unknown names still raise AttributeError -- so `from RIFT.interpolators.jax_gp import export` still resolves to the SUBMODULE via the import machinery's fallback. Checked both. test_coordinates.py had the same latent failure and no guard at all -- it is in the same package and dies the same way. Given the same treatment rather than left for the next reviewer. Verified in both environments: no jax : 2 skipped, with reasons naming the missing module (was: collection error) with jax : 2 passed, 1 skipped on optax -- unchanged from before this commit Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…an run
Reviewer: the missing-JAX skip guard runs too late -- pytest imports jax_gp/__init__.py
before test_interpolators.py, and that initializer imports jax unconditionally, so collection
still dies with ModuleNotFoundError before the guard is reached.
Correct, and reproduced. My guard only ever handled the case I could see: this host HAS jax
and lacks optax, so the optax path was exercised and the no-jax path never was. Blocking jax
with a sys.meta_path finder reproduces the report exactly.
THE FIX BELONGS IN __init__.py, because that is where the false claim lives. Its own
docstring has always said "This is an *optional* subpackage ... the JAX dependency stack ... is
not required for normal operation" while line 1 of its body was `import jax`. Importing the
package now works without jax; ASKING it for something is what needs the stack, and the error
a caller sees names jax rather than a shim:
>>> g.get_interpolator('rff') -> ModuleNotFoundError: No module named 'jax'
>>> g.BaseInterpolator -> ModuleNotFoundError: No module named 'jax' (PEP 562)
>>> g.nonexistent -> AttributeError
The x64 enable stays EAGER and ahead of every submodule import when jax is present. That
ordering is load-bearing: a backend imported before it runs silently gets float32, which reads
as a precision regression in the model rather than a config mistake. Verified unchanged --
x64 True, jnp.zeros(1).dtype float64.
test_coordinates.py had the SAME defect and no guard at all; it now has one.
Both guards also stopped lying about the direct-run path. They skipped whenever `import
pytest` succeeded, which is nearly everywhere, so `python -m RIFT.interpolators.jax_gp.
test_coordinates` -- the invocation both docstrings advertise -- died with a pytest `Skipped`
exception instead of the real ImportError, exactly contrary to the comment above it. They now
skip only when pytest is already in sys.modules, i.e. when pytest is the importer.
Four paths checked, having previously checked one:
no jax, under pytest -> both SKIP (the reported bug)
no jax, direct run -> ModuleNotFoundError (the docstring's promise, now true)
jax, no optax -> coordinates 2 pass, interpolators SKIP
jax present -> x64 True, float64, BaseInterpolator resolves
test-all-mod.py: 186 passed, unchanged against HEAD (the 5 failures are cupy/gpytorch absences
on this CPU host, pre-existing). RIFT.interpolators.jax_gp itself PASSES, and imports with jax
blocked. ci-roster-check PASS.
Note on the roster: its reasons for both files already read "skips cleanly without it". That
was FALSE for both when written -- I asserted behaviour I had not exercised. It is true now.
The census enforces that a reason EXISTS, not that it is correct; that limit is worth stating.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…lback_resolution Internal accuracy the certificate cannot see: a fully-covering cover that was 0.36 nats wrong
…ster-tests Fix the three tests the CI census found BROKEN, and gate the two that can be
…ming 4 GiB The cap added in c5b81dd is correct and still needed -- it exists because the laplace path asked XLA for a single 36.41 GiB buffer at chunk 4000 / npts 1193 and died RESOURCE_EXHAUSTED. But 4 GiB was sized against the 25 GiB per-UID cgroup of the machine the OOM was reproduced on, with a deliberate ~6x margin, and it is a bare constant with no device awareness. IT NOW THROTTLES FOR NO REASON ON THE CARDS WE ACTUALLY USE. cap = TARGET // (8192 * npts), so at a production npts of 1230 (a 0.15 s arrival-time window at 4096 Hz) it caps the eval chunk at 426 where the nominal chunk is 1000. `exact`, `laplace` and `peak-local` therefore run at under half the batch `grid` gets -- and small batches are precisely where their per-sample cost is worst: a companion scan measured exact at 1.83 s/sample at batch 8 against 1.04 at 512. The scheme we most want to afford is the one being throttled. Derived from jax's own device memory_stats() at 25% of the reported limit, falling back to the historical 4 GiB whenever the device cannot be interrogated -- no jax, no GPU, or an API that moved. A machine we cannot measure behaves exactly as it did before rather than getting a larger number by accident, and the fraction is deliberate: this bounds ONE buffer and the rest of the graph lives alongside it. Tests pin the BOUND, not the constant that used to express it: the original blowup is still refused at 4 GiB; the implied buffer stays within target at 4, 12 and 24 GiB across npts 614..32769; a 16 GiB device stops throttling at production npts while 4 GiB still does; `grid` is never capped; and a failed probe falls back to 4 GiB. Mutation-tested: removing the cap fails 5 of 7; hard-wiring the target back to the 4 GiB constant fails exactly the test that says a bigger device should lift the throttle. 7 pass restored. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…er target One conflict, two adjacent changes, both wanted: #246 (u-fallback resolution) replaced _jp.U_NODES_PER_CELL with the resolved n_u_live in peak-local's bytes-per-sample-point model -- its sizing is now derived rather than a constant. this PR replaced the hard-wired _ANGLE_MARG_BUFFER_TARGET with _angle_marg_buffer_target(), read from the device. They touch the same two lines and are independent: one fixes WHAT the buffer model counts, the other fixes HOW MUCH buffer we allow. Taking either side alone would have silently dropped a fix, so both are kept. Verified rather than assumed: n_u_live is defined in the merged file (it is #246's own local), syntax parses, and the 7 buffer-cap regressions still pass -- including the one pinning the original 36.41 GiB blowup as refused. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
RO'S: 25% is over-conservative. Agreed, and raised -- but the honest form of this
change is to say which part is measured and which is judgement.
MEASURED, and it is why the fraction cannot go to 1.0: these cards are SHARED. A
survey of ldas-pcdev11 while sizing this found all four GPUs at 100% utilisation
with 18-22 GiB of 24 GiB already held by other users. `bytes_limit` is what JAX
believes it may have at the moment it is asked, not a reservation, so sizing at
the full limit OOMs as soon as we share a card -- which here is the normal case.
NOT MEASURED: how much the rest of the graph needs alongside this one buffer. I
tried to measure it -- real laplace eval on a free Blackwell, polling device
memory -- and the run died twice in the JAX thread pool against the interactive
hosts' 500-thread cap, with 281 already held by other sessions. Peak reached 785
MiB before it died, which is not an answer. So 0.5 is a JUDGEMENT: twice the
first guess, still half the reported limit, and labelled as such in the code
rather than presented as a result.
Overridable for anyone who knows the card is theirs:
RIFT_ANGLEMARG_BUFFER_FRACTION=0.8
The regression tests pin the BOUND at any target, so raising the fraction cannot
reintroduce the 36.41 GiB blowup -- that is what makes relaxing it safe to do
before the overhead measurement exists.
7 tests pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Roll the independent sample-time point axis inside the Laplace distance/psi kernel so its QCH x dist_block x phi_chunk temporary is capped at 32 MiB instead of scaling as 8192*S*T bytes. Keep the conservative caller cap because coefficient tables remain O(S*T) and exact/peak-local still need the same treatment. Add an allocation model plus mutation-bearing shape and value/gradient tail tests.
…3 IFOs) Profiling the maintained GPU likelihood on the ILE-GPU-Paper demo shows the hand-written CUDA kernel is only 5.7% of DiscreteFactoredLogLikelihoodViaArrayVectorNoLoop; ~94% is cupy glue. Three stages -- SphericalHarmonicsVectorized (22.2%), ComputeDetAMResponse (17.1%) and TimeDelayFromEarthCenter (3.6%) -- were rebuilt once per detector inside the loop although none of them depends on the detector. They act on (n_extrinsic,) arrays, so that time is kernel-launch bound, not bandwidth bound, and the redundancy is paid in full. Split each of the two vectorized LAL tools into a source-only prologue and a per-detector half, and build the prologue (plus the Ylm array, which depends only on modes/incl/phiref) once per likelihood call. Also cache DetectorPrefixToLALDetector and its two host-to-device transfers, which were redone every call for values fixed per interferometer. The per-detector halves keep the identical `inner` contractions in the identical order, so lnL is bitwise unchanged -- verified by replaying captured production NoLoop arguments through both trees on GPU and on CPU. A single batched einsum over stacked detectors would be fewer launches still, but reassociates the contraction and agrees only to ~4e-16; that is deliberately not done. (ComputeDetAMResponse's advertised leading detector axis does not in fact work -- X * inner(X, R) fails to broadcast -- so nothing depended on it.) Two sharing hazards are handled explicitly: the phase-marginalization branch conjugates Ylms_vec in place while rho_sq_det above it needs the un-conjugated array, so each detector gets a copy when that branch is active; and lookupNKDict[det] may be a device array, so mode-list identity is memoized on the array object rather than compared per call, which would force a sync. Measured on an RTX PRO 4000 Blackwell, cupy 14.1.1 / CUDA 12.9, --interpolate-time nearest --n-chunk 10000, 100 calls per timing, 3 reps: H1 L1 12.41 -> 10.26 ms/call -17.4% H1 L1 V1 16.81 -> 13.06 ms/call -22.3% The saving scales with detector count, as expected. The CPU (numpy) path is unchanged within noise. Rationale and the measurements behind it are recorded in RIFT/likelihood/DESIGN_noloop_per_detector_glue.md. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…n-integration jax_ile: bound direct marginalization by error and memory budgets
…tained-fft-validated time marg: evaluate only the retained bandlimited FFT grid
Review round 3, both P1s upheld. THE SHIFTED COMPANION CLOSED ONE ALIAS, NOT THE FAMILY. conv_shift detects odd multiples of the coarse sampling frequency and shares every even one with the rule it is probing, and exp(F) is not band-limited, so all three rules can agree and still be wrong. The general form of that is stronger than the specific adversary: NO rule can see its own aliases in its own samples, so paying 96 extra evaluations to probe harmonic 96 only relocates the blind spot to 192. (The reviewer's c_2n case did not reproduce at kappa = 1000 -- I_192(1000) << I_96(1000), so the answer came back accurate -- but one failed construction is not a refutation and the mechanism is real.) So the extra grid is withdrawn and the evaluations are spent on the ANSWER. With an odd node count one grid is already nested: even indices are a trapezoid at half the density, odd indices are exactly its midpoints. Both probes become free, nothing evaluated is discarded, and the count rises to 193 so the answer rides a level finer than the rules the probes can certify. Measured on the aliasing counterexample, at IDENTICAL evaluation cost to the version this replaces: the answer goes from 0.02017 nats wrong to right within 1e-6, and it still declines because the 97-node rule the probes measure was bad. The pure cost saving -- nesting at 97 and halving the work -- is ruled out by measurement, not taste: there both probes read 1.1e-13 on that same 0.02-nat error, so it would ACCEPT. The node count is set by what the probes can reach, not by the accuracy of the answer, and that is now written down. THREE POINTS PER CURVATURE LENGTH IS NOT AN ERROR BOUND, and `bound_exact` was a name promoting an estimate into a certificate. Renamed u_sizing_ok. Review is also right that it uses the stationary scale 1/sqrt(M2u) where a boundary layer has the narrower 1/M1u; that stricter count is now computed and reported as n_u_understood_bound. It is deliberately NOT gated, because gating it declines the amplitude-19 row that is accurate to 1e-5 -- which is itself the evidence that this axis is empirically gated, so it belongs where a caller can see it rather than in a comment. WHAT ok ASSERTS IS NOW WRITTEN DOWN. One genuine bound (the omitted-mass margin, lifted by an exact second-order remainder) and two empirical gates. And the margin's own soundness runs through Fb, which the empirical gates are what stand behind, so the chain is empirical END TO END. The docstring says so and says this path must not be described as fail-closed. Bounds tight enough to replace the gates were looked for and do not appear to exist at usable tightness -- exact M2F demands 3.8e3-2.3e4 nodes for cases right to 1e-4, and 1/M1u declines cases right to 1e-5, both collapsing to "always decline" -- which is why the claims are narrowed rather than the gates replaced. 53 tests pass (31 joint, 10 algebraic, 12 wiring) in 621s, less than the wiring suite alone measured last round under a concurrent CI job, so the earlier 747s was contention and not this kernel. Gate re-collected: 329. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…% more) Per-operation timing at production shapes (n_extrinsic 10000, npts 614, three detectors) accounts for 98% of NoLoop and says the data term dominates -- not because of what is computed into it, but because of how it is stored. rho_sq is the <h|h> term and has no time dependence: each detector contributes an (npts_extrinsic,) vector, which was broadcast into a dense (npts_extrinsic, npts) accumulator. That is a 49 MB zero-fill plus one 49 MB read-modify-write per detector to store npts identical copies of each value. Sum the vector instead and expose the 2-D shape as a stride-0 broadcast view. Downstream arithmetic is elementwise and sees no difference; the additions happen in the same order on the same scalars. The calibration path already did this for rho_sq_cal. Consumers needing real backing memory -- the fused calmarg CUDA kernels, which index raw device pointers, and the non-Simpson quadrature helpers, which may write -- go through _dense_rho_sq() and pay what they did before. kappa_sq is 98 MB of complex128. It was zero-filled, and each detector's distance scaling allocated another full-size temporary before accumulating. Scale the Q kernel's own freshly allocated output buffer in place and take the first detector's buffer as the accumulator, which removes one full-size fill and one temporary per detector. Both are bitwise, verified by replaying captured production NoLoop arguments through base and patched trees on GPU and on CPU. One caveat for the record: 0.0 + x is exactly x for finite x, Inf and NaN, but 0.0 + (-0.0) is +0.0 while starting from the buffer preserves -0.0. A signed zero in kappa_sq is unobservable downstream (it survives .real, and exp(-0.0) == exp(+0.0)). Measured on an RTX PRO 4000 Blackwell, H1 L1 V1, --interpolate-time nearest, --n-chunk 10000, 100 calls per timing: rift_O4d 17.06 ms/call + hoist (previous commit) 13.08 -23.3% + rho_sq as a vector 12.10 -29.1% + kappa_sq in place 11.12 -34.8% test/test_noloop_accumulator_shapes.py pins both accumulators against a reference written the original way, using array_equal rather than a tolerance, at one, two and three detectors. That reference also passes against the unpatched tree, which is what makes it a check on the change rather than a transcription of it. Not done here, and not bitwise: simps is 1.765 ms/call and equals a matvec against precomputed weights at 0.049 ms, a 36x saving, but a gemv reassociates the summation. It needs its own accuracy argument. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
simps() over the (npts_extrinsic, npts) integrand was 1.765 ms/call, the largest remaining item after the accumulator work. It is a fixed linear functional at fixed dx, so it equals a matrix-vector product against precomputed weights: measured 0.049 ms, a 36x saving on that step. The fused calmarg path already built exactly these weights by hand with w_t = simps(eye(npts)). That is now one cached helper, _simps_weights, used by the hot path, by both calibration reductions and by the fused branch, so the tree carries one definition of the equivalence instead of two. UNLIKE THE REST OF THIS BRANCH THIS IS NOT BITWISE. A gemv reassociates the summation. It is the same RULE -- the weights come from the very simps implementation the call site would otherwise have used, so the even='avg' versus Cartwright distinction between the vendored GPU copy and scipy's is preserved exactly -- and only the order of the additions changes. Measured over 10000 real extrinsic samples spanning lnL from -2.2e6 to +116: max |dlnL| 2.8e-14 nats, median exactly 0, max relative 7.1e-14, against a float64 rounding scale for those values of 4.9e-10. Both paths are deterministic run to run. For physical scale, the errors already in this integral are eleven to sixteen orders of magnitude larger: the two simps variants in this tree disagree by 0.405 nats on an under-resolved peak, and the 'nearest' time stencil costs 200-443 nats at SNR 100. Simpson's accuracy limit here is sub-sample resolution of a peak whose width is set by the signal rather than the sample rate, which is what the time_quadrature and stencil work addresses -- not the order of its additions. The test now splits the two guarantees instead of blurring them: the accumulators are checked with array_equal at return_lnLt=True, before the integral, and the quadrature is checked separately against simps at a tolerance far tighter than anything physical, so a failure there means the rule changed rather than that rounding drifted. A third test pins the linearity the matvec rests on. Cumulative on an RTX PRO 4000 Blackwell, H1 L1 V1, --interpolate-time nearest, --n-chunk 10000, 100 calls per timing: rift_O4d 17.02 ms/call + hoist source-only geometry 13.08 -23.3% + rho_sq as a vector 12.10 -29.1% + kappa_sq in place 11.12 -34.8% + time integral as a matvec 9.50 -44.2% Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…evice_aware_cap anglemarg: derive the eval-buffer cap from the device instead of assuming 4 GiB
247 merges immediately after 250, so it has to be compatible with it rather than with the base it was written against. Merging surfaced three things. BIVARIATE_TRIG_STATIONARY SHIPS IN 250, NOT 252. The stronger enumerator -- BKK root count, Jacobian conditioning, torus classification, cross-projection agreement, fail-closed ok -- is in the branch landing directly ahead of this one, so the duplication is immediate and not deferred. joint_angle_algebraic.py, its ten tests and its CI gate are removed. So is the in-kernel algebraic_seeds path, and not only as deduplication: it solved the Sylvester resultant INSIDE the traced function, which is exactly the substitution bivariate_trig_stationary's own header forbids. The honest route back is a host-built fixed-capacity plan from enumerate_torus_maxima that declines on its ok, passed in as seeds; that is written down in the kernel and not implemented. Seeds are a grid again and are labelled one. GIT REPORTED ONE CONFLICT AND THE DANGEROUS ONE WAS SILENT. Only .travis/test-jax.sh conflicted textually. joint_anglemarg_peaklocal.py merged CLEANLY and placed 250's new header -- "it localizes u but retains a dense phi scan", "a sampled phi scan is not algebraic enumeration" -- directly above this branch's both-axis implementation. That contradiction would have merged green. The header now keeps 250's real claim (this kernel does not enumerate; the host module is authoritative) and states what phi_local_lnI does instead and which part of its ok is a bound. THE CI FLOOR WAS WRONG IN BOTH BRANCHES. Merged collection measures 424. 250 derived a provisional 408 from arithmetic and said in the same comment to replace it with a real collection; this branch measured 329 against a file set that no longer exists. 408 is sixteen low, and a low floor PASSES -- the silent direction the surrounding comment exists to warn about. 250 also inferred a standing "this environment collects one more than CI" offset and subtracted for it. There is no such offset: the 311 case documented immediately above was wrong by one because a harness sliced the script by line number and never ran the DESELECT loop. A one-off setup bug, not a property of the environment, and subtracting for it under-promises by one. 144 tests pass across test_joint_anglemarg_peaklocal, test_angle_marg_peaklocal_wiring, test_anglemarg_buffer_cap and test_joint_angle_peak_local. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
angle marg: finish QZ root polishing
…ocal_jax JAX joint angle peak-local: both axes localized. Merged after #250 (which carries the authoritative host enumerator bivariate_trig_stationary) and current rift_O4d, with both CI gates re-measured against the merged tree rather than derived: jax 424, integrate 36. phi is LOCALIZED, not enumerated -- the seeds are a grid and the branch says so. The duplicate in-kernel resultant seeder was removed rather than carried. ok = one bound (the omitted-mass margin) plus two empirical gates (nested-quadrature convergence, u sizing). The chain is empirical end to end and the path is documented as empirically gated, not fail-closed.
…per-detector-glue NoLoop: hoist source-only geometry out of the detector loop (-22% at 3 IFOs, bitwise identical)
ci-roster-check went red: the new test was reachable from no CI job, which is exactly the failure that gate was added to catch -- an unlisted test never runs and the job stays green forever. Add it to the core-unit-check FILES manifest, next to test_vectorized_lal_tools_split.py, rather than taking a roster exemption: it is an ordinary pytest suite needing only numpy / lal / lalsimulation, it builds synthetic inputs and calls the NoLoop likelihood on the CPU backend, so it needs no cupy and no GPU. Raise both pinned floors by 8. Measured 2026-09-05 on CIT: 8 collected, 8 passed, 0 skipped, 4.9 s -- two tests parametrized over three detector networks (1/2/3 IFOs) plus two unconditional. MAX_SKIPPED is unchanged because the file has no skip and no xfail. The absolute floors are NOT validated locally: neither available environment reproduces the CI editable install, so a dozen unrelated manifest files collect 0 tests here and the per-file floor exits before the totals. What is validated is the delta and the roster census, which now passes. CI checks the absolutes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The gate owns the filename pattern test/test_noloop_*.py, so the new file matched its manifest scope by NAME and went red as "neither registered nor explicitly excluded" -- which is the gate working: a new file in that namespace is meant to force a decision rather than be silently unrun. The decision is exclusion. It matches by name but not by subject: it pins NoLoop's rho_sq and kappa_sq ACCUMULATOR shapes against a reference, and its time-integral test is about the quadrature RULE, not about sub-sample interpolation of Q_lm. It is already registered with core-unit-check, whose FILES manifest carries it and whose floors count it, and there is no dual-registration precedent -- every marker-carrying file belongs to this gate alone and no core-unit file carries the marker. The two gates partition. Listed in EXCLUDED with a stated reason rather than renamed out of scope. The gate's own comment notes that a file under another prefix escapes the manifest entirely; renaming to dodge it is how these gates quietly stop covering things, and the reason belongs where the next such file will hit it. No floor changes here: exclusion adds no tests to this job. Verified locally through the gate's scope-pattern, exclusion and manifest phases, which are the three my change affects; the run then stops on an unrelated ImportError (no module named glue) in test_batchmode_stencil_default, an artifact of this environment rather than of the change. CI has the full install. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ocation was sized wrong An independent review ran a 37-configuration differential matrix over both PRs (phase and distance marginalization, n_cal>1 in loop and fused, the cal self-term fix, return_lnLt / return_cal_components / return_time_draw, explicit_time_values, all three stencils, all three quadratures, 1-3 detectors, heterogeneous mode lists, CPU and GPU). It found the CODE sound -- #255 bitwise across all 37, the whole of #256 within 1.8e-15 nats -- and the EVIDENCE unsound. This fixes the evidence and one real defect it surfaced. 1. THE ACCUMULATOR TEST NEVER EXERCISED kappa_sq. epoch was a whole second before tref, putting ifirst at ~3979-4152 against an n_time of 512, so every Q window was zero-extended and the data term was identically zero in all six parametrized cases. The file was named for the kappa_sq change and validated only rho_sq; the reviewer demonstrated it passing with the CPU Q producer aliased across detectors, which is precisely the hazard kappa_sq = Q_prod_result introduces. Fixed by placing the window inside the buffer, and pinned by a new test that asserts lnL(t) actually varies in time. Verified by sabotage: dropping either accumulator now fails 4 of 9, and the clean tree passes 9. 2. THE SPLIT TEST WAS TAUTOLOGICAL. After the split ComputeDetAMResponse IS SourcePolarizationBasis composed with ComputeDetAMResponsePrecomputed, so comparing them cannot fail. It passed with a sign flipped in the source-only half, the response matrix doubled in the per-detector half, and the speed of light wrong by 0.1%. Rewritten against a FROZEN copy of the pre-split bodies; all three sabotages now fail. The claim in DESIGN_noloop_per_detector_glue.md is corrected rather than deleted, because the general lesson is worth keeping: when a refactor splits a function, the halves are not an independent check on each other. 3. THE READ-ONLY TEST TESTED NUMPY, NOT THIS CODE, and its rationale was false. It called np.broadcast_to directly and passed on any tree; its docstring claimed cupy also returns a read-only broadcast, which is measurably wrong -- cupy's is writable and writes through to the base. Replaced with a test of what _dense_rho_sq actually returns, and the false claim removed. 4. _simps_weights BUILT AN (npts, npts) IDENTITY ON THE DEFAULT HOT PATH. npts is 2*window*srate and the driver's DEFAULT srate is 16384, so npts is 2457 in the default configuration, not the 614 of the srate-4096 runs everything here was measured at: a 48 MB identity and ~97 MB held in cupy's pool, inside the first likelihood call of every --vectorized --gpu run, in a function whose n_chunk is already bounded by device memory. Now built a block of rows at a time, capping it at 5 MB; verified bitwise against the whole-identity result at npts 614, 1000 and 2457. Also keyed the cache on the quadrature function, not just the backend: the GPU and CPU simps differ by 0.405 nats on an under-resolved peak, and a future caller passing a different rule at the same shape would silently be served the other one's weights. 5. THE loglikelihood= CALLBACK CONTRACT NARROWED SILENTLY. rho_sq now reaches the callback as a stride-0 view, so a callback writing in place raises on CPU and RACES on GPU -- cupy's broadcast is writable and every column aliases one address. No in-tree callback writes (_factored_lnL_helper and the driver's distmarg_loglikelihood both allocate), so nothing is broken; densifying at the callback boundary would undo the optimization, so this is documented in the NoLoop docstring instead. core-unit-check floors raised to 312/300: baseline 299/287, +4 as the split test goes 3 -> 7, +9 for the accumulator test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…-broadcast NoLoop: accumulator shapes and the time integral (-44% cumulative with #255)
oshaughnessy-junior
deployed
to
private-review-dispatch-rift-upstream
September 6, 2026 10:54 — with 
GitHub Actions
Active
GitHub Actions
Active
Author
|
Opened against the upstream repository by mistake. The development PR is oshaughnessy-junior#261. |
oshaughnessy-junior
deployed
to
private-review-dispatch-rift-upstream
September 6, 2026 10:56 — with
GitHub Actions
Active
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
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.
Adds an opt-in --q-time-pregrid-factor 8 path for ordinary vectorized NoLoop AV. It builds each cut Q bank once on an 8x reflected band-limited grid, keeps the geocentric integration cadence unchanged, and evaluates detector arrival times with a strided four-tap cubic CUDA/CPU gather. Factor 1 remains the default. Unsupported configurations fail closed; construction or transfer failures transactionally restore the original Q arrays and stencil with telemetry.
Why: an independent full-frequency time oracle isolated the existing finite-sinc Q interpolation as the dominant absolute error: delta lnZ = 0.0615, 16.4601, 0.0257, and 0.0595 nat across 22 low/high and HM low/high boundary cells. The band-limited time integrator itself agrees with the oracle to 9.91e-6 nat. The factor-8 candidate passes all 400 fixed posterior rows; worst pointwise error is 0.003535 nat and largest absolute delta lnZ is 0.001208 nat.
Validation:
Raw validation products remain outside the repository under /tmp; the branch diff contains only six implementation, test, and gate files.