Skip to content

batch mortie coverage in ShardMap.build (issue #396) - #400

Merged
espg merged 48 commits into
mainfrom
claude/396-batch-shardmap
Aug 10, 2026
Merged

batch mortie coverage in ShardMap.build (issue #396)#400
espg merged 48 commits into
mainfrom
claude/396-batch-shardmap

Conversation

@espg

@espg espg commented Aug 7, 2026

Copy link
Copy Markdown
Member

Closes #396.

ShardMap.build spends its wall clock in per-granule Python→Rust call overhead, not in geometry: the o5-vs-o9 datapoint on the issue (comment) shrank cell work ~16× but wall only 3.2×, so ~2/3 of a full-catalog build is fixed per-call cost. This PR eliminates that cost rather than amortizing it, per the premise correction: the original phase-1 "process pool" is not implemented and is not planned here. A pooled prototype was tried and abandoned — it spiked ~85 GB and took the machine down, which is exactly what a batch call into a GIL-releasing, rayon-parallel Rust core makes unnecessary.

Approach — batch, don't parallelize

mortie 0.9.4 ships polygons_to_morton_mocs(lats, lons, offsets, order=, normalize=, tolerance=, max_cells=) (espg/mortie#153): a ragged many→many coverage entry point, strict arrow offsets, GIL released for the whole batch, rayon across polygons, chunked internally so the ragged result assembles without a 2.5× concat peak. _intersect_mortie's HEALPix branch now:

  1. flattens every granule's rings once (_flatten_rings) into mortie's ragged layout plus an owners array mapping ring → record index;
  2. covers a block of rings per call (_batch_ring_mocs) instead of one morton_coverage_moc call per granule;
  3. resolves shard membership with searchsorted over the sorted shard array — the vectorized shape the non-HEALPix branch below it already used — replacing the scalar if s in all_shards test that ran once per MOC cell;
  4. regroups with a stable sort, which reproduces the old per-shard granule order exactly (see the identity argument below).

The ring → granule map is the load-bearing piece

_granule_footprints yields one ring per granule in swath mode but three per granule in beams mode (issue #65), while mortie's batch is one-ring-per-entry by design. Flattening without a back-map would silently turn "a granule's shards = the union over its rings" into "each ring is its own granule". So _flatten_rings returns owners, and the shard-side regroup dedups by owner:

granules = own[a:b]
out[int(cand[a])] = [int(g) for g in granules[_first_of_run(granules)]]

That consecutive-run dedup is complete, not approximate: records are visited in order and a granule's beam rings are adjacent, so owners is non-decreasing globally; a stable sort by shard therefore leaves each shard's owners non-decreasing, and every repeat of a granule within a shard is adjacent. This is the same set the old dict.fromkeys(v) produced, in the same order.

Blocking, and the block size

The batch is blocked rather than one giant call because the MOC words are the dominant allocation. _MOC_BATCH_RINGS is 32 — set from the min-of-3 real-catalog sweep in Measured, not from a synthetic fixture. Its two prior values (1024, then 256) were both chosen against synthetic footprints and were both too large; see What the real data corrected.

Peak memory has two terms and blocking bounds only one of them, so quoting it as "proportional to the block" (as an earlier revision of this body did) is wrong in the one direction that matters:

  • a block term_MOC_BATCH_RINGS × per-ring MOC words. California at order 13 peaks 57 MB at block 32, 251 at 64, 412 at 256, 616 at 2048;
  • a catalog term_flatten_rings concatenates every granule's vertices into one pair of flat arrays before any block runs. On the 555,867-granule clone that is 17,681,679 vertices × 16 B = 282.9 MB, plus 8.9 MB of offsets/owners: a ~292 MB floor no block size removes. Plus the hit_shards/hit_owners accumulators, which scale with pairs (the 88S serial row's 85 MB at 2.07 M pairs is that term alone).

So a build's peak ≈ catalog floor + block buffers, and the operator-scale figure cannot be read off the block alone.

The AOI filter also runs per ring, as each ring's cover lands, not once per block. Accumulating the block's densified cells first built a buffer that dwarfed the AOI hits it existed to produce, plus two np.concatenate copies of it. Filtering per ring is just as vectorized and keeps the accumulator bounded by hits, as the pre-#396 loop was; owners[start + r] is non-decreasing in r, so the sort/dedup invariant is untouched.

Failure semantics preserved

The serial path wrapped each granule in except Exception: continue, so one malformed footprint cost one granule. The batch call is fail-fast for the whole block (it raises naming the lowest-index offender). Two things restore the old contract:

  • _flatten_rings screens the documented rejection causes up front — fewer than 3 vertices, mismatched lat/lon lengths, a non-finite coordinate — dropping those rings quietly. Verified against mortie: scalar and batch reject exactly the same inputs and accept the same odd-but-legal ones (degenerate, collinear, self-intersecting, out-of-range latitude, unclosed rings all cover identically in both), and the malformed-ring test pins that equivalence against the serial oracle rather than a hard-coded expectation.
  • Anything undocumented that still raises (a captured kernel panic, per espg/mortie@3de8164) falls back to the per-ring scalar path for that block only, which is the old loop verbatim. That fallback emits one RuntimeWarning per build (not per block) naming the mortie exception — resolving question (3) below.

moc_to_order stays per-ring and keeps its try/except: its cell budget can refuse a huge expansion, and that dropped the ring before this PR too.

Dependency floor bump (existing dependency, not a new one)

mortie>=0.9.3mortie>=0.9.6 in pyproject.toml, in three steps: 0.9.4 for phase 1's polygons_to_morton_mocs, 0.9.5 for phase 3's mortie.arrow.from_wkbs, and 0.9.6 for phase 4's MOC batch twins. This is a floor bump on an existing core dependency — no new package, no new transitive deps, no footprint change.

Safety note: 0.9.5 deletes mortie/tools.py (espg/mortie#159's domain split) — the module mort2polygon/mort2geo used to live in. The bump is safe because #411 landed first: git grep 'mortie\.tools' origin/main returns nothing, and the six sites that existed (data/build_aoi_shardmap.py, data/conus/build_conus_shardmap.py, demo/05_california_read.ipynb, notebooks/aoi_mask.ipynb, src/zagg/grids/healpix.py ×2) came off this branch in the origin/main merge below. Confirmed against the installed 0.9.5 wheel: no tools.py on disk, and import mortie.tools raises ModuleNotFoundError.

pyproject.toml is the only version spec that needs to move (question (1), now resolved): deployment/aws/build_layer.sh:92-95 derives MORTIE_SPEC from [project.dependencies] via tomllib (issue #322), so the Lambda layer picks the floor up with no infra edit — which is why nothing under deployment/aws/ is touched here. uv.lock is gitignored and no CI job runs --frozen/--locked. CLAUDE.md §7 carried a stale >=0.7.2 in its prose description of the dependency; cd78699 moved it to >=0.9.4 per the "across all of zagg" ruling, 9005c3a carried it to >=0.9.5, and 5b74d70/6988e9e6 then removed the restatement from CLAUDE.md entirely (its §7 now points at pyproject.toml as the single source), so the 0.9.6 sweep (fbc67b2) has exactly one floor site to move. A fresh grep for mortie>= / mortie >= / mortie ≥ / MORTIE_SPEC / MIN_MORTIE_VERSION across the tree (excluding archive/) confirms it: the only floor spec is pyproject.toml's, now >=0.9.6; every other hit states when an API arrived (below). No CI workflow pins a mortie version.

Left alone deliberately: the mortie >= 0.8.3 / >= 0.8.4 / >= 0.9.3 mentions in docs/aoi_mask.md, docs/ragged_layout.md, src/zagg/grids/aoi.py (MIN_MORTIE_VERSION) and src/zagg/grids/morton.py. Those state when a particular API arrived, not the package floor, and they stay true; rewriting them to 0.9.6 would erase provenance. src/zagg/catalog/shardmap.py's mortie >= 0.9.4 note on polygons_to_morton_mocs and src/zagg/catalog/sources.py's mortie >= 0.9.5 note on from_wkbs are the same kind and stay. Flagging in case the ruling was meant to cover them too.

Coordination note: a concurrent local edit to pyproject.toml in the primary checkout (a comment block documenting the lambda extra's numpy/pandas pins) was inspected read-only before this sweep; it does not touch the mortie hunk, so the two changes cannot conflict and nothing from it is replicated here.

Merged origin/main

The branch was 92 commits behind, brought current by merging origin/main in (3d46aa0) rather than rebasing — §1 forbids force-pushing, so a rebase was not available. The merge was clean: no textual conflicts and no semantic ones either, because main touched nothing under src/zagg/catalog/ in those 92 commits (git diff --stat ad8aa30 origin/main -- src/zagg/catalog/ tests/test_shardmap.py bench/ pyproject.toml is empty; the only overlap in docs/ is the new docs/hive_layout.md). What the merge did bring is #411's removal of the last six mortie.tools imports — which is what makes the 0.9.5 floor safe (above).

Measured — real catalogs (phase 2)

Every number below comes from bench/shardmap_batch_vs_serial.py against real CMR catalogs in the tree. The previous synthetic figures are withdrawn — see What the real data corrected.

Grid is the shipped production one (tests/data/benchmark/configs/atl03_tdigest_healpix_o9.yaml: parent_order 9, child_order 19, chunk_inner 13), so a default build resolves the MOC order to 13 (#92); 9 is also swept because it is the legal floor the issue's baselines used. Every measurement runs in its own process, since ru_maxrss is a monotone high-water mark and serial and batch cannot share one. Batch output is asserted equal to the serial oracle on every row except the one row that has only a batch arm, which is labelled no-assert by the harness (see below).

Peak is measured against the load's resident plateau, not its high-water. These are not the same number: on the full case granule_records() leaves a transient above the plateau it settles at, and a maxrss-vs-maxrss delta silently absorbs the intersection's first few hundred MB and floors at 0 — which is why an earlier revision of this table reported 0 MB cells as if they were measurements. The harness now reports both (-- columns bat_MB/ser_MB against the resident plateau, bat_hw/ser_hw against the high-water). Where the hw delta is > 0 the resident-baseline figure is the intersection's exact increment; where hw is 0 the intersection never rose above the load's high-water, so the figure is an upper bound and is marked .

neon and 88s are committed fixtures (tests/data/benchmark/catalogs/) and run in any checkout — every path resolves relative to the script (previously they were absolute paths into one working copy, so they would have skipped anywhere else). california and full need the 305 MB ATL03 clone (data/atl03_v007/, not committed) and skip cleanly when it is absent via _available(); that guard is new here rather than a convention inherited from bench/neon_order_sweep.py, which reads the clone unguarded.

All numbers below were re-measured at the shipped block of 32 after the block change, on a machine at load average 6–20 (walls are therefore upper bounds; the block-size decision is made on min-of-3, below).

case catalog order granules shards pairs serial batch speedup serial peak batch peak
neon committed 9 2,089 4 286 1.05 s 0.26 s 4.0× ≤5 MB 7 MB
neon committed 13 2,089 4 286 19.01 s 4.71 s 4.0× 8 MB 35 MB
california clone cut 9 4,354 2,721 190,625 2.55 s 0.63 s 4.0× 44 MB 54 MB
california clone cut 13 4,354 2,721 190,625 44.23 s 9.66 s 4.6× 45 MB 100 MB
88s committed 9 35,639 564 2,074,785 15.77 s 5.06 s 3.1× 84 MB 192 MB
88s committed 13 35,639 564 2,074,785 171.06 s 59.60 s 2.9× 85 MB 209 MB
full clone 9 555,867 2,721 190,625 279.34 s 63.73 s 4.4× ≤267 MB 406 MB

The full row is the issue's headline baseline reproduced: 279.3 s against the issue's 310.7 s for the same single-pass mortie build, and it lands on the same 190,625 pairs / 2,721 shards as the bbox-cut california row — an independent check that the cut is not dropping assignments.

Speedup is 2.9–4.6× across every real fixture and both orders, with the operator-scale case at 4.4×.

The full row is at order 9 to line up with the issue's baseline. The order-13 arm of the same case — the order a default build actually resolves to — is run batch-only, because its serial arm is ~1 h. It is the one row in this PR with no oracle assert and the harness labels it no-assert (one arm only); what backs it instead is the california o13 row above, same AOI and order with both arms on identical output, plus TestMortieBatch's dict == pins. Reproduce it with --cases full --orders 13 --arms batch:

full @ order 13, 555,867 granules peak vs resident plateau peak vs high-water
_MOC_BATCH_RINGS = 32 (shipped) 545 MB 278 MB
_MOC_BATCH_RINGS = 256 (previous) 1,483 MB 1,217 MB

Same 190,625 pairs / 2,721 shards both ways. Walls (1,065 s and 929 s) are not quoted as a comparison — they ran at load average 18–29 against other work on the machine.

This is the measurement question (6) asked for, at the exact scale the withdrawn 143 GB was extrapolated to: 1.5 GB at the old block, 545 MB at the new one. Note that the old high-water-baseline figure for the 256 arm reproduces exactly (1,217 MB against the 1,216 MB previously reported); it is the baseline, not the run, that changed.

Block-size knee, re-validated on real footprints

Same harness, california at order 13, each row its own process, wall reported min-of-3 (--knee california --order 13 --reps 3). Min-of-N is not optional here: polygons_to_morton_mocs is rayon-parallel, and single-shot walls on this machine scattered up to 57% within a block — wider than the block-to-block effect being measured. The earlier single-shot version of this table was internally inconsistent for that reason (block 24 beat 32; block 512 beat 64).

block 8 16 24 32 48 64 96 128 256 512 1024 2048
wall min-of-3 (s) 12.62 11.06 10.12 9.89 9.84 9.67 9.52 9.57 9.58 9.63 9.75 9.66
peak (MB) 56 62 95 97 216 290 431 436 452 478 538 656
peak vs high-water (MB) 16 22 0 57 176 251 391 396 412 438 498 616

At order 9 the sweep is flat in both columns (16 → 0.73 s / 15 MB; 32 → 0.66 s / 15 MB; 64 → 0.64 s / 16 MB; 1024 → 0.58 s / 32 MB). At 88S order 13 the picture is different from California's and worth stating exactly, because it is the case that most constrains the choice: min-of-3 gives 32 → 63.13 s / 200 MB, 64 → 55.02 s / 210 MB, 256 → 53.74 s / 840 MB — but re-running 32 and 64 with the block order reversed gives 64 → 56.55 s, 32 → 57.37 s. So 88S's 32-vs-64 wall gap is ~4% (min of 6 across both orderings), not the 15% the block-major sweep suggested and not the "32 is faster" the previous single-shot row claimed. Block-major sweeps correlate block size with drifting machine load; that is now noted in shardmap.py.

Verdict: the knee did not hold, and _MOC_BATCH_RINGS moves 256 → 32 (question (7), resolved to 32 rather than 64). The decision is made on the peak column, which reproduces across three independent runs, rather than on wall differences that sit inside single-run scatter:

  • wall falls steeply only up to ~32 rings (12.62 s at 8 → 11.06 at 16 → 9.89 at 32) and is within 4% of its asymptote from 32 out — so the batch's fixed cost amortizes an order of magnitude sooner than the synthetic sweep suggested;
  • 32 costs 2.3% wall against 64 on California (9.89 vs 9.67 s) for 4.4× less peak (57 vs 251 MB), and ~4% on 88S for ~7% less peak;
  • against the previous 256 it is 7× less peak on California and 4.4× on 88S, for ~3% wall.

Memory is worth more than 2–4% of wall on a 2 GB Lambda, so 32 ships. The claim that peak "keeps climbing with the block all the way out" is withdrawn — 88S goes 1,057 → 1,010 MB from 256 to 1024 — as is "within ~6% of the asymptote in every case", which 88S violated at +6.4%.

What the real data corrected

  1. The synthetic fixture was miscalibrated by ~40×, and it is what set the block size. A synthetic quadrilateral covers ~220 MOC words at order 13. Real ATL03 CMR footprints — 1,000 sampled uniformly from the clone (seed 20260807), covered at order 13 — run min 4,373 / p25 7,179 / median 9,266 / mean 8,659 / p95 10,370 / max 10,881, at a median 27 vertices each. An earlier revision of this body quoted "median 10,352" from a NEON-weighted fixture pool rather than the catalog: ~12% high, and withdrawn. The distribution matters more than the median, because the block's steady memory follows the mean and its worst case the p99/max. The ~32,127 quoted before that is withdrawn too — it is 3.0× the largest footprint in the sample, which is what confirms it was a synthetic 90°-of-latitude envelope and not a CMR polygon.
  2. The withdrawn 5.7×/9.5× headline read high. Real fixtures give 2.9–4.6×. The synthetic number was inflated by amortizing the batch fixed cost over ~1/40 of the real per-footprint work.
  3. The "mortie retains its output buffer / ~143 GB at operator scale" merge-blocker is withdrawn, per espg's correction and confirmed at operator scale: the full build — 555,867 granules, the exact case the 143 GB was extrapolated to — peaks at 406 MB over its resident load plateau at order 9. The mechanism sentence that stood here before ("peak tracks the block size … not the catalog") was wrong and is replaced by the two-term model in Blocking, and the block size: a catalog-proportional _flatten_rings floor (~292 MB at this scale) plus block-proportional MOC buffers. The refutation is unaffected — 406 MB against 143 GB is five orders of magnitude — but the model the next extrapolation gets built on is now the right one.
  4. granule_records() is the real memory story at operator scale, not the intersection — but not for the reason stated here before. The plateau is real and slightly larger than reported: 137 MB interpreter → 1,353 MB after from_geoparquet4,038 MB resident after granule_records(), i.e. +2,684 MB. The coordinate arrays are only 282.9 MB of that, 7%. The dominant term is the seven whole-table to_pylist() calls at sources.py:525-552, all live simultaneously (~1.8 GB), of which assets alone is 1,147 MB — 2,064 B/row, 28% of the plateau — while the loop reads exactly two hrefs out of each dict; geometry WKB is 305 MB, the str values 304 MB, the dict objects 151 MB, id 49 MB, the two datetimes 63 MB. So the big lever is not vectorizing shapely.from_wkb (≤15% of the plateau, which is what the on-hold granule_records work targets) but not materializing assets/geometry table-wide — project the two href fields in Arrow, or batch the to_pylist() calls, and ~1.5 GB goes away with no API change. Scoping that is a separate call; flagging it rather than doing it here. (RSS also does not fall after those intermediates go out of scope or after del cat — allocator-retained, so peak and steady state are the same number at this scale.)

Phases

  • Phase 1 — batch rewire of _intersect_mortie (HEALPix branch): flatten rings + ring→granule map, blocked polygons_to_morton_mocs, vectorized searchsorted membership, stable-sort regroup; mortie floor to 0.9.4; identity tests.
  • Phase 2 (revised) — real-catalog benchmarks. bench/shardmap_batch_vs_serial.py replaces the synthetic numbers with the table above: four real catalogs from 2,089 to 555,867 granules, wall and peak RSS, serial vs batch, both orders, process-isolated. Block size re-validated on real footprints, min-of-N, with the blocks/order/reps all CLI-settable so every quoted row is regenerable. (The original phase 2 — vectorizing shapely.from_wkb in granule_records — is on hold pending an architecture decision and is not implemented here; item (4) below is why it is probably aimed at the wrong term.)
  • Phase 3 — footprint_cells catalog column (issue phase 2): per-granule morton MOC at a fixed order in the catalog geoparquet via the mortie arrow skin, build fast path = AOI MOC ∩ per-granule MOC set ops with no geometry work; refuse loudly when the shard order is finer than the column order; docs note it as a zagg-clone convention column whose staleness is structurally impossible (it rides in the same file as the geometry it was computed from). Section below.
  • Phase 4 — batch the stored-index intersection on mortie 0.9.6 (272de47, fbc67b2, 48e3c4d; review fold 669a09b…7efd38c6): swap phase 3's per-granule scalar moc_and/moc_to_order loop for one mocs_and + mocs_to_orders per block of records (Batch MOC set operations: mocs_and / mocs_intersect and the 1xN broadcast family espg/mortie#173), blocked at _CELLS_BATCH_RECORDS = 512 on the measured clone-scale memory sweep; mortie floor to 0.9.6 across the tree per the question (1) ruling; permanent scalar-parity test; bench harness grows --knee-arm cells and the measured tables are regenerated. Section Phase 4 below.
  • Phase 5 — mocs_intersect prefilter on the stored-index path (8347050): the phase-4 section's flagged future work, measured and landed — a blocked range-walk predicate over the row-aligned column gates the materializing mocs_and pass to surviving records only (~0.4% at clone scale). First production consumer of Batch MOC set operations: mocs_and / mocs_intersect and the 1xN broadcast family espg/mortie#173's mocs_intersect, closing the loop on that op's consumer-evidence rationale. Clone-scale wall 2.53 s → 1.70 s min-of-3 at an unchanged peak; the input-copy term verified at clone scale and blocked; parity pinned by the harness digest on every row plus new edge tests (review fold 21065c0…4753c317). Section Phase 5 below.

Phase 3 — the footprint_cells column

The per-granule footprint→cells cover is identical for every build against the same catalog, so the catalog carries it. Catalog.index_footprints(order) covers the whole geometry column with one mortie.arrow.from_wkbs call and appends a ragged large_list of morton MOC words, typed with mortie's morton_index extension (the same convention ShardMap.to_parquet and docs/morton_arrow.md use, and it survives stac_geoparquet.to_parquet — pinned by test_column_is_morton_typed_on_disk). The order lives in the catalog's own metadata as footprint_cells_order. python -m zagg.catalog --index-footprints ORDER does it while fetching, so a saved clone ships pre-indexed.

ShardMap.build then does no geometry at all (phase 4 shape — two batch calls per block of records):

aoi_moc = compress_moc(sorted(all_shards))                        # once per build
hit_vals, hit_off = mocs_and(aoi_moc, rec_vals, rec_off)          # one call per block
flat, flat_off = mocs_to_orders(hit_vals, hit_off, parent_order)  # already inside the AOI

Intersecting with the AOI first is what makes the searchsorted membership filter of phase 1 unnecessary: aoi_moc is a union of whole parent_order cells, so every cell mocs_to_orders yields is in all_shards by construction. The regroup is phase 1's, now shared as _regroup_hits so the two paths cannot drift.

Phase 4 — the per-granule loop is gone: mortie 0.9.6's batch twins

An earlier revision of this section recorded that mortie 0.9.5 had "no mocs_intersect predicate and no N-way variadic fold" and that the fast path therefore loops in Python calling scalar moc_and per granule. That is no longer true and the loop is no longer here. mortie 0.9.6 ships mocs_and — the 1×N broadcast of moc_and, one shared operand against a ragged column, GIL released, empty results kept as zero-width slots (espg/mortie#173/#174) — and phase 4 swaps the scalar loop for it, chained into the ragged mocs_to_orders (0.9.5, espg/mortie#156). Two Python→Rust crossings per block of records replace two calls per granule, and the AOI operand's BMOC is built once per block instead of once per granule. mocs_intersect was deliberately not used as the per-slot answer: this call site needs the intersection's cells (they are the shards after mocs_to_orders), never a bare emptiness test — empty slots ride through both batch calls for free. Phase 5 later adds it in the one role compatible with that, a prefilter that gates which records reach the materializing calls at all (section Phase 5 below).

The swap was measured before it was adopted — the phase-4 measurement comment carries the full method, profiles and parity assertion; its numbers against the reproduced scalar baseline on this machine:

  • California / o9 (4,354 granules, 190,625 pairs): arm wall 0.228 s → 0.054 s (4.2×), _intersect_footprint_cells alone 0.225 → 0.052 s, boundary crossings 4,354 + 2,356 → 1 + 1 per block. The moc_and term itself is 11× — honest agreement with Batch MOC set ops: mocs_and / mocs_intersect + scalar moc_intersects (issue #173) espg/mortie#174's ~15× tiny-operand floor, not a shortfall against its 121–169× at-scale row.
  • Clone / o9 (555,867 granules, same California AOI): 18.94 s (median, n=2) → 3.57 s (median, n=3) unblocked in the measurement; 2.53 s min-of-3 / 2.54 s median at the shipped block7.5× median-over-median, 7.4× min-over-min against the reproduced scalar baseline, above the measurement's 5.3× because blocking costs nothing on wall (below).
  • With mortie out of the way the regroup now dominates the fixture-scale arm (62% of the remaining 65 ms); the next win, if anyone wants it, is numpy-side, not more Rust.

Blocked, because the memory posture was measured too. The measurement's whole-catalog single call peaked at ~5.2 GB over the load plateau at clone scale — a record-aligned gather copy of the column plus mortie's documented input copy (mocs_and copies values/offsets before releasing the GIL), both catalog-proportional — against the scalar loop's ~1.7 GB. So the shipped swap wraps the batch calls in blocks of granules, _CELLS_BATCH_RECORDS = 512, the same shape of constant as phase 1's _MOC_BATCH_RINGS. The clone sweep (--knee full --knee-arm cells, each row its own process, min-of-3 at the finalists):

records/block 512 2048 4096 8192 16384 65536 262144 whole catalog
wall (s) 2.53 2.50 2.55 2.49 2.55 3.10 3.27 3.83
peak over plateau (MB) 1,737 1,770 1,828 2,080 2,286 3,650 6,407 5,198

Blocking is at worst free on wall — unblocked runs measured 3.83 s (this session, single shot) and 3.04/3.57/4.16 s (the measurement session, n=3), against 2.49–2.55 s at every block from 512 to 16384 — and ~1,736 MB is the o9 floor, the sweep's own minimum (1,736.5 MB at 1024, 1,736.9 at 512): column materialization plus plan bookkeeping no block size removes. So 512 puts the batch path's peak at the scalar loop's own ~1.7 GB while keeping the 7.4–7.5× above. 512 over 2048 is a bytes argument, not a records one: per-record MOC size grows ~40× from an order-9 column to an order-13 one, and on the anti-recommended o13-indexed configurations the smaller block is what bounds the damage (88S o13: 2,627 MB at 512 vs 4,063 at 2048, walls 4.62/4.59 s; California o13: 295 vs 1,261 MB). The per-block cost of rebuilding the shared AOI operand's BMOC is measured, not assumed, negligible: 104 µs/call at the real 2,721-cell California operand — ~113 ms across the clone's 1,086 blocks, ~4% of the arm.

One semantic change rides along, stated rather than hidden. The scalar loop wrapped moc_to_order in except Exception: continue, so a cell-budget refusal silently dropped that one granule. mocs_to_orders applies the same per-MOC budget but refuses the whole call (ValueError naming the lowest-index offender), so a refusal now fails the build loudly — and since phase 4's review fold (290bfcc) the re-raise names the block's record range so the offender is identifiable. Reaching it requires a single granule spanning ~10⁶ shard cells inside the AOI: the expansion is bounded by the AOI-clipped footprint densified at parent_order, not zero — compressed operands mean mocs_to_orders can genuinely expand, so "unreachable" would be too strong (the review thread on 669a09b carries the exact bound). Loud-over-silent is the intended trade, recorded in the _intersect_footprint_cells comment; the geometry path's per-ring refusal still drops just the ring, a deliberate divergence also recorded there. (A malformed stored word still panics in Rust and aborts the build in both versions, unchanged.)

Parity is a permanent test now, not just the measurement's assertion: test_batched_cells_equal_the_scalar_loop keeps the pre-phase-4 scalar loop verbatim in the test file as _intersect_cells_serial (the same one-implementation-ships pattern as _intersect_mortie_serial) and asserts exact dict equality — same keys, same granule lists, same order — over both a matched-order column and a coarsening one (index 13 against parent 11, the fold's 52e2125, which is what exercises dropping the scalar loop's np.unique), at blocks 1, 5, 7 and 24 over a fixture with zero-width slots, a dropped non-polygonal row (so the block gather walks non-contiguous column spans) and ragged-tail block boundaries. The harness digest assert is the same check at scale: identical digests across arms on every asserted row below, including batch-vs-cells on the full 555,867-granule clone (-8201871529712101455, the measurement comment's digest reproduced).

Phase 5 — the mocs_intersect prefilter (the flagged future work, measured)

Phase 4 flagged it; this phase lands it. At clone scale the batch calls scanned all 555,867 stored MOCs against the AOI with ~99% of slots coming back empty, paying a record-aligned gather (plus mortie's input copy) for slots that materialize nothing. _intersect_footprint_cells now runs mortie 0.9.6's mocs_intersectespg/mortie#173's range-walk predicate, espg/mortie PR 174's implementation — over the column before the blocked mocs_and pass, and only surviving records are gathered and materialized. This is that op's first production consumer, closing the loop on the consumer-evidence rationale it shipped under. (This is distinct from question (4)'s prefilter_order= idea, which is a coarse-order geometry prefilter on the geometry path; that question stands.)

Two structural points:

  • The predicate walks the column, not the records. The column's own arrow offsets make every predicate block a contiguous zero-copy slice — gathering per record is exactly the copy the predicate exists to avoid. Rows granule_records dropped are walked too (index_footprints gives them zero-width runs, which cost nothing) and hits[rows] discards them.
  • The owner mapping is the load-bearing edit. Survivors are surv = np.flatnonzero(hits[rows]) — original record indices, strictly increasing — and the gather blocks now cut at survivor counts with owners surv[start : start + B], never start + i. surv increasing is what keeps _regroup_hits' non-decreasing-owners invariant. Parity is pinned by the existing block sweep (test_batched_cells_equal_the_scalar_loop, whose interleaved empties make survivor blocks genuinely differ from record blocks) plus a new test_prefilter_edges_all_empty_all_hit_and_middle_run (named for what its survivor shape really is — a contiguous middle run {4, 5} behind leading and ahead of trailing empties, not an interleave): all-empty (a disjoint but non-empty AOI, so the empty-inputs gate does not fire and the predicate itself must drop every record), all-hit (the survivor gather is the identity; every record pinned assigned), and the middle run, where surv[0] != 0 — the off-by-one shape a block-local owner mapping would fail — each against the scalar oracle at blocks 1, 3, 4 and 10. The review fold hardened this further: the refusal message is exercised on a gapped, non-0-based survivor set (surv == [3, 6], test_batch_refusal_range_reads_the_survivor_owners), an over-reporting predicate is pinned to still return {} rather than die in the regroup (test_predicate_overreport_still_returns_empty, restoring a one-branch guard), and the edge fixture's cell-quantization margin is argued from the granule gap with cos(lat) accounted (0.101° arc ≈ 3.5 order-11 cells).

The input-copy term was verified at clone scale, not assumed — and it blocks the predicate too. mortie documents mocs_intersect's peak as the binding's input copy; on the clone's order-9 column (values are 1,551 MB) the whole-column single call measured 1,096 MB over plateau in-process, and 3,305 MB vs 1,739 MB at block 512 in the harness knee (--knee full --knee-arm cells --blocks 512,4096,65536,555867; walls 1.75 / 1.55 / 1.69 / 1.77 s — flat, single-shot scatter). So the predicate reuses _CELLS_BATCH_RECORDS as contiguous row-slices: ~0.1 s of predicate wall (0.88 s whole-column vs 0.98 s blocked, min-of-3 in-process) buys ~1.1 GB less peak, and the sizing argument is the same bytes-not-records one as the gather pass (512 rows ≈ 1.2 MB/slice on an order-9 column, ~30 MB on an order-13 one).

Blocking the survivor pass stays too. At clone scale survivors are 2,356 of 555,867 (0.42%) — five blocks, where one call would also be fine. But survivors are AOI-proportional, not catalog-proportional: an AOI covering the catalog passes everything, and an unblocked survivor pass is then the ~5.2 GB whole-catalog call phase 4 blocked away. Blocking measured at-worst-free on wall, so _CELLS_BATCH_RECORDS = 512 stays for both passes; its justifying comment is updated in place with these numbers.

Measured — against the phase-4 blocked baseline (same harness, same process isolation; digest asserted green between arms on every row below, and the clone row's digest is the measurement comment's -8201871529712101455 exactly):

case order phase-4 cells prefiltered cells phase-4 cel_MB prefiltered cel_MB
full 9 2.53 s min-of-3 1.70 s min-of-3 (1.66–1.84 across five runs) 1,737 1,738–1,741
neon 9 0.01 s 0.01 s 18 14
california 9 0.06 s 0.06 s 57 53
88s 9 0.51 s 0.55 s 237 237
neon 13 0.26 s 0.18 s 458 308
california 13 0.68 s 0.73 s 312 429
88s 13 4.61 s 5.99 s 2,627 2,528

(Phase-4 fixture rows were min-of-3 at order 9 and min-of-2 at order 13; the prefiltered rows are min-of-3 throughout, batch↔cells arms in the same run. The batch arms reproduced within ~1–4% of their phase-4 values in the same children — 88s o13 batch 55.93 vs 56.47 s — so the sessions are comparable.)

The verdict, stated honestly: the win is where the empty slots are, and only there. The operator-scale case — the clone against a regional AOI, 99.6% of slots empty — goes 2.53 → 1.70 s (−33%) at an unchanged peak: both sit on the ~1.74 GB column-materialization floor that no block size or prefilter removes. Fixture populations that mostly hit pay the predicate as pure overhead: +8% on 88s o9 (0.04 s absolute) and +30% on 88s o13 (4.61 → 5.99 s min-of-3) — the fat-column configuration the order section above already anti-recommends indexing at. Neon, mostly-empty like the clone, gets faster at both orders. Landed because the win falls on the recommended configuration (index at the shard order) at the scale that motivated the phase, and the regression is confined to the anti-recommended one — recorded rather than smoothed, and if an o13-indexed column ever becomes a supported posture the prefilter should be re-measured there.

With the prefilter in, the clone arm's remaining ~1.7 s is the predicate's own blocked walk over the 555,867-MOC column (~1.0 s in-process) plus the id→row plan alignment; the survivor gather + mocs_and + mocs_to_orders are ~50 ms combined. mortie is the majority term again — a faster path, if ever needed, is upstream, not more zagg-side numpy.

Engagement gate, and the refusal

The stored cover is mortie MOCs at the column's own order, so it may only answer a build that asked for exactly that. The fast path engages when all of: the resolved backend is mortie; footprint="swath"; mortie_order was not pinned by the caller; the grid is HEALPix; the column is present. Anything else takes the geometry path unchanged — in particular an exact-S2 spherely run is never silently swapped for a MOC one (the ~0.01% polar omission, espg/mortie#32). metadata["footprint_cells"] records the verdict: True when the index answered the build, False when the catalog is indexed but the build took geometry anyway. It is a bool rather than a present/absent key because the catalog's own footprint_cells_order rides into the manifest either way, so a bare absent key beside it read as if the index had answered (a74a4fc9). A manifest with neither key came from a catalog that was never indexed.

A column coarser than the grid's parent_order raises. Answering it would refine every cell onto all 4^(parent_order − order) descendants and put ~every granule in ~every shard — the #92 failure — and silently falling through to geometry would hide that the index the operator paid for is useless for this grid. Pinned by test_column_coarser_than_the_shard_order_is_refused.

Row alignment is the load-bearing piece

granule_records() skips rows whose geometry is empty or non-polygonal, while the column has one entry per table row. Record index is therefore not row index, and a fast path that assumed it would hand every granule after the first gap its neighbour's footprint. Two things close that:

  • index_footprints screens those rows with the same shapely predicate granule_records uses and gives them a zero-length run, so a catalog carrying a stray Point indexes rather than raising (mortie's coverage refuses a point outright, naming the blob). The screen costs 0.02 s against 2.65 s for the order-9 cover on the 35,639-granule 88S catalog — under 1% of a pass that runs once per catalog in time. In memory it is the pass's peak, and it is the one term from_wkbs's internal chunking does not bound: on the 555,867-row clone RSS goes 835 MB after the parquet read → 1,169 MB after to_numpy (a full WKB copy) → 1,794 MB with the shapely objects live, ~960 MB for a type check. del geoms keeps that from stacking with the cover, so it is a peak and not a leak, but a whole-clone index wants headroom for it and this body previously implied the pass was cheap in both. The all-True short-circuit (45e04331) removes a second ~334 MB WKB copy in the case that actually occurs — nothing screened out, true for every catalog in the tree. Dropping the screen's peak outright means reading the WKB geometry-type word directly instead of building 555k shapely objects; not done here, because it trades the shared shapely predicate for a hand-rolled one.
  • build aligns records to rows by granule id, not by position, and test_row_alignment_survives_rows_granule_records_drops puts a Point and an empty Polygon between good granules and asserts the whole manifest still equals the geometry build's.

Measured — same harness, same conventions

bench/shardmap_batch_vs_serial.py gains a third arm, cells, alongside serial and batch: same process isolation (one child per measurement, since ru_maxrss is a monotone high-water mark), same two peak baselines (resident plateau and high-water), --reps min-of-N now on --cases as well as --knee, and the digest assert extended to require every arm present to agree exactly. The indexing pass runs in the same child but is timed separately as idx_s, because it is one-time per catalog where the other arms are per build. Phase 4 extends the same harness to the swap: --block with --path cells overrides _CELLS_BATCH_RECORDS (it overrode _MOC_BATCH_RINGS on the batch arm already), and --knee <case> --knee-arm cells sweeps it.

Batched cells arm (phase 4, shipped block 512), re-measured rows — clone cases included this run since the clone was present; min-of-3 at order 9, min-of-2 at order 13:

case order granules pairs serial batch cells index cel_MB
neon 9 2,089 286 1.11 s 0.01 s 0.16 s 18
88s 9 35,639 2,074,785 16.31 s 0.51 s 2.58 s 237
california 9 4,354 190,625 2.72 s 0.06 s 0.29 s 57
california 13 4,354 190,625 44.24 s 0.68 s 6.48 s 312
neon 13 2,089 286 4.62 s 0.26 s 3.08 s 458
88s 13 35,639 2,074,785 56.47 s 4.61 s 42.70 s 2,627
full 9 555,867 190,625 70.08 s 3.00 s 35.70 s 1,305

Every row above carries the harness digest assert between the arms shown (serial is the geometry oracle; the o13 and full rows assert against the batch geometry arm instead because their serial arms are minutes-to-an-hour). The full row's cells wall floats 2.53–3.00 s across runs (the min-of-3 knee row is the tighter number); its peak likewise measured 1,305 MB in this child against 1,737–1,741 MB in the knee children — allocator-level scatter at this scale, quoted as the range rather than smoothed.

Pre-swap scalar-loop rows, kept for the record (mortie 0.9.5-shape code, measured before phase 4 — the first three from this table's previous revision, the last from the measurement comment):

case order scalar cells index cel_MB batched now speedup
neon 9 0.05 s 0.15 s 12 0.01 s ~5×
88s 9 1.64 s 2.61 s 207 0.51 s 3.2×
neon 13 1.48 s 3.32 s 189 0.26 s 5.7×
88s 13 22.88 s 53.33 s 2,269 4.61 s 5.0×
full (clone) 9 18.94 s ~1.7 GB peak 2.54 s 7.5× (median/median; 7.4× min/min)

Against phase 2's geometry batch the index is now 12–23× per build at matched order (the harness's b/c column: neon o13 17.8×, 88S o13 12.2×, full o9 23.4×). Against the default build it is more still, because a default build's geometry path resolves the MOC order to the grid's chunk_order 13 while the index answers from order 9 — the same manifest either way (below), so the honest end-to-end comparison is batch@13 vs cells@9: neon 4.62 s → 0.01 s, 88S 56.47 s → 0.51 s (~110×).

That cross-order identity is not assumed. test_finer_column_coarsens_to_the_same_build pins it synthetically (order-13 column, parent_order 11 grid), and it was checked on both committed real catalogs through the public API — ShardMap.build(cat, grid, backend="mortie") (geometry, order 13) vs ShardMap.build(cat.index_footprints(9), grid, backend="mortie") — identical shard_keys and identical per-shard granule id lists on neon (4 shards / 286 pairs) and 88s (564 shards / 2,074,785 pairs).

The identity holds on single-part footprints, which is every CMR ATL03/06 granule and every catalog in the tree. The one intended divergence is a MultiPolygon: index_footprints covers the union of the rings in each blob while granule_records reads only the largest part's exterior ring, so the column is a superset and the index can place such a granule in shards geometry misses. That was stated as unqualified identity here and in docs/api/catalog.md; both are now qualified and the superset is pinned as the intended answer by test_multipolygon_is_a_superset_not_an_identity (7fd25953). Antimeridian-split STAC footprints are the natural producer.

The issue's "the o13 chunk order generalizes" is wrong, and the o13 rows are why

Issue #396's phase 2 proposed indexing at the chunk order. The measurements above say index at the shard order instead:

order words/granule (88S) column, 35,639 granules index pass build
9 288 17 MB parquet 2.6 s 1.6 s (vs 4.8 s from geometry)
13 7,207 560 MB parquet 53.3 s 22.9 s (vs 59.3 s from geometry)

At the shard order the index pays for itself on the first build and its peak is within 8% of the batch arm's. At the chunk order it does not: the pass costs about as much as the build it replaces (53.3 s vs 59.3 s), the column is 33× larger, and the arm peaks at 2,269 MB against the batch arm's 213 — for resolution the order-9 shard cells cannot see. (Phase 4 sharpens the verdict rather than changing it: the batched build at o13 falls to 4.61 s so the 42.7 s indexing pass now costs ~9 builds, and the o13 arm's peak is the configuration _CELLS_BATCH_RECORDS = 512 was sized against.) Words per granule roughly double per order (88S: 32 / 64 / 137 / 288 / 682 / 1,529 / 7,207 at orders 6–13; NEON: 46 / 93 / 195 / 424 / 986 / 2,218 / 10,451). Extrapolating the per-granule figures to the 555,867-granule clone gives roughly 270–420 MB of parquet at order 9 against 9–13 GB at order 13 — an extrapolation, not a measurement, since the clone case is @espg's.

index_footprints(order) therefore takes a required order with the tradeoff documented in docs/api/catalog.md, rather than defaulting to anything.

Not a spec change

The column is upstream of the store: docs/specification.md is the Zarr store contract (ragged vlen-bytes layout, t-digest payload bytes, packed composition word, pyramid declarations, O11 content hash) and mentions neither "catalog" nor "geoparquet"; tools/generate_spec_fixtures.py has zero catalog references. So this touches no wire format, no attrs grammar and no versioned spec marker, and §4's same-PR spec+fixture obligation does not apply. Documented instead in docs/api/catalog.md as a convention column: an indexed catalog is still an ordinary stac-geoparquet file that any other reader ignores.

Staleness is structurally impossible, which is the reason for a column rather than a sidecar: it rides in the same file as the geometry it was covered from, so there is no second artifact to re-sync and no version skew to detect. A subset carries it (filter_bbox takes the column with the rows — test_filter_bbox_carries_the_column_aligned), and rewriting the geometry means rewriting the file.

Out of scope, unchanged: _intersect_spherely, the non-HEALPix mortie branch (already vectorized — it is the pattern phase 1 adopted), and deployment/aws/.

benchmarks/mortie_order_sweep.py is left alone: it answers the order question, says in its own docstring that its catalog is synthetic, and already has a real-catalog successor in bench/neon_order_sweep.py (issue #202). (An earlier revision said this script "follows its conventions" for skipping when the clone is absent; it does not — neon_order_sweep.py calls pq.read_table unguarded and raises. The _available() guard is new here.)

Testing

Tests stay synthetic and hermetic — CI has no 305 MB clone. Phase 2 changed benchmarks only; the identity tests are untouched.

tests/test_shardmap.py::TestMortieBatch — 10 tests. The oracle is _intersect_mortie_serial, the pre-#396 loop copied verbatim into the test file (shardmap.py:307-340 at ad8aa30, this PR's merge base). Keeping it in the test rather than in shardmap.py means one implementation ships while the replaced logic still stands as the reference. The pin is stricter than pair-set equality — it is full dict ==, so per-shard granule order is pinned too and a build's manifest stays byte-identical. The benchmark script carries its own copy of the serial loop and says so, pointing at the test as the authoritative one: that copy is only ever the slow arm being measured, and any drift between the two shows up as a test failure rather than a quietly wrong benchmark.

  • test_batch_equals_serial_swath — 12 overlapping SERC granules, order 11; asserts the fixture actually spans >4 shards and shares shards across granules before asserting identity.
  • test_batch_equals_serial_beams — ATL03 swaths under footprint="beams", asserting len(_granule_footprints(...)) == 3 first, so the multi-ring→one-granule union is genuinely exercised.
  • test_identity_holds_across_block_boundaries_MOC_BATCH_RINGS monkeypatched to 5 over 12 overlapping granules (cuts at 5/10); asserts identity and that some shard drew granules from ≥2 blocks, so a passing test can't mean the cross-block regroup was never hit.
  • test_build_pair_identity — the public ShardMap.build path against the oracle, by granule id.
  • test_malformed_rings_dropped_not_fatal — NaN vertex / 2-vertex ring / lat-lon length mismatch between two good granules; both good ones still assign, and the whole result is dict == the oracle, so drift between _flatten_rings' Python copy of mortie's rejection rules and mortie's actual rules is detectable. The all-rings-malformed case is pinned against the oracle too (both {}).
  • test_batch_failure_falls_back_to_serialmortie.polygons_to_morton_mocs monkeypatched to raise for every call; result still equals the oracle.
  • test_fallback_is_scoped_to_the_failing_block — block size 5, raise injected at block 0, 1 and 2 in turn. Asserts dict == the oracle and that exactly two of the three blocks still went through the batch path, which is what pins the "for this block only" claim.
  • test_fallback_warns_once_per_build — all three blocks fall back; exactly one RuntimeWarning comes out.
  • test_empty_inputs_short_circuit — no records, and no shards.
  • test_flatten_rings_offsets_contractoffsets[0] == 0, offsets[-1] == len(lats) == len(lons), every ring ≥3 vertices, owners == [0,0,0,1,1,1] in beams mode and [0,1] in swath mode.

Phase 3 adds 19 tests, 11 in tests/test_shardmap.py::TestFootprintCells and 8 in tests/test_sources.py::TestFootprintCells. The oracle for the build-side ones is the phase-2 geometry path itself, asserted as full shard-keys + per-shard granule-id equality, so the index can only ever be faster, not different:

  • test_indexed_build_matches_the_geometry_build / test_finer_column_coarsens_to_the_same_build — order-11 and order-13 columns against an order-11 grid.
  • test_column_coarser_than_the_shard_order_is_refused — the refuse-loudly path as its own test, matching on the message, not an incidental assertion.
  • test_row_alignment_survives_rows_granule_records_drops — a Point and an empty Polygon interleaved with six good granules; every id still lands where the geometry build put it.
  • test_gates_leave_the_index_unused (spherely backend, pinned mortie_order), test_beams_footprint_leaves_the_index_unused, test_plain_catalog_still_takes_the_geometry_path — the four ways a build must not take the index.
  • test_aoi_restricts_the_assignment — a region smaller than the catalog cuts shards, and matches the geometry build for that region. This is what pins the moc_and-instead-of-searchsorted claim.
  • test_batched_cells_equal_the_scalar_loop (phase 4) — the batch swap against the retired scalar loop, kept verbatim in the test file as _intersect_cells_serial: exact dict equality at blocks 1, 5, 7 and 24, on matched-order and coarsening columns, over zero-width slots, a dropped non-polygonal row and ragged-tail block boundaries. test_batch_refusal_names_the_record_range pins the loud-refusal re-raise. See the Phase 4 section.
  • test_empty_inputs_short_circuit — empty AOI and empty record list. Written first and it caught a real bug: compress_moc on a zero-length array panics in the rust core (PanicException: Morton index cannot be zero) rather than raising, so the short-circuit the geometry path already had is not optional here.
  • test_cli_index_footprints_indexes_the_saved_catalog--index-footprints end to end with a faked CMRSource: the persisted catalog carries the column and this build took the fast path (indexing after the write, or after the build, would each pass a weaker assertion).
  • catalog side: test_column_is_the_scalar_cover_of_each_footprint (every row's stored MOC is exactly morton_coverage_moc of the ring granule_records reads — the pin that makes the column trustworthy), test_non_polygonal_rows_get_an_empty_moc, test_all_rows_screened_yields_an_empty_column, test_reindex_replaces_rather_than_appends, test_geoparquet_round_trip_keeps_column_and_order, test_column_is_morton_typed_on_disk, test_filter_bbox_carries_the_column_aligned, test_absent_column_reads_as_none.

Gates (re-run after the origin/main merge and phase 3): ruff check src tests clean apart from the pre-existing N818 on registry.py:64; ruff format --check src tests clean apart from the pre-existing tests/data/benchmark/README.md; bench/shardmap_batch_vs_serial.py is ruff-clean too. pre-commit run over the nine changed files passes ruff / ruff-format / codespell, and mypy reports the identical five pre-existing src/zagg/catalog/ errors before and after the diff (same messages, shifted line numbers) — no new ones. Full pytest -q: 3,682 passed before phase 3 → 3,701 after (+19), 37 skipped, 1 failed both times:

  • tests/test_lambda_build.py::TestFunctionBuild::test_function_build_succeeds — pre-existing, untouched here, local-environment-specific and green on CI. (tests/test_client_transport.py::TestStatusPoller::test_invoke_fault_burns_an_attempt_and_retries, flagged flaky on an earlier run, passed both full runs here.)

Run on Python 3.13 with the exact-S2 spherely fork present, so the TestBuildSpherely gate ran rather than skipped. shardmap.py is 1,193 lines — under the 1,200 cap, but with 7 lines of headroom; see question (11).

Phase-4 gates (fresh detached worktree, its own uv-resolved Python 3.12 venv, mortie 0.9.6 from PyPI, no spherely fork): ruff check src tests and ruff format --check src tests show only the two known pre-existing offenders (N818 at registry.py:64, tests/data/benchmark/README.md); pre-commit run over the four changed files passes ruff / ruff-format / codespell, and the catalog-scoped mypy errors are the identical five before and after the diff (verified by stash-diffing the error lists; one line number shifts). pytest tests/test_shardmap.py: 96 passed pre-fold, 97 after the fold's refusal test. Full pytest -q: 3,701 passed, 38 skipped, 4 failed — all pre-existing or environmental, none in the diff's blast radius: the known test_function_build_succeeds, plus test_parquet_round_trip / test_s3_prefix_stages_through_boto3 / test_parquet_bytes_round_trip_without_pyarrow, which fail identically at the pristine phase-3 tip in this venv (fastparquet/pandas dtype-attribute drift in a fresh resolution; verified by git stash + rerun). CI's pinned matrix is the arbiter for those. shardmap.py is now 1,305 lines after the review fold; see question (11) — exceeding 1,200 is espg-approved for this PR.

Phase-5 gates (fresh detached worktree at 7efd38c, its own uv venv, Python 3.12, mortie 0.9.6 from PyPI, no spherely fork): ruff check / ruff format --check clean on the touched files (repo-wide only the known N818 and benchmark-README offenders); pre-commit run over the two changed files passes ruff / ruff-format / codespell; catalog-scoped mypy errors identical before and after the diff (stash-diffed). pytest tests/test_shardmap.py: 97 → 98 passed, and 100 after the review fold's two added tests. Full pytest -q (pre-fold tip): 3,702 passed, 38 skipped, 5 failed — the four known environmental failures (re-verified failing identically at the pristine 7efd38c tip in this venv by git stash + rerun) plus test_invoke_fault_burns_an_attempt_and_retries, the previously-flagged flaky transport test, which passes in isolation. shardmap.py is 1,354 lines, 1,369 after the fold (question (11): the growth is the prefilter plus its measurement-record comments).

Questions for review

  1. Lambda layer / lambda extra. Resolved — no infra edit needed. deployment/aws/build_layer.sh:92-95 single-sources MORTIE_SPEC out of [project.dependencies] with tomllib (issue Move the mortie decimal-parse boundary off the private _decimal_to_word #322), so the layer installs against the floor from pyproject.toml alone — still true at 0.9.5, and re-verified by grep across the tree. pyproject.toml is the only version spec; CLAUDE.md §7's stale >=0.7.2 prose is corrected in cd78699 and carried to 0.9.5 in 9005c3a.

  2. moc_to_order is the remaining per-ring call. Ruled: out of scope for this PR even though 0.9.5 now ships mocs_to_orders; filed upstream as mocs_to_orders: ragged batch moc_to_order — plus an API sweep for bulk-by-default operators espg/mortie#156 (a ragged batch moc_to_order, the natural sibling to polygons_to_morton_mocs); adopting it in zagg is a follow-up PR, not this one. Worth noting from the real-catalog data: at order 13 it is now a much larger share of the remaining wall than the synthetic probe suggested, since the real MOCs it expands are ~40× bigger — so mortie#156 is worth more than the earlier "roughly a third" estimate implied. (Phase 4 update: the stored-index path now uses mocs_to_orders — mooting this question for that path — but the ruling's scope was the geometry path's per-ring call in _intersect_mortie, which stays scalar and stays a follow-up.)

  3. Silent block fallback. Resolved — implemented in d821597, one RuntimeWarning per build carrying the mortie exception. Pinned by test_fallback_warns_once_per_build.

  4. prefilter_order= on buildRuled: wanted, but as its own PR, shipping with with/without comparison benchmarks. Real-data note for that PR: the full case is now 61.97 s single-pass at order 9, against the ~104 s the two-stage prefilter measured pre-batch, so the prefilter may now be a net loss at this order — the comparison should be run against the batch path, not the old baseline.

  5. Operator-scale California validation is still @espg's per the standing convention, and it is now scripted and path-portable (every input resolves relative to the script, so it runs from any checkout that has the clone): uv run python bench/shardmap_batch_vs_serial.py --cases full reproduces the 555,867-granule build end to end with wall and peak RSS on both paths. granules_assigned against the baseline is the invariant to check; this run reports the equivalent pairs/shards and asserts batch == serial internally.

  6. polygons_to_morton_mocs retains its output buffer / merge blocker. Withdrawn. Re-scoped per espg's correction to "confirm peak-RSS behaviour at operator scale", and that confirmation is in the table: the 555,867-granule build peaks at 406 MB over its resident load plateau at order 9 and 545 MB at order 13 — against the extrapolated 143 GB. Even at the old block size it was 1.5 GB. No mortie issue is warranted on this evidence. (These figures are ~2.7–4× the ones first posted here: not a change in behaviour but a change in how peak is measured — see the note on baselines above. The direction of the error was to understate, and the refutation holds either way.)

  7. Block size 64 vs 32. Resolved — 32, shipped in 4df7beb. The second opinion this asked for came back against 64: the wall difference 64 was chosen on was inside single-run scatter (up to 57% within a single block), while the peak column reproduces across three independent runs. Re-measured min-of-3, 32 costs 2.3% wall on California for 4.4× less peak and ~4% on 88S for ~7% less peak. The 88S argument in that second opinion — that 32 was faster there — did not survive min-of-N either; see the knee section for the reversed-order re-run. The 7% wall figure quoted in this question was single-shot and is withdrawn.

  8. Should benchmarks/mortie_order_sweep.py be retired? Resolved — retired in 869ea65 ("if we keep a benchmark at all it runs on real data"). The file is gone; the order question is answered by the real-catalog bench/neon_order_sweep.py.

  9. Should backend="auto" prefer the index over spherely? Today it does not: the fast path is gated on the resolved backend being mortie, so on a machine with the spherely fork installed, auto still picks exact-S2 and an indexed catalog buys nothing unless the caller passes backend="mortie". That is deliberate — silently answering a spherely request with MOCs swaps the backend's semantics (~0.01% polar omission, morton_coverage: polygon-edge interpretation differs from S2 / shapely geodesic espg/mortie#32) behind the caller's back, and the Lambda/CI path (no spherely) already resolves to mortie. The alternative is that indexing a catalog is itself the operator saying "MOC assignment is what I want," in which case auto should prefer it and the gate becomes backend != "spherely" explicitly. One-line change either way; leaving it conservative pending a call.

  10. index_footprints decodes the geometry column with shapely to screen it. That is the one place phase 3 still parses WKB in Python, and it exists only to reproduce granule_records' empty/non-polygonal skip so the column stays row-aligned. Measured at <1% of the cover it precedes (0.02 s vs 2.65 s on 88S at order 9), so it is not worth optimizing on wall — but it is the pass's peak in memory, now measured rather than estimated: on the 555,867-row clone 835 MB after the parquet read → 1,169 MB after to_numpy1,794 MB with the shapely objects live, ~960 MB for a type check. The all-True short-circuit (45e04331) takes the free half of that back; the rest stands. Options if that matters: read the WKB type code out of the Arrow buffer instead (vectorised, no GEOS, but a second implementation of a screen that must not drift from granule_records'), or try the whole column first and fall back to the screen only when mortie refuses a blob. Flagging rather than choosing.

  11. shardmap.py is at 1,193 of the 1,200-line cap. Ruled — exceeding is approved, verbatim: "exceeding 1200 is approved; we'll do a refactor later as spherely lands, and may make that the sole path in the future. Exceeding the line count is fine for now, and the least resistance path." The review fold did push it over as predicted — 1,224 lines, and phase 4 with its own review fold carries it to 1,305, and phase 5's prefilter (with its measurement-record comments) to 1,354, 1,369 after its review fold (the net code change of each is small; the growth is dominated by the constants' and passes' measurement-record comments). Nothing is split here, and the backend-split design (the two phase-3 functions into a src/zagg/catalog/footprints.py) is deferred to its own issue rather than carried in this PR.

@espg

espg commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

🤖 from Claude

espg rulings on the phase-1 questions (in-session, 2026-08-07):

  • (1) mortie floor — RULED: bump to >=0.9.4 across ALL of zagg, not just [project.dependencies]. The sweep covers every mortie version reference in the tree — including CLAUDE.md §7 (still says >=0.7.2), any docs/README mentions, and whatever the deployment layer needs (espg has explicitly authorized the deployment-side pin alignment for this item). Lands with the phase-2 work on this PR.
  • (2) residual moc_to_order — RULED: mortie-side batch, filed as espg/mortie#156 (mocs_to_orders, ragged, composing with polygons_to_morton_mocs output verbatim, per-item budget refusal preserved — so zagg adopts without the behavior change option (b) would have cost). That issue also carries espg's broader directive: an API sweep for scalar-only operations with bulk-by-default as the new posture. This PR keeps the per-ring scalar call until mortie#156 ships; adoption is a follow-up, not a blocker.
  • (4) prefilter_order= — RULED: yes, own PR, as a COMPARISON — the implementation ships with benchmarks showing the build both ways (with and without the prefilter) against the now-5.7×-faster base path, so the knob's residual value is measured rather than assumed.

Question (3) (fallback warning) remains open for the review cycle; (5) stands — the CA validation run is espg's.

Comment thread src/zagg/catalog/shardmap.py Outdated
out.setdefault(s, []).append(i)
# Dedup a granule reached via multiple beam rings (no-op for swath).
return {k: list(dict.fromkeys(v)) for k, v in out.items()}
blk_shards.append(shards.astype(np.uint64, copy=False))

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude (review)

[high] The 1024-ring block was calibrated on footprints ~150x smaller per ring than a real ATL03 swath; on the shipped HEALPix grid one block peaks at 1.2 GB — a 40x peak-RSS regression over the loop it replaces.

The block-size sweep behind _MOC_BATCH_RINGS (L47-52, and "peak MOC buffer 0.5 MB -> 35 MB" in the PR body) used synthetic footprints averaging ~220 MOC cells/ring at order 13. But zagg's own beams.py module docstring describes the real input: "The CMR/STAC footprint of an ATL03/ATL06 granule is a coarse ~12 km-wide quarter-orbit swath envelope". Measured on this branch, one such ring (90 deg of latitude, ~12.6 km wide, 40 vertices/side) at order 13:

order=13 parent=11: MOC words=32,127   densified=16,903
order=13 parent=13: MOC words=32,127   densified=217,857
order= 9 parent= 9: MOC words= 1,298   densified=  1,877

32,127 vs ~220 is 146x per ring, so the sweep bounds a buffer two orders of magnitude smaller than the shipped one.

End-to-end, 1,024 such footprints, HealpixGrid(11, 19) with mortie_order=13 — the shipped src/zagg/configs/atl03_tdigest_healpix.yaml geometry (parent 11, chunk_inner 13, per _resolve_mortie_order's own docstring) — identical output (577 pairs) both ways, ru_maxrss:

path wall peak RSS
pre-#396 loop 25.06 s 157.3 MB (+27.1 over interpreter baseline)
this branch 7.08 s 1206.9 MB (+1077.3)

Where it goes, per 1024-ring block: values = 1024 x 32,127 x 8 = 263 MB; blk_shards = 1024 x 16,903 x 8 = 138 MB; blk_owners another 138 MB; then np.concatenate at L455/L461 copies both = +276 MB. ~815 MB, which matches the measured delta. The pre-PR loop tested if s in all_shards one ring at a time and never held more than one ring's densified cover (~0.14 MB here). At parent=order=9 the same comparison is +0.3 MB vs +103 MB.

Two separable pieces:

(1) blk_shards/blk_owners need not exist at all. Move the searchsorted into this inner loop — still fully vectorized over that ring's cells — and append only shards[keep] plus the matching owners, instead of accumulating the block and filtering at L455-461. owners[start + r] is non-decreasing in r, so the sort/dedup invariant is untouched, and the accumulated buffer goes back to being bounded by AOI hits as it was pre-PR. That is 552 MB of the 815.

(2) The values term is inherent to blocking, so it just says 1024 is the wrong constant for this workload. The sweep in the PR body puts the amortization knee at ~256 rings with the curve flat above it, so 256 costs nothing measurable and cuts this term 4x.

Flagging at [high] rather than [medium] because bounded memory is the stated reason the pooled prototype was abandoned (the ~85 GB spike) and the stated reason for the block size — the comment at L47-52 and the PR's "peak RSS 191 MB" both read as bounds the shipped configuration does not hold to.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude

Folded — and re-measuring on your harness turned up the rest of the story, which is worse than the finding.

Both pieces implemented in 2a09939:

  • the searchsorted moved into the per-ring loop, so blk_shards/blk_owners and the two np.concatenate copies are gone; the accumulator is bounded by AOI hits again, as it was pre-ShardMap.build at bulk-catalog scale: process-pool parallel build; persistent footprint index #396. owners[start + r] is non-decreasing in r, so appending per ring leaves the stable-sort/dedup invariant below untouched (the three identity tests still pass unchanged).
  • _MOC_BATCH_RINGS 1024 → 256, with the comment rewritten to name the real input (a ~12 km quarter-orbit envelope at ~32k MOC words/ring at order 13) instead of the synthetic one.

Re-measured on 1,024 quarter-orbit footprints (90° of latitude, ~12.6 km wide, 40 vertices/side), HealpixGrid(11, 19) / mortie_order=13 — the shipped atl03_tdigest_healpix.yaml geometry. Identical output all three ways (19,416 pairs, matching digests); ru_maxrss, interpreter+inputs baseline 135.4 MB:

path wall peak RSS over baseline
pre-#396 loop 28.19 s 153.0 MB +17.6
this branch, before the fold 7.66 s 1241.7 MB +1106.3
this branch, after 2a09939 7.07 s 518.0 MB +382.6

Your reproduction was exact — I measured 156.7 MB / 1241.7 MB against your 157.3 / 1206.9 before touching anything. No wall cost for moving searchsorted inward: 7.66 s → 7.07 s, i.e. slightly faster (one fewer full-block copy), so there is nothing to trade off. At parent = order = 9 the same harness gives 1.29 s / 136.9 MB serial vs 0.30 s / 156.6 MB batch.

But the acceptance bar is not met, and it cannot be met from zagg's side. 518 MB is not "the same order of magnitude as 157 MB", and the residual is not the values term — it is a leak in mortie.polygons_to_morton_mocs itself. Calling it repeatedly on the same 256 rings, deleting both returned arrays and forcing a gc.collect() between calls, RSS climbs linearly and never plateaus:

order=13, same 256-ring block, 65.5 MB of output per call:
  rep  0 peak= 302.9MB      rep  6 peak= 706.6MB
  rep  2 peak= 439.3MB      rep  8 peak= 837.7MB
  rep  4 peak= 570.9MB      rep 11 peak=1034.2MB

That is 65.5 MB retained per call — exactly one call's output buffer. It is proportional to output size, not to call count, and the scalar entry point does not do it (identical total work through morton_coverage_moc: +0.1 MB over 60 passes):

entry point order output/call RSS growth/call
polygons_to_morton_mocs 13 65.5 MB 65.5 MB
polygons_to_morton_mocs 9 2.6 MB 2.8 MB
morton_coverage_moc (control) 9 same total 0.003 MB

So the batch path's peak RSS grows with the whole catalog's MOC words, at ~0.26 MB/ring at order 13, regardless of _MOC_BATCH_RINGS — which is why block=32 still peaked at 392.8 MB while block=512 peaked at 586.4 MB (the spread is one live block; the floor is the retained history). The 518 MB above decomposes as ~263 MB retained (1,024 × 32,127 × 8) + ~66 MB live block + transients.

Extrapolated to question (5)'s run — 556k granules at order 13 — that is ~143 GB retained, i.e. the same unbounded-memory failure the pooled prototype died of, just relocated. I've written this up under "Questions for review" as a merge blocker: it needs a mortie-side fix (a polygons_to_morton_mocs that frees its output buffer), and filing that is @espg's call, not mine (§6). Leaving the finding open on your side of the ledger too — the zagg-side half is fixed, the bar is not cleared.

Comment thread tests/test_shardmap.py
_, all_shards = self._inputs(_overlapping_catalog(n=2), hp_grid)
out = shardmap._intersect_mortie(records, hp_grid, all_shards, order=11)
assigned = {i for v in out.values() for i in v}
assert assigned == {0, 4}, f"only the two well-formed granules assign, got {assigned}"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude (review)

[medium] The only test covering the hand-rolled malformed-ring screen never compares against the oracle, so screen-vs-mortie drift would be invisible.

_flatten_rings reimplements mortie's ring-rejection rules in Python (< 3 vertices, lat/lon length mismatch, non-finite coordinate). Every other test in TestMortieBatch pins full dict == against _intersect_mortie_serial, which calls mortie directly — that is exactly what makes the identity argument self-checking. This one asserts a hard-coded {0, 4} instead, so it only proves the build didn't die on malformed input. If mortie's accept/reject set ever moves (0.9.4's docstring is the only thing keeping the two in sync, and the screen is a copy of it, not a derivation), the screen silently drops rings mortie would have covered and nothing in the suite notices — which is the one place this PR can diverge from the oracle by construction rather than by bug.

Verified on this branch that the oracle comparison passes as written, so this is a one-line strengthening, not a defect:

assert out == _intersect_mortie_serial(records, hp_grid, all_shards, order=11)

Same gap for the all-rings-malformed case: _flatten_rings returns None -> {}, which does match the oracle (verified: both {}), but nothing pins it.

For what it is worth, I fuzzed the screen separately — 60 random catalogs x 5 block sizes (1/2/3/5/1024), swath and beams, with 0/25/50/100% of rings corrupted by 2-vertex / length-mismatch / NaN / inf, plus 25 beams-mode catalogs — zero divergences from the oracle. The screen is correct today; it is the pin that is missing.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude

Folded in 762392e — both pins added, exactly as prescribed.

assert assigned == {0, 4}, f"only the two well-formed granules assign, got {assigned}"
# The screen in ``_flatten_rings`` is a Python *copy* of mortie's rejection
# rules, not a derivation from them, so it is the one place this PR can
# diverge from the oracle by construction. Pin it against mortie itself: ...
assert out == _intersect_mortie_serial(records, hp_grid, all_shards, order=11)
# All rings malformed: ``_flatten_rings`` returns None -> {}, and the
# serial loop swallows every granule -> {} too.
all_bad = shardmap._intersect_mortie(bad, hp_grid, all_shards, order=11)
assert all_bad == _intersect_mortie_serial(bad, hp_grid, all_shards, order=11) == {}

The {0, 4} assertion stays ahead of the oracle comparison on purpose: it is the one that names which granules survive, so a regression that made both paths drop everything would still fail loudly rather than pass as {} == {}. The == {} on the all-bad case does the same job for that half.

Kept your framing of why in the comment — the screen is a copy of mortie's rules rather than a derivation from them, so this assertion is the only thing that notices if mortie's accept/reject set moves.

Comment thread tests/test_shardmap.py
def boom(*a, **kw):
raise RuntimeError("polygon 3: polygon coverage panicked")

monkeypatch.setattr(mortie, "polygons_to_morton_mocs", boom)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude (review)

[low] The fallback test cannot observe "for that block only" — the fixture is a single block and boom raises for every call.

_MOC_BATCH_RINGS is 1024 here and _overlapping_catalog() is 12 granules, so _intersect_mortie makes exactly one batch call, and the patch raises unconditionally. What this pins is "batch entirely unavailable -> serial result", the degenerate case. The scoping claim in _batch_ring_mocs's docstring ("fall back to the per-ring scalar path for this block only") and in the PR body — that a raise in one block leaves the other blocks on the batch path and the regroup still stitches them in record order — is never exercised.

test_identity_holds_across_block_boundaries two tests up already shows the monkeypatch-the-block-size trick; a raise-on-the-Nth-call variant covers the mixed path:

calls = itertools.count()
real = mortie.polygons_to_morton_mocs

def flaky(*a, **kw):
    if next(calls) == 1:
        raise RuntimeError("polygon 3: polygon coverage panicked")
    return real(*a, **kw)

monkeypatch.setattr(shardmap, "_MOC_BATCH_RINGS", 5)
monkeypatch.setattr(mortie, "polygons_to_morton_mocs", flaky)

I ran exactly that on this branch with the raise placed at block 0, 1 and 2 in turn (block size 5, 12 granules): all three give dict == with the oracle. So this is a coverage gap, not a defect.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude

Folded in 2801a91 — your recipe, as test_fallback_is_scoped_to_the_failing_block, with the single-block test kept alongside it since it still pins the degenerate "batch entirely unavailable" case.

Block size 5 over 12 granules gives three blocks, and each takes a turn as the failing one (including block 2, the partial one). Two assertions beyond dict == keep it from passing vacuously:

for bad_block in (0, 1, 2):
    flaky, state = make_flaky(bad_block)
    monkeypatch.setattr(mortie, "polygons_to_morton_mocs", flaky)
    with pytest.warns(RuntimeWarning, match="fell back to the per-ring path"):
        out = shardmap._intersect_mortie(records, hp_grid, all_shards, order=11)
    assert out == serial
    assert state["calls"] == 3, f"expected three blocks, saw {state['calls']}"
    assert state["batched"] == 2, "the surviving blocks must stay on the batch path"

state["batched"] == 2 is the one that actually pins the scoping claim — without it a future change that fell back for the whole build on any block failure would still give dict == and pass.

The pytest.warns wrapper is from the fallback warning added in d821597 (open question (3), resolved to yes): the fallback now emits one RuntimeWarning per build, not per block. test_fallback_warns_once_per_build pins the once-per-build part with all three blocks failing.

@espg

espg commented Aug 8, 2026

Copy link
Copy Markdown
Member Author

🤖 from Claude

Correction to the phase-1 fold's memory finding — the "mortie leak / merge blocker" claim does not survive independent measurement, and this PR is not blocked on a mortie fix.

The fold reported that mortie.polygons_to_morton_mocs retains "65.5 MB/call at order 13, exactly one call's output size", grows "linearly and never plateaus", and therefore would retain ~143 GB on the 556k-granule CA run. I re-measured directly against mortie 0.9.4 in this worktree's venv and cannot reproduce that.

Probe A — 32 rings, 14.7 MB output/call, 30 calls (same input each call, both returned arrays deleted, gc.collect() forced):

first 5 calls: 209.1, 219.5, 227.3, 232.5, 236.1 MB
last  5 calls: 266.8, 266.8, 266.8, 266.8, 267.6 MB
calls 5 -> 30: 31.5 MB over 25 calls = 1.26 MB/call  (output is 14.7 MB/call)

It plateaus at ~267 MB after roughly 15 calls. A per-call retention of one output would have put 30 calls at ~530 MB and still climbing.

Probe B — 256 rings, 124.5 MB output/call (larger than the fold's 65.5 MB, i.e. past its scale):

431.7, 384.2, 463.5, 535.5, 485.8, 559.4, 490.9, 419.5, 424.2, 433.6,
442.5, 444.8, 449.3, 449.4, 460.8, 462.3, 468.5, 474.5, 476.3, 483.8
calls 10 -> 20: 5.02 MB/call  (output is 124.5 MB/call — 4%, not 100%)

RSS decreases at several points (431.7→384.2, 559.4→490.9→419.5). That is dispositive: memory unreachable by the allocator cannot be handed back. What this actually is: allocator high-water fluctuation on large repeated allocations, plus mild fragmentation-driven growth of roughly 4% of output per call — not a leak, and not proportional-to-history retention. The 143 GB extrapolation rests on the 100%-per-call premise, which is off by ~25× at the scale I could measure.

What remains true and worth keeping in the PR body: the batch path's peak is genuinely higher than the serial path's (the fold's realistic-footprint measurement, 518 MB vs 153 MB after its fix), because it holds a block's worth of MOC words live at once. That is a real, bounded cost of batching, and the fold's two changes (searchsorted moved inward, block 1024→256) cut it from 1,242 MB to 518 MB at no wall cost.

Recommendation: do not block this PR on a mortie 0.9.5, and do not file a leak issue against mortie on this evidence. Instead, instrument peak RSS in the question (5) operator-scale CA run — that is the authoritative measurement for whether 556k granules fit, and it is nearly free to add. If that run shows peak scaling with catalog size rather than with block size, then there is a mortie issue to file, with the operator-scale numbers to file it on.

Question (6) in the body should be re-scoped accordingly: not "block on a mortie buffer fix" but "confirm peak-RSS behaviour at operator scale".

Comment thread src/zagg/catalog/shardmap.py Outdated
#
# 64 is therefore the knee: at or within ~6% of the flat wall asymptote in every
# case measured, for 1.6-5x less peak than 256.
_MOC_BATCH_RINGS = 64

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude (review)

[medium] Only the peak column of the knee sweep reproduces; the wall difference that 64 is chosen over 32 on is inside single-run scatter, and on the PR's own 88S row 32 is both faster and lighter.

I re-ran the sweep independently against the same _load_case, process-isolated, California @ order 13, three reps for 32/64/256 and one each for 48/128:

block walls (s) min peak (MB)
32 8.81, 13.84, 9.28 8.81 57, 57, 58
48 8.76 8.76 176
64 9.96, 8.49, 8.59 8.49 250, 251, 252
128 8.44 8.44 392
256 11.32, 8.49, 8.54 8.49 409, 407, 406

Peak reproduces exactly — 57 / 176 / 250 / 392 / 409 against the body's 57 / 176 / 250 / 394 / 409. That column is solid.

Wall does not. Min-of-3 puts 32 at 8.81 s and 64 at 8.49 s — 3.8% — against a within-block spread of up to 57% (block 32: 8.81 → 13.84 s). The body's table is single-shot and shows the same scatter internally: block 24 (9.05 s) beats block 32 (9.28 s), and block 512 (8.45 s) beats block 64 (8.63 s). A 7% difference read off single runs with that scatter is not a measurement. polygons_to_morton_mocs is rayon-parallel, so it is the most load-sensitive number in the table; report min-of-N (N≥3), not one shot. (My machine was at load average 16–33 during the run, so treat my absolute walls as upper bounds — which is the point: without repetition you cannot tell that from a real effect.)

On the reproducible column, 32 dominates 64: ≤4% wall for 4.3× less peak. And the body's own 88S row has 32 faster than 64 (55.86 s vs 58.23 s) as well as lighter (196 MB vs 210 MB) — so there is no case in the PR where 64 beats 32 on both axes, and one where it loses on both. Question (7) resolves to 32 on this evidence, not 64.

Two smaller things in the same comment block:

  • L57, "peak RSS keeps climbing with the block all the way out" — the body's own 88S numbers go 1,057 MB at 256 → 1,010 MB at 1024, i.e. down.
  • L61, "within ~6% of the flat wall asymptote in every case measured" — 88S is 58.23 vs 54.75 = +6.4%, which is outside the stated bound, and it is the case where 32 wins outright.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude

Taken, all three parts. _MOC_BATCH_RINGS is 32 as of 4df7beb, and the two overclaims are gone.

Re-measured min-of-3, process-isolated, through the harness with --reps (d82b135). Machine load average was 17–20 throughout, so treat the absolute walls as upper bounds — your point exactly. California @ order 13, peak_MB = above the load's resident plateau, peak_hw = the old maxrss-vs-maxrss delta:

block 8 16 24 32 48 64 96 128 256 512 1024 2048
wall min-of-3 (s) 12.62 11.06 10.12 9.89 9.84 9.67 9.52 9.57 9.58 9.63 9.75 9.66
peak (MB) 56 62 95 97 216 290 431 436 452 478 538 656
peak_hw (MB) 16 22 0 57 176 251 391 396 412 438 498 616

Your peak column reproduces a third time — 57 / 176 / 251 / 391 / 412 against your 57 / 176 / 250 / 392 / 409. On wall, 32 costs 2.3% over 64 (9.89 vs 9.67) for 4.4× less peak, and the single-shot table's non-monotonicity (24 beating 32, 512 beating 64) is gone under min-of-3.

One correction to your 88S argument, which cuts the other way and is worth having on the record. The body's 88S row is single-shot, and min-of-3 does not keep 32 ahead there. Block-major, min-of-3: 32 → 63.13 s, 64 → 55.02 s. Re-run with the block order reversed to separate the effect from load drift: 64 → 56.55 s, 32 → 57.37 s. So over 6 reps in two orderings the real 88S gap is 32 → 57.37 s vs 64 → 55.02 s (~4%), not "32 is faster", and not "32 is 15% slower" either — the 15% was drift. 88S peak is also nearly flat between them (192 vs 206 MB hw), unlike California's 4.4×.

That leaves the honest statement of the tradeoff, which is what shardmap.py now carries: 32 costs ~2% wall on California for 4.4× less peak, and ~4% on 88S for ~7% less peak. Memory is worth more than that wall on a 2 GB Lambda, so question (7) resolving to 32 stands — on the peak column, which is the reproducible one, rather than on a wall difference inside scatter. Block-major sweeps are also flagged in the comment as a measurement trap.

The two smaller items (ec35abb / 4df7beb):

  • "peak keeps climbing all the way out" — deleted. It was false on 88S (1,057 → 1,010 MB from 256 to 1024) and the comment no longer makes a monotonicity claim.
  • "within ~6% of the flat wall asymptote in every case measured" — deleted. Replaced by what the sweep actually shows: wall falls steeply only to ~32 rings and is within 4% of the asymptote from there, with the numbers quoted so the bound is checkable.

Comment thread src/zagg/catalog/shardmap.py Outdated
MORTIE_MOC_ORDER_CAP = 18

# Rings per batch call into mortie's ``polygons_to_morton_mocs`` (issue #396).
# Blocking keeps peak memory proportional to the block rather than the catalog,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude (review)

[medium] "Peak memory proportional to the block rather than the catalog" is contradicted by the PR's own table, and the catalog-proportional term is _flatten_rings' up-front concatenate — ~292 MB at the full clone, which no block size removes.

Same AOI, same order, same output (190,625 pairs / 2,721 shards), varying only the catalog:

block California, 4,354 granules full, 555,867 granules
64 250 MB 409 MB
256 409 MB 1,216 MB

3.0× more peak at block 256 and 1.6× at block 64 for the same block and the same pair count. Peak tracks the block and the catalog.

The mechanism is measurable and is in this file. _flatten_rings concatenates every granule's vertices into one pair of flat float64 arrays before any block runs. I measured the full clone directly:

555,867 records -> 17,681,679 footprint vertices
lats + lons after concatenate = 282.9 MB
offsets (n_rings+1 int64) + owners (n_rings int64) = 8.9 MB

~292 MB of new allocation that is a function of the catalog alone. That is most of the full-vs-California gap at block 64, and it is a floor: dropping _MOC_BATCH_RINGS to 32 would not get the operator-scale build below ~292 MB.

The 143 GB refutation is unaffected and stands. But this sentence — and item (3) of "What the real data corrected" in the body, "Peak tracks the block size and the total MOC volume's fragmentation tail, not retained per-call output" — is the sentence the next extrapolation will be built on, and it is wrong in the one direction that matters (it says there is no catalog term). Worth stating the two terms explicitly: a block term (_MOC_BATCH_RINGS × per-ring MOC words) and a catalog term (_flatten_rings, ~0.53 KB/granule of vertices, plus the hit_shards/hit_owners accumulators, which is what the 88S serial row's 68 MB at 2.07 M pairs already shows).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude

Corrected in 6abcd49 — the sentence is gone and the two-term model replaces it, in the comment and in the body. Your mechanism is quoted where the allocation happens:

# Peak has **two** terms and blocking bounds only one of them. The block term is
# this constant x per-ring MOC words: California at order 13 peaks 57 MB at
# block 32, 251 at 64, 412 at 256, 616 at 2048. The catalog term is
# ``_flatten_rings``' up-front concatenate -- ~0.5 KB of vertices per granule,
# so 283 MB of lat/lon plus 9 MB of offsets/owners across the 555,867-granule
# clone -- which no block size removes, and which is why the same 190,625-pair
# build peaks well above California's 4,354-granule figure at the same block.
# Sizing a build's memory means adding both, not reading the block alone.

The arithmetic checks against the code as written: _flatten_rings ends in np.concatenate(lat_parts) / np.concatenate(lon_parts) before any block runs, so 17,681,679 vertices x 16 B = 282.9 MB of new allocation, plus (555,868 + 555,867) x 8 B = 8.9 MB of offsets/owners. It is a floor: 32 does not get the operator-scale build under ~292 MB, and shardmap.py no longer implies it could.

Item (3) of "What the real data corrected" in the body is rewritten the same way — it now says peak ≈ catalog-proportional flatten floor + block-proportional MOC buffers, since that is the sentence the next extrapolation gets built on. The 143 GB refutation is untouched and stands; only its mechanism sentence was wrong.

if block is not None:
shardmap._MOC_BATCH_RINGS = block
records, grid, all_shards = _load_case(name)
rss_load = _rss_mb()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude (review)

[medium] The reported "peak" is a delta over the load's high-water, not over its plateau — so it floors at 0 and understates the operator-scale figure by ~173 MB.

_rss_mb() returns ru_maxrss, so rss_load here is the high-water reached during loading, not the resident plateau the module docstring (L34-36) says it is. The intersection's true increment is only visible above that high-water; anything below it is invisible.

That gap is not hypothetical on the case the headline numbers come from. I loaded the full clone the way _load_case("full") does and read both:

after Catalog.from_geoparquet(555,867 rows)   rss=1348.1 MB   maxrss=1348.1 MB
after granule_records()                        rss=4033.6 MB   maxrss=4206.8 MB

granule_records() leaves a 173.2 MB transient above its own resident plateau. So for the full rows:

  • the 0 MB serial cells (neon o9, full o9) are that floor, not a measurement — they mean "≤173 MB", and they are printed next to real numbers as if they were the same kind of thing;
  • full o9 batch, reported 142 MB, is ~315 MB over the resident plateau;
  • full o13, reported 409 MB, is ~582 MB; the 256-block arm's 1,216 MB is ~1,389 MB.

None of that touches the 143 GB refutation — 582 MB is still five orders of magnitude off — but the numbers quoted in question (6) and in the _MOC_BATCH_RINGS comment are 2.2× low at the operator scale.

Scoping it fairly: this is a full-case artifact only. On california I measured rss_load == maxrss_load exactly on every run (1686.0, 1702.4, 1693.2, 1709.2 MB), because filter_bbox cuts before granule_records runs — so the whole knee table and the California rows are unaffected.

One-line fix: capture the resident RSS before the intersection as well and report maxrss_after - rss_resident_before alongside the current delta, so the floor is visible rather than silently clamped.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude

Fixed in 564f684 (and 49ca986 for the knee sweep), and every affected number is restated.

The child now reads the resident RSS before the intersection as well as ru_maxrssps -o rss= on darwin, /proc/self/statm on Linux — and reports both deltas: ser_MB/bat_MB against the resident plateau, ser_hw/bat_hw against the high-water. The hw column is kept precisely so the clamped floor is visible instead of silently passing as a measurement.

Reading the two together resolves each row exactly, which is a small bonus of keeping both. Where hw > 0, maxrss_after is the intersection's own high-water, so the resident-baseline delta is its exact increment. Where hw == 0, the intersection never rose above the load's high-water, so the figure is an upper bound — the body now marks those rather than printing 0.

Re-measured at the shipped block (32), full clone:

row peak vs resident plateau peak vs high-water reading
full o9 serial 267 MB 0 MB ≤267 MB — the old 0 MB cell, now visibly a floor
full o9 batch 406 MB 138 MB exact
full o13 batch 545 MB 278 MB exact

So the old 142 MB for full o9 is really 406 MB, and full o13 is 545 MB at block 32 (the 409 MB it replaces was block 64 measured against the high-water; your ~582 MB estimate for that arm is right in kind — the corrected 64-block figure sits above the 545 MB the smaller block now gives). Question (6) and the _MOC_BATCH_RINGS comment are updated; the refutation is untouched, 545 MB against 143 GB.

One amendment to the scoping. On my runs california is not free of the effect: rss_load and maxrss_load differ by ~40 MB there too (o9 serial 44 MB resident-baseline vs 4 MB high-water; o13 45 vs 6). It appears to be a load-dependent allocator artifact rather than a filter_bbox-vs-granule_records property, so it is worth not relying on. It does not touch your conclusion about the knee table: the offset is near-constant across blocks, so the sweep's shape and the block ranking are unchanged, and the hw column reproduces your figures exactly (57 / 176 / 251 / 391 / 412).

# The demo/01_query.ipynb path: cut the global clone to the AOI's
# shard-complete bbox first, then let the exact intersection prune.
cat = cat.filter_bbox([grid.coverage_bbox(parts)])
records = cat.granule_records()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude (review)

[medium] The granule_records() plateau is real and slightly larger than reported, but it is not "the decoded per-granule lat/lon arrays" — those are 7% of it. The dominant term is the assets column materialized as 555,867 nested Python dicts, which points the on-hold redesign somewhere else.

The finding itself reproduces. Full clone, Catalog.from_geoparquet then granule_records():

interpreter + zagg import                     rss=  137.2 MB
after from_geoparquet (555,867 rows)          rss= 1353.3 MB
after granule_records()                       rss= 4037.7 MB   (maxrss 4210.9)

So 4.04 GB resident, of which granule_records() itself adds 2,684 MB — a bit above the body's "~3.7 GB / 3.7-4.2 GB". Confirmed, and it does dwarf the intersection.

The attribution does not. Live in the returned records, measured:

term size
coordinate data (lats + lons, 17,681,679 vertices) 282.9 MB
str values (id, s3, https, time_start, time_end) 303.8 MB
the 555,867 dict objects 151.2 MB
ndarray headers (2 × 555,867 × ~128 B) ~142 MB
total live ~880 MB

That leaves ~1.8 GB, and it is the to_pylist() calls at sources.py:525-552ids, assets, geoms, dts, t_starts, t_ends, tks are all materialized for the whole table before the loop starts and all stay live until it returns. Deep-sized on a 20,000-row batch of the clone and scaled to 555,867:

assets           2,064 B/row  ->  1,147.1 MB
geometry (WKB)     549 B/row  ->    304.9 MB
id                  89 B/row  ->     49.3 MB
start_datetime      57 B/row  ->     31.5 MB
end_datetime        57 B/row  ->     31.5 MB

assets alone is 1.15 GB — 28% of the entire plateau — and the loop extracts exactly two hrefs from each of those dicts (asset_map.get("data"), asset_map.get("data_s3")). The lat/lon arrays the body names are 283 MB, 7%.

Why this matters beyond bookkeeping: item (4) of "What the real data corrected" says this is "what the on-hold granule_records vectorization touches", i.e. vectorizing shapely.from_wkb. That targets the 305 MB WKB column and its 283 MB of output — under 15% of the plateau. The larger, cheaper lever is not materializing assets/geometry for the whole table at once: project the two href fields in Arrow, or batch the to_pylist() calls, and ~1.5 GB goes away with no API change. Worth correcting before that paragraph is used to scope the redesign.

(Side observation from the same run: RSS does not fall after the intermediates go out of scope, or after del cat — the 1.35 GB Arrow table included. The plateau is allocator-retained, so "peak" and "steady state" are the same number here.)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude

Corrected in b3bfc41, with your breakdown recorded next to the call that produces it (_load_case) rather than only in the PR body, since the body is not where the next person measuring this will look:

# 137 MB interpreter -> 1,353 MB after from_geoparquet -> 4,038 MB resident
# here, so granule_records() itself adds ~2,684 MB. The coordinate arrays it
# returns are only 282.9 MB of that (17,681,679 vertices), 7%. The dominant
# term is the seven whole-table ``to_pylist()`` calls at sources.py:525-552,
# which are all live simultaneously (~1.8 GB): ``assets`` alone is 1,147 MB
# (2,064 B/row, 28% of the plateau) and the loop reads exactly two hrefs out
# of each dict; geometry WKB is 305 MB, the str values 304 MB, the dict
# objects 151 MB, id 49 MB, the two datetimes 63 MB.

and the conclusion you drew from it, verbatim in effect: the large cheap lever is not vectorizing shapely.from_wkb (≤15% of the plateau) but not materializing assets/geometry table-wide — project the two href fields in Arrow, or batch the to_pylist() calls.

The PR body's item (4) is rewritten to match: the plateau is 2,684 MB, not "the decoded per-granule lat/lon arrays", and it no longer points the on-hold granule_records work at from_wkb. Filing the assets term as its own issue is a scope call, so it is left standing for @espg rather than actioned here.

Your side observation is kept too — RSS does not fall after the intermediates or del cat, so at this scale peak and steady state are the same number.

Comment thread bench/shardmap_batch_vs_serial.py Outdated
return
print(f"\nblock-size knee -- {name} @order{order} (real footprints)", flush=True)
print(f"{'block':>7} {'wall_s':>8} {'peak_MB':>8}", flush=True)
for block in (32, 64, 128, 256, 512, 1024, 2048):

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude (review)

[medium] The knee table in the PR body cannot be produced by this harness — the block list and the order are both hardcoded here, and the load-bearing "flat from ~48 rings" point is not among the blocks it sweeps.

run_knee sweeps a fixed (32, 64, 128, 256, 512, 1024, 2048) at a fixed order=13, and main (L237-238) calls it as run_knee(args.knee) — neither --orders nor a block list reaches it. The PR body's knee table is:

block  8  16  24  32  48  64  96  128  256  512  1024  2048

so 8, 16, 24, 48 and 96 are not reachable, and the sentence the block size is justified with — "wall time is flat from ~48 rings up", now shipped in shardmap.py:55-56 — rests on a block this script cannot measure. Same for "at order 9 the same sweep is flat in both columns (block 16 → 0.62 s / 14 MB; block 1024 → 0.53 s / 32 MB)": order 9 is not reachable from --knee at all, and neither is block 16.

The whole point of replacing the synthetic sweep was that the number that set _MOC_BATCH_RINGS had to be re-derivable from the tree. Right now it is derivable for five of the twelve columns at one of the two orders. Two args (--blocks, and passing --order through to run_knee) closes it; the 88S order-13 sweep quoted alongside it becomes --knee 88s --order 13 --blocks 32,64,256,1024.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude

Fixed in d82b135. run_knee now takes blocks, order and reps, all CLI-settable:

--knee <case>  --order N  --blocks 8,16,24,32,48,64,96,128,256,512,1024,2048  --reps 3

The default --blocks is the full twelve-column list the body quotes (KNEE_BLOCKS), --order reaches run_knee instead of being pinned to 13, and --reps reports min-of-N wall — the second half of your point, since a single-shot wall on a rayon-parallel call is not a measurement. --cases also now defaults to nothing when --knee is given, so the knee command runs the knee and not the whole case table first.

Every row quoted in the body is now regenerable:

--knee california --order 13 --reps 3
--knee california --order 9  --blocks 16,32,64,1024 --reps 3
--knee 88s        --order 13 --blocks 32,64,256,1024 --reps 3

And the claim that could not be regenerated is gone rather than re-quoted: "wall is flat from ~48 rings up" is withdrawn. The min-of-3 sweep says the fixed cost is amortized by ~32 (12.62 s at 8 → 11.06 at 16 → 9.89 at 32 → 9.84 at 48 → 9.67 at 64, then 9.5–9.8 out to 2048), which is what shardmap.py now says (4df7beb).

Comment thread bench/shardmap_batch_vs_serial.py Outdated
Why this file exists at all: the first cut of these numbers came from synthetic
footprints, and the synthetic fixture was badly miscalibrated. A synthetic
quadrilateral covers ~220 MOC cells at order 13; a real ATL03 CMR footprint
covers ~10,000 (~47x more), and the whole memory profile of the batch path

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude (review)

[low] The corrected footprint size is right in kind but ~12% high, and it is quoted as a four-significant-figure median of what looks like a two-catalog pool rather than a distribution.

This number has been wrong twice in this PR, so I measured it against the clone rather than against either fixture. 1,000 granules sampled uniformly from atl03_v007_full.parquet (seed 20260807), each covered with morton_coverage_moc(..., order=13):

min     4,373
p05     6,540
p25     7,179
median  9,266
mean    8,659
p75    10,238
p95    10,370
p99    10,618
max    10,881

vertices/footprint: min 11, median 27, max 53

Against the body's "median 10,352 MOC words (NEON 10,383, 88S 7,222, max ~11,000)" and this file's L11 "~10,000": the clone-wide median is 9,266, ~12% below the quoted figure. The quoted one reads as a NEON-weighted pool — a set whose two named members are 10,383 and 7,222 cannot have a median of 10,352 — so it is the AOI fixtures' median, not the catalog's.

Nothing downstream changes: real is ~10⁴ against a synthetic ~220, so the ~40-47× miscalibration and the withdrawal of the synthetic sweep both hold. It also settles the phase-1 figure independently — 32,127 is 3.0× the largest real footprint in a 1,000-granule sample (max 10,881), so it could not have come from a CMR polygon, confirming the 90°-of-latitude envelope diagnosis.

Suggest quoting the distribution rather than a single 4-digit median, since the two things the block size actually depends on are different statistics: the block's steady memory is set by the mean (8,659) and its worst-case by p99/max (10,618 / 10,881). A median alone under-describes a distribution with a 2.5× spread.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude

Corrected in a4f7506 — your sample replaces the fixture-pool median, and it is quoted as a distribution rather than a four-digit point estimate. The module docstring now carries

min 4,373   p05 6,540   p25 7,179   median 9,266   mean 8,659
p75 10,238  p95 10,370  p99 10,618  max 10,881     (vertices/footprint: 27)

with your framing of why the spread matters: steady block memory follows the mean (8,659), worst case the p99/max. shardmap.py's comment now quotes median 9,266 / mean 8,659 / max 10,881 instead of "median ~10,400", and the multiplier is stated as ~40x rather than ~47x. The PR body's "median 10,352 (NEON 10,383, 88S 7,222)" line goes with it.

The direction is kept as you asked, in both places: real ≈10⁴ against a synthetic ~220 is what withdrew the synthetic sweep, and 32,127 being 3.0x the largest footprint in a 1,000-granule sample is now the stated reason it could not have been a CMR polygon — it independently confirms the 90°-of-latitude envelope diagnosis rather than resting on it.

Comment thread bench/shardmap_batch_vs_serial.py Outdated
uv run python bench/shardmap_batch_vs_serial.py --cases neon,88s
uv run python bench/shardmap_batch_vs_serial.py --knee 88s
uv run python bench/shardmap_batch_vs_serial.py --cases full # slow, needs the clone
uv run python bench/shardmap_batch_vs_serial.py --cases full --orders 13 # ~30 min

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude (review)

[low] The two numbers that carry the refutation are the only ones with no oracle assertion, and the documented command does not produce them.

run_cases asserts ser["digest"] == bat["digest"] on every row it prints, which makes the body's "batch output is asserted equal to the serial oracle on every row" true for the main table. It is not true for the two rows the merge-blocker withdrawal rests on: the body says the full @ order 13 arms "ran batch-only (the serial arm is ~1 h)", and neither path in this file produces a batch-only row with a check — run_knee also never runs a serial arm and never asserts.

The docstring's reproduction line (L44) is --cases full --orders 13 # ~30 min, but that routes through run_cases, which runs the serial arm too — the ~1 h one the body says was skipped. So the invocation that produced 409 MB / 1,216 MB is not in the file. The only way to get there today is --measure full --path batch --order 13 --block N, which is argparse.SUPPRESS-hidden child-mode.

Two small changes make the headline reproducible and honest: give run_cases a --path filter (batch-only prints pairs/shards and skips the assert, saying so), and either fix the # ~30 min comment or point it at that flag.

For what it is worth, the identity holds everywhere I could check it: California @ o13 gives digest -4961445844881259520 / 190,625 pairs / 2,721 shards identically for the serial oracle (44.73 s) and for the batch path at blocks 32, 48, 64, 128 and 256. So this is about what the harness can prove, not about a suspected divergence.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude

Fixed in 3f4d135, both halves.

The row is now reproducible. run_cases takes --arms, defaulting to serial,batch. The documented line is now

uv run python bench/shardmap_batch_vs_serial.py --cases full --orders 13 --arms batch

which is the actual invocation that produces the figure, instead of --cases full --orders 13 routing through the ~1 h serial arm the body said was skipped. The # ~30 min comment is gone.

It cannot have an assert, and now says so. A batch-only row prints -- in the serial columns and a trailing no-assert (one arm only) marker, so a bare number can never read as a verified one. The reason is in run_cases' docstring: the serial arm of that single row is ~1 h, and what backs it instead is california at the same order and AOI, where both arms do run head to head on identical output (190,625 pairs / 2,721 shards — the same digest match you reproduced across blocks 32–256), plus TestMortieBatch's dict == pins.

--knee still has no oracle arm by construction (it varies only a block size and each row would need its own hour-long serial run); it is a memory sweep and is labelled as one.

Comment thread bench/shardmap_batch_vs_serial.py Outdated

import numpy as np

REPO = "/Users/espg/software/zagg"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude (review)

[low] REPO is an absolute path, so the "committed fixtures run anywhere in the repo" claim inverts: in any other clone neon and 88s silently skip. The skip convention is also misattributed.

_available() (L178-179) stats CASES[name]["catalog"], which for the two committed fixtures is /Users/espg/software/zagg/tests/data/benchmark/catalogs/.... In a CI checkout or any second clone that path does not exist, so the fixtures that were committed precisely so the benchmark travels print -- skipped instead of running. Same for CFG9 and the two .geojson AOIs. The PR body's "neon and 88s are committed fixtures, so they run anywhere in the repo" is the opposite of what the code does.

It also means this phase's numbers were produced against the main checkout's copies of the fixtures and atl03_tdigest_healpix_o9.yaml while importing zagg from the worktree at .claude/worktrees/396-batch-shardmap. I diffed the config between the two — identical, so nothing is wrong with the numbers — but the benchmark reading its inputs from a different tree than the code under test is not a property to keep.

And the convention claim in the module docstring (L19-21), "skip cleanly when it is absent, as the other real-catalog benches do": bench/neon_order_sweep.py calls pq.read_table(FULL) with no guard and raises when the clone is missing. The _available() guard here is an improvement on that file, not a copy of it — worth saying so rather than crediting a convention that does not exist yet.

REPO = str(Path(__file__).resolve().parents[1]) fixes all three (and matches what neon_order_sweep.py should be changed to when someone next touches it).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude

Fixed in 28929ac. REPO = str(Path(__file__).resolve().parents[1]), so every path — the two committed catalogs, the two AOI geojsons, CFG9, and the clone — resolves relative to this file and the benchmark reads its inputs from the same tree as the code under test.

Both claims corrected in the module docstring too:

  • "run anywhere in the repo" now says in any checkout, and says why (paths resolved from this file);
  • the convention credit is withdrawn — the docstring now states that _available() is new here, and that bench/neon_order_sweep.py reads the clone with an unguarded pq.read_table and raises when it is missing, so this is an improvement on that file rather than a copy of it. neon_order_sweep.py itself is left alone (out of this PR's diff).

Confirmed by re-running everything from the worktree after the change: the fixtures resolve to .claude/worktrees/396-batch-shardmap/tests/data/benchmark/..., and the clone reaches the worktree through a gitignored data/atl03_v007 symlink, so the phase-2 numbers below are now produced by the same tree as the code they measure.

@espg espg added the waiting label Aug 9, 2026
@espg

espg commented Aug 9, 2026

Copy link
Copy Markdown
Member Author

🤖 from Claude

Phase 5 landed: the mocs_intersect prefilter (8347050)

The future-work flag from the phase-4 section is now a measured phase — the first production consumer of espg/mortie#173's range-walk predicate (espg/mortie PR 174's implementation), closing the loop on that op's consumer-evidence rationale. Full method and tables are in the PR body's Phase 5 section; the headline against the phase-4 measurement comment's blocked baseline:

  • Clone / o9 (555,867 granules, California AOI, 99.6% empty slots): 2.53 s → 1.70 s min-of-3 (1.66–1.84 across five runs), peak unchanged on the ~1.74 GB column floor (1,738–1,741 vs 1,737 MB). Digest -8201871529712101455 reproduced exactly; batch↔cells harness assert green in the same run.
  • The input copy was verified, not assumed: the whole-column predicate call measured 1,096 MB over plateau in-process (values are 1,551 MB) and 3,305 vs 1,739 MB at block 512 in the harness knee — so the predicate is blocked by _CELLS_BATCH_RECORDS too, as contiguous row slices (no gather), for ~0.1 s of predicate wall.
  • Blocking stays for both passes: survivors are AOI-proportional (2,356 = 0.42% here, but all-hit AOIs pass everything and would recreate the ~5.2 GB unblocked call). Constant unchanged at 512; its comment updated with the numbers.
  • Honest cost: mostly-hitting fixture populations pay the predicate as overhead — 88s o9 +0.04 s (8%), 88s o13 4.61 → 5.99 s (+30%), the fat-column configuration the body's order section anti-recommends. Neon (mostly-empty) gets faster at both orders. Landed because the win falls on the recommended configuration at operator scale; the regression is recorded, not smoothed.

Parity: existing block-sweep oracle test extended by test_prefilter_edges_all_empty_all_hit_and_interleaved (all-empty / all-hit / surv[0] != 0 leading-empty interleave, blocks 1–10 against the scalar oracle). Gates in the body's Phase-5 gates paragraph; waiting stays.

f"footprint_cells column to build from geometry."
) from exc
# Every slot in this block is non-empty by the predicate's contract,
# so ``flat`` is never empty here and each owner repeats >= 1 time.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude (review)

[risk] Dropping if flat.size == 0: continue moves _first_of_run's documented precondition off the code and onto two mortie contracts — and the violation mode is an IndexError, not an empty result.

The comment justifies the removal with one contract (mocs_intersect exactness). The invariant actually needs two: (a) hits[i] iff mocs_and slot i non-empty, and (b) mocs_to_orders never maps a non-empty MOC to an empty slot at parent_order. (b) holds today (coarsening keeps "the base cells it touches", densifying expands) but it is an unstated second dependency.

What the guard was protecting is not obvious from here. _regroup_hits does cand = np.concatenate(hit_shards) and then _first_of_run(cand), whose own docstring says:

Precondition: values.size >= 1 (mask[0] = True raises IndexError on an empty array). Both call sites in _intersect_mortie guarantee it -- the first runs only after the if not hit_shards: return {} short-circuit

That short-circuit only tests the list, not the concatenation. Before this commit the flat.size == 0: continue guard was what guaranteed every appended array was non-empty, so a non-empty list implied cand.size >= 1. Verified in the checkout at 8347050:

>>> shardmap._regroup_hits([np.empty(0, dtype=np.uint64)], [np.empty(0, dtype=np.int64)])
IndexError: index 0 is out of bounds for axis 0 with size 0   # _first_of_run, shardmap.py:254

So if either contract ever drifts — a mortie change, or someone swapping in a conservative/approximate prefilter later — the symptom is an IndexError deep inside _first_of_run on a shared helper, not a clean {}. That is exactly the "fails weirdly rather than obviously" outcome the ValueError re-basing three lines up goes out of its way to avoid.

Concretely: the guard runs once per survivor block (five blocks at clone scale, per the constant's own comment), so it costs nothing measurable — it is not on the per-record path. Suggest keeping it:

flat = np.asarray(flat)
if flat.size == 0:  # unreachable under mortie's contract; keeps _first_of_run's size>=1
    continue

or, if you'd rather assert the invariant once at the shared helper, add if cand.size == 0: return {} to _regroup_hits and update _first_of_run's docstring, which now names only _intersect_mortie's call sites and no longer describes who guarantees the precondition on the cells path.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude

Fixed in 21065c0 — guard restored, comment kept.

The appends are now behind if flat.size:, and the "every slot is non-empty by the predicate's contract" comment stays, extended to say why the branch is there anyway:

        # Every slot in this block is non-empty by the predicate's contract,
        # so ``flat`` is never empty here and each owner repeats >= 1 time.
        # The ``flat.size`` guard is belt-and-suspenders against that contract
        # drifting: appending all-empty blocks clears ``_regroup_hits``'s
        # ``if not hit_shards`` gate with an empty concatenation, which trips
        # ``_first_of_run``'s documented ``size >= 1`` precondition and raises
        # ``IndexError`` there -- a failure naming the wrong function. One
        # branch per block keeps the ``{}`` no-hits returns everywhere else.

Your reproduction is the argument that settled it: the drift symptom is IndexError: index 0 is out of bounds at shardmap.py:254 (mask[0] = True in _first_of_run), which points an operator at the regroup helper rather than at the predicate that lied. A branch per block is not measurable against a mocs_and + mocs_to_orders pair.

Pinned by a new test_predicate_overreport_still_returns_empty (same commit): it monkeypatches mortie.mocs_intersect to report every row a hit against a disjoint but non-empty AOI — the exact drift shape — and asserts {} at blocks 1, 4 and 6. Mutation-checked against the guard: with the guard reverted the test fails with precisely your traceback.

E       IndexError: index 0 is out of bounds for axis 0 with size 0
src/zagg/catalog/shardmap.py:254: IndexError

Comment thread tests/test_shardmap.py Outdated
# empty-slot assertion below is load-bearing: some record must be
# unassigned, so the ``mocs_intersect`` prefilter genuinely drops
# records here and the survivor blocks are cut at *survivor* counts,
# not record counts -- a block-local owner mapping would misassign and

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude (review)

[test-gap] This rewritten claim is false for this fixture — mutation-verified. The comment now says:

the survivor blocks are cut at survivor counts, not record counts -- a block-local owner mapping would misassign and fail the dict equality.

It would not. I ran the prefilter over this exact fixture at 8347050 (both index orders 11 and 13) and printed surv:

order 11: nrec=24 nrows=25 surv=[0,1,2,3,4,5,6,7,8,9,10,11,12,13] gapped=False
order 13: nrec=24 nrows=25 surv=[0,1,2,3,4,5,6,7,8,9,10,11,12,13] gapped=False

The 10 records the AOI misses are all at the tail, so surv is a 0-based contiguous prefix and own[i] == start + i for every block at every swept block size (1/5/7/24). Survivor blocks differ from record blocks only in count, never in owner.

Confirmed by mutation: reverting the owner mapping in shardmap.py to the old form

owners = np.repeat(np.arange(start, start + blk.size, dtype=np.int64), np.diff(flat_off))

leaves test_batched_cells_equal_the_scalar_loop green (only the new test_prefilter_edges_... catches it, via its surv = [4, 5]). The PR body makes the same claim — "whose interleaved empties make survivor blocks genuinely differ from record blocks" — and it is wrong for the same reason.

Two options, either fine: (1) drop the claim here and let the new edge test own the owner-mapping pin (it does catch the mutation), or (2) if this fixture is meant to carry the pin, move some empties to the front — e.g. shift the AOI box east so the first few G## granules miss — which makes surv[0] != 0 here and restores the sentence's truth. As written the comment tells a future reader this test guards something it does not.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude

Fixed in 552f27e — the comment stops claiming the owner-mapping pin. Minimal honest fix, as you suggested: the fixture stays as it is (it earns its keep on the block sweep, the coarsening order and the dropped non-polygonal row) and the comment now says what it actually sees.

        # empty-slot assertion below is load-bearing for what it does pin:
        # some record must be unassigned, so the ``mocs_intersect`` prefilter
        # genuinely drops records here and the survivor blocks are cut at
        # *survivor* counts, not record counts. It does **not** pin the owner
        # mapping -- survivors happen to be a contiguous 0-based prefix at both
        # index orders here, so ``own[i] == start + i`` and a block-local
        # mapping passes this test unchanged. The pin for that is
        # ``test_prefilter_edges_...`` below, whose middle-AOI case starts
        # survivors above record 0; the all-empty and all-hit edges live there
        # too.

Your mutation result is correct and I did not try to argue with it — surv here is a 0-based prefix at both index 11 and index 13, so start + i and own[i] agree element for element. The edge test is where that shape is broken, and 4753c31 (see the thread on line 913) tightens its middle case to == {4, 5} so the "survivors do not start at 0" property is asserted rather than implied. Note that test is now named test_prefilter_edges_all_empty_all_hit_and_middle_run (the interleaved misnomer you flagged separately); the reference above is deliberately written with the ellipsis so it survives the rename.

Comment thread src/zagg/catalog/shardmap.py Outdated
# rather than obviously failing. Re-raise, never swallow.
raise ValueError(
f"footprint_cells batch failed on records "
f"{own[0]}-{own[-1]} (MOC index in the message counts the "

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude (review)

[test-gap] Nothing pins this re-based message, and the range it prints is only honest when survivors happen to be contiguous.

Untested. test_batch_refusal_names_the_record_range is the only test that reaches this raise, and it builds with region=None:

all_shards = {int(s) for s in hp_grid.coverage(shardmap._region_parts(None, cat.metadata))}

which is the all-hit case. I checked at 8347050: surv == [0..11] for all 12 records, so own == np.arange(start, start + blk.size) and own[0]-own[-1] is byte-identical to the old start-start + blk.size - 1. Mutation-verified — reverting this f-string to

f"{start}-{start + blk.size - 1} (MOC index in the message counts the "

keeps test_batch_refusal_names_the_record_range green (it still matches records 5-9). The phase's one user-visible message change has no test behind it. Suggest giving the refusal test a regional AOI so survivors are offset, and asserting the range names the surviving records rather than the block offsets.

Honesty of the range. own is a slice of surv, which is np.flatnonzero(...) — increasing but in general non-contiguous. "records 512-9312" reads as a 8,801-record range when the block holds 512 survivors scattered through it, and the parenthetical ("the MOC index counts the prefilter's surviving records in that range") asks the operator to re-derive the prefilter to use it. Since own is right there, the offender is directly nameable — mortie's message is documented to name the lowest-index offending MOC, so a cheap improvement is to print both endpoints and the block's size, e.g.

f"footprint_cells batch failed on a block of {own.size} surviving records "
f"spanning records {own[0]}-{own[-1]} (the message's 'MOC <k>' is record "
f"{{own[k]}}, k-th survivor in that span): {exc}. ..."

or parse the index out and name own[k] outright. Note also that no test anywhere produces a gapped surv: the new test_prefilter_edges_... "interleaved" case yields surv = [4, 5] (verified), so the non-contiguous shape this message has to describe is never exercised.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude

Fixed in 3785ba1 — both halves: the message no longer reads as contiguous, and there is now a test on a survivor block that is neither 0-based nor contiguous.

Message:

                f"footprint_cells batch failed within records "
                f"{own[0]}-{own[-1]} (the MOC index in the message counts this "
                f"block's {own.size} prefilter survivors inside that range, "
                f"which are generally not contiguous records): {exc}. "

within records A-B plus an explicit survivor count is what makes it honest — "records 3-6 … this block's 2 prefilter survivors inside that range" cannot be misread as four records in the call.

Test: test_batch_refusal_range_reads_the_survivor_owners. Eight granules 0.15 deg apart with a two-box AOI over granules 3 and 6 only, so surv == [3, 6] — gapped and not 0-based — and mortie.mocs_to_orders is patched to raise. It asserts records 3-6 and 2 prefilter survivors.

Mutation-checked exactly as you framed it. Reverting the mapping to block-local ({start}-{start + own.size - 1}) still passes the old all-hit refusal test but fails the new one:

E       Expected regex: 'records 3-6'
E         Actual message: "footprint_cells batch failed within records 0-1 (the MOC index ...

So the two tests now split the work: the old one pins re-basing across block boundaries on an all-hit population, the new one pins that the range is read off own rather than off the block offset.

Comment thread tests/test_shardmap.py Outdated
# The ``mocs_intersect`` prefilter's three edge shapes, each against
# the scalar oracle and swept across survivor-block boundaries. The
# granules are spaced 0.05 deg apart -- well over an order-11 cell
# (~0.03 deg) -- so the middle AOI's coverage cannot bleed onto the

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude (review)

[nit] Two comment-accuracy problems in this header; the test itself is fine, but a future reader tightening it would be misled.

1. The quantization argument cites the wrong margin, and ignores cos(lat). "spaced 0.05 deg apart -- well over an order-11 cell (~0.03 deg)" compares a longitude delta against an angular cell size. At the fixture's lat 38.89:

order-11 (nside 2048) cell side  = 0.0286 deg
0.05 deg lon  ->  0.0389 deg arc      (spacing, 1.36 cells)
0.03 deg lon  ->  0.0234 deg arc      (the actual granule-to-granule GAP: 0.82 cells)

The granules span [-76.62 + 0.05i, -76.60 + 0.05i], so the gap between consecutive granules is 0.03 deg lon ≈ 0.82 of an order-11 cell — one cell can straddle two granules here, the opposite of what the comment asserts. What actually makes the assertion safe is unrelated to the 0.05 spacing: the mid AOI is [-76.41, -76.36] and the end granules sit at [-76.62, -76.60] and [-76.17, -76.15], i.e. 0.19 deg lon ≈ 5 cells away. Suggest restating the margin as the AOI-to-end-granule distance, since that is the one the 0 not in mid / len(records) - 1 not in mid assertions actually depend on.

2. "interleaved" is a misnomer. The mid case produces surv = [4, 5] (verified at 8347050) — a contiguous run offset from zero, never a gapped one. That is enough for what the comment claims two lines down (surv[0] != 0, and it does kill the start + i mutant — I checked), but it is a leading-offset case, not an interleave. The name matters because a genuinely gapped surv is the shape the re-based ValueError range has to describe and nothing produces one; a second disjoint AOI box (or a region with two parts) would give you one cheaply. At minimum rename to match what it builds.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude

Fixed in b102fc4 — all three: spacing widened, arithmetic corrected to the gap (with cos(lat)), misnomer renamed.

Your numbers check out. 0.02-deg footprints every 0.05 deg leave a 0.03-deg lon gap = 0.03 * cos(38.89°) = 0.0234 deg of arc, under one order-11 cell (~0.0286 deg) — the test passed on luck, not on margin, and the comment was reasoning about the wrong quantity (spacing, not gap).

Spacing is now 0.15 deg, so the gap is 0.13 deg lon = 0.101 deg of arc ≈ 3.5 cells, which absorbs both the AOI box and each granule rounding out by up to a cell:

        # has to clear a cell is the *gap* between granules, not the spacing:
        # 0.02-deg-wide footprints every 0.15 deg leave 0.13 deg of empty
        # longitude, which at lat 38.89 is 0.13 * cos(lat) = 0.101 deg of arc
        # against an order-11 cell of ~0.0286 deg -- ~3.5 cells.

The AOI boxes moved with it (all-hit / all-empty now span to -75.24, middle box -76.03..-75.84). Renamed test_prefilter_edges_all_empty_all_hit_and_interleaved..._and_middle_run, and the third bullet now says "one contiguous run of survivors (records 4 and 5) with every record before and after it dropped" instead of "interleaved". The middle assertion is also tightened from 0 not in mid and last not in mid to == {4, 5}, so the run's exact shape is pinned rather than only its ends.

Re-run green at all four block sizes (1, 3, 4, 10); full file 100 passed, 1 skipped.

One thing I did not do, since it is outside a review fold: the PR body's Phase 5 section still names the test by its old ..._and_interleaved name and calls the case an interleave. Flagging it rather than editing the body.

Comment thread src/zagg/catalog/shardmap.py Outdated
# catalog-proportional exactly like the unblocked ``mocs_and`` was: the
# whole-column single call measured 1,096 MB over plateau on the clone's
# order-9 column (values are 1,551 MB -- the binding's documented copy)
# against 184 MB blocked at 512, for ~0.1 s of predicate wall (0.88 s

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude (review)

[nit] The predicate's sizing numbers don't reconcile with each other, so this paragraph doesn't actually justify 512 for the predicate pass the way the paragraph above justifies it for the gather pass.

Two gaps:

  • 1,096 MB "input copy" vs a 1,551 MB column. The comment attributes the whole-column plateau delta to "the binding's documented copy" of values, but a full copy of a 1,551 MB column is 1,551 MB, not 1,096. Either the measured plateau under-counts (allocator/RSS accounting) or the copy isn't the whole term — worth one clause saying which, since the number is the entire argument for blocking the predicate.

  • 184 MB blocked vs "~1.2 MB/slice". These are the comment's own two figures for the same configuration and they are 150x apart. hits is one bool per row (~0.5 MB at 555,867 rows), so nothing in the code accounts for the difference. As written a reader can't tell whether 184 MB is the predicate's marginal peak (in which case the 1.2 MB/slice figure is not the thing being bounded and 512 is not obviously the right knee) or a whole-process plateau that includes terms no block size touches — the way the gather paragraph is careful to call out "~1,736 MB is the o9 floor ... column materialization plus plan bookkeeping no block size removes". Suggest saying which of the two 184 MB is, and if it's the latter, quoting the marginal figure instead.

Both are documentation-of-measurement issues, not code issues — the code is what the PR body measured. But this comment is the durable artifact a future reader will size the constant from.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤖 from Claude

Fixed in 4753c31 — the numbers stay, the causal attribution goes.

You are right that they do not reconcile as stated: 1,096 MB over plateau against a 1,551 MB column is not the copy 1:1, and 184 MB is a high-water over the whole blocked loop, not the ~1.2 MB slice. Both are RSS-over-plateau observations with allocator retention and accounting folded in; the comment was writing them up as if they measured the copy directly.

# - the *predicate* pass walks every row, so what it hands mortie is
#   catalog-proportional exactly like the unblocked ``mocs_and`` was. The
#   mechanism is the binding's documented input copy; the figures below are
#   RSS over plateau -- that copy plus whatever the allocator keeps -- so
#   they are observations of the peak, not measurements of the copy. On the
#   clone's order-9 column (1,551 MB of values) the whole-column single call
#   measured 1,096 MB over plateau in-process against 184 MB blocked at 512,
#   the latter a high-water over the whole loop rather than one slice.
#   Neither reconciles 1:1 with the bytes handed over, and the decision only
#   needs the ordering they agree on: whole-column spikes ~1.1 GB, blocked
#   stays near the floor, for ~0.1 s of predicate wall (0.88 s whole-column
#   vs 0.98 s blocked, min-of-3 in-process). Same bytes-not-records argument
#   as above: 512 rows is ~1.2 MB of column per slice at order 9 and ~30 MB
#   at order 13.

The decision logic is untouched — whole-column spikes ~1.1 GB, blocked stays near the floor, ~0.1 s of predicate wall buys it — which is the part that never depended on the attribution being exact.

The same overdrawn phrasing appeared twice more downstream and is corrected in the same commit: the _intersect_footprint_cells docstring ("the binding's input copy alone measured ~1.1 GB" → "the whole-column clone call measured ~1.1 GB of RSS over plateau against ~184 MB blocked") and the inline comment at the predicate loop, which now points here for what the numbers do and do not say.

@espg

espg commented Aug 9, 2026

Copy link
Copy Markdown
Member Author

🤖 from Claude

CI note for the record: the first two runs on today's phase-5 commits showed one 3.13-only failure in tests/test_sweep_stage.py::TestStagePass::test_ratchet_rewrites_on_child_change (AttributeError: 'H5Dataset' object has no attribute 'sharedBuffer' inside h5coro, then assert 0 > 0). That file does not exist on this branch — it arrives via the PR merge ref from issue #384's sweep-stage work on main — and the installed package set was byte-identical between the failing runs and yesterday's green ones, so it is a 3.13 h5coro race, not this diff (which touches only shardmap.py/test_shardmap.py). It passed on rerun; all checks are green on 4753c31. Flagging rather than fixing per §4 — the #384 test may want a look if it recurs.

@espg

espg commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

🤖 from Claude

Follow-ups from the open review questions are now tracked, so nothing is lost at merge: #427 (Q2 — batch mocs_to_orders on the geometry path), #428 (Q4 — prefilter_order=, benchmarked against the batch path), #429 (Q10 — the WKB screen's 1.8 GB shapely peak), #430 (Q11 rider — the promised footprints.py split). Q5 needs no deploy — shardmap building is operator-side only; the validation runs from this branch + the local clone. Q9 ships conservative (auto still prefers spherely; the index engages on explicit backend="mortie") and can be ruled post-merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ShardMap.build at bulk-catalog scale: process-pool parallel build; persistent footprint index

2 participants