Skip to content

Commit 4eed4f8

Browse files
author
Mike Johnson
committed
Fix add_measures join: key divide area by flowpath_id, not divide_id
add_measures joined incremental catchment area onto flowpaths with `by = c("flowpath_id" = "divide_id")`, assuming the legacy 1:1 convention where divide_id == flowpath_id. In the current schema these differ (e.g. cat-* vs fp-*), so the join matched nothing: areasqkm came back NA and was forced to 0, which then propagated through accumulate_downstream to make total_dasqkm uniformly 0 in flowpath/network outputs. Prefer the explicit divide->flowpath link (`flowpath_id` on divides) when present; fall back to the legacy divide_id join otherwise. Verified on ngen_vpu09: 22977/22989 flowpaths now receive correct areas (the remaining 12 are genuinely divide-less). Adds regression tests for both join paths.
1 parent a706041 commit 4eed4f8

2 files changed

Lines changed: 54 additions & 4 deletions

File tree

R/utils.R

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,22 @@ add_measures <- function(flowpaths, divides) {
4747
flowpaths$lengthkm <- add_lengthkm(flowpaths)
4848
divides$areasqkm <- add_areasqkm(divides)
4949
flowpaths$areasqkm <- NULL
50-
flowpaths <- dplyr::left_join(flowpaths,
51-
dplyr::select(sf::st_drop_geometry(divides), divide_id, areasqkm),
52-
by = c("flowpath_id" = "divide_id")
53-
)
50+
div_tab <- sf::st_drop_geometry(divides)
51+
# Join incremental catchment area onto flowpaths. Prefer the explicit
52+
# divide->flowpath link (`flowpath_id` on divides), required by the current
53+
# schema where divide_id != flowpath_id (e.g. cat-* vs fp-*); joining on
54+
# divide_id there matches nothing and silently zeroes areasqkm (and any
55+
# downstream-accumulated total area). Fall back to the legacy 1:1 convention
56+
# (divide_id == flowpath_id) when divides carry no flowpath_id.
57+
if ("flowpath_id" %in% names(div_tab)) {
58+
flowpaths <- dplyr::left_join(
59+
flowpaths, dplyr::select(div_tab, flowpath_id, areasqkm),
60+
by = "flowpath_id")
61+
} else {
62+
flowpaths <- dplyr::left_join(
63+
flowpaths, dplyr::select(div_tab, divide_id, areasqkm),
64+
by = c("flowpath_id" = "divide_id"))
65+
}
5466
list(
5567
flowpaths = rename_geometry(flowpaths, "geometry"),
5668
divides = rename_geometry(divides, "geometry")

tests/testthat/test-add-measures.R

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
test_that("add_measures joins divide area by flowpath_id when divide_id != flowpath_id", {
2+
skip_if_not_installed("sf")
3+
fp <- sf::st_sf(
4+
flowpath_id = c("fp-1", "fp-2"),
5+
geometry = sf::st_sfc(
6+
sf::st_linestring(matrix(c(0, 0, 1000, 1000), ncol = 2, byrow = TRUE)),
7+
sf::st_linestring(matrix(c(1000, 1000, 2000, 2000), ncol = 2, byrow = TRUE))),
8+
crs = 5070)
9+
# cat-* divides carry an explicit flowpath_id link; divide_id != flowpath_id
10+
div <- sf::st_sf(
11+
divide_id = c("cat-1", "cat-2"),
12+
flowpath_id = c("fp-1", "fp-2"),
13+
geometry = sf::st_sfc(
14+
sf::st_polygon(list(matrix(c(0,0, 1000,0, 1000,1000, 0,1000, 0,0), ncol = 2, byrow = TRUE))),
15+
sf::st_polygon(list(matrix(c(0,0, 2000,0, 2000,2000, 0,2000, 0,0), ncol = 2, byrow = TRUE)))),
16+
crs = 5070)
17+
res <- add_measures(fp, div)
18+
expect_true(all(res$flowpaths$areasqkm > 0)) # not zeroed by a bad join
19+
expect_equal(res$flowpaths$areasqkm[2] / res$flowpaths$areasqkm[1], 4, tolerance = 1e-6)
20+
})
21+
22+
test_that("add_measures falls back to legacy divide_id == flowpath_id", {
23+
skip_if_not_installed("sf")
24+
fp <- sf::st_sf(
25+
flowpath_id = c("1", "2"),
26+
geometry = sf::st_sfc(
27+
sf::st_linestring(matrix(c(0, 0, 1000, 1000), ncol = 2, byrow = TRUE)),
28+
sf::st_linestring(matrix(c(1000, 1000, 2000, 2000), ncol = 2, byrow = TRUE))),
29+
crs = 5070)
30+
div <- sf::st_sf( # no flowpath_id column -> legacy join on divide_id
31+
divide_id = c("1", "2"),
32+
geometry = sf::st_sfc(
33+
sf::st_polygon(list(matrix(c(0,0, 1000,0, 1000,1000, 0,1000, 0,0), ncol = 2, byrow = TRUE))),
34+
sf::st_polygon(list(matrix(c(0,0, 2000,0, 2000,2000, 0,2000, 0,0), ncol = 2, byrow = TRUE)))),
35+
crs = 5070)
36+
res <- add_measures(fp, div)
37+
expect_true(all(res$flowpaths$areasqkm > 0))
38+
})

0 commit comments

Comments
 (0)