Skip to content

Fix compute_spillfield_graph dropping isolated trap-bottom vertices - #11

Merged
Oddan merged 1 commit into
SUrbAreafrom
fix-compute-spillfield-graph-missing-vertices
May 23, 2026
Merged

Oddan merged 1 commit into
SUrbAreafrom
fix-compute-spillfield-graph-missing-vertices

Conversation

@Oddan

@Oddan Oddan commented May 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • compute_spillfield_graph was filtering out self-loops (correct) but not compensating with add_vertices!, so isolated cells — concretely, trap bottoms entirely surrounded by masked building cells — were silently missing as vertices from the returned graph.
  • Added Graphs.add_vertices!(g, xmax * ymax - Graphs.nv(g)) before the return, exactly matching the pattern already used in the spillregions flowgraph construction.

Background

_spillfield_flow_edges! intentionally adds self-loop edges (n, n) for trap-bottom and boundary-exit cells that have no inflow, so that _determine_connected_components can include them in watershed regions. Callers that build a final SimpleDiGraph are expected to strip those self-loops and then pad the vertex count. spillregions did this correctly; compute_spillfield_graph did the stripping but skipped the padding.

Test plan

  • Existing test suite passes (Pkg.test())
  • Manually verify on a grid with building-masked cells adjacent to a trap bottom that the returned graph vertex count equals the number of grid cells

🤖 Generated with Claude Code

Isolated cells (e.g. trap bottoms entirely surrounded by masked building
cells) produce no flow edges and were silently absent from the returned
graph. Add the same add_vertices! call that spillregions already uses to
guarantee every grid cell is represented as a vertex.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@Oddan
Oddan changed the base branch from main to SUrbArea May 23, 2026 14:16
@Oddan
Oddan merged commit 4b2325e into SUrbArea May 23, 2026
0 of 2 checks passed
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