Single-pass patch vectorization (fixes OOM on large-extent rasters)#29
Merged
NewGraphEnvironment merged 5 commits intoJul 7, 2026
Merged
Conversation
dft_transition_vectors() looped over transition classes, allocating full-grid vectors per class and two full-grid logicals per patch — ncell x n_patches scaling that OOM-killed a 102.6M-cell, 56-class floodplain (1.87% non-NA; trim() useless when data follows the mainstem across the whole bbox). Replace the loop with terra::patches(x, directions = 8, values = TRUE) — one C++ pass computing 8-connected components of same-valued cells — then as.polygons() and a sparse pid-to-label map that touches only the non-NA cells. Output verified identical on fixtures (185 patches, 123.11 ha, 57 at patch_area_min = 1000; pinned as a regression test captured from the old implementation). Benchmark at 24M cells: 1.9 s vs 122 s. patch_id numbering is now scan-order. terra floor >= 1.8-10: earlier versions falsely connected patches across the left/right raster edges with values = TRUE. Fixes #27 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Relates to #27 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
Summary
dft_transition_vectors()on large-extent rasters: the per-class loop allocated full-grid vectors per class and two full-grid logicals per patch (ncell × n_patches scaling — ~18+ GB extrapolated on the 102.6M-cell UFRA floodplain). Replaced with a singleterra::patches(x, directions = 8, values = TRUE)pass plus a sparse patch→label map that touches only the ~2% non-NA cells.patch_area_min = 1000) were captured from the old code before the rewrite. Onlypatch_idnumbering / row order changes (now raster scan order).>= 1.8-10: older versions had an edge-wraparound bug inpatches(values = TRUE)that silently merged patches touching opposite raster edges.dft_rast_transition()has the same disease (6+ full-grid vectors incl. two character vectors) and needs its own refactor.Fixes #27
Related Issues
Test plan
devtools::test(): 211 pass, 0 fail — all 10 pre-existing tests unchanged (behavior contract), plus new brute-force decomposition test (engineered topology: diagonal merges, class boundaries, crossed diagonals, code-0 class, holes), all-NA edge case, and fixture regression guardlintrclean on touched files (one pre-existing vignette lint, untouched)/code-checkclean on each commitNotes
Approach was selected empirically during planning — the issue's tiling and sparse-window options benchmarked worse (sparse-window degenerates on mainstem-shaped data where every class's bounding window spans the grid) and
as.polygons+explode was disqualified (GDAL polygonize is 4-connected, silently splits diagonal joins). Full analysis inplanning/archive/2026-07-issue-27-transition-vectors-oom/.🤖 Generated with Claude Code