Added theta12 as to extract_param - #51
Open
chadhenshaw wants to merge 13 commits into
Open
Conversation
When one of the spins is non-zero but very small (e-06) calculating chi_pavg throws a divide by zero error. In this limit chi_pavg = chi_p, so now this is calculated instead.
This is done to eliminate calculation error during PE instead of when using plot_posterior_corner
This was done to eliminate calculation error during PE instead of while plotting
Updated calculation criteria for chi_pavg. Also includes changes to faciliate usage of chi_pavg in CIP.
Updates to allow usage of chi_pavg as parameter-implied.
reverting due to weirdness
Updated to include chi_pavg as parameter-implied.
oshaughn
pushed a commit
that referenced
this pull request
Aug 7, 2026
…k; force the full-dim warm GMM Three PR #51 findings. [P1] Standalone AV was knowingly biased and a warning did not fix it. The warm pass is an estimate over TRUNCATED support: the seed provably contains the peak the cold pass found, and nothing about what it did not reach. Deliberately NARROW change. The rescue still runs exactly as before -- removing it by default would strip the high-SNR n_eff rescue from every existing AV run, a certain regression traded against a bias so far shown only on an adversarial synthetic. What changes is only the case with POSITIVE EVIDENCE of lost mass: the cold pass had full support, so if the warm lnZ lands more than --sampler-l0-rescue-reject-dlnZ (0.5) below it, the cold result is kept and the reason printed. --sampler-l0-rescue-accept-truncated restores the old behaviour. Detection is imperfect -- a missed mode need not produce that ordering -- so this NARROWS the failure rather than closing it, and says so. Not "fixed" by giving AV its own defensive component. That needs per-sample mixture densities, which is mcsamplerPortfolio: q_mix, defensive members and their coverage bookkeeping already exist there. A second implementation of the same mathematics is how the bugs in this very review arise -- a capability flag that lied, a defensive component absorbed by an update, config dropped on re-setup -- when one of two parallel paths is updated and the other is not. The architectural answer is that coverage-critical runs use the portfolio; AV stays a fast single-proposal sampler with the limitation documented at its call site. Also corrects the --sampler-warmstart-retry-neff help, which claimed the rescue "cannot bias the result". [P1] Pooling assumed raw importance records. In production _rvs may already be thresholded or fairdraw-resampled, so sum_i w_ki over retained rows is not Z_k * n_k and a 1/n_k rescale mis-weights the block (a fairdraw record is already posterior-resampled, hence weighted twice). Given the reported per-replica lnZ, each block is now renormalized to contribute exactly Z_k/K -- correct for raw, pruned or resampled rows. The new test prunes a replica and asserts both that the fix is exact and that the naive rescale would have been wrong. [P2] Setup memory defeated the full-dimensional warm GMM. A remembered explicit gmm_dict survived bootstrap_from_samples, so correlate_all_dims=True no longer rebuilt the single full-dim group that path exists to build (capturing sky<->phase<->distance correlations at high SNR). It now passes gmm_dict=None explicitly; verified [(0,), (1,2)] -> [(0,1,2)] with coverage settings preserved. All suites pass: 22 portfolio, 6 pooling, 7 confirm-on-fail, 5 escaped-mass, 6 seq-gmm. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
oshaughn
pushed a commit
that referenced
this pull request
Aug 7, 2026
…_coverage Pool MC-error replicas for export; warn on L0 truncated support; general setup() kwarg memory
oshaughn
pushed a commit
that referenced
this pull request
Aug 13, 2026
…e run The flag-ON probe reuses the shape gate's thresholds and its evaluate(), so it inherits the same near-threshold realization sensitivity -- but unlike the comparison it had NO confirmation step, so a single noisy row failed a PR. Measured: `adaptive_alloc ON / d4_n1_s303` reported base=PASS flag=FAIL on four consecutive gate runs during PR #51, reproduced identically against an unrelated base checkout, and the same cell has read n_eff 422 (PASS) and 46 (STARVED) on IDENTICAL code. Re-running the same seed reproduces the same false verdict; only fresh seeds separate "the flag broke this" from "this cell responds to its realization". Flagged rows are now re-run at fresh seeds with the flag off and on, and reported as a regression only if the flag arm is worse in a majority. Fails closed, like confirm_regressions.py: a flag arm that produces no record counts AGAINST the flag, and too few usable pairs is INCONCLUSIVE with a nonzero exit rather than a silent clear. --no-confirm restores the previous immediate-fail behaviour. The probe's regression rule is now ONE function (is_probe_regression) used by both the summary and the confirmation. It deliberately differs from compare_shape_results.is_blocking -- this probe tolerates flag=STARVED, since an opt-in path may trade efficiency on a target the default already resolves -- so it is defined once here rather than duplicated, which is the failure mode that produced most of the findings in the #47/#51 series. Verified live: the d4_n1_s303 row clears at 3 fresh seeds with the two arms BIT-IDENTICAL (36/36, 84/84, 131/131), probe exit 1 -> 0. At one seed both arms FAIL, so the cell is marginal on its own merits, not because of the flag. Adds test_probe_confirm.py (6 checks, all aimed at the direction that ships a bug -- a false clear of a real opt-in regression). Also records the outstanding integrator follow-ups in FOLLOWUPS.md, since issues are disabled on this fork. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
oshaughn
pushed a commit
that referenced
this pull request
Aug 13, 2026
…eights cache Follow-up sweep from PR #51. Two real defects, both of the one-of-two-paths kind. [1] 'log_weights' does not mean one thing. mcsamplerPortfolio stores the true importance weight (lnL + ln p - ln p_s); mcsamplerGPU stores tempering_exp*lnL + ln p - ln p_s -- the ADAPTATION weight. That exponent is not 1 in production: helper_LDG_Events sets it from the SNR (helper_LDG_Events.py:1472/1477), and --no-adapt drives it to 0, removing the likelihood from the column entirely. The .dgrid and calibration-posterior exporters both PREFERRED that cache, so with the GPU/AC sampler they reweighted their output by L^(e-1) -- and by 1/L under --no-adapt. The extrinsic-proposal fit in the same file already derived for exactly this reason, with a comment naming the hazard; these two sites never got the same treatment. Adds ln_weights_from_rvs() as the single definition -- log components, then the linear mcsamplerEnsemble form with out-of-support rows at -inf, and an explicit exception when neither is present, because a loud failure beats a plausible wrong number in a science output. Five call sites now share it, including the two inline copies added in #51. The cache is not read on any weight path. [2] mcsamplerGPU.py:1194 appended new weights onto joint_s_prior instead of weights, corrupting that column from the second chunk on. mcsampler.py:571 carries the identical block WITH the fix and a BUGFIX comment; the GPU copy never received it. Reachable -- mcsamplerGPU:1536 reads _rvs['weights']. Fixed, with a comment naming its twin. Adds test_rvs_weight_derivation.py (5 checks): derives from components rather than the cache and asserts the cache was materially different; --no-adapt loses the likelihood entirely; a portfolio-style cache agrees but is still not read; the linear form sends out-of-support rows to -inf, not NaN; a cache-only record raises instead of guessing. Full verdict table for every writer and reader in RVS_CACHE_AUDIT.md, including one divergence NOT chased: mcsampler.py:1138 notes _rvs['weights'] is sorted as a side effect, and two callers already work around it by recomputing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
oshaughn
pushed a commit
that referenced
this pull request
Aug 13, 2026
…ive-alloc regression
BUDGET. GMM mix_d6_n3_s303 sits on the n_eff=100 floor at the matrix budget, so
as a merge-BLOCKING row it is close to a coin flip. Measured over 8 fresh seeds:
budget n_eff min/med/max clears 100 median |bias|
x1 59 / 105 / 159 5/8 0.014
x2 105 / 169 / 214 8/8 0.010
x4 209 / 293 / 473 8/8 0.012
Bias is flat, so this is threshold margin rather than a defect.
Neither option previously proposed was expressible: the matrix budget is
nmax_per_dim*d for EVERY cell and --strict-samplers is per-SAMPLER, so "re-budget
this cell" and "drop this row from strict" both needed a mechanism that did not
exist. Adds CELL_BUDGET_MULT, in the same shape as the existing per-case
WARM_CASES budgets, and sets this cell to x4. x2 clears 8/8 but its MINIMUM
(105) is 5% above the floor -- not a margin worth trusting for a row that has
read 66 and 119 on unchanged code. x4 gives min 209 (2.1x) for ~4% of the gate's
evaluations, being one cell of ~96.
ADAPTIVE-ALLOC. Re-running the probe confirmation on the FIXED probe reverses a
claim I made repeatedly across #47/#51/#55: the `adaptive_alloc ON / d4_n1_s303`
row is a CONFIRMED opt-in regression, not realization noise. The earlier "not
confirmed" came from the probe's patching bug, which compared the flag against
itself and could not report "worse". At 5 fresh seeds the flag arm fails at
EVERY seed, frequently with HIGHER n_eff than the default arm -- so it degrades
posterior SHAPE, not efficiency. Opt-in, default off, never set by the pipeline,
so production is unaffected; recorded as FOLLOWUPS item 4 with the scoping work
needed before the flag could ever be promoted.
Also marks FOLLOWUPS items 1-3 resolved with their outcomes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
oshaughn
pushed a commit
that referenced
this pull request
Aug 13, 2026
…ropped Review of #70 (P3): the previous commit narrowed "AV passes at every budget" to "all 8 seeds at each of the three budgets measured" in FOLLOWUPS item 7, but left the duplicate of that same claim in the CELL_BUDGET_MULT comment reading "at every budget". Now "at each measured budget". Which is, precisely, the defect this PR's item 2 fix and the whole #47/#51/#55 series keep finding: one fact with two representations, the secondary copy goes stale, both plausible read in isolation. Committing it while editing the file whose purpose is to warn about it is the joke telling itself. Swept the rest of the suite for the same shape rather than fixing only the reported line. One other latent instance: item 5's "PASS is 1/8 at every budget from x2 up" could be read as covering untested budgets above x32; now "every measured budget from x2 up". The remaining hits are correctly scoped already -- "no budget tested up to x32 ... at every seed" and "passes 8/8 at every budget measured". Reflowed the comment to the file's ~100 col convention. No numbers, decision or behaviour changed. 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
1 participant
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.
Shouldn't change anything else, just added theta12 as a viable parameter to extract. Tested locally without any issues.