Updates to make util_AMRgrid.py run - #63
Open
spfanning wants to merge 2 commits into
Open
Conversation
Owner
|
@spfanning : there are some code conflicts with this. Also, we have the annoying problem that our higher-level code uses different field names, so we'll need to carefully distinguish between using full names and short names ... |
oshaughn
pushed a commit
that referenced
this pull request
Aug 13, 2026
Two strands, developed in parallel in the same tree and validated together. === PR #63 review findings === 1. [P1] The production caller ignored the collapse verdict. The sampler set live_volume_collapsed but the ILE never read it, so a known-degenerate export entered downstream posterior assembly as an ordinary likelihood row -- strictly worse than the pre-fix crash, which at least kept it out. The ILE now reads the verdict, prints "*** LIVE VOLUME COLLAPSED ***", feeds it to the existing --mc-error-replicas trigger list, and offers --reject-collapsed-live-volume to drop the event entirely. That gate is OFF by default on purpose: dropping the event silently thins the posterior in an SNR-dependent way, which is the failure this whole branch exists to remove. Exported-and-loud is the better default; the flag is there when a contaminated point is worse than a missing one. 2. [P1] The empty-cycle check used the CUMULATIVE live-set size. `ninj` is measured after old survivors and new samples are combined, so `ninj == 0` only ever caught LEADING empty chunks. Once one sample survived, a later chunk contributing nothing sailed past it and re-thresholded the recycled live set. Reproduced before fixing: 20 live points decaying 19, 18, 17, 16, ... with ln V falling -0.05, -0.11, -0.16, -0.22, ... over chunks that each returned ZERO finite samples -- contraction, and a biased lnZ, on no new evidence at all. Now the admissions from the current chunk are counted before the append and contraction is skipped when that count is zero. Pinned by an invariant test: extra chunks that contribute nothing must not move lnZ or the live-set size. 3. [P2] The legacy empty-reduction classifier was too broad. Any exception whose text contained "zero-size array" and "no identity" was labelled an AV collapse, although the enclosing handler also covers waveform generation, data conditioning and the whole likelihood stack. The named LiveVolumeCollapse is now matched by isinstance, and the string fallback additionally requires the traceback to name mcsamplerAdaptiveVolume. === The L0 warm-pass gap (documented as known in the PR body) === Root-caused: integrate() writes an 'integrand' key into _rvs AFTER integrate_log returns -- i.e. after the arrays have been moved to the host and, if a fair draw ran, truncated. On a SECOND integrate() on the same sampler (exactly the warm-start rescue) that key is stale in both length and backend, integrate_log repopulates every other key but not it, and the fair-draw gather then indexes a host array with a device index array: "Implicit conversion to a NumPy array is not allowed". The pass aborted mid-way, so `res, var, neff, dict_return = sampler.integrate(...)` never completed and the ILE reported the COLD pass's lnZ/k-hat/ESS beside the WARM pass's exported samples. * drop the stale key on entry (as mcsamplerPortfolio.integrate_log already does); * build the fair-draw weights on the sampler's own backend and gather on the host; * snapshot _rvs rather than aliasing it, and restore the cold pass in full if the warm pass raises -- the L0 handler now says "*** FAILED ***", not "skipped". With the pass no longer aborting, the verdict reaches it -- and finds the OPPOSITE failure, which none of the existing rules could see: seeded from too few points the grid contracts onto a sliver, the integrand is flat across it, and the pass terminates in one cycle looking excellent (n_eff at target, small k-hat) while lnZ is short by the mass outside the sliver. Measured over 12 rho_net=146.8 rescue replicates: eleven seeded from 2000 puffed points warm-started at V = 7.5e-9..1.5e-8 and returned ln(Z/Lmax) = -27.0..-30.6; the one seeded from 2 points warm-started at V = 9.2e-36 and returned -80.7 -- about 50 nats low, with eff_samp 9789 of 10010. So the verdict gains a geometric rule, n_warm_seed <= ndim+1 (fewer points than a simplex cannot define a volume), with seed provenance carried through bootstrap_from_samples/save_state/ load_state, and the report distinguishes an over-contracted warm start from underflow because the two need opposite advice. === Validation === * test_av_empty_live_volume.py: 41 passed, 2 GPU-only skips. * Healthy-run bit-identity re-verified against the pinned pre-fix tree (ba2b38d) after both strands: all five well-conditioned integrals reproduce lnZ, eff_samp, ESS, k-hat and ntotal to every digit. * Shape-recovery merge gate (RIFT/integrators/TESTING.md): 0 blocking regressions, all 25 AV rows bit-identical to base, same 4/24/29 strict/warn/starved baseline. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
oshaughn
pushed a commit
that referenced
this pull request
Aug 13, 2026
Second round of PR #63 review. [P1] Collapsed results were still indistinguishable downstream. The verdict only reached stdout; both --reject-collapsed-live-volume and --mc-error-replicas default off, after which a collapsed run writes an ordinary .dat/.grid/XML/posterior with no marker on it. The .dat and .grid schemas are POSITIONAL and read by CIP, so the marker cannot be a new column without breaking every reader. It goes in a sidecar instead: <output>_<indx>_integrator_status.json written on EVERY run beside the other artifacts, carrying collapsed (bool), collapse_reason, lnL/sigma_lnL/neff/ntotal and whichever of pareto_khat, n_ESS, n_live_final, n_empty_cycles, n_live_collapses, n_warm_seed(_rank) the sampler reported. Written unconditionally so "collapsed": false is an explicit statement rather than an inference from a missing file -- downstream can require the file and fail loudly against an integrator too old to write one. A failure to write it can never abort a run that otherwise succeeded. Verified on the real pipeline: a rho_net=146.8 export writes it beside the .dat and .xml.gz with "collapsed": true and the reason; a rho_net=51.4 export writes "collapsed": false with ESS 18.6, k-hat 1.31, 1000 live points. Rejection is still not the default, for the reason the branch exists: dropping the event silently thins the posterior in an SNR-dependent way. With the sidecar the default is now exported-and-labelled rather than exported-and-silent. [P2] Warm-seed adequacy was a row count, which is neither necessary nor sufficient. Duplicated or collinear points span the same degenerate subspace two points do and produce the identical near-zero-volume failure however many rows there are; and the n <= ndim+1 boundary flagged a simplex, which is exactly enough to define a volume in ndim dimensions. Both directions were wrong. The invariant is the AFFINE RANK of the seed cloud: the rank of the mean-centred points over the adaptive axes, each scaled by its box extent so the test is unit-free (a distance in Mpc and an angle in radians must not get different tolerances). _grid_from_points now records n_seed_rank and n_seed_dim alongside n_seed, they travel through save_state/load_state, and the verdict tests rank < dim. Rank subsumes the count anyway, since n points span at most n-1 affine dimensions. Where rank is absent (state written before this) the count fallback now uses the correct simplex boundary, n <= dim. Tests: a 500-point cloud confined to a plane in 6-D is measured rank 2 and flagged; a full-rank cloud is not; ndim+1 independent points are not; duplicated and collinear seeds are flagged whatever their row count; rank survives save/load. Two tests that had encoded the old ndim+1 boundary are corrected. 52 passed, 2 GPU-only skips. Healthy-run bit-identity re-verified against ba2b38d (lnZ, eff_samp, ESS, k-hat, ntotal identical on all five well-conditioned integrals); shape-recovery merge gate 0 blocking regressions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
oshaughn
pushed a commit
that referenced
this pull request
Aug 13, 2026
Third round of PR #63 review. [P1] Replica collapse status was discarded. The mc-error replication loop kept each replica's lnZ, sigma and samples but dropped its dict_return, and the status sidecar reads the ORIGINAL dict_return -- so a healthy first run followed by a collapsed replica exported the pooled posterior while recording "collapsed": false. The pooled export is a mixture over every replica in the pool, so its status is the OR over them and its reason names each collapsed member individually. Aggregated right after _pool_replica_rvs, folded back into dict_return (which is what the sidecar and the downstream reporting read), with n_replicas_pooled / n_replicas_collapsed recorded. Verified on a real rho_net=146.8 run with --mc-error-replicas 1: [mc error] *** LIVE VOLUME COLLAPSED in 2 of 2 pooled replicas *** "collapse_reason": "run 1: 322 cycle(s) with no finite in-volume sample; ESS=1.00; replica 1: 116 cycle(s) with no finite in-volume sample; ..." Note for readers of the sidecar, stated in the file: lnL/sigma_lnL are the POOLED values while the per-run diagnostics (ESS, k-hat, live-set counts) are the first run's, because those are per-integration quantities the pool has no single value for. [P2] --sim-xml without --event suppressed the whole sidecar. int(None) raises, and because the write is deliberately wrapped so a diagnostic cannot abort a good run, that turned into a silently missing marker -- the one outcome the sidecar exists to prevent. Now uses the same convention as the .dat writer, including its explicit `opts.event is None -> -1` case. [P2] Rank could be computed from points excluded from the grid. _build_grid_from_points filters `pts` to the prior box but took `resolution_pts` unfiltered, so out-of-box rows could make a degenerate in-box seed look full-rank. Callers do not always clip: bootstrap_from_samples clips only when inflating, and the ILE's puffed fallback seed is an unclipped Gaussian about the peak. The core is now filtered by the same rule, which also keeps the grid RESOLUTION honest -- out-of-box rows widen `ext`, and V_extent could exceed 1, under-resolving the grid. Falls back to the (already filtered, non-empty) full cloud if the core lies entirely outside. Tests: an in-box line plus out-of-box scatter is measured rank 1 and flagged; adding far-outside rows leaves V and the grid identical to the clean core; the replica aggregation and the event-id convention are pinned structurally. Two ILE wiring tests that used byte-offset windows now bound themselves by the enclosing try/except, so adding a line cannot move an assertion off the end of the window. 56 passed, 2 GPU-only skips. Healthy-run bit-identity re-verified against ba2b38d; shape-recovery merge gate 0 blocking regressions, portfolio_warm included. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
oshaughn
pushed a commit
that referenced
this pull request
Aug 13, 2026
Fourth round of PR #63 review. --reject-collapsed-live-volume was checked once, BEFORE replication, against the first run's status only. Replica statuses are aggregated afterwards, so a healthy first run whose replica collapsed still exported the pooled, collapsed result with the flag set -- bypassing the gate for exactly the case pooling introduced. The check is now a helper applied TWICE, on purpose: * before replication, as a fast path -- if the first run already collapsed and the user wants such events dropped, there is nothing to learn from spending GPU on replicas; * immediately after the replica statuses are folded into dict_return, so the gate sees the pooled verdict. Both calls precede every artifact the gate exists to suppress (status sidecar, .dat, .grid, XML), and the only write of dict_return['live_volume_collapsed'] now sits between them. The raise carries which stage rejected, so the log says "first run" or "pooled over N replicas". Verified end to end at rho_net=146.8 with --reject-collapsed-live-volume: the named exception reaches the handler, is classified correctly ("the INTEGRATOR's live volume collapsed"), the binary is skipped, and the output directory is EMPTY -- no .dat, no XML, no sidecar. Tests pin the invariants rather than the line numbers: at least two gate calls exist, at least one is after the aggregation, at least one is before the replication trigger (so the fast path cannot be dropped), every write of the collapse status is followed by a gate call, and the last gate call precedes both the sidecar and the .dat writer. 58 passed, 2 GPU-only skips. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
oshaughn
pushed a commit
that referenced
this pull request
Aug 13, 2026
AV: fix the high-SNR empty-live-volume crash, and report the collapse
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.
No description provided.