Skip to content

Bound import_bag memory: tile eviction (#92) + correct-at-record Welford backscatter (#93)#94

Merged
rolker merged 11 commits into
jazzyfrom
feature/issue-92
Jun 29, 2026
Merged

Bound import_bag memory: tile eviction (#92) + correct-at-record Welford backscatter (#93)#94
rolker merged 11 commits into
jazzyfrom
feature/issue-92

Conversation

@rolker

@rolker rolker commented Jun 29, 2026

Copy link
Copy Markdown
Owner

Summary

Bounds import_bag's memory so a large or heavily-oversampled survey no longer
OOM-kills the importer. Two tightly-coupled parts:

Closes #92. Closes #93.

Correctness

  • Correct-at-record == correct-at-extract for a fixed curve, proven bit-equivalent
    by a new test_node equivalence test (~13 digits). All existing backscatter test
    values unchanged.
  • Eviction/reload lossless via the Welford triplet (ResurveyedBackscatterBlendsLosslessly).
  • O(1) per-cell intensity proven (500k-beam constant-footprint test).
  • 418 tests, 0 failures. Static analysis clean.

Tradeoff (recorded in ADR-0007/0001)

Correct-at-record bakes the curve in at record time → re-correcting with a different
curve now requires reprocessing from bags. Accepted (curve fixed at configure); the
ADRs are updated to supersede the prior raw-retention / cube#15 re-derivability note.

Review

Pre-push Deep review (containerized), Round 2 approved — the 3 Round-1 must-fixes
(stale docstring, ADR reconciliation, reload-failure comment+logging) are resolved.
5 remaining suggestions (robustness/doc polish) are tracked as a follow-up, not blockers.

Validation

Full June 12–26 Massabesic build: RSS plateaus ~7 GB (O(area)) vs the unbounded run's
12 GB-by-20%-heading-to-60 GB.


Authored-By: Claude Code Agent
Model: Claude Opus 4.8 (1M context)

Claude Code Agent added 11 commits June 29, 2026 03:06
…ion (#92)

import_bag held the whole survey's resident GeoMapSheet tile set in RAM for
the entire pass and only wrote at the end, so memory scaled with surveyed
AREA -- a large multi-day survey OOM-killed the importer. Port the live
node's cube#70 bounded-RAM scheme to the offline path.

New cube::ImportAccumulator (in the shared store_import lib, so both the tool
and its unit test exercise the same orchestration; the node is untouched):

- Persist-then-drop: when resident tiles exceed --max-resident-tiles, each
  cold tile is written to the -o store (bathy via geoGridToTile + saveTile)
  BEFORE it is dropped. A tile whose persist throws is left resident, never
  dropped -- RAM stays transiently over budget rather than losing data.
- Reload-on-revisit: before a batch's soundings that land on a previously-
  evicted tile are next persisted, the tile's settled depth+uncertainty is
  reloaded (loadWindow + primeFromTile), keyed off the dirty set (not
  sounding centres) so a GGGS-seam neighbour is not missed. Faithful for a
  settled tile; an approximation if evicted mid-disambiguation (ADR-0001 --
  competing hypotheses + sample counts are not persisted, so uncertainty is
  re-derived from the reseed).
- Backscatter under eviction: co-estimated intensity lives only in the in-RAM
  node records, not the bathy tile, so reload cannot restore it. Eviction
  persists the evicted tile's backscatter to --bs-store with a newest-finite-
  wins on-disk merge (loadTile + overwrite only finite cells), so a later
  partial revisit never NaN-outs earlier intensity. CAVEAT: a cell resurveyed
  AFTER eviction keeps only the newest visit's intensity, not a Welford blend
  of both visits (overlap cells are exactly these) -- a documented divergence
  from an unbounded build, not data loss.

--max-resident-tiles defaults to 256 (generous offline; small/medium surveys
never evict -> identical output to the old path); 0 = unbounded (old behavior).

Tests (test_import_eviction): bounded==unbounded equivalence on a disjoint
multi-tile survey (bit-reproduced); revisit equivalence with disjoint cells
(no cell lost; depth+backscatter reproduced, uncertainty within the reload
re-derivation bound); resurveyed-cell backscatter divergence measured
explicitly (bounded = newest visit, unbounded = blend); and a persist-failure
case proving no data-bearing tile is dropped.
… spill/restore (#92)

The first cut bounded import_bag's RAM but left a backscatter approximation: a
cell resurveyed after its tile was evicted kept only the newest visit's
intensity (newest-finite-wins), because the reload restored depth but not the
co-estimated intensity. Replace that with a true multi-pass blend that matches a
never-evicted build (coordinator-approved Option A).

CUBE associates each beam's raw {intensity, angle, range} with a depth
hypothesis and corrects at extract time; the bathy tile and bs summary keep only
the corrected/settled result, so reload-from-summary can't rebuild the
estimator. So spill and restore the SAMPLES, not the summary:

- Spill on evict: ImportAccumulator writes each evicted tile's per-cell raw
  intensity_samples (winning hypothesis) to a unique temp scratch dir (one file
  per tile, overwritten -- the in-RAM list is always the complete population).
  Cleaned up in finalize() and via the destructor (RAII) so an exception can't
  leak it.
- Reload-BEFORE-add: addBatch now reloads any evicted tile the batch will touch
  (GeoMapSheet::gridIndicesForSoundings -- the same expanded window addSoundings
  uses, so seam neighbours aren't missed) BEFORE the soundings land. primeFromTile
  restores depth, then the spilled samples are restored onto the reseeded
  hypothesis; the batch's beams then accrete onto that SAME hypothesis, so the
  node-output intensity is the full pre+post-eviction blend.
- Simplified the bs persist to a plain overwrite (dropped the newest-finite-wins
  on-disk merge): with samples restored, the in-RAM tile holds the complete
  population at every eviction, so its corrected summary is already complete.

New additive accessors (node runtime behaviour unchanged -- the live node does
not call them; only the importer reorders):
  Node::chosenIntensitySamples / setSettledIntensitySamples
  GeoGrid::nodeIntensitySamples / setSettledIntensitySamplesAt
  GeoMapSheet::gridIndicesForSoundings / setSettledIntensitySamplesAt
addSoundings refactored to share boundsForSoundings (behaviour-preserving).

Remaining approximation (documented): only the bathy depth UNCERTAINTY is
re-derived on reload (single-hypothesis reseed); the depth VALUE is faithful and
backscatter is lossless. Transient disk cost = retained raw samples for evicted
tiles (scratch-only, deleted after).

Tests (test_import_eviction): ResurveyedBackscatterIsNewestWinsNotBlended ->
ResurveyedBackscatterBlendsLosslessly (bounded == unbounded blend on the overlap
cell, intensity + variance, tol 1e-3); new ScratchDirCleanedUpAfterFinalize;
no-revisit and disjoint-revisit equivalence + never-drop-on-persist-fail kept.
416 tests pass (incl. node/geo_map_sheet/geo_grid/persistence regressions).
…rect-at-record (#93)

Supersedes cube#92's Option A raw-sample spill. The Massabesic build still OOM'd
because tile eviction never fired (47 tiles < the 256 budget) and the real hog is
per-cell backscatter retention: Hypothesis kept every beam's raw {intensity,
angle, range} so the angle/TL correction could run at EXTRACT -- per-cell memory
grew with TOTAL beams (~1.5 GB/min, ~10^9 beams over 14 days). Depth state was
already O(1); this makes intensity O(1) too.

Correct-at-record + streaming Welford:
- Hypothesis: replace `std::vector<BeamIntensitySample> intensity_samples` with an
  IntensityWelford {uint32_t n; double mean; double m2;} (16-24 bytes, fixed).
- Correction MOVED record<-extract: factored the exact per-beam math out of
  Node::extractNodeRecord into `correctBeamIntensity` (angular_response_curve.*,
  alongside the moved-verbatim `curveRelativeDb`). recordBeam now takes Parameters,
  corrects the beam, and Welford-folds the corrected dB. NaN semantics preserved
  exactly (NaN intensity skipped; NaN angle counted, no angle term; NaN/<=0 range
  counted, no tier-2 TL term).
- extractNodeRecord reads (mean, M2/(n-1)/n) -- NO re-correction. Same node-output
  mean + estimate variance as before (variance-of-the-mean semantics unchanged).

cube#92 spill simplified: per-cell state is 3 numbers, so eviction/reload spills
the Welford TRIPLET (24-byte fixed record) instead of raw samples. Because
(n,mean,M2) is a perfect sufficient statistic, restore-then-continue is bit-
identical to never-evicting -- the resurvey-blend test now holds exactly. Removed
the BeamIntensitySample format + sample accessors; chosenIntensitySamples/
setSettledIntensitySamples -> chosenIntensityWelford/setSettledIntensityWelford,
nodeIntensitySamples -> nodeIntensityWelford, etc.

Live node: no live-node code change (it already passes Parameters to insert); its
backscatter output is unchanged, confirmed by the estimator suite.

Equivalence proof + tests:
- New test_node CorrectAtRecordMatchesCorrectAtExtract: 8 varied beams through
  recordBeam, asserts node-output mean+variance == the OLD correctBeamIntensity +
  naive sum/sum-of-squares reference (tol 1e-5; Welford vs naive agree to ~13+
  digits).
- New test_hypothesis IntensityStateIsConstantSizeRegardlessOfBeamCount: 500k
  beams folded, sizeof(Hypothesis) unchanged (O(1) per-cell intensity state).
- ALL existing backscatter OUTPUT values unchanged (ARA cube#81, tier-2 cube#87,
  store_import cube#80): EXPECT_NEAR 1e-4 / FLOAT_EQ assertions pass as written.
  The ARA/tier-2 helpers now pass the curve params to recordBeam (correction moved
  to record); the asserted intensity/variance VALUES are identical.
- cube#92 ResurveyedBackscatterBlendsLosslessly still passes (now via the triplet).

White-box tests updated to the new representation (NOT a value change): the
test_hypothesis recordBeam tests and the test_node exclusion-on-intervention test
inspected the raw `intensity_samples` vector; they now assert the Welford state
(intensity.n / intensity.mean) with the same semantics.

418 tests pass, 0 failures (incl. node/hypothesis/store_import/quantize/eviction).
Copilot AI review requested due to automatic review settings June 29, 2026 09:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@rolker rolker merged commit 0a6eefe into jazzy Jun 29, 2026
1 of 2 checks passed
@rolker rolker deleted the feature/issue-92 branch June 29, 2026 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants