88S latitude sweep: ring AOI family + reassessment benchmark matrix - #526
88S latitude sweep: ring AOI family + reassessment benchmark matrix#526espg wants to merge 12 commits into
Conversation
espg
left a comment
There was a problem hiding this comment.
🤖 from Claude (review)
Adversarial pass over phase 1 (f77885f) — the ring AOI family — against CLAUDE.md and issue #148's acceptance criteria.
What holds up. The reference test is the right one: generating the committed antarctic_88s.geojson geometry and comparing, rather than asserting properties of the generator in isolation, means the sweep bands provably share the shipped ring's shape and the granule counts stay comparable to the pinned row. test_regenerating_a_band_is_byte_stable covering the serialization (not just the parse) is the right call for a fixture that will be regenerated. The float arithmetic is clean — every band edge serializes exactly (-84.7, -85.2, … -87.7), no -86.69999999999999 in the committed bytes, checked directly against the files.
Three findings, inline.
tools/make_lat_ring_aoi.py— the writer overwritesantarctic_88s.geojsonwith generic prose if88is in the latitude list, which the documented reassessment list is. Geometry survives, so no test fires. Diff-scoped, should be fixed.tools/make_lat_ring_aoi.py—sector_ring(step=…)is unreachable flexibility that silently rounds a non-dividing step (§4). Diff-scoped.tests/test_lat_ring_aoi.py— one test carries three claims and covers the MISSING branch anonymously. Diff-scoped.
One thing deliberately not raised as a finding. The 45° sector seams sit on the HEALPix base-cell boundary that espg/mortie#103 mis-fills, and the generator inherits that on purpose. That is the right call for this PR — matching the shipped ring is what makes the sweep comparable — and it is already standing as question (2) in the PR body, where it belongs: moving the seams would re-pin healpix_o9_88s / healpix_o10_88s, which is a deliberate fixture move and not a reviewer's to make.
Scope check. Nothing under deployment/aws/ or .github/ is touched, no dependency is added, no wire format or spec marker moves, and nothing in the diff can reach AWS or CMR — the whole phase is geometry and JSON.
Generated by Claude Code
| failed = True | ||
| continue | ||
| path.write_text(dumps(doc)) | ||
| print( |
There was a problem hiding this comment.
🤖 from Claude (review)
The writer will clobber the one file it is supposed to be generated against.
main() writes unconditionally. Run the documented invocation with 88 in the list — uv run python tools/make_lat_ring_aoi.py 85 85.5 86 86.5 87 87.5 88, which is the exact latitude list the reassessment comment gives and therefore the list a reader is most likely to paste — and antarctic_88s.geojson is overwritten with the generator's prose:
- committed:
"name": "Antarctic 88S dense (ICESat-2 turning-latitude stress target)", with the note explaining the turning-latitude stress rationale and the~576 at o9figure - generated:
"name": "Antarctic 88S ring (ICESat-2 latitude sweep band, issue #148)", with the generic sweep-band note
The geometry survives (that is what test_generator_reproduces_the_committed_88s_ring guarantees) so no test catches it, and the module docstring's own claim — "the committed 88S ring stays the reference the rest of the family is generated to match" — is what silently stops being true. The prose that goes is exactly the prose issue #148 relies on.
A guard is cheap: refuse to overwrite an existing file whose bytes differ, and say what to do instead. That also makes the writer idempotent, which is worth having on its own.
Generated by Claude Code
There was a problem hiding this comment.
🤖 from Claude
Fixed in f722d5a. A plain run now refuses to replace a committed band whose bytes differ (EXISTS and differs -- re-run with --force to replace it, exit 1) and --force is the deliberate way through; rewriting a band that is already identical stays silent, so the writer is idempotent.
Two tests carry it: test_writing_88_leaves_the_shipped_ring_alone runs the exact documented latitude list including 88 and asserts the shipped file is byte-unchanged and still carries turning-latitude stress target, and test_force_replaces_a_committed_band covers the guard, the --force path, and the idempotent re-write. The module docstring now states the rule instead of only implying it.
Generated by Claude Code
| lon_end: float, | ||
| lat_south: float, | ||
| lat_north: float, | ||
| step: float = LON_STEP_DEG, |
There was a problem hiding this comment.
🤖 from Claude (review)
step is a knob nothing can turn. ring_geometry never threads it, no test varies it, and n = round((lon_end - lon_beg) / step) silently rounds a step that does not divide 45° — so the only reachable effect of the parameter existing is a wrong ring if someone ever does pass one. CLAUDE.md §4: "No dead code, no speculative abstraction."
Read LON_STEP_DEG directly and drop the parameter. (band on ring_geometry is the same shape of thing but earns its place — the band height is the quantity the docstring argues about, and naming it at the call site is what makes ring_geometry(88.0) in the reference test readable.)
Generated by Claude Code
There was a problem hiding this comment.
🤖 from Claude
Fixed in 8d19bcd. sector_ring now takes only the four coordinates and reads LON_STEP_DEG directly; the docstring says why the spacing is a module constant rather than an argument (a per-call step could only round to a different vertex count than the rest of the family shares). --check across all seven bands still reports geometry matches, so nothing moved.
band on ring_geometry is left as-is, for the reason the finding itself gives.
Generated by Claude Code
| """``--check`` is the guard: it agrees with every committed band, and never writes.""" | ||
| argv = [str(v) for v in SWEEP_LATS] | ||
| assert ring.main(["--check", *argv]) == 0 | ||
| assert ring.main(["--check", "--out-dir", str(tmp_path), "88"]) == 1 |
There was a problem hiding this comment.
🤖 from Claude (review)
Two unrelated claims share one test, and the name only advertises one of them. Line 111 exercises the MISSING branch (--check against an empty dir returns 1); lines 110 and 112 exercise "agrees with the committed family" and "writes nothing". A failure here does not say which of the three broke, and the MISSING branch — the one that catches a band that was never committed at all — is covered anonymously, inside a test whose name promises something else.
Split the missing-file case out under its own name, beside test_check_mode_reports_a_band_whose_geometry_moved. The two --check failure modes then read as the pair they are.
Generated by Claude Code
There was a problem hiding this comment.
🤖 from Claude
Fixed in 36d2fb8. The missing-band case is now its own test, test_check_mode_reports_a_band_that_was_never_committed, sitting beside test_check_mode_reports_a_band_whose_geometry_moved so the two --check failure modes read as the pair they are. What remains under test_check_mode_passes_on_the_committed_family is the one claim its name makes, plus the "writes nothing" assertion that has to ride along with it (it can only be checked against a run that had something to write) — now carrying its own message.
Generated by Claude Code
espg
left a comment
There was a problem hiding this comment.
🤖 from Claude (review)
Adversarial pass over phase 2 (10e0dc2) — the sweep matrix manifest — against CLAUDE.md and issue #148's acceptance criteria.
What holds up. Keeping the unpinned rows in pending_targets rather than targets is the right shape and for the right stated reason: run_target reads shardmap_meta["shard_key"] unconditionally, so a pin-less row in targets would die mid-dispatch with a bare KeyError instead of being visibly not-ready. test_pinned_and_unpinned_bands_stay_consistent closes both halves of that (pinned != (pin == "unpinned") rejects an entry that is neither as well as one that is both). test_the_88s_row_is_the_committed_pin_not_a_fork is the one that matters most long-term — the mission's worst shard behind two pins that can drift, with only one covered by the drift guard, is a trap worth nailing shut. And asserting the config properties (hive, parent_order 9, granule_workers 4, get_aoi_mask false) rather than only the target keys is what actually checks the words "hive" and "o9" in the reassessment's base configuration; run_benchmark never sets those per target.
Four findings, inline. One is a real bug:
tests/test_88s_latitude_sweep.py:210— the dry-dispatch assertion hard-codeshealpix_o9_88s's shard key while looping over all pinned rows, so it goes red the moment someone pins a second band, presenting as a dispatch bug in their new row. Diff-scoped, should be fixed.tests/test_88s_latitude_sweep.py:76— in-testsys.path.insertfortools/, which the neighbouring module moved to module scope on purpose. Diff-scoped.tests/test_88s_latitude_sweep.py:197—monkeypatch.chdir(REPO)is inert;run_targetresolves offbase. Diff-scoped.targets_88s_latitude_sweep.json:2— the fleet figures carry no version or comment provenance, and they are version-dependent in a way that changes their meaning. Diff-scoped.
Two things deliberately not raised as findings.
- The twelve
pending_targetsrows repeat ten identical keys each. That is verbose, but it is exactly howtargets.jsonspells its own targets out, andtest_every_row_is_the_reassessment_base_configurationiterates every row and asserts each key — so a half-applied edit fails loudly rather than drifting. Matching the surrounding data file beats de-duplicating it (§4). - The manifest records
temporal.endas2025-06-01(thecat_88s.parquetsnapshot's window) wheretargets.json's top-level default is2026-03-15andhealpix_o9_88scarries no override. That inconsistency is pre-existing intargets.json, latent because the offlinecatalog_parquetpath ignores the manifest window entirely.test_the_sweep_temporal_window_matches_the_snapshot_that_pins_itpins the correct value here and documents why; changingtargets.jsonis out of this PR's scope (§4 on pre-existing failures) and is worth its own look.
Scope check. Data and tests only. No .github/, no deployment/aws/, no dependency, no wire format or spec marker. run_target is exercised only through dry_run=True, which returns before any zagg.runner.agg import — nothing in the suite can reach Lambda, S3 or CMR.
Generated by Claude Code
| dry_run=True, | ||
| ) | ||
| assert record["target"] == name | ||
| assert record["shard_key"] == manifest["shardmaps"]["healpix_o9_88s"]["shard_key"] |
There was a problem hiding this comment.
🤖 from Claude (review)
This assertion is written to break at exactly the moment the deferred work lands.
The loop iterates manifest["targets"] — every pinned row — but the assertion hard-codes one band's key:
for name in manifest["targets"]:
...
assert record["shard_key"] == manifest["shardmaps"]["healpix_o9_88s"]["shard_key"]Today targets holds only the two 88S rows so it passes. The whole point of _pin_recipe step 4, though, is that pinning a band moves its two rows into targets — and the first person to do that gets a red test asserting their 85°S row dispatched the 88°S shard. The failure will look like a dispatch bug in the row they just added, not like a stale assertion here, which is the expensive kind of wrong.
Look the expectation up through the row: manifest["shardmaps"][manifest["targets"][name]["shardmap"]]["shard_key"]. Then the test grows with the matrix instead of blocking it, and it starts actually checking that each row dispatches its own band — which is what the assertion is trying to say.
Generated by Claude Code
There was a problem hiding this comment.
🤖 from Claude
Fixed in 7a08675. The expectation is now looked up through the row:
band = manifest["shardmaps"][manifest["targets"][name]["shardmap"]]
assert record["shard_key"] == band["shard_key"]so the test grows with the matrix as bands get pinned, and it now asserts what it was trying to say — each row dispatches its own band, not a fixed key that happens to be the only one there today. A comment at the assertion names _pin_recipe step 4 as the reason.
Generated by Claude Code
| ``bench_metrics.resolve_aoi_temporal_cmr``'s NEON fallback). That only holds | ||
| while every entry actually carries one. | ||
| """ | ||
| sys.path.insert(0, str(REPO / "tools")) |
There was a problem hiding this comment.
🤖 from Claude (review)
In-test sys.path.insert is the pattern the neighbouring module explicitly moved away from. tests/test_benchmark_shardmap.py puts it at module scope and says why in a comment:
tools/is not an installed package, so it goes on the path ONCE here — matching thebench_metricspattern above — rather than on every_driver()call, which stacked one identical entry per test.
This file already inserts .github/scripts at module scope for exactly that reason, then does the tools/ one inside a test body. Hoist it next to the other insert (CLAUDE.md §4: match the surrounding code).
Generated by Claude Code
There was a problem hiding this comment.
🤖 from Claude
Fixed in e04addb. Both inserts now sit together at module scope with one comment covering them, and make_lat_ring_aoi is imported at the top beside bench_metrics and run_benchmark rather than inside the test body.
Generated by Claude Code
| backend/streaming/worker injection and record building all run, and a real | ||
| dispatch would be the only remaining step. | ||
| """ | ||
| monkeypatch.chdir(REPO) |
There was a problem hiding this comment.
🤖 from Claude (review)
monkeypatch.chdir(REPO) does nothing here. run_target resolves every path off the base argument (_resolve(base, …)), which this test passes as the absolute BENCH; the config, the shard map and the grid all come from that. Dropping it also drops the monkeypatch fixture from the signature.
A chdir that is not needed is worse than neutral in a test: it reads as "this test depends on the working directory", which is the first thing someone debugging a path failure will believe.
Generated by Claude Code
There was a problem hiding this comment.
🤖 from Claude
Fixed in fa2c1f5 — the chdir and the monkeypatch fixture are both gone. Verified the claim rather than assuming it: the module passes run from /tmp with an explicit --project, so nothing in that test reads the working directory.
Generated by Claude Code
| @@ -0,0 +1,309 @@ | |||
| { | |||
| "description": "88S reassessment latitude sweep (issue #148): one o9 shard per 0.3-deg latitude band at 85 / 85.5 / 86 / 86.5 / 87 / 87.5 / 88 degrees SOUTH, each run on TWO worker arms -- 4 GB with disk and 8 GB with disk. Every arm is the shipped per-merge sidecar configuration (tdigest, store_layout hive, sidecar read, spill streaming, sharded output, granule_workers=4, no AOI mask) pointed at a ring band instead of the NEON box, so the sweep measures LATITUDE and WORKER SIZE against a pipeline that is otherwise byte-identical to the one that ships. The question it answers is where the tractable band ends: the last recorded fleet result had 85S/8GB completing at 865 s and everything poleward timing out, with the split read 77% / aggregate 22% / write 1% -- read-bound, memory not binding (2.2 GB peak of 8192, spill holding it). Run on demand by explicit --target; this is NOT an every-merge matrix, and a timeout is an expected result here, not a red build.", | |||
There was a problem hiding this comment.
🤖 from Claude (review)
Measured numbers with no provenance, in a file that will outlive the thread they came from. 865 s, 190M obs, 2.2 GB peak of 8192, read 77% / aggregate 22% / write 1% and "timed out on both arms" are all real measurements, but the manifest attributes them only to "the last recorded fleet run" — and does not say which code point they were measured at. They were taken on 0.36.0, after the store-cache fix, and that matters: the same sweep on 0.35.0 timed out at every band including 85°S, so a reader who takes these as properties of the bands rather than of a version will draw the wrong conclusion about what changed.
CLAUDE.md §6 asks that claims be grounded so the reasoning can be reconstructed later. Name the zagg version and the issue #148 comment the figures come from, in the description and in the healpix_o9_85s note. It costs one clause each and makes the numbers falsifiable.
Generated by Claude Code
There was a problem hiding this comment.
🤖 from Claude
Fixed in a01fbbc. The description now names the code point and the source comment, keeps the phase split in seconds rather than percentages (read 671 s / aggregate 194 s / write 10 s), and states outright that the figures are a property of 0.36.0 and not of the bands — with the 0.35.0 contrast and the store-cache fix (issue #287 / PR #288) as the evidence, plus an instruction to re-record on the next run. The healpix_o9_85s note carries the same attribution and the exact obs count (189,852,320); the other five band notes now say "on 0.36.0" instead of "on the last fleet run".
Also normalised _pin_recipe.how_to_pin to (1)…(4) while in the file, per CLAUDE.md §6 on enumerated list items.
Generated by Claude Code
espg
left a comment
There was a problem hiding this comment.
🤖 from Claude (review)
Adversarial pass over phase 3 (9652ba4) — the docs — against CLAUDE.md and issue #148's acceptance criteria.
What holds up. The README section leads with the thing a reader actually needs to know (what varies and what does not), and states the pending_targets rationale in terms of the mechanism — run_target reads shard_key unconditionally — rather than as an assertion. Pointing the "Add a target over a different AOI" recipe at the generator, right where it currently tells you to hand-drop a polygon, is the edit most likely to keep the next ring band consistent with this family. The blob/main link style matches what docs/aoi_mask.md, docs/quickstart.md and docs/specification.md already use, and the relative in-docs links are untouched.
Two findings, inline. Both are about the same failure mode — a fact stated in two places where only one will get updated:
tests/data/benchmark/README.md:269— the new section sits flush against the older## The 88°S stress targetssection, whose closing bullet still says to expect an OOM at 2 GB "until the issue #148 streaming/cached-read work lands". It landed; the two sections now contradict each other on the same issue, on adjacent lines. Diff-scoped, should be fixed — created by this phase's placement, even though the stale sentence predates it.docs/deployment/benchmark.md:210— the fleet figures are hand-copied from the manifest, which asks for them to be re-recorded on the next run, with nothing naming the record of truth. Diff-scoped.
One thing deliberately not raised as a finding. The Layout table's new row uses a brace expansion (antarctic_{85,85_5,…}s.geojson) for six real files. That is not a literal path, but the table already globs (configs/*.yaml, shardmaps/*.json) and six separate rows for one family would bury the two rows that matter.
Scope check. Documentation only — no code, no data, no .github/, no deployment/aws/. The one pre-existing ruff format diff in this README (a python example block) is confirmed present on main at line 176 and is untouched; it moved to 178 only because the Layout table gained two rows.
Generated by Claude Code
| is the *expected* baseline result until the issue #148 streaming/cached-read | ||
| work lands. | ||
|
|
||
| ## The 88°S latitude sweep (issue #148 reassessment) |
There was a problem hiding this comment.
🤖 from Claude (review)
The new section leaves the one directly above it stale, and now they contradict each other about the same issue.
## The 88°S stress targets (issue #148) ends, two lines up, with:
Run on demand via explicit
--target… OOM/timeout at 2 GB / 900 s is the expected baseline result until the issue #148 streaming/cached-read work lands.
The streaming work landed — spill is what the sweep runs on, and it is now the shipped per-merge configuration. The reader gets "expect an OOM at 2 GB until streaming lands" immediately followed by "on 0.36.0, spill held 2.2 GB of 8,192 and 85°S completed in 865 s". Both sentences are about issue #148, they are adjacent, and only one of them is still true.
This is fair to charge to this phase: the older section was merely out of date on its own, and putting a current section flush against it is what turns that into a contradiction. Reconcile it — the older section's pins and pruned map and build-the-catalog-once content is all still correct and load-bearing, so the fix is to update its last bullet and point it at the sweep, not to delete it.
Generated by Claude Code
There was a problem hiding this comment.
🤖 from Claude
Fixed in 771978d, by updating the bullet rather than deleting the section — the pins, the pruned-map note and the build-the-catalog-once note are all still correct.
The bullet now says why 2 GB / 900 s is still the right expectation for those two specific rows (they predate the streaming work and dispatch the hierarchical and cached read paths on the base worker, not spill on a -disk worker), then states that the streaming half of issue #148 has landed and points the reader at the sweep below for the current polar picture — including that memory is no longer the wall anywhere as of 0.36.0. The two sections now read as history and current state instead of as a contradiction.
Generated by Claude Code
| and the numbers are directly comparable to the per-merge series. | ||
|
|
||
| What it answers is where the tractable band ends. On zagg 0.36.0 the crossover | ||
| was **85°S at 8 GB** (865 s, 190 M obs, 2,221 MB peak of 8,192 — memory is not |
There was a problem hiding this comment.
🤖 from Claude (review)
Same measurements, two files, nothing linking them. 865 s, 190 M obs, 2,221 MB of 8,192 and the 671 / 194 / 10 split now live here and in the manifest's description, which says of itself "re-record them when the sweep is next run". Whoever does that run will update one of the two. The published page and the data file will then disagree, and there is no way to tell from either which one is current.
Every other series on this page is backed by a rendered artifact regenerated from the recorded data, so numbers here are normally derived. These are hand-copied, which makes them the exception, and the exception should at least say where the record of truth is: name the manifest as the thing to update, so the drift is a one-line fix instead of an archaeology problem.
Generated by Claude Code
There was a problem hiding this comment.
🤖 from Claude
Fixed in f85ed73. The section now says outright that these figures are the exception on that page — not derived from a rendered artifact, because an on-demand sweep has no recorded series — names the manifest's description as the record of truth and the thing to update on the next run, and tells the reader to treat the page as a copy of it.
Generated by Claude Code
|
🤖 from Claude Status: all three phases landed, all nine self-review findings folded, applying What is here: the seven-band ring AOI family and its generator ( The one decision that blocks the rest — where do the six band catalogs live? Only the 88°S row is pinned (it reuses the committed
Recommendation: (1). It keeps the rebuild deterministic and offline the way the 88°S row already is, the ring prefilter is exact so one fetch genuinely serves all seven bands, and it is one artifact to regenerate rather than seven. The reason it is a question and not a commit is CLAUDE.md §3 on large binaries — ~40 MB is a deliberate addition. Once that is answered the remaining work is mechanical and already written down as Two smaller things also standing, both in the PR body under "Questions for review": whether the 45° sector seams should move off the HEALPix base-cell grid (it would re-pin Not run, by construction: no Lambda invoke, no standup, no S3 write, no CloudFormation. Everything in this PR is offline — the only dispatch path exercised anywhere in the tests is Generated by Claude Code |
Closes #148
What this does
Turns the 88S reassessment ask — the 2026-07-18 comment: "things have changed … we should update this and reassess if polar processing is now more tractable", with the matrix
[85, 85.5, 86, 86.5, 87, 87.5, 88]degrees south × {4 GB-disk, 8 GB-disk} on sidecar + hive + spill + sharded + o9 + no AOI mask — from an ad-hoc sweep into committed, reproducible benchmark data.Today the sweep exists only as numbers on the issue thread. Every band except 88°S was measured against a scratch AOI and a scratch catalog that were never committed, so nothing in the tree can rebuild them, and the pinned 88°S stress row (
healpix_o9_88s) has no siblings to be compared against. This PR lands the half of that which is offline and deterministic:The ring AOI family.
tools/make_lat_ring_aoi.pygenerates the seven[-L, -L+0.3]latitude bands to one set of rules, and the six new bands are committed beside the shippedantarctic_88s.geojson. The generator's acceptance test is that it reproduces the committed 88°S ring geometry exactly — so the sweep rows are the same shape as the pinned row they extend, and their granule counts stay comparable. The rules it encodes are the non-obvious ones: the 0.3° band height every issue add 88 south stress test shard + benchmark #148 count was measured over, the eight-sector split (a single-180..180rectangle collapses to an antimeridian sliver under spherical polygon fill), and the 1°-of-longitude vertex sampling. It refuses to overwrite a committed band whose bytes differ, so a run that includes88cannot silently replace the shipped ring's prose with the generic sweep note.The sweep matrix as data. A
targets_88s_latitude_sweep.jsonmanifest undertests/data/benchmark/, alongside the existingtargets_order_sweep_neon.json/targets_full_aoi_neon.jsonauxiliary manifests. Per that directory's own README, "this directory is the benchmark matrix … adding, removing, or reshaping benchmarks is a data edit undertests/data/benchmark/, with no change to the workflow or the runner" — so the matrix lands without touching.github/.Docs. The README's sweep section (bands, generator, pinning recipe), an on-demand section in
docs/deployment/benchmark.md, and a reconciliation of the older 88°S stress-target section, which still told readers to expect an OOM at 2 GB "until the streaming work lands".Approach
The sweep arm is exactly the shipped per-merge sidecar arm (
tdigest_healpix_o9_hive_sidecarintargets.json) pointed at a ring band instead of NEON, so no new pipeline config is needed —run_benchmark.run_targetalready setsindex_backend,streaming_mode,shardedandworkerper target off the manifest:run_benchmark.resolve_variantturns thatworkerblock into the pre-provisionedprocess-shard-8192-diskvariant, and the 4 GB arm into-4096-disk, which is the "4 GB worker with disk" / "8 GB worker with disk" pair the reassessment asks for.The 88°S row reuses the committed
sm_healpix_o9_88s.jsonmap and pin verbatim, so the drift guard and the issue #444 re-pin driver keep covering the mission's worst shard in the one place they already do; the manifest does not fork it (test_the_88s_row_is_the_committed_pin_not_a_fork).Phases
f77885f).tools/make_lat_ring_aoi.py+ the six new band geojsons +tests/test_lat_ring_aoi.py.10e0dc2).targets_88s_latitude_sweep.json(14 rows: 7 bands × 2 worker arms) +tests/test_88s_latitude_sweep.py.9652ba4). Thetests/data/benchmark/README.mdsweep section and thedocs/deployment/benchmark.mdon-demand section.Adversarial self-review ran after each phase; all nine findings are folded, one commit each (
f722d5a,8d19bcd,36d2fb8,7a08675,e04addb,fa2c1f5,a01fbbc,771978d,f85ed73), with a reply on every thread.Deferred (not landed here, deliberately)
The six unpinned bands and the live fleet run. A benchmark row is dispatchable only once its shard map is built and its densest shard pinned. For the 85–87.5°S bands that needs a CMR catalog those bands do not have:
catalogs/cat_88s.parquetsnapshot is the CMR result for bbox[-180, -88.0, 180, -87.7](35,639 granules) — it is not a superset of the equatorward bands. Every granule in it reaches north of 85°S (verified:ymax >= -85.0for all 35,639 rows), but granules that turn at, say, 86°S and never reach 87.7°S are absent, so rebuilding an 85°S map from it would silently under-count.So the 88°S row of the matrix is fully pinned and dispatchable today, and the six equatorward rows land as a complete, documented definition awaiting their catalog snapshots. Their targets sit in
pending_targetsrather thantargetson purpose:run_targetreadsshardmap_meta["shard_key"]unconditionally, so a pin-less row intargetswould die mid-dispatch with a bareKeyErrorinstead of being visibly not-ready._pin_recipein the manifest is the four-step recipe for moving one across, andtest_pinned_and_unpinned_bands_stay_consistentfails loudly on a half-done pin.How it was tested
uv run pytest -q→ 4893 passed, 38 skipped (606 s). The two pre-existing test failures flagged for this branch (test_client_transport.py::TestStatusPoller::test_invoke_fault_burns_an_attempt_and_retriesandtest_lambda_build.py::TestFunctionBuild::test_function_build_succeeds) did not reproduce in this environment — reporting that rather than claiming credit for it.tests/test_lat_ring_aoi.py(30) +tests/test_88s_latitude_sweep.py(12). Both offline — geometry, JSON, and onerun_target(dry_run=True)wiring check that returns before anyzagg.runner.aggimport.uv run python tools/make_lat_ring_aoi.py --check 85 85.5 86 86.5 87 87.5 88→geometry matcheson all seven.ruff check src tests toolsandruff format --check src testsare clean apart from two pre-existing items, both confirmed present onmainand untouched: theregistry.py:64N818, and one formatter diff in apythonexample block intests/data/benchmark/README.md(at line 176 onmain; it moved to 178 only because the Layout table gained two rows). The CIruffandbuildchecks are green; thetest (3.12)/test (3.13)matrix was still running when this was written.Questions for review
How should the six band catalogs be obtained and stored? They are the one thing standing between this manifest and a runnable sweep.
[-180, -88.0, 180, -84.7](the union of all seven bands), committed as onecatalogs/cat_85_88s.parquet, with each band's map built by prefiltering it offline — exact for complete rings, per the README's own note that lat-ring maps rebuild offline "only because complete-ring lat-overlap is exact". One fetch, one file, but the file is ~40 MB.cat_88s.parquetis stored today. Consistent with the existing convention; ~100 MB of new binaries and ~2 h of CMR fetching.catalog_parquet-less path inbench_metrics.rebuild_shardmapalready supports this). Zero bytes added; the sweep is then non-deterministic across runs and re-fetches every time.My recommendation is (1): it is one artifact, it keeps the rebuild deterministic and offline the way the 88°S row already is, and the prefilter is exact for these ring AOIs. It still adds a ~40 MB binary, which is why it is a question rather than a commit.
Should the sector seams move off the 45° base-cell grid? The committed 88°S ring splits at multiples of 45°, which is exactly the geometry
espg/mortie#103mis-fills; the 2026-07-09 analysis found that offsetting the seams by 22.5° makes mortie's fill match point-sampled ground truth (1,560 vs 1,560 cells at 83°S) where the unoffset split returns 9,154. The generator keeps the 45° seams on purpose — matching the shipped ring is what makes the sweep comparable to the pinned row — but that means the whole family inherits the defect, and correcting it would re-pinhealpix_o9_88s/healpix_o10_88s. Re-pinning committed benchmark fixtures is a deliberate act, so it is left standing here.Does the sweep belong in its own manifest or in
targets.json'sprovisional_targets? This PR uses its own manifest (followingtargets_order_sweep_neon.json) to keep the drift check and manifest-consistency test's blast radius at zero. The alternative is adding the 14 rows toprovisional_targets, which would put them underbench_metrics.MANIFESTand give the pinned 88°S rows drift coverage in the standard place.A pre-existing inconsistency this work surfaced, not fixed.
targets.json'shealpix_o9_88scarries notemporaloverride, so it resolves to the top-level2018-10-13 .. 2026-03-15— but the entry also carriescatalog_parquet, andcatalogs/cat_88s.parquetwas fetched over2018-10-13 .. 2025-06-01. The offline rebuild path ignores the manifest window entirely, so the mismatch is latent rather than wrong today. This PR's manifest records the snapshot's window explicitly (andtest_the_sweep_temporal_window_matches_the_snapshot_that_pins_itpins it against the parquet's own metadata), buttargets.jsonis left alone — out of scope here, and worth its own look.