DRAFT: give the retained set and the export resample separate names

[oshaughnessy-junior]

Draft, not for merge. Opened to argue about a design against something concrete. Nothing
reads the new code — this branch is a no-op on every output.

Follows from #87, where four successive review rounds each found a defect in the boolean
bookkeeping around _rvs, and none in the physics.

The problem

sampler._rvs means two things at two times in one function:

self._rvs[key]                              # the RETAINED SET, with real importance weights
...
self._rvs[key] = self._rvs[key][indx_list]  # WITH REPLACEMENT, proportional to weight
self._rvs[key]                              # an EXPORT RESAMPLE, ~1.5*eff_samp equal-weight rows

The name doesn't change. The type doesn't change. A consumer written against the first meaning
keeps working, silently, against the second.

Why this is design, not luck

Nine defects of one shape. Five before the audit (CIP export; L0 seed #78; reject gate #79;
reserve cap and its logarithm #84), three found by the mechanical sweep in #87, and then four
more in review of that fix — every one in the bookkeeping, none in the physics:

round	defect
1	correct in isolation, wrong once pooling ran after it
2	one flag answering two questions (rows resampled vs globally equal-weight)
2	the CLI option used where "what this pass actually did" was needed
3	a marker cleared only on the normal return, surviving a raised event

Each was a second source of truth that some site touching the first failed to maintain.

What's here

RvsRecord carries rows and provenance together, replacing the booleans with named
questions:

rec.rows_are_resampled()      # per BLOCK   -- survives pooling
rec.is_equal_weight()         # whole RECORD -- pooling destroys it
rec.blocks_were_flattened()   # a fact about the pooling STEP

Three methods with three names, because the failure was never that the answer was hard to
compute — it was that one name suggested one question while a caller asked another. Provenance
is per-block, so a mixture of raw and resampled replicas is representable at all.

Deliberately not a dict subclass: that would let every existing sampler._rvs[...] keep
working against an object whose meaning it doesn't check — the original problem with more steps.

The test suite is one section per review-round failure shape; each test would have caught its
round had provenance lived with the rows from the start.

Blast radius, and why option A

Measured by audit_rvs_fairdraw.py: 306 reads, 131 post-rebind, 7 rebind sites.

• A (here): two names, _rvs keeps its meaning, migrate one consumer at a time.
• B (target): the fair draw returns a new object. Correct end state, makes the error
  unrepresentable — but needs all 131 post-rebind reads told which object they want, in one
  change, to code that writes science products.
• C: keep the booleans and the CI gate. That gate stays either way; it's the only thing
  that catches a new consumer.

Recommendation: A now, B later, C regardless.

Three open questions I'd like answered before going further

1. rvs_record vs _rvs_record — public reads better for something consumers should use,
   but every comparable sampler attribute is underscored.
2. Should the record hold the RETAINED rows too? It would close the last BROKEN entry
   (L0 rescue: the reject gate was comparing two different estimators (stacked on #78) #79's cross-source lnZ fallback) and let .dslice reweight properly instead of falling back
   to all-fresh. It costs memory on a portfolio, whose _rvs holds every draw. _warm_seed_reserve
   shows a bounded, finite-stratified copy is affordable; whether the full set is, I have not
   measured and would want to before committing.
3. Does the LISA twin follow, or diverge on purpose? It carries 36 of the 131 post-rebind
   reads and none of the helpers this work added.

Verification

132 passed, 3 skipped across the AV/L0/portfolio suites. A collapsed AV pass records
n_retained=288 against 1 exported row — the case this whole line of work is about.

Depends on nothing in #87; both are off rift_O4d. If #87 lands first this rebases cleanly
(disjoint files bar the one AV hunk).

[oshaughnessy-junior]

All three answered; rebased onto rift_O4d with #87 merged in.

1. Naming → _rvs_record

Done, underscored throughout.

2. Retained rows → measured, and the answer splits by sampler

Agreed it's an operations question, so I measured rather than argued. Run with no fair draw so
_rvs is the retained set (measure_retained_set_memory.py, log committed):

sampler	nmax	ntotal	retained rows	cols	record MB
AV	200k	200,886	7,934	9	0.5
AV	800k	322,587	25,374	9	1.7
portfolio	200k	200,000	199,641	12	18.3
portfolio	800k	800,000	799,637	12	73.2

AV ~0.9 MB per million nmax (4 MB at 4e6); portfolio ~92 MB per million (384 MB
at 4e6). They diverge because AV keeps only the in-volume subset, which grows far more slowly
than ntotal, while the portfolio's _rvs holds every draw.

Recommendation: don't hold it unbounded. 384 MB per ILE process is real when many ILE jobs
share a node — and the portfolio's retained set is mostly ballast: on the collapsed pass this
work is about, the finite fraction is ~1e-5, so nearly all of it is -inf rows no consumer can
use. make_warm_seed_reserve already keeps a bounded, finite-stratified copy with the exact
pre-cap weight total. So: have the record reference the reserve rather than take its own
copy, and treat full retention as an AV-only opt-in where it costs ~4 MB. That gets what #79's
lnZ fallback needs at a cost already being paid.

3. LISA — you're right, and my question was badly posed

There is no separate integrator, and there shouldn't be. Both drivers import the identical
set (mcsampler, Ensemble, GPU, AdaptiveVolume, Portfolio), so _rvs_record reaches
LISA for free the moment the samplers set it. No LISA-side decision exists in this design. I
should have said "driver", not "integrator".

The divergence is the driver script: ..._batchmode_lisa is 2,526 lines against the main
driver's 4,563, a fork of an older ILE with zero occurrences of ln_weights_from_rvs,
_pool_replica_rvs, _lnZ_of_rvs, _kish_neff_of_rvs, the L0 rescue, the sequential warm
start, replicas, .dgrid, or the proposal breadcrumb.

So LISA has nothing to migrate — its 36 post-rebind reads are the MAP-seed/export pattern,
already BENIGN/PER_ROW in the ledger, and it never pools or reweights. The real issue is two
forks of one ILE where one silently misses every fix. Larger than this design; flagged so it
isn't mistaken for part of it.

One thing worth noting

The --check gate that merged with #87 caught this draft's own new _rvs read, on the very
next change to touch one — a case nobody wrote it for. Classified PER_ROW (the record takes
the rebound columns as a view plus the pre-draw count, and reads no statistic of them). 133
sites now carry verdicts; 186 passed, 3 skipped.

Still a no-op — nothing reads the record. Ready for a decision on whether option A is worth
pursuing, and if so I'd do the reserve-reference version of Q2 rather than full retention.

[oshaughnessy-junior]

All three settled and recorded in the branch (0324ce7b).

2 — retained rows. Agreed: the record will reference the existing bounded reserve
(make_warm_seed_reserve, finite-stratified, n_max=20000) rather than take its own copy of
the retained rows. Full retention stays an AV-only option where it costs ~4 MB. That's the
concrete next step if A proceeds.

3 — LISA. Agreed, and logged as recurring maintenance rather than a one-off. The measured
drift table is recorded, along with the standing rule: every ILE driver PR states whether the
LISA driver needs the same change — "does not apply" is a fine answer, silence is not. A
catch-up session is queued to close the accumulated gap; it's scoped to decide per item (port /
does-not-apply-with-reason / needs-physics-decision) rather than port blindly, since several
genuinely won't apply, and to add a drift check so the gap is visible next time. Merging the two
drivers is explicitly off the table.

A vs B. Agreed — B is parked as long-term in #95, with the blast-radius numbers
(306 reads / 131 post-rebind / 7 rebind sites), why it can't be staged, and a definition-of-done.
The doc now says so at the option itself as well as in the recommendation, so the reasoning is
where someone will find it rather than in a PR comment.

Worth stating explicitly, since it changes how A should be judged: A is what makes B cheap
later. Once consumers ask a record instead of indexing a dict, B stops being a 306-site rename
and becomes a change to what the record's default view returns. So A's value isn't only the
flags it subsumes — it's that it converts the dangerous change into a safe one. If A doesn't
proceed, #95 stays permanently out of reach, which is a fine outcome but worth choosing
deliberately.

Still a no-op — nothing reads the record. 13 record tests pass, gate green at 133 sites.
Awaiting a decision on whether to pursue A; if yes I'd do the reserve-reference version first
and migrate one consumer as a worked example before touching the other six samplers.

[oshaughnessy-junior]

Option A started — ea55b92e. One worked example, not a sweep, so the shape is judgeable before
the mechanical part. No longer a no-op: one consumer is converted, and the conversion is
proven to change no number.

What's in it

Reserve by reference. retained_points() / retained_lnL() / n_retained() point at the
bounded, finite-stratified _warm_seed_reserve — never a copy. A pooled record carries no
reserve: it's a mixture of several passes, so there is no single retained set, and pointing at
one arbitrary pass's would be worse than None.

AV records both paths — fair_draw when the draw fires, retained when it doesn't.
"Absent" and "not resampled" are different statements; a consumer forced to distinguish them is
back to combining conditions by hand, which is the failure being removed.

Pooling builds a pooled record carrying _rep_fairdraw per block — exactly what the two
booleans cannot express, and the reason a raw/resampled mixture needed a special case in
_pool_replica_rvs. The record represents it directly.

First consumer migrated: ln_weights_for_posterior — chosen because it is the precise site
of the one-flag-two-questions defect, so the conversion demonstrates the point rather than
merely exercising the API.

How the migration is kept honest

Two descriptions of one thing is A's real cost, and all four #87 review rounds were "two
descriptions drifted apart". So it's asserted, not promised:

• test_the_record_and_the_flags_agree_in_every_state — across retained / fair draw / pooled /
  pooled-mixed / pooled-raw.
• test_the_migration_changes_no_number — on a real collapsed AV pass, the record path and the
  flag path return bit-identical weights, on both branches. Converting a consumer is a
  refactor, and stays checkable until the flags are deleted.
• The migrated consumer trusts a record only when .columns is rvs. _rvs is a mutable dict
  that may have been replaced since the record was built, so a stale description falls back to
  the flags instead of being believed.

Next, in order

1. The sibling consumers (.dgrid, breadcrumb, .dslice guard) ask the record.
2. The other six samplers set it — mechanical; audit_rvs_fairdraw.py already enumerates the
   rebind sites.
3. Only then delete _rvs_is_fairdraw / _rvs_is_pooled.
4. Long-term: make the fair draw return a new object so _rvs always means the retained set #95 becomes tractable at that point, not before.

26 record tests; 213 passed, 3 skipped; --check green at 134 sites — it caught the new
sampler-side read again, which is twice now on changes nobody wrote it for.

Nothing here touches the LISA driver; that's the separate catch-up session, and its 36
post-rebind reads are all BENIGN/PER_ROW since it never pools or reweights.

Still marked draft — say the word and I'll take it out of draft, or keep going to step 2 first.

[oshaughnessy-junior]

Steps 2 and 3 done — 0f718861. Still draft.

Step 2: every consumer asks the record

.dgrid and the breadcrumb moved with ln_weights_for_posterior; the .dslice guard and the
pooled n_eff now ask directly. Each asks a different question, which is the point:

consumer	question
.dslice guard	rows_are_resampled() — survives pooling; reweighting resampled rows double-counts either way
pooled n_eff	blocks_were_flattened() — a fact about the pooling step; keying it on either other question is what made that branch dead code in round 2
weights	is_equal_weight() — whole-record

All three go through one lookup, _rvs_record_for(sampler, rvs), which declines a record
whose .columns isn't the dict being held — _rvs is replaced in place, so "the sampler has a
record" and "the record describes these rows" are different questions. The producer at the
pooling site asks a third (_sampler_keeps_records) and got its own name rather than an
exemption: it's about to replace sampler._rvs, so the consumer question would answer "no
record" and it would silently skip building the pooled one.

Step 3: all seven rebind sites

Wired by one patcher against #87's own markers, so they're identical rather than seven hand
edits. There's a structural test asserting that — one patcher means one mistake would be
replicated everywhere, which is the case worth testing rather than eyeballing a diff.

Two things found doing it

n_retained had to be captured eagerly. RvsRecord.retained(self._rvs) holds a reference
to the live dict, which the draw then rebinds — so len(record) afterwards returns the post-
draw count. Reading it made a collapsed pass report n_retained == rows, i.e. "nothing was
discarded", the exact opposite of the truth. This project's own bug class, in the code written
to prevent it. Caught only because the end-to-end check printed n_ret == rows and that looked
wrong; now pinned by a test.

mcsampler/mcsamplerEnsemble take a linear integrand, AV/portfolio a log one. Wrong kind
→ negative fair-draw weights → raise. I confirmed it fails identically on the pristine file
before concluding anything, so it's a harness contract, not a defect. Recorded because it cost
time and will again.

State

31 record tests (every wired sampler agreeing with its flags on both draw settings); 248
passed, 3 skipped; --check green at 141 sites.

Remaining: step 4 deletes _rvs_is_fairdraw / _rvs_is_pooled once nothing reads them — but I'd
hold off. The flags are currently what makes step 3 checkable: the agreement tests compare
record against flag, and deleting the flags removes that cross-check at exactly the moment the
mechanical change is newest. My inclination is to leave both in place for a release cycle, let
the agreement tests run in CI, and delete the flags once they've been green against real runs.
Happy to do it now if you'd rather not carry the duplication.

[oshaughnessy-junior]

Flags kept, as agreed. And I took the backend point seriously — b775c6de.

It's a separate problem from the naming one, and RvsRecord does not fix it. So the first
step is to stop it being invisible: audit_backend_contracts.py records what each backend
actually does, and --check (now in CI) fails when one changes without the recorded table
changing with it.

What it found is worse than I'd assumed

I'd said "log vs linear integrand". It's three things, not two:

_rvs['integrand'] holds	backends
linear L	mcsampler, mcsamplerGPU
lnL (aliased from log_integrand)	mcsamplerAdaptiveVolume, mcsamplerNFlow, mcsamplerPortfolio
L or lnL, per the return_lnI kwarg	mcsamplerEnsemble

The last is the dangerous one: for that backend the column's meaning is a runtime property of
how the pass was called, so no amount of reading the consumer tells you which it is. That is
precisely why ln_weights_from_rvs demands use_lnL explicitly and why it must be the stored
convention rather than opts.internal_use_lnL — a constraint already documented at that
function, but nowhere discoverable from the backends themselves.

The failure is asymmetric, which is what makes it a landmine rather than a nuisance: a log
callable into a linear entry point makes the fair draw compute negative weights and raise; the
same mistake downstream does not raise — it takes log() of a log and returns a plausible,
almost-flat weight vector.

Two more differences recorded, because consumers must cope with them: only AV and the portfolio
keep a _warm_seed_reserve; and the portfolio's _rvs holds every draw against AV's retained
subset (~92 vs ~0.9 MB per million nmax), so n_retained means different things per backend.

Deliberately not "fixed"

The gate does not forbid the differences — several are load-bearing, and none should be
tidied without a decision that's yours, not mine. It makes a change show up as a diff instead of
as a wrong number months later. Same philosophy as the _rvs gate, which has now caught two
additions nobody wrote it for.

Verified by removing NFlow's integrand aliasing: it names the exact field that moved and the
exact before/after, then passes again on restore.

State

251 passed, 3 skipped; both gates green (141 _rvs sites, 6 backend contracts). Still draft, and
the flags stay.

If you want the divergence actually reduced rather than just visible, that's a third piece of
work and I'd want it scoped separately — the mcsamplerEnsemble kwarg case is the one I'd
target first, since it's the only one whose meaning isn't statically knowable.

[oshaughnessy-junior]

That framing is better than the draft had it, and it subsumes the backend divergence instead of
merely documenting it. Implemented — 30571754, still draft.

rec = sampler.samples()      # public; RvsRecord or None

rec.log_likelihood()         # ln L -- the SAME thing on all six backends
rec.log_prior()  /  rec.log_sampling_prior()
rec.log_weights()            # lnL + ln pi - ln q, and NO use_lnL argument

All log space: the only convention all six can express without loss, since the linear column
underflows to 0 at ~745 nats — exactly the regime this work is about.

How return_lnI goes stale

log_likelihood() prefers the unambiguous log_integrand column, which already covers AV,
NFlow, portfolio, GPU, and Ensemble when it ran under use_lnL. Only two cases have a bare
integrand whose meaning isn't on the record, and both are recorded where the answer is
actually known:

backend	recorded
mcsampler	integrand_is_log=False (writes no log columns at all)
mcsamplerEnsemble	integrand_is_log=bool(use_lnL) (log columns only under use_lnL)

That's the whole trick. The convention was always a runtime property recoverable only by the
sampler — so the sampler states it once, instead of every caller threading use_lnL through
and one of them eventually passing opts.internal_use_lnL by mistake, which is a bug already
documented at ln_weights_from_rvs. Once consumers are on this API, return_lnI is an
implementation detail of one backend rather than something the ILE has to know about.

A record with a raw integrand and no recorded convention raises rather than guessing. I
know that path works rather than merely exists because Ensemble hit it during wiring, before its
convention was recorded.

Shape

SamplerOutputMixin, because the six MCSampler classes share no base — five are
class MCSampler(object), and only mcsamplerNFlow inherits MCSamplerGeneric. Giving them a
real common base is a bigger change than this draft should make; the mixin gets the API onto all
six without one. That inconsistency is itself worth knowing about and is now in the recorded
backend contracts.

State

45 record tests, 1 skipped — mcsamplerNFlow needs the optional nflows package, so that case
asserts the class declaration in source and skips only the import. A plain skip would have
quietly stopped covering a backend the day a dependency dropped out. 262 passed, 4 skipped
overall; both gates green.

Still draft, flags still in place.

Two honest limits: nothing in the ILE calls samples() yet — the consumers still go through
_rvs_record directly, and moving them is the next increment. And the API can only disambiguate
what a sampler chooses to record, so it makes the divergence harmless to consumers rather than
smaller — reducing it is still the separate piece of work.

[oshaughnessy-junior]

Done — d5f1c61d, still draft. You were right that it touches a lot, so I enumerated first:
7 sampler self-reads (the producer reading its own attribute — those stay), 11 in the
ILE, 14 in the tests.

The two ILE writes were the interesting part

Replica pooling legitimately produces a record the sampler cannot build (it's a mixture of
several passes). With no public writer, that code had to assign sampler._rvs_record directly —
reaching into another object's private attribute, which is the habit this design exists to end.
So the mixin grew set_samples(). A writer needs an API as much as a reader does, and that
only became obvious once the reads were migrated.

I also folded the record into _snapshot_pass_state/_restore_pass_state. A stale record was
already declined by the identity check, so it's belt-and-braces — but "everything describing a
pass moves together" is the invariant, and carving an exception into it is how review round 1
happened.

Getting the boundary test right took three attempts

Worth reporting, because attempt 2 was actively dangerous:

1. Substring search — counts the comments explaining the hazard, which in these files are
   most of the occurrences. Same false alarm as _rvs fair-draw audit: enumerate every consumer, fix what it found, gate the rest #87.
2. Strip comments, count tokens — misses getattr(sampler, '_rvs_record'), where the
   name is a string literal, and that is exactly the form a consumer reaching inside would use.
   This version passed against a deliberately reintroduced violation. It was worse than no
   test: it would have reported a clean boundary while the boundary was broken. Found only by
   revert-checking it.
3. AST — attribute access where the object isn't self, plus getattr/setattr/hasattr
   with the name as a string constant. Verified to fail on both violation forms and pass on
   restore.

The LISA driver has its own case: as a deliberate fork it may legitimately have none of this,
but "none" and "half" are different, and half is how a fork rots.

State

271 passed, 4 skipped; both gates green (141 _rvs sites, 6 backend contracts); the ensemble
script-style test still recovers AC/GMM/AV to ~1.0.

The interface is now what you described: _rvs and _rvs_record are internal, consumers call
samples(), and the boundary is enforced rather than documented.

What I have not done: consumers still call ln_weights_from_rvs(rvs, use_lnL=...) in places
rather than rec.log_weights(). Migrating those removes the last reason for the use_lnL
parameter to exist — which is the step that actually lets return_lnI go stale rather than
merely allowing it to. I'd do that next unless you want to look at this first, since it's
where the remaining breakage risk lives.

[oshaughnessy-junior]

Agreed, so I wrote the validation plan before starting the change rather than after —
1dcabd27, RIFT/integrators/VALIDATION_rvs_weight_migration.md. The migration itself is not
started.

One measured fact changes the whole approach

At a fixed --run-seed the shape gate is deterministic to the bit. Two runs of the quick
preset on identical code differ in exactly one field:

field	run A	run B
wallclock	8.455	3.753
js, n_eff, lnI, mean_pull, width_ratio, corr_diff_max, bias_ln, rel_err, n_ess, n_eval	identical	identical

(Worth having checked rather than assumed — the first comparison printed DIFFER and looked like
it had killed the approach, until the diff turned out to be the timer.)

So for a refactor the acceptance criterion is bit-identity, not "within tolerance". That's
far more sensitive than compare_shape_results.py's own thresholds (js 0.005, pull 0.05, width
0.05), and it sidesteps the stochastic-flip problem run_shape_recovery.sh warns about at
length — its --confirm-repeats machinery exists for cells on the n_eff >= 100 floor, and a
refactor should never produce a differing cell at all. Any non-wallclock difference is a signal,
not something to compare against a tolerance.

The tier that can actually falsify it

shape_recovery.py carries its own log_weights_from_rvs() — a third implementation,
independent of both ln_weights_from_rvs and RvsRecord.log_weights(), written to be "tolerant
of the heterogeneous _rvs conventions". Asserting the three agree per backend, including
mcsamplerEnsemble in both use_lnL modes, tests the migration against an independent route
instead of against itself.

The trap in the sequencing

shape_recovery.py is itself an _rvs consumer, so it's both the ruler and a migration
target. Migrate the ILE weight path first, validate with the gate unchanged, migrate the gate
afterwards as its own step. Changing ruler and subject in one commit destroys the independence
that makes the falsification tier worth anything. I'd have walked into that if I'd just started
migrating consumers in file order.

Practical constraints, stated now

Tier 3 (test-run.sh, test-run-alts.sh) clones ILE-GPU-Paper and runs
make test_workflow_batch_gpu_lowlatency — network access and GPU-shaped. On CIT it must run on
a different host from the session, one campaign per host. If it can't be run, the plan says
the migration is provisional and says so, rather than shipping on the cheap tiers and calling
it validated.

Also recorded: what would make me stop — a confirmed differing cell, any disagreement among the
three weight implementations, or an unrunnable tier 3. Better decided now than under pressure
later.

Say the word and I'll start on tier 0 + the ILE weight path; or if you'd rather run the full ILE
tier yourself somewhere with a GPU, that's the piece I'm least able to close from here.