Skip to content

Single-WSG downstream-state guard: fail loud when downstream dams aren't persisted #227

Description

@NewGraphEnvironment

Status

The public.wsg_outlet half of this issue is resolved — by removal, not repair.

NewGraphEnvironment/fresh#214[email protected] rebuilt frs_wsg_drainage() on per-group outlet points (blue_line_key + downstream_route_measure) tested with the measure-aware whse_basemapping.fwa_downstream(). Outlets now ship with fresh as inst/extdata/wsg_outlet.csv (246 rows, generator at data-raw/wsg_outlet.R) and reach the database as a VALUES list, so no table is required in the target database — only the fwa_downstream() routine. There is nothing left to materialize ad-hoc and nothing to lose on a docker volume rebuild. Adopted here in [email protected] (#238); the closure-mode tests in test-lnk_wsg_resolve.R pass again.

Worth recording: the old ad-hoc derivation was also wrong, and the missing table was masking it. Selecting on nlevel(wscode_ltree) ASC picked the shallowest code appearing anywhere in a group, including slivers of neighbouring systems — MORR (Morice) clips a 2-segment, 0 km, order-1 piece of the Bulkley-coded line 400.431358 alongside 1,236 segments and 275 km of the order-8 Morice line, so the Bulkley appeared to drain through the Morice. Province-wide, 62/246 groups resolved to an over-shallow outlet and 14 Fraser groups collapsed onto 100, where the alphabetical tiebreak put LFRA seventh. Closures are now tighter and correct: PARS + BULK resolves to 9 WSGs, not 15.

What remains is the second half: the single-WSG downstream-state guard.

Reconciled on merge (v0.46.0, #245). Building this disproved part of the design
below by measurement. The corrections are marked inline; the problem statement and the
three-tier shape held, but the tier-1 predicate, the dam-location join, and the
persistence check were all wrong as specified
, and the study_area_run.sh acceptance
criterion was unachievable as written. Left in place rather than rewritten, so the
reasoning is visible.

The problem

data-raw/wsg_run_one.R states its own precondition in the header (lines 9–12):

Run the WSGs of a drainage DS-first (most-downstream first) so a WSG's downstream dam barriers are already persisted when its access / mapping_code is computed — that is what makes cross-WSG ;DAM appear without any post-consolidate recompute.

Nothing enforces it. Accessibility is not computed from the focal WSG alone — link reads the already-persisted barriers of the WSGs downstream. Run Rscript wsg_run_one.R BULK against an empty or partial persist schema and the access query finds no downstream dam barriers, because LSKE / KISP / KLUM have not been run. It concludes nothing is blocking, writes streams_access and streams_mapping_code with segments marked accessible that are in fact dammed off, prints done in N min, and exits 0.

No error, no warning — a wrong answer indistinguishable from a right one. It surfaced during #223 validation only because the full provincial state happened to pre-exist.

Note the script already carries a defensive skip for species presence (#157), commented "belt-and-suspenders." The far more consequential precondition has no equivalent.

Why this is buildable now and wasn't before

A guard is only as good as the closure it checks against. Under the old predicate, BULK's closure wrongly contained MORR — so the guard would have demanded the Morice be persisted before the Bulkley, blocking correct runs with false alarms. Nobody would have left it enabled. With [email protected] the closure is exact and validated (12 hydrology fixtures, 0 transitivity violations across 3,537 checks), so the guard can be strict without crying wolf.

Cost of the naive answer

"Just run the downstream WSGs first" is not cheap. wsg_run_one.R runs the full pipeline per group — every stream, habitat classification for every species — not a mainstem-only pass. Measured from this repo's own run logs (47 WSG runs): median 113 s, mean 112 s, max 252 s per WSG. Twenty downstream groups is ~40 minutes, and those logs predate #223, which grew segment counts 2–3.5×.

But the information the guard needs is cheap. The question is only "is there a blocking dam on the path downstream" — a point query, not a modelling run. A spatial join of cabd.dams to whse_basemapping.fwa_watershed_groups_poly runs province-wide in 2.4 s.

Corrected. fwa_watershed_groups_poly is unusable here — its code columns are NULL on docker fwapg (#222). The shipped probe takes watershed_group_code from the snapped fwa_stream_networks_sp row, which is both what .lnk_pipeline_prep_dams itself filters on and ~8× faster (~0.3 s).

Proposed design — three tiers, not a boolean

  1. Auto-pass. Resolve the downstream closure, check for blocking dams in those groups. None → nothing downstream can block → proceed silently. This is the common case, and it verifies rather than asks the operator to assert.

    Corrected. That is a membership test and it cries wolf. Measured live: BULK's closure holds 18 blocking dams across LSKE/KISP/KLUM and none of them sit below its outlet — so the guard would fire on this issue's own motivating example, operators would learn to reach for the override, and it would stop meaning anything. Shipped instead as a path test: each dam checked with the measure-aware whse_basemapping.fwa_downstream() from the outlet in fresh::frs_wsg_outlets(). PARS → its three real dams (Peace Canyon, Site C, W.A.C. Bennett); SLOC → Brilliant Dam; BULK → none. Complete rather than merely cheaper: access walks downstream from every segment and every focal segment exits through the focal outlet, so the out-of-WSG barriers reachable from any focal segment are exactly those below it. ~0.5 s. Recorded in RUNBOOK.md §8c so it is not simplified back.

  2. Fail loud. Blocking dams exist downstream and those groups are not persisted → stop, naming the specific WSGs and dams. Not "closure not persisted" but "3 blocking dams in LFRA, MFRA — not yet modelled."
  3. Explicit override for when dams exist but are known passable, remediated, or irrelevant. Proceeds, and records the assumption in the #127 run log so lnk_log_read() can later report that this network was built assuming those dams do not block.

Tier 3 is what keeps the override from being a hole. Without provenance an overridden run yields a schema whose access values depend on an unrecorded assumption — precisely the failure class #127 exists to prevent.

Caveat that will make or break it

The 2.4 s join uses raw cabd.dams geometry. link's pipeline snaps dams to blue lines via cabd_blkey_xref.csv and applies cabd_exclusions plus passability overrides, so a raw spatial hit is not the same as "a barrier link would actually model." For a guard, erring toward flagging is correct — a false "run downstream first" costs 40 minutes, a false all-clear costs a silently wrong answer. But the check must apply the same passability and exclusion filters the pipeline does, or it will flag on dams link already treats as passable, operators will reach for the override reflexively, and the guard stops meaning anything.

Pieces that already exist

  • lnk_wsg_resolve(cfg, loaded, wsgs = <wsg>) — DS-first closure, species-filtered
  • .lnk_wsg_persisted(conn, cfg, aoi) — answers exactly "is this WSG in the persist schema"

    Corrected. Right question, wrong granularity — it cannot distinguish a WSG persisted with dams = FALSE, which would pass a schema holding the streams but not the barriers. Persistence is checked per dam in <persist>.barriers (barrier_source = 'CABD', id_barrier = the CABD uuid).

  • #127 run provenance tables + lnk_log_read()[email protected]

Acceptance

  • wsg_run_one.R <WSG> on an empty persist schema, with blocking dams downstream, fails non-zero and names the offending WSGs and dams
  • Same invocation where the closure is genuinely dam-free proceeds with no override and no prompt
  • Override parameter proceeds and writes the assumption into the #127 run log; lnk_log_read() surfaces it — and a bare TRUE is rejected: the justification is the mechanism
  • Dam check honours cabd_exclusions and passability overrides — solved by construction: the cabd / matched CTEs are shared with .lnk_pipeline_prep_dams(), not copied. Refactor verified behaviour-preserving against a golden capture (ADMS/KOTL/PARS byte-identical)
  • Guard is a no-op cost (< ~5 s) — measured ~0.5 s
  • study_area_run.sh DS-first runs pass unchanged — via LNK_GUARD_DOWNSTREAM=warn on both legs, not by the guard passing

    Corrected. Unachievable by a hard pre-flight: on multi-host runs downstream groups are legitimately mid-flight on another cypher, and because per-WSG failures soft-fail with a WARN the blocked WSG would be skipped entirely — which lnk_access(merge = TRUE) cannot repair, since it cannot fix a WSG that was never modelled. Strictly worse than the bug. Shipped as warn mode on both the dispatcher leg and inside the ssh string, with the note still written to the run log, plus a post-condition in wsg_recompute_one.R that re-runs the guard in error mode after consolidate. That makes it a deferral rather than a hole.

Refs


Shipped

v0.46.0 / #245. Also fixed a v0.45.0 defect found in passing: .lnk_log_create_tables() built the run-log tables but never the schema, so a brand-new persist schema failed — the log opens before lnk_persist_init by design, and every schema tested until then already existed.

Follow-up #244: cabd_additions dams carry barrier_ind = t but the usa CTE hardcodes passability_status_code NULL and the barrier_status CASE has no NULL arm, so the US placeholders can never become barriers. The guard mirrors this; a fix changes modelled access in the Columbia and needs a parity check, so it is a decision rather than a bug fix.

Known bound: inherits frs_wsg_drainage()'s one-outlet-per-group model — a WSG draining by two independent paths would be under-covered. Recorded in RUNBOOK.md §8c.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions