Skip to content

DGGSpace as a regridding source descends its own subtree (42x) - #62

Open
asinghvi17 wants to merge 7 commits into
mainfrom
claude/dgg-source-subtree
Open

DGGSpace as a regridding source descends its own subtree (42x)#62
asinghvi17 wants to merge 7 commits into
mainfrom
claude/dgg-source-subtree

Conversation

@asinghvi17

@asinghvi17 asinghvi17 commented Aug 21, 2026

Copy link
Copy Markdown
Member

Stack position: 1 of 4main#62 claude/dgg-source-subtree#63 claude/subzone-store#64 claude/copdem-production#65 claude/perf-ladder (draft)

This is a stacked PR chain. Each PR's base is the previous branch, so each diff shows only its own commits. Do not squash-merge out of order; merge bottom-up and retarget children after each merge.


What

A DGGSpace used as a regridding source no longer falls back to a generic cap tree over its linear index range. It descends its own subtree, through a new grid seam.

Measured on the benchmarked path: ≈42×, bit-identical output.

Why

GR.subtree(::DGGSpace, inds) had two fast paths and both were shut for a Copernicus DEM source — _iswholespace is false for a chunk, and _chunkcursor requires treeify(grid) to be a HierarchicalGridCursor, which a CopernicusDEMSystem grid's is not. Every block build therefore got a GR.CellCapTree, which bisects the linear index range. With row-major pixel order, every node above about depth 11 of 17 is a band of complete pixel rows spanning the tile's whole degree of longitude, and its cap stays ~200 km wide however thin the band. Nothing prunes on longitude, and the dual-tree candidate search re-descends the destination at every one of those surplus nodes. Against a RasterGrid over the same pixels, get_all_candidate_pairs cost 228× more for the same clipping work.

The four commits

  • acf6909 — pin ConservativeRegridding and GeometryOps to SHAs, not branch names. Both [sources] revisions had moved under main: claude/budget-frontier resolved to 89ec5ca5, which predates Trees.split_weight and so cannot precompile GlobalRegridding; as/intersection_area resolved to 02750768, which predates intersection_area itself and fails at the first weight block. A fresh instantiate of main hits both today. Pinned to the revisions the campaigns actually measured against: CR 6a4b997, GO 2825c476. Still temporary pins (regrid-notes/cr-pin-state, go-pin-state still apply) — but a SHA cannot move. Also makes bench a workspace member sharing the root Manifest, with the Zarr dependency the store work needs.

  • 4d55dd4 (F1)DGG.subcursor(grid, inds) -> tree | nothing, a new unexported grid seam (src/interface/grid.jl), tried by GR.subtree(::DGGSpace, inds) before _chunkcursor. Default is nothing; a hierarchical grid needs no method, since regridding already descends to the chunk's ancestor. CopernicusDEM implements it by reusing _block_cursor's run-to-rectangle test over the window rather than over the whole grid. A BlockCursor node is an index rectangle whose cap is an exact O(1) box and whose Bisected split takes the longer axis — the same shape RasterCellTree gives the raster arm, so this closes the gap rather than narrowing it. Ranges that are not one lattice rectangle keep the cap-tree fallback.

    The seam deliberately takes the grid, not the root tree — the only shape that works for PartialGrid sources, where the whole-grid tree is a HierarchicalGridCursor.

    Sparse sources work out of the box: DGGSpace(PartialGrid(land tiles); chunklevel=0) gives one chunk per held tile and windows normally. Verified with pole, band-edge and equatorial tiles; matrices equal the CellCapTree arm and the full lazy regrid is bit-identical.

  • ecd4f34 (F4)CapCachedTree gains an index offset (caps[p - offset], offset = 0 whole-space, first(inds) - 1 for a chunk), so the same wrapper serves a chunk cursor. Previously a chunk's _chunkcursor handed the dual-tree search a bare cursor whose node_extent and child_indices_extents both re-derived every cell cap from Fallbacks.cell_cap → cell_boundary — an inverse projection per cell, once per opposing node. On the Copernicus source A/B that destination-side recomputation was 43 % of the whole block build. subtree wraps chunk cursors up to _CHUNK_CAP_CACHE_MAX = 2^16; above that the fill costs more than the revisits it saves. Caps, leaf entries and cells are the raw cursor's bit for bit, and the tests check that against _chunkcursor directly.

  • 660640f (F2)_mergecaps folds the smallest cap containing two caps (the same construction GO.UnitSpherical._merge and DGG.Fallbacks.merge_caps use) instead of centring on the mean of its inputs' centres and taking max(distance + radius). The old form inflates a node by the spread of its children, and _cellcapnode then merges child extents bottom-up, so the inflation compounds level by level. On one 1200×1200 GLO-90 tile at 45°N: root cap 274.9 → 94.6 km (true 68.1 km), depth-3 median 224.2 → 45.9 km, depth-11 maximum 203.9 → 39.4 km. Caps only prune, so this is a speed change, not a semantic one — and it is the fallback every system without a windowed cursor still takes.

Numbers

At -t 8, load 19–20:

case before after
A/B box:10,10.05,45,45.05, IGeo7 L12, ancestor 7 61 s (cap-tree fallback) 1.39–1.50 s (≈42×, 33.6–36.2k cells/s)
same A/B, forced cap-tree arm (F4 then F2) 61.9 → 32.8 → 13.0 s
zarr-spike run-D region 234.2 s 7.6–7.8 s

Raster path unchanged (179–188 s against 183.6 s recorded).

Tests

Full suite 987,238 pass / 17 broken / 0 fail (+76 new assertions); GlobalRegridding 2,202 pass.

Known gotcha recorded in the notes: a chunk tree's Trees.ncells answers for the chunk while its leaves hold global positions, so weights only flow through a block build's index maps — the chunk-level test is checktree, not a matrix comparison.

Notes

Full record: regrid-notes/2026-08-20-copdem-zarr-spike.md §13.

Deferred there and not in this PR: D1's remaining ~39 km cap floor (the split is on the linear index range, so a shallow node is a band of complete pixel rows spanning the tile's full degree of longitude — a separate change), plus F3/F5/F6/F7 (§13.4).

🤖 Generated with Claude Code

https://claude.ai/code/session_01AC7rbi9eqZypWseMjBGSJ2

asinghvi17 and others added 4 commits August 20, 2026 14:14
Both `[sources]` revisions had moved under us. `claude/budget-frontier`
resolved to `89ec5ca5` ("Bump version from 0.2.8 to 0.2.9"), which predates
`Trees.split_weight` and so cannot precompile GlobalRegridding;
`as/intersection_area` resolved to `02750768` ("Bump patch version to
v0.1.44"), which predates `intersection_area` itself and fails at the first
weight block with `UndefVarError: intersection_area not defined in
GeometryOps`. A fresh instantiate of main hits both.

Pin the two revisions the campaigns actually measured against:
CR `6a4b997` and GO `2825c476`. They are still temporary — the note at
`regrid-notes/cr-pin-state` still applies — but a SHA cannot move.

Also make `bench` a workspace member so it shares the root Manifest, and
give it the Zarr dependency the store work needs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AC7rbi9eqZypWseMjBGSJ2
`GR.subtree(::DGGSpace, inds)` had two fast paths and both were shut for a
Copernicus DEM source: `_iswholespace` is false for a chunk, and
`_chunkcursor` requires `treeify(grid)` to be a `HierarchicalGridCursor`,
which a `CopernicusDEMSystem` grid's is not. So every block build got a
`GR.CellCapTree`, which bisects the linear index range: with row-major pixel
order every node above about depth 11 of 17 is a band of complete pixel rows
spanning the tile's whole degree of longitude, and its cap stays ~200 km wide
whatever the band's height. Nothing prunes on longitude, and the dual-tree
candidate search re-descends the destination at every one of those surplus
nodes. Measured against a `RasterGrid` over the same pixels:
`get_all_candidate_pairs` cost 228x more for the same clipping work.

Add `subcursor(grid, inds)`, a grid seam returning the tree restricted to a
run of grid positions, or `nothing`. The default is `nothing`; a hierarchical
grid needs no method, since regridding already descends to the chunk's
ancestor. `CopernicusDEM` implements it by reusing `_block_cursor`'s
run-to-rectangle test over the window rather than over the whole grid, which
also makes the test independent of whether the grid is complete: a partial
holding of scattered tiles -- what Copernicus actually ships, land only --
gets the generic cursor for the whole grid and a windowed `BlockCursor` for
each of its per-tile chunks. Ranges that are not one lattice rectangle keep
the cap-tree fallback.

A `BlockCursor` node is an index rectangle whose cap is an exact O(1) box and
whose `Bisected` split takes the longer axis, which is what `RasterCellTree`
does for the raster arm, so this closes the gap rather than narrowing it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AC7rbi9eqZypWseMjBGSJ2
`CapCachedTree` existed only for the whole space: a chunk's `_chunkcursor`
handed the dual-tree search a bare cursor, whose `node_extent` at a leaf and
whose `child_indices_extents` both re-derive every cell cap from
`Fallbacks.cell_cap` -> `cell_boundary`, an inverse projection per cell, once
per opposing node. On the Copernicus source A/B that destination-side
recomputation was 43 % of the whole block build, because the destination tree
is re-descended once per source node.

Carry an index offset in `CapCachedTree` so the same wrapper serves a chunk:
position `p` reads `caps[p - offset]`, with `offset = 0` for the whole space
and `first(inds) - 1` for a chunk. `subtree` wraps chunk cursors up to
`_CHUNK_CAP_CACHE_MAX` cells; above that the fill costs more than the revisits
it saves. On the lazy path the destination reaches this through `TileCells`,
which keeps one tree per tile, so the caps are filled once and read by every
source chunk paired with it.

Caps, leaf entries and cells are the raw cursor's, bit for bit; the tests
check that against `_chunkcursor` directly.

Measured on the box:10,10.05,45,45.05 / IGEO7 L12 / ancestor 7 A/B, t8:
the windowed-cursor arm 1.5 -> 1.4 s, and the forced cap-tree arm 61.9 ->
32.8 s, which is the 43 % showing up where it was diagnosed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AC7rbi9eqZypWseMjBGSJ2
`_mergecaps` centred on the mean of its inputs' centres and took
`max(distance + radius)`. That covers, but a node is inflated by the spread of
its children and `_cellcapnode` then merges child EXTENTS bottom-up, so the
inflation compounds level by level. Measured on one 1200x1200 GLO-90 tile at
45 deg N: root cap 274.9 km against a true 68.1 km, and a maximum still above
70 km at depth 17 of 17. Every factor of two in radius is about two levels of
pruning depth.

Fold the smallest cap containing two caps instead — the same construction
`GO.UnitSpherical._merge` and `DGG.Fallbacks.merge_caps` use — so an internal
node is minimal given its children rather than minimal-plus-spread. Same tile,
same tree: root 94.6 km, depth 3 median 224.2 -> 45.9 km, depth 11 maximum
203.9 -> 39.4 km. What is left at ~39 km is the other defect: the split is on
the linear index range, so a shallow node is a band of complete pixel rows
spanning the tile's full degree of longitude, 39.4 km wide at 45 deg N however
thin the band. That is a separate change.

Caps only prune, so this is a speed change and not a semantic one: the
Copernicus A/B's forced-cap-tree arm goes 32.8 -> 13.0 s at t8 and its output
stays bit-identical to the windowed-cursor arm's. It is the fallback every
system without a windowed cursor still takes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AC7rbi9eqZypWseMjBGSJ2
Comment thread lib/GlobalRegridding/src/conservative.jl Outdated
Comment thread lib/GlobalRegridding/test/test_conservative.jl Outdated
Comment thread src/interface/grid.jl Outdated
Comment thread src/cap_cached_tree.jl Outdated
Comment thread src/cap_cached_tree.jl Outdated
Comment thread src/regridding.jl Outdated
Comment thread test/systems/CopernicusDEM/runtests.jl Outdated
asinghvi17 and others added 3 commits August 21, 2026 08:04
`caps` is now indexed by raw grid position: the whole-space tree passes a plain
Vector, a chunk's tree passes _ShiftedCaps, an offset-indexed AbstractVector
whose axes are the chunk's positions. Bounds-checked, @propagate_inbounds.

Also spells the chunk cap-cache limit 2^16.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Claude-Session: https://claude.ai/code/session_01AC7rbi9eqZypWseMjBGSJ2
…block

All five [sources] blocks on this branch pinned GeometryOps at
2825c476647cfe6791dbb89fffb8592697b46995, two commits behind the tip of
GO's `claude/perf-ladder-predicates` (GO PR #476).  PR #65 further up the
stack had already moved the ROOT block to 36c853e0 and left the other
four behind, so merging the stack forward produced a workspace whose
members disagree on which GeometryOps they want.  Move all five to the
tip so a fresh resolve of #62-#64 (and of #65 after the merge) sees one
GeometryOps.

  GeometryOps  2825c476647cfe6791dbb89fffb8592697b46995
            -> 36c853e0de865251e1395ff01e9d34cda9e9bfa8
  in Project.toml, docs/, test/, lib/GlobalRegridding/,
     lib/DiscreteGlobalGridsConformanceTesting/

The two commits in between are the `spherical_orient` fast path and the
`SphericalCap` squared-chord intersection - perf only, bit-identical
results, measured during the perf-ladder campaign.

ConservativeRegridding stays at 6a4b997ab2e66dea45b5b62b5b2f32f0a3b279b0,
re-checked rather than changed: it is the current tip of CR
`claude/budget-frontier`, reachable from that remote ref, and its tree
does define `Trees.split_weight` (src/trees/interfaces.jl:52).  The
"a fresh resolve loses split_weight" report does not reproduce - the
fresh depot below resolved and precompiled it.

Evidence, throwaway depot, sharing nothing with ~/.julia:

  JULIA_DEPOT_PATH=<tmp> julia --project=. -e \
    'using Pkg; Pkg.instantiate(); Pkg.precompile("DiscreteGlobalGrids")'

resolved both git sources from scratch (bare clones fetched fresh), wrote
a Manifest carrying repo-rev 36c853e0.../6a4b997... beside Pkg's own
git-tree-sha1, precompiled 92 dependencies including GeometryOps,
ConservativeRegridding, GlobalRegridding and DiscreteGlobalGrids, and
`isdefined(GeometryOps, :intersection_area)` and
`isdefined(ConservativeRegridding.Trees, :split_weight)` both hold.

Targeted suites on the new pins (julia -t 4):

  GlobalRegridding                        2202 pass /  1 broken / 0 fail
  crosssystem/regrid.jl                    116 pass /  0 broken / 0 fail
  crosssystem/regridding_conservation.jl    76 pass / 12 broken / 0 fail
  systems/CopernicusDEM                  16162 pass /  3 broken / 0 fail
  crosssystem/regrid_acceptance.jl          22 pass /  0 broken / 0 fail

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AC7rbi9eqZypWseMjBGSJ2
asinghvi17 added a commit that referenced this pull request Aug 21, 2026
Brings up the [sources] fix from PR #62: GeometryOps
2825c476647cfe6791dbb89fffb8592697b46995 -> 36c853e0de865251e1395ff01e9d34cda9e9bfa8
in all five blocks, so a fresh instantiate of this branch resolves one
GeometryOps.  ConservativeRegridding stays at 6a4b997....

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AC7rbi9eqZypWseMjBGSJ2
asinghvi17 added a commit that referenced this pull request Aug 21, 2026
Brings up the [sources] fix from PR #62 through #63: GeometryOps
2825c476647cfe6791dbb89fffb8592697b46995 -> 36c853e0de865251e1395ff01e9d34cda9e9bfa8
in all five blocks, so a fresh instantiate of this branch resolves one
GeometryOps.  ConservativeRegridding stays at 6a4b997....

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AC7rbi9eqZypWseMjBGSJ2
@asinghvi17

Copy link
Copy Markdown
Member Author

Fixed the [sources] pins so a fresh instantiate of this stack works.

GeometryOps 2825c476647cfe6791dbb89fffb8592697b4699536c853e0de865251e1395ff01e9d34cda9e9bfa8 (tip of GO claude/perf-ladder-predicates, GO PR #476), in all five blocks — root, docs/, test/, lib/GlobalRegridding/, lib/DiscreteGlobalGridsConformanceTesting/. The five were previously consistent at 2825c476, but #65 had already moved only the root block to 36c853e0, so merging the stack forward gave a workspace whose members disagreed on the GeometryOps rev. The two commits in between are the spherical_orient fast path and the SphericalCap squared-chord intersection: perf only, bit-identical, measured during the perf-ladder campaign.

ConservativeRegridding stays at 6a4b997ab2e66dea45b5b62b5b2f32f0a3b279b0 — re-checked, not changed. It is the current tip of CR claude/budget-frontier, reachable from that remote ref, and its tree does define Trees.split_weight (src/trees/interfaces.jl:52). The "a fresh resolve loses split_weight" report did not reproduce.

Fresh-instantiate evidence, throwaway depot sharing nothing with ~/.julia (both git sources cloned from scratch):

JULIA_DEPOT_PATH=<tmp> julia --project=. -e \
  'using Pkg; Pkg.instantiate(); Pkg.precompile("DiscreteGlobalGrids")'

92 dependencies precompiled, including GeometryOps, ConservativeRegridding, GlobalRegridding and DiscreteGlobalGrids; the generated Manifest carries repo-rev = 36c853e0… / 6a4b997… beside Pkg's own git-tree-sha1; and both isdefined(GeometryOps, :intersection_area) and isdefined(ConservativeRegridding.Trees, :split_weight) hold. (No Manifest is tracked in this repo — .gitignore line 2 — so the fix is Project.toml only.)

Targeted suites on the new pins, julia -t 4:

suite pass broken fail
lib/GlobalRegridding (full) 2202 1 0
test/systems/crosssystem/regrid.jl 116 0 0
test/systems/crosssystem/regridding_conservation.jl 76 12 0
test/systems/CopernicusDEM/runtests.jl 16162 3 0
test/systems/crosssystem/regrid_acceptance.jl 22 0 0

Pushed here as 1183e6f, then merged forward (no rebases, no force-pushes): claude/subzone-store55e676d (#63), claude/copdem-productione08860f (#64). claude/perf-ladder (#65) is untouched; its root block already pins 36c853e0, so that merge stays trivial.

asinghvi17 added a commit that referenced this pull request Aug 21, 2026
Three review-fix commits on top of e2f90d1:

  - the _wavesize threshold becomes a keyword and its docstring shrinks to
    what the function does (lib/GlobalRegridding/src/lazy.jl, shared.jl);
  - the LeafCells docstring drops its provenance narrative
    (src/systems/CopernicusDEM/cursor.jl);
  - the MemoBlockCursor extent cache is explained as a mechanism
    (src/systems/CopernicusDEM/cursor_memo.jl).

No overlap with the perf-ladder tip; merged without conflict.
asinghvi17 added a commit that referenced this pull request Aug 22, 2026
…block

All five [sources] blocks on this branch pinned GeometryOps at
2825c476647cfe6791dbb89fffb8592697b46995, two commits behind the tip of
GO's `claude/perf-ladder-predicates` (GO PR #476).  PR #65 further up the
stack had already moved the ROOT block to 36c853e0 and left the other
four behind, so merging the stack forward produced a workspace whose
members disagree on which GeometryOps they want.  Move all five to the
tip so a fresh resolve of #62-#64 (and of #65 after the merge) sees one
GeometryOps.

  GeometryOps  2825c476647cfe6791dbb89fffb8592697b46995
            -> 36c853e0de865251e1395ff01e9d34cda9e9bfa8
  in Project.toml, docs/, test/, lib/GlobalRegridding/,
     lib/DiscreteGlobalGridsConformanceTesting/

The two commits in between are the `spherical_orient` fast path and the
`SphericalCap` squared-chord intersection - perf only, bit-identical
results, measured during the perf-ladder campaign.

ConservativeRegridding stays at 6a4b997ab2e66dea45b5b62b5b2f32f0a3b279b0,
re-checked rather than changed: it is the current tip of CR
`claude/budget-frontier`, reachable from that remote ref, and its tree
does define `Trees.split_weight` (src/trees/interfaces.jl:52).  The
"a fresh resolve loses split_weight" report does not reproduce - the
fresh depot below resolved and precompiled it.

Evidence, throwaway depot, sharing nothing with ~/.julia:

  JULIA_DEPOT_PATH=<tmp> julia --project=. -e \
    'using Pkg; Pkg.instantiate(); Pkg.precompile("DiscreteGlobalGrids")'

resolved both git sources from scratch (bare clones fetched fresh), wrote
a Manifest carrying repo-rev 36c853e0.../6a4b997... beside Pkg's own
git-tree-sha1, precompiled 92 dependencies including GeometryOps,
ConservativeRegridding, GlobalRegridding and DiscreteGlobalGrids, and
`isdefined(GeometryOps, :intersection_area)` and
`isdefined(ConservativeRegridding.Trees, :split_weight)` both hold.

Targeted suites on the new pins (julia -t 4):

  GlobalRegridding                        2202 pass /  1 broken / 0 fail
  crosssystem/regrid.jl                    116 pass /  0 broken / 0 fail
  crosssystem/regridding_conservation.jl    76 pass / 12 broken / 0 fail
  systems/CopernicusDEM                  16162 pass /  3 broken / 0 fail
  crosssystem/regrid_acceptance.jl          22 pass /  0 broken / 0 fail

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AC7rbi9eqZypWseMjBGSJ2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant