Add dependency graph correctness and performance gates - #70
Open
asinghvi17 wants to merge 3 commits into
Open
Conversation
Task G1. Production graph construction is unchanged; this adds the oracles and the harness a later builder swap needs, and uses them to answer the performance question PR #69 bypassed. The actual-cell oracle builds real cell geometry for both spaces and finds the pairs with genuine positive spherical intersection area, through the same kernel the conservative weight builder uses. No cap, no chunk extent and no chunk index takes part, so it is independent of everything the builder does. It runs over toy, raster, polar, antimeridian, nonuniform, complete/rooted/sparse DGG and cross-system pairs, at zero and nonzero support radius. `chunk_dependency_graph` holds every contributing pair in every case. The cap join is an exact identity only on the generic packed-R-tree index, a strict upper bound on the DGG hierarchical descent, and neither on the RasterGrid quadtree or the CopernicusDEM level-0 frontier, both of which hold pairs the cap join rejects. The tests pin all four. `benchmark/chunk_graph_gates.jl` reimplements the deleted latitude-sorted cap join as a second arm and reports plan/graph time, allocations, graph bytes and peak memory per case, stamped with the Julia version, thread count and the manifest's GeometryOps/GeometryOpsCore/ConservativeRegridding revisions. Verdict in regrid-notes/2026-08-23-g1-graph-oracles.md: the indexed builder is 5.7x slower than the latitude join on the production pair, and up to 60x per destination on a shallow raster source, so G2's performance gate fails on its literal wording. It is waived, not passed: 0.12 s against an 8.81 h run, in exchange for a relation a refcount can actually be derived from. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019DuGKTmvs5B4EynwZddKMg
Production graph construction is still untouched; every change here is to a gate, a harness, or the record. S1. The harness gate counted unchecked cases as passes: `oracle_missing` is -1 when the O(ncells^2) sweep is skipped, and `demand_missing` is 0 on the `:indexed` arm by construction, since post-#69 the builder issues exactly the `candidatechunks!` queries `demanded_pairs` replays. A run over nothing but skipped cases printed an unqualified PASS having verified nothing geometric. The summary now separates oracle-checked from skipped cases, names the skipped ones, says why `demand_missing` is not evidence on that arm, and prints `NOT CHECKED` rather than `PASS` when no case had a real verdict. G3 and G4 use this line to prove they did not change the relation, so it has to mean something. S1b. Of the 28 new assertions, 21 were one-directional (`truth subset graph`) and would pass on a builder returning the complete bipartite relation. Post-#69 the contract is an equality between the graph and the demanded relation; that equality now runs on all thirteen new cases in both suites, and on the four DGG-source cases the cap-join containment is asserted per case too, which retires the separate two-pair loop. S2. `contributing_pairs` was hand-copied three times and the copies had already diverged. All four relations now live once in `lib/GlobalRegridding/test/graphoracles.jl` (module `ChunkGraphOracles`), included by path from the GlobalRegridding suite, the root suite and the harness. It is the innermost of the three and both outer sites already depend on GlobalRegridding, so no dependency edge runs the wrong way; it follows the `test/helpers.jl` convention. S3. The nonzero-radius oracle was vertex-to-vertex, which under-approximates: two cells can lie within `radius` edge-to-edge with no vertex pair that close. Replaced with a cell-to-cell distance -- every vertex of one against every edge of the other, both directions, which is exact for rings with no positive intersection, the only case the branch is reached from. `raster-support`'s truth set grows 209 -> 222 pairs; all three arms hold all 222, so no builder was wrong, but the gate was 6% weaker than it read. S5. `benchmark/regridding_plan_baseline.jl`'s header documented the deleted builder's figures. It now states what the script measures today (326,064 edges, 0.229 s at t4) and marks the old numbers as superseded. Behaviour unchanged. S6. The note's "the latitude prefilter is worth 47x, any future cap-join-shaped builder must keep it" read as a live prescription. It is the deleted builder measured against itself in a configuration that ships nowhere. Reworded to past tense: a flat O(ndst x nsrc) cap scan is the defect and the band is a bandaid on it; the shipped builder needs none because pruning happens inside the source space's own index, and it beats the unprefiltered join by 8.2x while losing to the prefiltered one by 5.7x. S8. "The CopernicusDEM frontier crosses the cap join by 72 pairs" was asserted only in a comment and reproducible only from a local tile list. Reduced to a runnable CI window: the whole level-0 GLO-90 frontier into IGeo7 L3/chunk-2, 0.36 s, 6 pairs the index holds and the cap join rejects and 10 the other way. The 72 is now labelled harness-only where it appears. Harness arms: `:latjoin` stays and gains a sunset condition in the file header (it goes when G2's waiver is retired, or when the production builder stops being comparable to a cap join). `:latjoin_raw` is opt-in behind `DGG_GRAPH_GATE_RAW=1`. Also fixed the comment on "the dependency graph holds every pair the chunk index answers": post-#69 it compares `candidatechunks!` against a graph built from `candidatechunks!`, so it still catches a mis-assembled CSR but can no longer catch a wrong choice of index. The note was honest about this; the test was not. GlobalRegridding 3737 pass / 1 broken / 0 fail (3730 before; the seven are the new equality assertions). `crosssystem/regrid.jl` clean. The harness reproduces the note's section 3 unchanged apart from the raster-support truth count. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019DuGKTmvs5B4EynwZddKMg
Bookkeeping only; no work card is added or reworded beyond what has already happened. The checkpoint said "Current implementation phase: 1A", seven cards stale. A1 `47a7cf7`, A2 `41fb204`, A3 `a07de52`, A4 `e821c00`, B1/B2/B3 `714f101` + `93e836d`, B4 `d194e2d` and C1 `5e91b0c` have all landed, each under the commit subject its card names; the SHAs are now tabulated beside the cards they shipped. Current phase is 2. G2 is rewritten in place and closed. It landed early, out of order, as PR #69 (`da9e737`) -- a correctness fix, not the planned cutover, and therefore without either gate this card said to pass first. G1/PR #70 ran both retroactively: correctness PASSED, performance FAILED AND WAIVED at 5.7x (t8) / 6.1x (t4) and 1.6x allocations, waived on 0.12 s against an 8.81 h run in exchange for a relation a refcount can be derived from. It is not an implementation card and is not to be reopened. The dependency diagram gains a note that the real order was `C1 -> G2 -> G1 -> G3 -> G4`. G1's second action instructed a future agent to "treat the flat cap relation as an upper bound for raster/DGG native hierarchies". #70 measured and pinned the opposite: the generic index equals the cap join exactly, the DGG descent is a strict subset, and both the RasterGrid quadtree and the CopernicusDEM level-0 frontier cross it in both directions. Corrected, with both 2026-08-23 notes linked. G3 and G4 are amended lightly; their action lists were otherwise still accurate. G3 gains the fact that `chunkindex(src_space)` is rebuilt on every `chunk_dependency_graph` call (`discovery.jl:40`), so a per-column rebuild now costs a full index build and row views are worth strictly more than when the card was written. G4 absorbs the deletion of `connectedchunkpairs` (`discovery.jl:203-217`): after #69 it is line-for-line `_chunkgraph`'s row loop with one caller in `test_lazy.jl`, so the Phase 4 schedule for it moves forward to where it is free. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019DuGKTmvs5B4EynwZddKMg
This was referenced Aug 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Task G1 of
regrid-notes/2026-08-21-regridding-simplification-plan.md. Production graph construction is unchanged — this adds gates and instrumentation only, and then uses them to answer the performance question PR #69 bypassed.Record:
regrid-notes/2026-08-23-g1-graph-oracles.md.The oracles
contributing_pairs(dst, src; radius)builds real cell geometry for both spaces and returns the chunk pairs holding at least one cell pair with genuine positive spherical intersection area, through the same kernel the conservative weight builder uses. At nonzero support, cells withinradiusof each other count too — measured cell to cell, every vertex of one against every edge of the other in both directions. No cap, no chunk extent and no chunk index takes part in the construction, so it is independent of everything the builder does — which is what the existing candidate-relation tests are not.All four relations (
contributing_pairs,demanded_pairs,capjoin_pairs,graph_pairs) are defined once, inlib/GlobalRegridding/test/graphoracles.jl, and included by path from all three call sites.lib/GlobalRegridding/test/test_chunkgraph.jl— testset "no geometrically contributing pair is dropped": generic packed-R-tree source at radius 0 and 0.2, the shippedRasterGridquadtree at 0 and 0.1, nonuniform raster chunks, a polar source band, an antimeridian regional source.test/systems/crosssystem/regrid.jl— same oracle over real DGG geometry: complete IGeo7 L2/chunk-1 from IGeo7 L3/chunk-2 (all twelve pentagons, both poles), IGeo7 from S2, a rooted subtree destination, a scattered non-rooted subset, and a chunked raster source into a DGG destination at radius 0 and 0.05.Every case asserts
truth ⊆ graph,graph == demand, andtruth ⊆ capjoin. The equality is the one that matters for G3/G4: post-#69 the builder issues exactly thecandidatechunks!queriesdemanded_pairsreplays, so the contract is an equality and a containment alone would pass on a builder returning the complete bipartite relation. The cap-join assertion catches a chunk cap that is too tight, which is achunkextentsbug the graph would inherit.Brute-force cap identity, and where it stops
The card asks to treat the flat cap relation as an upper bound for native hierarchies. Measured, that is false, and the tests now pin the real picture:
_packedchunkindexDGGSpacevia_dggcandidatechunks!RasterGridquadtreeCopernicusDEMlevel-0 frontierThe harness
benchmark/chunk_graph_gates.jl, two arms by default: production:indexed, plus:latjoinreimplementing the latitude-sorted cap join deleted in #69 (verbatim fromba2bbfa^, private helpers copied in so it cannot drift).:latjoin_raw— the same with the prefilter off — is opt-in behindDGG_GRAPH_GATE_RAW=1. Reports complete plan time, graph time, graph bytes, allocations and peak memory, plus demand-domination and oracle columns, over small raster chunks, complete/rooted/sparse DGGs, polar and antimeridian cases, nonzero support, nonuniform coverage, and the production Copernicus→IGeo7 pair.The gate line distinguishes oracle-checked cases from skipped ones and prints
NOT CHECKED, neverPASS, for a run with no geometric verdict.:latjoincarries a sunset condition in the file header: it goes when G2's waiver is retired, or when the production builder stops being comparable to a cap join, whichever comes first.Every ndjson row is stamped with the Julia version, thread count, and the manifest's GeometryOps / GeometryOpsCore / ConservativeRegridding revisions and tree hashes — read at runtime, so a re-run after a pin bump relabels itself. The GeometryOps spherical-predicate work in flight will move clipping cost materially; these are pre-change baselines. It never downloads: the production case is skipped without a local tile list.
Verdict on G2's skipped performance gate
Correctness: PASS, on 9 oracle-checked cases; 3 too large for the
O(ncells²)sweep and reported as unchecked rather than as passes.Performance: FAIL on the literal wording, waived. Production pair, t8: indexed 0.1219 s vs latjoin 0.0213 s (5.7×); t4: 0.2289 s vs 0.0376 s (6.1×). Per destination cap on a shallow raster source it is worse — 60× at 4320×2160 with 162 chunks, which is #69's residual risk, measured. The ratio falls as the source side grows, so the indexed builder is the one that scales; it is losing a large constant factor. Against an 8.81 h run, 0.12 s buys a relation a refcount can be derived from (72 → 0 unheld demanded pairs) and 322 fewer edges.
The
:latjoinarm reproduces the archived baseline exactly — 66,175 × 26,475 chunks, 326,386 edges, 3,352,520-byte graph.What the oracles caught
Nothing wrong with the current builder. They do catch the deleted one on cases nobody had run it against: it misses demanded pairs on the raster path at every size (92 of 405 small, 8 of 6,640 at 162 chunks, 120 of 16,090 at 1,800, 72 of 326,064 in production) and on polar (25) and antimeridian (22) regional sources.
The latitude prefilter was worth 47× on the production pair — read as a diagnosis of the deleted builder, not a requirement on a future one. A flat
O(ndst × nsrc)cap scan is the defect; the band is a bandaid over it. The shipped builder needs none, and beats the unprefiltered join by 8.2× while losing to the prefiltered one by 5.7×.They also caught a weakness in themselves: the nonzero-support branch was vertex-to-vertex, which under-approximates. The cell-to-cell fix grew
raster-support's truth set 209 → 222 pairs — 13 contributing pairs ther > 0gate had not been demanding. All arms hold all 222, so no builder was wrong; the gate was 6% weaker than it read.Review fixes (
07d24ea)oracle_missing == -1on skip;demand_missing == 0on:indexedby construction). Now separates checked from skipped and refuses to printPASSfor a run that verified nothing.graph == demandnow runs on all thirteen new cases; the four DGG-source cases assertgraph ⊆ capjoinper case, retiring the separate loop.contributing_pairswas hand-copied three times and the copies had diverged. One definition inlib/GlobalRegridding/test/graphoracles.jl, included by all three sites.benchmark/regridding_plan_baseline.jl's header documented the deleted builder. Updated to what it measures today; behaviour unchanged.:latjoinkept with a sunset condition;:latjoin_rawbehindDGG_GRAPH_GATE_RAW=1.candidatechunks!against a graph built fromcandidatechunks!, so it can no longer catch a wrong choice of index.Plus one bookkeeping commit to
regrid-notes/2026-08-21-regridding-simplification-plan.md(c13eca9): checkpoint moved from phase 1A to 2 with the landed SHAs tabulated, G2 rewritten in place and closed, G1's "cap join is an upper bound" action corrected, G3/G4 lightly amended.Suites
GlobalRegridding 3737 pass / 1 broken / 0 fail (3730 before the review fixes; the seven are the new equality assertions. Baseline on this tip was 3702/1/0; the broken one is pre-existing).
test/systems/crosssystem/regrid.jlclean in 47 s, the new testset at 30 assertions and the cap-crossing window taking the demand testset from 7 to 9.crosssystem/runtests.jl5222 pass / 0 fail (untouched).🤖 Generated with Claude Code
https://claude.ai/code/session_019DuGKTmvs5B4EynwZddKMg