gate: measure the quick-preset shape floor; it is a GMM defect, not a budget - #70
Conversation
… budget
Two FOLLOWUPS items, batched because both are documentation-only.
Item 5 asked whether `quick`'s skipped d=4 GMM cell needs a bigger budget
(estimated "roughly 2.5x" from the one default-seed reading). Measured over 8
fresh run seeds at x1/x2/x3/x4/x8/x16/x32:
budget n_eff min/med/max clears 100 PASS median width_ratio[1]
x1 200k 11 / 50 / 101 1/8 0/8 0.989
x4 800k 28 / 44 / 179 2/8 1/8 0.919
x32 6.4M 28 / 139 / 217 6/8 1/8 0.889
n_eff grows as ~nmax**0.3, so no budget clears the floor -- at x32, eight times
the STANDARD preset's own d=4 budget, it still starves 2/8. And the failures
are not starvation: width_ratio[1] degrades monotonically with budget while the
other three dims stay at 1.00, and mean_pull[1] grows +0.023 -> +0.069. The
evidence bias stays at 0.03-0.07 nats throughout, so the integral looks fine
while the posterior does not.
AV on the identical target passes 8/8 at every budget with width_ratio 1.000,
converging TO 1.000 on the dimension GMM diverges from, so the target and the
gate thresholds are sound. Sweeping GMM over the six standard d=4 cells shows
every other one scales n_eff ~4x for a 4x budget and holds width_ratio within
1%; only n2_s101 fails to scale. So it is target-specific, and ncomp=3 cells
are fine, meaning plain multimodality is not the trigger.
Item 5 is therefore resolved as a preset question -- accept the skip, keep the
cell (it is the only detector for this), do not re-budget it -- and the defect
it uncovered is filed as item 6 with the geometry and next steps. The sweep
table is repeated in a comment at CELL_BUDGET_MULT, because #59 makes "fix
item 5" look like a one-line ("GMM", 4, 2, 101): 3 entry that would convert a
documented skip into a merge-blocking FAIL.
Item 2's header still read "needs a decision, not a code fix" while its body
recorded the decision as MEASURED AND RESOLVED with the x4 entry landed. Fixed
-- and it is itself an instance of the pattern this series keeps finding: two
representations of one fact, the secondary copy goes stale, both plausible in
isolation.
No behaviour change: CELL_BUDGET_MULT is untouched, cell_budget() returns the
same values, and `RIFT_RUN_EXPENSIVE=1 pytest test_shape_recovery.py` is still
3 passed / 1 skipped -- now by documented design.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review of #70 (P2): FOLLOWUPS claimed dropping the quick d=4 GMM row would "delete the only detector" for item 6 and called its skip "containment". Both overclaim. evaluate() returns STARVED at the n_eff floor BEFORE it examines JS, pull, width or correlation, and the pytest wrapper skips that result -- so at quick's budget the row asserts nothing about shape. It is no better under the canonical driver: classify() maps STARVED/STARVED to BOTH-STARVED and returns before the metric-regression branch, and STARVED->FAIL to NEWLY-TESTABLE-FAIL, which is explicitly flag-don't-block. A permanently-starved row catches only REGRESSION(missing-in-candidate), i.e. a candidate that emits no record at all. And `standard` runs ncomps=[1,3], so the ncomp=2 geometry never reaches the merge gate. Net: no preset detects the item 6 defect, and none did before. Reworded rather than papered over: * Item 5 keeps its decision (accept the skip) but re-justifies it on COST, not coverage. The real reason not to drop the row is that dropping it is not expressible -- the matrix is dims x ncomps x seeds x samplers with no per-cell exclusion (the same wall item 2 hit), so the only expressible drop removes d=4 from quick["dims"], taking AV d=4 with it, a row that passes 8/8 at every budget measured. Added an explicit paragraph on what the retained row does and does not buy, and stated plainly that the skipped quarter IS an untested corner. * Item 6 now leads with "ungated -- no preset detects it", explains the two short-circuits, and carries a verified ~20 s reproducer that exits 1 and prints width_ratio[1]=0.854 directly. Next steps gained the two coverage options (gate it with the fix / characterize it non-blocking with an explicit expiry) instead of implying coverage already exists. * The CELL_BUDGET_MULT comment loses "merge-blocking FAIL" -- quick is not the merge gate -- and says outright that the row is a reproducer, not a detector. Also records the PYTHONPATH trap next to the reproducer: without it the suite measures whichever RIFT is installed. On this cell at seed 987654 that is n_eff 42.3 (branch) versus 4.6 (CVMFS igwn). run_shape_recovery.sh exports it; the pytest entry point does not. Tracked separately. Still documentation-only: CELL_BUDGET_MULT untouched, cell_budget() returns the same values, pytest still 3 passed / 1 skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Confirmed, and it is worse than the review states — fixed in 53a553c. You are right that
So a permanently-starved row catches exactly one thing: Taking the reword option, since the alternative (gating an unfixed defect) should ship with the fix, not ahead of it: Item 5 keeps its decision but re-justifies it on cost rather than coverage. The real reason not to drop the row is that dropping it is not expressible: the matrix is Item 6 now leads with "ungated — no preset detects it", explains both short-circuits, and carries a verified ~20 s reproducer: Next steps gained the two coverage options explicitly — gate it in Source comment drops "merge-blocking FAIL" ( One thing the reproducer surfaced that is worth its own fix: the Still documentation-only: |
…ard branch Review of #70 (P2): the reproducer block was not runnable as written. It used an undefined $CHECKOUT, which expands to an incorrect /MonteCarloMarginalizeCode path, and invoked `python shape_recovery.py` with no cd, so it only worked from the script's own directory. Now derives CHECKOUT from `git rev-parse --show-toplevel` and invokes the script by absolute path, so it runs from anywhere in the checkout. Also `python3`, not `python` -- several IGWN/conda environments provide only the former, which is why run_shape_recovery.sh says so at its exec line. Verified from an unrelated cwd on ldas-pcdev2: exits 1 and prints width_ratio[1]=0.854 (tol 0.055), identical to the in-directory run. Renumbered the new defect entry 6 -> 7, with every cross-reference (item 5 in three places, the CELL_BUDGET_MULT comment). Branch claude/shape-recovery-pytest-checkout-guard independently adds a FOLLOWUPS item 6 for the PYTHONPATH provenance hole this PR flagged, and both branches append after item 5. Renumbering here leaves a pure adjacency conflict in FOLLOWUPS.md -- resolve by keeping both, 6 then 7 -- instead of two different items claiming one number. shape_recovery.py now merges clean against that branch; verified with merge-tree. The PYTHONPATH caveat next to the reproducer now points at that branch rather than describing the hole as unfiled. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Correct on both counts — fixed in 2c954fb. Now derives the checkout and uses an absolute script path, so it runs from anywhere inside the tree: Took On the conflictBranch Renumbered this PR's defect entry 6 → 7, including all four cross-references (item 5 in three places, plus the
I did not rebase this PR onto that branch, since it has no PR open yet and I would rather not make #70 depend on unmerged work. Either merge order works after the trivial resolution. The caveat next to the reproducer now points at that branch instead of describing the hole as unfiled — with the guard in place, a wrong |
…defect) PR #71 landed the pytest-provenance guard this PR flagged, adding its own FOLLOWUPS item 6. Both branches appended after item 5, so FOLLOWUPS.md conflicted; shape_recovery.py auto-merged, since #71 touches the docstring, the helpers after MIN_NEFF_FOR_SHAPE and main(), while this branch touches the comment at CELL_BUDGET_MULT. Resolved by keeping both, in numeric order -- this branch had already renumbered its defect entry 6 -> 7 in anticipation, so there was no semantic collision to settle, only adjacency. Took the merged guard's mechanism into the item 7 reproducer while here: it now exports RIFT_SHAPE_CHECKOUT as well as PYTHONPATH, so the checkout is CHECKED rather than merely reported and a mistyped path fails loudly instead of quietly measuring the installed RIFT. The expected output gained the "# RIFT under test:" provenance line #71 prints. The caveat paragraph now credits item 6 in the past tense instead of pointing at an unmerged branch. Verified on the merged tree: the reproducer runs verbatim from an unrelated cwd (RIFT/integrators), prints the provenance line, exits 1 with width_ratio[1]=0.854 (tol 0.055). test_shape_recovery.py + the guard's test_rift_provenance_guard.py: 12 passed, 1 skipped -- the skip still being the item 7 cell, by design. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review of #70 (P3): "No budget clears the floor" overstates the data sitting directly above it -- x32 clears 6/8, and nothing measured here rules out some far larger budget clearing all eight. Narrowed to the supported claim in FOLLOWUPS item 5 and in the CELL_BUDGET_MULT comment. The correction also surfaced a stronger measured fact that was being obscured by the sloppy version. It is not the median that refuses to move -- that grows sublinearly, ~nmax**0.3 -- it is the across-seed MINIMUM, which stops improving after x4: 11, 16, 24, 28, 41, 33, 28 for x1..x32. Over a 32x range the worst seed never gets past ~40. What widens with budget is the spread, not the floor margin. That is both narrower than the old claim and more useful, since it is the worst seed that decides whether a merge-blocking row is reliable. The "no larger budget could work" question is now stated as untested and moot, because the second bullet already settles it: the budgets that DO make the row testable turn it into a failure, not a pass. Also qualified two neighbouring absolutes: the "raise the budget" rejection now says "no budget up to x32" rather than "any budget", and the AV control says "all 8 seeds at each of the three budgets measured" rather than "every budget". Numbers, decision and behaviour unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Agreed — fixed in 59c516a. The claim contradicted the table printed directly above it, which is the worst place to overstate something. Narrowed to the supported form in both places it appeared (item 5's bullet and the Writing it precisely surfaced a stronger fact the sloppy version was hiding. It is not the median that refuses to move — that grows sublinearly, ~ Over a 32x range the worst seed never gets past ~40. What widens with budget is the spread, not the floor margin. That is both narrower than the old claim and more useful, since it is the worst seed that decides whether a merge-blocking row is reliable — the same point item 2 turned on when it chose x4 over x2 for Swept two neighbouring absolutes while here:
Numbers, decision and behaviour unchanged. |
…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>
Fixed in 11a5bc7 — now "at each measured budget". Worth naming what happened: I narrowed the claim in the follow-up and left the duplicate in the guard comment stale. That is exactly the defect item 2's fix addressed and the whole #47/#51/#55 series keeps finding — one fact, two representations, the secondary copy goes stale, both plausible read in isolation. Committing it into the very comment whose job is to warn about it is the joke telling itself. So I swept the suite for the same shape rather than fixing only the reported line. One other latent instance, now fixed: 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 already correctly scoped ("no budget tested up to x32 ... at every seed", "passes 8/8 at every budget measured"). Also reflowed the comment to the file's ~100 col convention. No numbers, decision or behaviour changed; Holding here — not merging, per your note that the merge is being done externally. |
Two low-priority FOLLOWUPS items in
test/expensive_before_merging/integrators/, batched because both are documentation-only. No behaviour change —CELL_BUDGET_MULTis untouched,cell_budget()returns identical values, andRIFT_RUN_EXPENSIVE=1 pytest test_shape_recovery.pyis still 3 passed / 1 skipped.Item 5 — measured, and the premise was wrong
The entry asked whether
quick's skipped d=4 GMM cell (mix_d4_n2_s101, n_eff 42 against the 100 floor) needs a bigger budget, estimating "roughly 2.5x" and asserting "not a defect in the sampler — the same cell passes at the standard preset's budget". Both halves came from the single default-seed reading.Measured over 8 fresh run seeds per budget (987654 + 988654..994654, CPU, branch on
PYTHONPATH):width_ratio[1]quick)standard's per-dim budget)nmax**0.3. At x32 — 6.4M evaluations, 8x the standard preset's own d=4 budget — it still starves 2/8, minimum 28.width_ratio[1]degrades monotonically with budget while the other three dims sit at 1.00;mean_pull[1]grows +0.023 → +0.069. Evidence bias stays at 0.03–0.07 nats throughout — the integral looks fine while the posterior does not.Decision: accept the skip, justified on cost. Raising the budget is measured not to work. Dropping the row is not expressible — the matrix is
dims × ncomps × seeds × samplerswith no per-cell exclusion (the same wall item 2 hit), so the only expressible drop removesd=4fromquick["dims"], taking AV d=4 with it, a row that passes 8/8 at every budget measured.Item 6 (new) — the defect it uncovered
AV on the identical target, same seeds, passes 8/8 at every budget with
width_ratio= 1.000, converging to 1.000 on the dimension GMM diverges from — so the target, truth pool and thresholds are sound.Sweeping GMM over the six
standardd=4 cells at x1→x4: every other cell scales n_eff ~4x for a 4x budget, holdswidth_ratiowithin 1% in all dims, and reaches 7–8/8 PASS. Onlyn2_s101fails to scale (50 → 44). The ncomp=3 cells are fine, so plain multimodality is not the trigger.The one target that breaks it has two near-equal-weight components (0.479/0.521) whose widths along x1 differ 2.5x with only 0.95σ of separation there; the pull is toward the narrow component, i.e. the proposal abandons the broad component's tail.
n_eff = Σw/max(w)cannot see that, which is why the row reads as starvation.This defect is ungated — no preset detects it, and none did before.
standardrunsncomps=[1,3], so the geometry never reaches the merge gate.quickruns it at a budget whereevaluate()short-circuits toSTARVEDbefore examining width, and both consumers of that verdict discard it: the pytest wrapper skips, andclassify()maps STARVED/STARVED toBOTH-STARVED, returning before the metric-regression branch. The retained row is a reproducer plus a missing-record canary, not a detector. Item 6 records the two ways to add real coverage (gate it instandardalongside the fix; or pinwidth_ratio[1]in a non-blocking characterization test with an explicit expiry) and defers the choice, since gating an unfixed defect should ship with the fix.Verified reproducer, ~20 s, exits 1:
Item 2 — stale header
**Status:** needs a decision, not a code fix.contradicted its own body, which records the decision as MEASURED AND RESOLVED with the x4 entry landed. One line. It is itself an instance of the pattern this series keeps finding: two representations of one fact, secondary copy goes stale, both plausible in isolation.Guard
The sweep table is repeated in a comment at
CELL_BUDGET_MULT, because #59 makes "fix item 5" look like a one-line("GMM", 4, 2, 101): 3entry — visually identical to the fix item 2 landed formix_d6_n3_s303, but that cell's bias was flat across budgets so more budget bought real margin, whereas this one's width deficit grows with budget.Noted for a separate fix
The
PYTHONPATHline is load-bearing: without it the suite measures whichever RIFT is installed, not the branch. On this cell at seed 987654 that is n_eff 42.3 (branch) vs 4.6 (CVMFS igwn).run_shape_recovery.shexports it; the pytest entry point its docstring advertises as equivalent does not.Verification
RIFT_RUN_EXPENSIVE=1 pytest test_shape_recovery.py→ 3 passed, 1 skipped (unchanged).cell_budget()spot-checked:d6n3s303standard = 4800000,d4n2s101quick = 200000, explicit-override path = 1200000.🤖 Generated with Claude Code