diff --git a/DESCRIPTION b/DESCRIPTION index b5031c9..54b5bdf 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: link Title: Crossing Connectivity Interpretation -Version: 0.6.0 +Version: 0.7.0 Authors@R: person("Allan", "Irvine", , "airvine@newgraphenvironment.com", role = c("aut", "cre"), diff --git a/NEWS.md b/NEWS.md index 73548fd..8c7ab74 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,11 @@ +# link 0.7.0 + +`user_barriers_definite` no longer eligible for observation-based override ([#48](https://github.com/NewGraphEnvironment/link/issues/48)). + +- `.lnk_pipeline_prep_natural()` previously unioned `barriers_definite` into `natural_barriers`, which `lnk_barrier_overrides()` iterates over. Net effect: the 227 reviewer-added user-definite positions (EXCLUSION zones, MISC detections the model misses) could be re-opened by observations clearing the species threshold. Confirmed active on ELKR pre-fix — 4 override rows at Erickson Creek exclusion and Spillway MISC positions that bcfishpass keeps as permanent barriers. +- bcfishpass's `model_access_*.sql` builds the barriers CTE from gradient + falls + subsurfaceflow only and appends `barriers_user_definite` post-filter via `UNION ALL`. Observations and habitat filters never see user-definite rows, so they're never overridable. link now matches this shape: `natural_barriers` is gradient + falls only; `barriers_definite` stays consumed separately as a break source in `lnk_pipeline_break()` and as a direct `UNION ALL` entry into `fresh.streams_breaks` via `lnk_pipeline_classify()`. +- ELKR rollup shifts toward bcfishpass: BT spawning +3.4% → +2.8%, WCT spawning +4.0% → +2.6%, WCT rearing +1.6% → +0.3%. Other four WSGs unchanged (ADMS/BABL/DEAD have empty `barriers_definite`; BULK has 87 rows but no observation-threshold matches to any of them). + # link 0.6.0 Honour `user_barriers_definite_control.csv` at the observation-override step. diff --git a/R/lnk_pipeline_prepare.R b/R/lnk_pipeline_prepare.R index 4979e6a..7e8f6c6 100644 --- a/R/lnk_pipeline_prepare.R +++ b/R/lnk_pipeline_prepare.R @@ -11,8 +11,10 @@ #' [fresh::frs_break_find()], pruned against the control table, #' enriched with `wscode_ltree` and `localcode_ltree` for #' `fwa_upstream()` joins -#' - A natural-barriers table (gradient + falls + definite) used by -#' `lnk_barrier_overrides()` to compute the per-species skip list +#' - A natural-barriers table (gradient + falls) used by +#' `lnk_barrier_overrides()` to compute the per-species skip list. +#' User-definite barriers are intentionally excluded here and +#' consumed by later phases directly — bcfishpass parity. #' - Per-model barrier tables reduced to the minimal downstream-most #' set via [fresh::frs_barriers_minimal()], then unioned into #' `gradient_barriers_minimal` for segmentation @@ -214,7 +216,7 @@ lnk_pipeline_prepare <- function(conn, aoi, cfg, schema, } -#' Build natural-barriers table (gradient + falls + definite) with ltree +#' Build natural-barriers table (gradient + falls) with ltree #' @noRd .lnk_pipeline_prep_natural <- function(conn, schema) { .lnk_db_execute(conn, sprintf( @@ -240,16 +242,15 @@ lnk_pipeline_prepare <- function(conn, aoi, cfg, schema, AND f.downstream_route_measure >= s.downstream_route_measure AND f.downstream_route_measure < s.upstream_route_measure", schema, schema)) - .lnk_db_execute(conn, sprintf( - "INSERT INTO %s.natural_barriers - SELECT d.blue_line_key, round(d.downstream_route_measure), - 'blocked', s.wscode_ltree, s.localcode_ltree - FROM %s.barriers_definite d - JOIN whse_basemapping.fwa_stream_networks_sp s - ON d.blue_line_key = s.blue_line_key - AND d.downstream_route_measure >= s.downstream_route_measure - AND d.downstream_route_measure < s.upstream_route_measure", - schema, schema)) + # NOTE: `barriers_definite` is NOT unioned into `natural_barriers`. + # bcfishpass appends user-definite post-filter in + # `model_access_*.sql`, so upstream observations and habitat never + # re-open them. link mirrors this by consuming `barriers_definite` + # separately: + # - `lnk_pipeline_break()` applies it as its own sequential break + # source (so segmentation still places a boundary there) + # - `lnk_pipeline_classify()` UNION ALLs it directly into + # `fresh.streams_breaks` (so it blocks access gating) invisible(NULL) } diff --git a/data-raw/logs/20260423_07_tar_make_48_run1.txt b/data-raw/logs/20260423_07_tar_make_48_run1.txt new file mode 100644 index 0000000..b4e0623 --- /dev/null +++ b/data-raw/logs/20260423_07_tar_make_48_run1.txt @@ -0,0 +1,365 @@ ++ cfg dispatched +✔ cfg completed [244ms, 296.48 kB] ++ comparison_BABL dispatched +NOTICE: schema "fresh" already exists, skipping + +NOTICE: table "barriers_definite" does not exist, skipping + +NOTICE: table "streams_blk" does not exist, skipping + +NOTICE: table "gradient_barriers_raw" does not exist, skipping + +NOTICE: table "natural_barriers" does not exist, skipping + +NOTICE: table "barrier_overrides" does not exist, skipping + +NOTICE: table "barriers_bt" does not exist, skipping + +NOTICE: table "barriers_bt_min" does not exist, skipping + +NOTICE: table "barriers_ch_cm_co_pk_sk" does not exist, skipping + +NOTICE: table "barriers_ch_cm_co_pk_sk_min" does not exist, skipping + +NOTICE: table "barriers_st" does not exist, skipping + +NOTICE: table "barriers_st_min" does not exist, skipping + +NOTICE: table "barriers_wct" does not exist, skipping + +NOTICE: table "barriers_wct_min" does not exist, skipping + +NOTICE: table "gradient_barriers_minimal" does not exist, skipping + +NOTICE: table "streams" does not exist, skipping + +NOTICE: table "streams_habitat" does not exist, skipping + +NOTICE: table "observations_breaks" does not exist, skipping + +NOTICE: table "habitat_endpoints" does not exist, skipping + +NOTICE: table "crossings_breaks" does not exist, skipping + +NOTICE: table "streams_breaks" does not exist, skipping + +NOTICE: relation "streams_id_segment_idx" already exists, skipping + +NOTICE: table "streams_acc_015" does not exist, skipping + +NOTICE: table "streams_acc_02" does not exist, skipping + +NOTICE: table "streams_acc_025" does not exist, skipping + +NOTICE: table "streams_acc_025_ovr_bt" does not exist, skipping + +NOTICE: table "streams_acc_015_ovr_ch" does not exist, skipping + +NOTICE: table "streams_acc_015_ovr_co" does not exist, skipping + +NOTICE: table "streams_acc_015_ovr_sk" does not exist, skipping + +NOTICE: table "streams_acc_02_ovr_st" does not exist, skipping + +NOTICE: table "frs_clusters_bt" does not exist, skipping + +NOTICE: table "frs_clusters_ch" does not exist, skipping + +NOTICE: table "frs_clusters_co" does not exist, skipping + +NOTICE: table "frs_clusters_sk" does not exist, skipping + +NOTICE: table "frs_qual_spawn_sk" does not exist, skipping + +NOTICE: table "frs_trace_lfid_sk" does not exist, skipping + +NOTICE: table "frs_clusters_st" does not exist, skipping + +✔ comparison_BABL completed [1m 47.6s, 416 B] ++ comparison_ELKR dispatched +NOTICE: schema "fresh" already exists, skipping + +Override validation: working_elkr.pscis_fixes vs working_elkr.crossings + Total overrides: 324 + Valid (matched): 288 + Orphans: 36 <-- not found in crossings + Duplicates: 0 +Updated 288 of 7306 rows (barrier_status) +NOTICE: table "barriers_definite_control" does not exist, skipping + +NOTICE: table "streams_blk" does not exist, skipping + +NOTICE: table "gradient_barriers_raw" does not exist, skipping + +NOTICE: table "natural_barriers" does not exist, skipping + +NOTICE: table "barrier_overrides" does not exist, skipping + +NOTICE: table "barriers_bt" does not exist, skipping + +NOTICE: table "barriers_bt_min" does not exist, skipping + +NOTICE: table "barriers_ch_cm_co_pk_sk" does not exist, skipping + +NOTICE: table "barriers_ch_cm_co_pk_sk_min" does not exist, skipping + +NOTICE: table "barriers_st" does not exist, skipping + +NOTICE: table "barriers_st_min" does not exist, skipping + +NOTICE: table "barriers_wct" does not exist, skipping + +NOTICE: table "barriers_wct_min" does not exist, skipping + +NOTICE: table "gradient_barriers_minimal" does not exist, skipping + +NOTICE: table "streams" does not exist, skipping + +NOTICE: table "streams_habitat" does not exist, skipping + +NOTICE: table "observations_breaks" does not exist, skipping + +NOTICE: table "habitat_endpoints" does not exist, skipping + +NOTICE: table "crossings_breaks" does not exist, skipping + +NOTICE: table "streams_breaks" does not exist, skipping + +NOTICE: relation "streams_id_segment_idx" already exists, skipping + +NOTICE: table "streams_acc_02" does not exist, skipping + +NOTICE: table "streams_acc_025" does not exist, skipping + +NOTICE: table "streams_acc_025_ovr_bt" does not exist, skipping + +NOTICE: table "streams_acc_02_ovr_wct" does not exist, skipping + +NOTICE: table "frs_clusters_bt" does not exist, skipping + +NOTICE: table "frs_clusters_wct" does not exist, skipping + +✔ comparison_ELKR completed [2m 36.6s, 318 B] ++ comparison_BULK dispatched +NOTICE: schema "fresh" already exists, skipping + +Override validation: working_bulk.pscis_fixes vs working_bulk.crossings + Total overrides: 580 + Valid (matched): 514 + Orphans: 66 <-- not found in crossings + Duplicates: 0 +Updated 514 of 5568 rows (barrier_status) +NOTICE: table "streams_blk" does not exist, skipping + +NOTICE: table "gradient_barriers_raw" does not exist, skipping + +NOTICE: table "natural_barriers" does not exist, skipping + +NOTICE: table "barrier_overrides" does not exist, skipping + +NOTICE: table "barriers_bt" does not exist, skipping + +NOTICE: table "barriers_bt_min" does not exist, skipping + +NOTICE: table "barriers_ch_cm_co_pk_sk" does not exist, skipping + +NOTICE: table "barriers_ch_cm_co_pk_sk_min" does not exist, skipping + +NOTICE: table "barriers_st" does not exist, skipping + +NOTICE: table "barriers_st_min" does not exist, skipping + +NOTICE: table "barriers_wct" does not exist, skipping + +NOTICE: table "barriers_wct_min" does not exist, skipping + +NOTICE: table "gradient_barriers_minimal" does not exist, skipping + +NOTICE: table "streams" does not exist, skipping + +NOTICE: table "streams_habitat" does not exist, skipping + +NOTICE: table "observations_breaks" does not exist, skipping + +NOTICE: table "habitat_endpoints" does not exist, skipping + +NOTICE: table "crossings_breaks" does not exist, skipping + +NOTICE: table "streams_breaks" does not exist, skipping + +NOTICE: relation "streams_id_segment_idx" already exists, skipping + +NOTICE: table "streams_acc_015" does not exist, skipping + +NOTICE: table "streams_acc_02" does not exist, skipping + +NOTICE: table "streams_acc_025" does not exist, skipping + +NOTICE: table "streams_acc_025_ovr_bt" does not exist, skipping + +NOTICE: table "streams_acc_015_ovr_ch" does not exist, skipping + +NOTICE: table "streams_acc_015_ovr_co" does not exist, skipping + +NOTICE: table "streams_acc_015_ovr_pk" does not exist, skipping + +NOTICE: table "streams_acc_015_ovr_sk" does not exist, skipping + +NOTICE: table "streams_acc_02_ovr_st" does not exist, skipping + +NOTICE: table "frs_clusters_bt" does not exist, skipping + +NOTICE: table "frs_clusters_ch" does not exist, skipping + +NOTICE: table "frs_clusters_co" does not exist, skipping + +NOTICE: table "frs_clusters_sk" does not exist, skipping + +NOTICE: table "frs_qual_spawn_sk" does not exist, skipping + +NOTICE: table "frs_trace_lfid_sk" does not exist, skipping + +NOTICE: table "frs_clusters_st" does not exist, skipping + +✔ comparison_BULK completed [2m 55s, 438 B] ++ comparison_ADMS dispatched +NOTICE: schema "fresh" already exists, skipping + +NOTICE: table "barriers_definite" does not exist, skipping + +NOTICE: table "streams_blk" does not exist, skipping + +NOTICE: table "gradient_barriers_raw" does not exist, skipping + +NOTICE: table "natural_barriers" does not exist, skipping + +NOTICE: table "barrier_overrides" does not exist, skipping + +NOTICE: table "barriers_bt" does not exist, skipping + +NOTICE: table "barriers_bt_min" does not exist, skipping + +NOTICE: table "barriers_ch_cm_co_pk_sk" does not exist, skipping + +NOTICE: table "barriers_ch_cm_co_pk_sk_min" does not exist, skipping + +NOTICE: table "barriers_st" does not exist, skipping + +NOTICE: table "barriers_st_min" does not exist, skipping + +NOTICE: table "barriers_wct" does not exist, skipping + +NOTICE: table "barriers_wct_min" does not exist, skipping + +NOTICE: table "gradient_barriers_minimal" does not exist, skipping + +NOTICE: table "streams" does not exist, skipping + +NOTICE: table "streams_habitat" does not exist, skipping + +NOTICE: table "observations_breaks" does not exist, skipping + +NOTICE: table "habitat_endpoints" does not exist, skipping + +NOTICE: table "crossings_breaks" does not exist, skipping + +NOTICE: table "streams_breaks" does not exist, skipping + +NOTICE: relation "streams_id_segment_idx" already exists, skipping + +NOTICE: table "streams_acc_015" does not exist, skipping + +NOTICE: table "streams_acc_025" does not exist, skipping + +NOTICE: table "streams_acc_025_ovr_bt" does not exist, skipping + +NOTICE: table "streams_acc_015_ovr_ch" does not exist, skipping + +NOTICE: table "streams_acc_015_ovr_co" does not exist, skipping + +NOTICE: table "streams_acc_015_ovr_sk" does not exist, skipping + +NOTICE: table "frs_clusters_bt" does not exist, skipping + +NOTICE: table "frs_clusters_ch" does not exist, skipping + +NOTICE: table "frs_clusters_co" does not exist, skipping + +NOTICE: table "frs_clusters_sk" does not exist, skipping + +NOTICE: table "frs_qual_spawn_sk" does not exist, skipping + +NOTICE: table "frs_trace_lfid_sk" does not exist, skipping + +✔ comparison_ADMS completed [1m 7.3s, 379 B] ++ comparison_DEAD dispatched +NOTICE: schema "fresh" already exists, skipping + +NOTICE: table "barriers_definite" does not exist, skipping + +NOTICE: table "streams_blk" does not exist, skipping + +NOTICE: table "gradient_barriers_raw" does not exist, skipping + +NOTICE: table "natural_barriers" does not exist, skipping + +NOTICE: table "barrier_overrides" does not exist, skipping + +NOTICE: table "barriers_bt" does not exist, skipping + +NOTICE: table "barriers_bt_min" does not exist, skipping + +NOTICE: table "barriers_ch_cm_co_pk_sk" does not exist, skipping + +NOTICE: table "barriers_ch_cm_co_pk_sk_min" does not exist, skipping + +NOTICE: table "barriers_st" does not exist, skipping + +NOTICE: table "barriers_st_min" does not exist, skipping + +NOTICE: table "barriers_wct" does not exist, skipping + +NOTICE: table "barriers_wct_min" does not exist, skipping + +NOTICE: table "gradient_barriers_minimal" does not exist, skipping + +NOTICE: table "streams" does not exist, skipping + +NOTICE: table "streams_habitat" does not exist, skipping + +NOTICE: table "observations_breaks" does not exist, skipping + +NOTICE: table "habitat_endpoints" does not exist, skipping + +NOTICE: table "crossings_breaks" does not exist, skipping + +NOTICE: table "streams_breaks" does not exist, skipping + +NOTICE: relation "streams_id_segment_idx" already exists, skipping + +NOTICE: table "streams_acc_015" does not exist, skipping + +NOTICE: table "streams_acc_02" does not exist, skipping + +NOTICE: table "streams_acc_025" does not exist, skipping + +NOTICE: table "streams_acc_025_ovr_bt" does not exist, skipping + +NOTICE: table "frs_clusters_bt" does not exist, skipping + +NOTICE: table "frs_clusters_ch" does not exist, skipping + +NOTICE: table "frs_clusters_co" does not exist, skipping + +NOTICE: table "frs_clusters_sk" does not exist, skipping + +NOTICE: table "frs_qual_spawn_sk" does not exist, skipping + +NOTICE: table "frs_trace_lfid_sk" does not exist, skipping + +NOTICE: table "frs_clusters_st" does not exist, skipping + +✔ comparison_DEAD completed [40.6s, 420 B] ++ rollup dispatched +✔ rollup completed [1ms, 869 B] +✔ ended pipeline [9m 7.7s, 7 completed, 0 skipped] diff --git a/data-raw/logs/20260423_08_tar_make_48_repro.txt b/data-raw/logs/20260423_08_tar_make_48_repro.txt new file mode 100644 index 0000000..4465781 --- /dev/null +++ b/data-raw/logs/20260423_08_tar_make_48_repro.txt @@ -0,0 +1,365 @@ ++ cfg dispatched +✔ cfg completed [250ms, 296.48 kB] ++ comparison_BABL dispatched +NOTICE: schema "fresh" already exists, skipping + +NOTICE: table "barriers_definite" does not exist, skipping + +NOTICE: table "streams_blk" does not exist, skipping + +NOTICE: table "gradient_barriers_raw" does not exist, skipping + +NOTICE: table "natural_barriers" does not exist, skipping + +NOTICE: table "barrier_overrides" does not exist, skipping + +NOTICE: table "barriers_bt" does not exist, skipping + +NOTICE: table "barriers_bt_min" does not exist, skipping + +NOTICE: table "barriers_ch_cm_co_pk_sk" does not exist, skipping + +NOTICE: table "barriers_ch_cm_co_pk_sk_min" does not exist, skipping + +NOTICE: table "barriers_st" does not exist, skipping + +NOTICE: table "barriers_st_min" does not exist, skipping + +NOTICE: table "barriers_wct" does not exist, skipping + +NOTICE: table "barriers_wct_min" does not exist, skipping + +NOTICE: table "gradient_barriers_minimal" does not exist, skipping + +NOTICE: table "streams" does not exist, skipping + +NOTICE: table "streams_habitat" does not exist, skipping + +NOTICE: table "observations_breaks" does not exist, skipping + +NOTICE: table "habitat_endpoints" does not exist, skipping + +NOTICE: table "crossings_breaks" does not exist, skipping + +NOTICE: table "streams_breaks" does not exist, skipping + +NOTICE: relation "streams_id_segment_idx" already exists, skipping + +NOTICE: table "streams_acc_015" does not exist, skipping + +NOTICE: table "streams_acc_02" does not exist, skipping + +NOTICE: table "streams_acc_025" does not exist, skipping + +NOTICE: table "streams_acc_025_ovr_bt" does not exist, skipping + +NOTICE: table "streams_acc_015_ovr_ch" does not exist, skipping + +NOTICE: table "streams_acc_015_ovr_co" does not exist, skipping + +NOTICE: table "streams_acc_015_ovr_sk" does not exist, skipping + +NOTICE: table "streams_acc_02_ovr_st" does not exist, skipping + +NOTICE: table "frs_clusters_bt" does not exist, skipping + +NOTICE: table "frs_clusters_ch" does not exist, skipping + +NOTICE: table "frs_clusters_co" does not exist, skipping + +NOTICE: table "frs_clusters_sk" does not exist, skipping + +NOTICE: table "frs_qual_spawn_sk" does not exist, skipping + +NOTICE: table "frs_trace_lfid_sk" does not exist, skipping + +NOTICE: table "frs_clusters_st" does not exist, skipping + +✔ comparison_BABL completed [1m 47.4s, 416 B] ++ comparison_ELKR dispatched +NOTICE: schema "fresh" already exists, skipping + +Override validation: working_elkr.pscis_fixes vs working_elkr.crossings + Total overrides: 324 + Valid (matched): 288 + Orphans: 36 <-- not found in crossings + Duplicates: 0 +Updated 288 of 7306 rows (barrier_status) +NOTICE: table "barriers_definite_control" does not exist, skipping + +NOTICE: table "streams_blk" does not exist, skipping + +NOTICE: table "gradient_barriers_raw" does not exist, skipping + +NOTICE: table "natural_barriers" does not exist, skipping + +NOTICE: table "barrier_overrides" does not exist, skipping + +NOTICE: table "barriers_bt" does not exist, skipping + +NOTICE: table "barriers_bt_min" does not exist, skipping + +NOTICE: table "barriers_ch_cm_co_pk_sk" does not exist, skipping + +NOTICE: table "barriers_ch_cm_co_pk_sk_min" does not exist, skipping + +NOTICE: table "barriers_st" does not exist, skipping + +NOTICE: table "barriers_st_min" does not exist, skipping + +NOTICE: table "barriers_wct" does not exist, skipping + +NOTICE: table "barriers_wct_min" does not exist, skipping + +NOTICE: table "gradient_barriers_minimal" does not exist, skipping + +NOTICE: table "streams" does not exist, skipping + +NOTICE: table "streams_habitat" does not exist, skipping + +NOTICE: table "observations_breaks" does not exist, skipping + +NOTICE: table "habitat_endpoints" does not exist, skipping + +NOTICE: table "crossings_breaks" does not exist, skipping + +NOTICE: table "streams_breaks" does not exist, skipping + +NOTICE: relation "streams_id_segment_idx" already exists, skipping + +NOTICE: table "streams_acc_02" does not exist, skipping + +NOTICE: table "streams_acc_025" does not exist, skipping + +NOTICE: table "streams_acc_025_ovr_bt" does not exist, skipping + +NOTICE: table "streams_acc_02_ovr_wct" does not exist, skipping + +NOTICE: table "frs_clusters_bt" does not exist, skipping + +NOTICE: table "frs_clusters_wct" does not exist, skipping + +✔ comparison_ELKR completed [2m 35s, 318 B] ++ comparison_BULK dispatched +NOTICE: schema "fresh" already exists, skipping + +Override validation: working_bulk.pscis_fixes vs working_bulk.crossings + Total overrides: 580 + Valid (matched): 514 + Orphans: 66 <-- not found in crossings + Duplicates: 0 +Updated 514 of 5568 rows (barrier_status) +NOTICE: table "streams_blk" does not exist, skipping + +NOTICE: table "gradient_barriers_raw" does not exist, skipping + +NOTICE: table "natural_barriers" does not exist, skipping + +NOTICE: table "barrier_overrides" does not exist, skipping + +NOTICE: table "barriers_bt" does not exist, skipping + +NOTICE: table "barriers_bt_min" does not exist, skipping + +NOTICE: table "barriers_ch_cm_co_pk_sk" does not exist, skipping + +NOTICE: table "barriers_ch_cm_co_pk_sk_min" does not exist, skipping + +NOTICE: table "barriers_st" does not exist, skipping + +NOTICE: table "barriers_st_min" does not exist, skipping + +NOTICE: table "barriers_wct" does not exist, skipping + +NOTICE: table "barriers_wct_min" does not exist, skipping + +NOTICE: table "gradient_barriers_minimal" does not exist, skipping + +NOTICE: table "streams" does not exist, skipping + +NOTICE: table "streams_habitat" does not exist, skipping + +NOTICE: table "observations_breaks" does not exist, skipping + +NOTICE: table "habitat_endpoints" does not exist, skipping + +NOTICE: table "crossings_breaks" does not exist, skipping + +NOTICE: table "streams_breaks" does not exist, skipping + +NOTICE: relation "streams_id_segment_idx" already exists, skipping + +NOTICE: table "streams_acc_015" does not exist, skipping + +NOTICE: table "streams_acc_02" does not exist, skipping + +NOTICE: table "streams_acc_025" does not exist, skipping + +NOTICE: table "streams_acc_025_ovr_bt" does not exist, skipping + +NOTICE: table "streams_acc_015_ovr_ch" does not exist, skipping + +NOTICE: table "streams_acc_015_ovr_co" does not exist, skipping + +NOTICE: table "streams_acc_015_ovr_pk" does not exist, skipping + +NOTICE: table "streams_acc_015_ovr_sk" does not exist, skipping + +NOTICE: table "streams_acc_02_ovr_st" does not exist, skipping + +NOTICE: table "frs_clusters_bt" does not exist, skipping + +NOTICE: table "frs_clusters_ch" does not exist, skipping + +NOTICE: table "frs_clusters_co" does not exist, skipping + +NOTICE: table "frs_clusters_sk" does not exist, skipping + +NOTICE: table "frs_qual_spawn_sk" does not exist, skipping + +NOTICE: table "frs_trace_lfid_sk" does not exist, skipping + +NOTICE: table "frs_clusters_st" does not exist, skipping + +✔ comparison_BULK completed [2m 56.5s, 438 B] ++ comparison_ADMS dispatched +NOTICE: schema "fresh" already exists, skipping + +NOTICE: table "barriers_definite" does not exist, skipping + +NOTICE: table "streams_blk" does not exist, skipping + +NOTICE: table "gradient_barriers_raw" does not exist, skipping + +NOTICE: table "natural_barriers" does not exist, skipping + +NOTICE: table "barrier_overrides" does not exist, skipping + +NOTICE: table "barriers_bt" does not exist, skipping + +NOTICE: table "barriers_bt_min" does not exist, skipping + +NOTICE: table "barriers_ch_cm_co_pk_sk" does not exist, skipping + +NOTICE: table "barriers_ch_cm_co_pk_sk_min" does not exist, skipping + +NOTICE: table "barriers_st" does not exist, skipping + +NOTICE: table "barriers_st_min" does not exist, skipping + +NOTICE: table "barriers_wct" does not exist, skipping + +NOTICE: table "barriers_wct_min" does not exist, skipping + +NOTICE: table "gradient_barriers_minimal" does not exist, skipping + +NOTICE: table "streams" does not exist, skipping + +NOTICE: table "streams_habitat" does not exist, skipping + +NOTICE: table "observations_breaks" does not exist, skipping + +NOTICE: table "habitat_endpoints" does not exist, skipping + +NOTICE: table "crossings_breaks" does not exist, skipping + +NOTICE: table "streams_breaks" does not exist, skipping + +NOTICE: relation "streams_id_segment_idx" already exists, skipping + +NOTICE: table "streams_acc_015" does not exist, skipping + +NOTICE: table "streams_acc_025" does not exist, skipping + +NOTICE: table "streams_acc_025_ovr_bt" does not exist, skipping + +NOTICE: table "streams_acc_015_ovr_ch" does not exist, skipping + +NOTICE: table "streams_acc_015_ovr_co" does not exist, skipping + +NOTICE: table "streams_acc_015_ovr_sk" does not exist, skipping + +NOTICE: table "frs_clusters_bt" does not exist, skipping + +NOTICE: table "frs_clusters_ch" does not exist, skipping + +NOTICE: table "frs_clusters_co" does not exist, skipping + +NOTICE: table "frs_clusters_sk" does not exist, skipping + +NOTICE: table "frs_qual_spawn_sk" does not exist, skipping + +NOTICE: table "frs_trace_lfid_sk" does not exist, skipping + +✔ comparison_ADMS completed [1m 8.7s, 379 B] ++ comparison_DEAD dispatched +NOTICE: schema "fresh" already exists, skipping + +NOTICE: table "barriers_definite" does not exist, skipping + +NOTICE: table "streams_blk" does not exist, skipping + +NOTICE: table "gradient_barriers_raw" does not exist, skipping + +NOTICE: table "natural_barriers" does not exist, skipping + +NOTICE: table "barrier_overrides" does not exist, skipping + +NOTICE: table "barriers_bt" does not exist, skipping + +NOTICE: table "barriers_bt_min" does not exist, skipping + +NOTICE: table "barriers_ch_cm_co_pk_sk" does not exist, skipping + +NOTICE: table "barriers_ch_cm_co_pk_sk_min" does not exist, skipping + +NOTICE: table "barriers_st" does not exist, skipping + +NOTICE: table "barriers_st_min" does not exist, skipping + +NOTICE: table "barriers_wct" does not exist, skipping + +NOTICE: table "barriers_wct_min" does not exist, skipping + +NOTICE: table "gradient_barriers_minimal" does not exist, skipping + +NOTICE: table "streams" does not exist, skipping + +NOTICE: table "streams_habitat" does not exist, skipping + +NOTICE: table "observations_breaks" does not exist, skipping + +NOTICE: table "habitat_endpoints" does not exist, skipping + +NOTICE: table "crossings_breaks" does not exist, skipping + +NOTICE: table "streams_breaks" does not exist, skipping + +NOTICE: relation "streams_id_segment_idx" already exists, skipping + +NOTICE: table "streams_acc_015" does not exist, skipping + +NOTICE: table "streams_acc_02" does not exist, skipping + +NOTICE: table "streams_acc_025" does not exist, skipping + +NOTICE: table "streams_acc_025_ovr_bt" does not exist, skipping + +NOTICE: table "frs_clusters_bt" does not exist, skipping + +NOTICE: table "frs_clusters_ch" does not exist, skipping + +NOTICE: table "frs_clusters_co" does not exist, skipping + +NOTICE: table "frs_clusters_sk" does not exist, skipping + +NOTICE: table "frs_qual_spawn_sk" does not exist, skipping + +NOTICE: table "frs_trace_lfid_sk" does not exist, skipping + +NOTICE: table "frs_clusters_st" does not exist, skipping + +✔ comparison_DEAD completed [40.4s, 420 B] ++ rollup dispatched +✔ rollup completed [1ms, 869 B] +✔ ended pipeline [9m 8.6s, 7 completed, 0 skipped] diff --git a/inst/extdata/vignette-data/rollup.rds b/inst/extdata/vignette-data/rollup.rds index ff55b92..c0ad41a 100644 Binary files a/inst/extdata/vignette-data/rollup.rds and b/inst/extdata/vignette-data/rollup.rds differ diff --git a/inst/extdata/vignette-data/sub_ch.rds b/inst/extdata/vignette-data/sub_ch.rds index 67f42ef..061fd7c 100644 Binary files a/inst/extdata/vignette-data/sub_ch.rds and b/inst/extdata/vignette-data/sub_ch.rds differ diff --git a/inst/extdata/vignette-data/sub_ch_bcfp.rds b/inst/extdata/vignette-data/sub_ch_bcfp.rds index 26c20a0..729f9ec 100644 Binary files a/inst/extdata/vignette-data/sub_ch_bcfp.rds and b/inst/extdata/vignette-data/sub_ch_bcfp.rds differ diff --git a/man/lnk_barrier_overrides.Rd b/man/lnk_barrier_overrides.Rd index e585691..8e0bd51 100644 --- a/man/lnk_barrier_overrides.Rd +++ b/man/lnk_barrier_overrides.Rd @@ -41,12 +41,21 @@ observations are removed before counting.} \item{control}{Character or \code{NULL}. Schema-qualified table of barrier controls with columns: \code{blue_line_key}, \code{downstream_route_measure}, \code{barrier_ind}. Barriers in this table with \code{barrier_ind = TRUE} cannot -be overridden.} +be overridden \strong{by observations} — but only for species where +\code{params$observation_control_apply} is TRUE. Resident species routinely +inhabit reaches upstream of anadromous-blocking falls (post-glacial +connectivity, no ocean-return requirement), so their observations still +count unless this flag says otherwise. Habitat confirmations +(\code{habitat} argument) are higher-trust than observations — they bypass +the control table entirely, for all species.} \item{params}{Data frame with per-species parameters. Must have columns: \code{species_code}, \code{observation_threshold}, \code{observation_date_min}, -\code{observation_buffer_m}, \code{observation_species}. See -\code{configs/bcfishpass/parameters_fresh.csv} for format.} +\code{observation_buffer_m}, \code{observation_species}. Optional column +\code{observation_control_apply} (logical) — when TRUE, the \code{control} table +blocks overrides for this species; when FALSE/NA/missing, the species +ignores control. Bcfishpass defaults: TRUE for CH/CM/CO/PK/SK/ST, +FALSE for BT/WCT. See \code{configs/bcfishpass/parameters_fresh.csv}.} \item{cols_index}{Character vector. Column names to index on the barriers table for \code{fwa_upstream()} performance. Indexes are created diff --git a/man/lnk_pipeline_prepare.Rd b/man/lnk_pipeline_prepare.Rd index 746f339..ea7efb4 100644 --- a/man/lnk_pipeline_prepare.Rd +++ b/man/lnk_pipeline_prepare.Rd @@ -45,8 +45,10 @@ habitat confirmation CSVs from the config bundle \code{\link[fresh:frs_break_find]{fresh::frs_break_find()}}, pruned against the control table, enriched with \code{wscode_ltree} and \code{localcode_ltree} for \code{fwa_upstream()} joins -\item A natural-barriers table (gradient + falls + definite) used by -\code{lnk_barrier_overrides()} to compute the per-species skip list +\item A natural-barriers table (gradient + falls) used by +\code{lnk_barrier_overrides()} to compute the per-species skip list. +User-definite barriers are intentionally excluded here and +consumed by later phases directly — bcfishpass parity. \item Per-model barrier tables reduced to the minimal downstream-most set via \code{\link[fresh:frs_barriers_minimal]{fresh::frs_barriers_minimal()}}, then unioned into \code{gradient_barriers_minimal} for segmentation diff --git a/planning/active/findings.md b/planning/active/findings.md index 3ee943e..a2b486e 100644 --- a/planning/active/findings.md +++ b/planning/active/findings.md @@ -1,60 +1,61 @@ -# Findings: Wire barriers_definite_control (#44) +# Findings — #48 (user_barriers_definite bypass) -## Where the gap lives +## Pre-fix defect on ELKR (2026-04-23, via post-#47 tar_make state) -Three places where `"barriers_definite_control"` is referenced in link today: +Query: `SELECT * FROM working_.barrier_overrides bo INNER JOIN working_.barriers_definite bd ON bd.blue_line_key = bo.blue_line_key AND abs(bd.downstream_route_measure - bo.downstream_route_measure) < 1` across 5 WSGs: -1. **`R/lnk_pipeline_prepare.R` → `.lnk_pipeline_prep_load_aux`** — loads the per-AOI filtered CSV rows into `.barriers_definite_control` when `cfg$overrides$barriers_definite_control` is non-NULL. Already correct. -2. **`R/lnk_pipeline_prepare.R` → `.lnk_pipeline_prep_gradient`** — `information_schema` probe for the table, then `DELETE FROM gradient_barriers_raw g USING barriers_definite_control c WHERE ... c.barrier_ind::boolean = false`. Already correct. Removes **passable** positions from gradient set before minimal reduction. -3. **`R/lnk_pipeline_prepare.R` → `.lnk_pipeline_prep_overrides`** — does NOT pass `control` to `lnk_barrier_overrides`. This is the gap. `lnk_barrier_overrides` accepts a `control` parameter but is called without one from this site. +- ADMS: 0 rows in barriers_definite → no matches possible +- BULK: 87 rows in barriers_definite, 0 matches in barrier_overrides +- BABL: 0 rows in barriers_definite → no matches possible +- ELKR: 7 rows in barriers_definite, **4 matches** in barrier_overrides +- DEAD: 0 rows in barriers_definite → no matches possible -## Latent bug in lnk_barrier_overrides control filter +ELKR matches: -Read `R/lnk_barrier_overrides.R` lines 140–153. The implementation: +| Species | Position | Type | Name | +|---------|----------|------|------| +| BT | (356549622, 42) | EXCLUSION | Erickson Creek exclusion (mining impacts per CSV note) | +| WCT | (356549622, 42) | EXCLUSION | Erickson Creek exclusion | +| WCT | (356553439, 574) | MISC | Spillway | +| WCT | (356560765, 2935) | MISC | Spillway | -```r -ctrl_where <- sprintf("LEFT JOIN %s c ON b.blue_line_key = c.blue_line_key - AND abs(b.downstream_route_measure - c.downstream_route_measure) < 1", control) -ctrl_filter <- "AND c.blue_line_key IS NULL" -``` - -Filter treats ANY control row as blocking override — including `barrier_ind = FALSE` rows. Docstring (lines 27–30) says only `barrier_ind = TRUE` rows block. - -In practice on bcfishpass input this is masked because `.lnk_pipeline_prep_gradient`'s upstream DELETE removes `barrier_ind = FALSE` positions from `gradient_barriers_raw` before they reach the override step. But falls and user-definite positions are not pruned by control at load time — they stay in `natural_barriers`. If a control row with `barrier_ind = FALSE` exists for a fall or definite-barrier position, the current filter blocks observation overrides on it. Should not block. - -Fix: `"AND (c.blue_line_key IS NULL OR c.barrier_ind::boolean = false)"`. - -## Manifest-driven gating decision +These are user-definite positions that link's pipeline treats as overridable. Post-fix they should stay as permanent blockers (matching bcfishpass). Current ELKR rollup: BT spawn +3.4% / rear -0.7%; WCT spawn +4.0% / rear +1.6%. Fixing this should bring spawning numbers down toward 0. -`.lnk_pipeline_prep_overrides` could probe `information_schema.tables` to discover whether the control table exists (same pattern used there for habitat). Decided against: the manifest key is the direct contract. If `cfg$overrides$barriers_definite_control` is non-NULL the load step wrote the table; if it's NULL no table exists. Manifest gate, not DB probe. +## Architecture comparison -Scope discipline: the existing `information_schema` probe for the habitat table in the same function, and the similar probe in `.lnk_pipeline_prep_gradient` for the control table, work correctly today. Leaving them alone in this PR; filing a follow-up issue for consistency. Using the manifest as the contract is well preferred across the package. +**bcfishpass** — `model_access_*.sql`: -## Tests that need to exist - -- `tests/testthat/test-lnk_barrier_overrides.R` does not exist. Creating new. -- `tests/testthat/test-lnk_pipeline_prepare.R` exists — extending with prep_overrides control pass-through tests. - -## No `information_schema` probe in the new code +```sql +barriers CTE = gradient + falls + subsurfaceflow -- NO user_definite +... (observation filter, habitat filter, control filter all operate on barriers CTE) +barriers_filtered as (... where n_obs < threshold and h.species_codes is null) +INSERT INTO barriers_ + SELECT * FROM barriers_filtered + UNION ALL + SELECT * FROM bcfishpass.barriers_user_definite WHERE wsg = :wsg -- appended post-filter +``` -`.lnk_pipeline_prep_overrides`'s new control guard reads `cfg$overrides$barriers_definite_control` directly. That field is populated by `lnk_config()` when the manifest declares the key. No DB round-trip needed. +**link today** — `.lnk_pipeline_prep_natural()`: -## Expected rollup direction +```r +CREATE TABLE natural_barriers FROM gradient_barriers_raw -- base +INSERT INTO natural_barriers SELECT FROM falls -- + falls +INSERT INTO natural_barriers SELECT FROM barriers_definite -- + user-definite (WRONG — subjects to override) +``` -Running the pipeline pre-fix vs post-fix on bcfishpass config: +Then `.lnk_pipeline_prep_overrides()` passes `natural_barriers` to `lnk_barrier_overrides()`, which emits per-species override rows for any barrier meeting threshold — including user-definite. -- WSGs with `user_barriers_definite_control.csv` rows having `barrier_ind = TRUE` and upstream observations at those positions → rollup `link_km` shrinks for affected species (positions that were wrongly overridden are no longer overridden). Moves toward bcfishpass reference. -- WSGs with no such rows → rollup unchanged. +## Shape A (chosen) -Magnitude: unknown. Control-TRUE rows on the four validated WSGs are uncommon, so likely small. Direction matters more than magnitude. +1. Drop the `INSERT INTO natural_barriers SELECT FROM barriers_definite` block in `.lnk_pipeline_prep_natural()`. +2. In `.lnk_pipeline_prep_minimal()`, after `frs_barriers_minimal()` emits each per-model reduced table, append `barriers_definite` rows (already WSG-filtered at load time) via `INSERT ... SELECT ... ON CONFLICT DO NOTHING`. Also append to the union that produces `gradient_barriers_minimal` so segmentation breaks include user-definite. -## Reproducibility +## natural_barriers callers -The change is a deterministic additional filter clause on a `LEFT JOIN`. No new randomness, no schedule-dependent behaviour. Two back-to-back `tar_make()` runs must produce bit-identical rollups. Will verify with `digest::digest()`. +Grep confirms `natural_barriers` only referenced in: -## Cross-refs +- `.lnk_pipeline_prep_natural()` (builds) +- `.lnk_pipeline_prep_overrides()` (passes to `lnk_barrier_overrides()`) +- `.lnk_pipeline_prep_minimal()` (reads into `frs_barriers_minimal()`) -- Plan file: `/Users/airvine/.claude/plans/stateful-hopping-feather.md` -- Issue: link#44 -- Parallel cleanup issue (separate PR): link#45 (gradient classes) -- Follow-up to file at end of this PR: "Migrate remaining pipeline probes to manifest-driven gating" +Shape A is safe — no external consumers. diff --git a/planning/active/progress.md b/planning/active/progress.md index 2b5960d..97d9c85 100644 --- a/planning/active/progress.md +++ b/planning/active/progress.md @@ -1,25 +1,9 @@ # Progress -## Session 2026-04-23 +## Session 2026-04-23 — #48 kickoff -- Archived `2026-04-23-targets-pipeline/` — link#38 closed via PRs #41/#42/#43. Three consecutive `tar_make()` runs produced bit-identical rollups. All species within 5% of bcfishpass reference on all four WSGs. -- Branched `44-barriers-definite-control` off main. -- Plan approved. PWF initialized for #44. -- Pre-flight complete: identified the `ctrl_filter` bug in `lnk_barrier_overrides` (all rows block, not just `barrier_ind = TRUE`), and confirmed `.lnk_pipeline_prep_overrides` doesn't pass `control`. Same PR fixes both — filter semantics + missing pass-through. -- Next: Phase 1 — fix `R/lnk_barrier_overrides.R` `ctrl_filter` and add `tests/testthat/test-lnk_barrier_overrides.R`. -- Phase 1 committed (d1a7109) — `NOT EXISTS` control filter, 11 tests, 269 PASS. -- Phase 2 committed (53bedbd) — manifest-gated `control` pass-through in `.lnk_pipeline_prep_overrides`, fixed asymmetric load_aux (schema-valid empty table), 271 PASS. -- Post-Phase-2 `tar_make()` (log: `data-raw/logs/20260423_01_tar_make_post_44.txt`) showed 11–22pp drift AWAY from bcfishpass on ADMS/BABL; BULK/ELKR unchanged. Root cause: bcfishpass applies control filter only in CH/CM/CO/PK/SK and ST models (not BT/WCT/CT/DV/RB). My implementation applied it across all species in the `params` loop. -- Phase 2a: new `observation_control_apply` column in `parameters_fresh.csv` (TRUE for CH/CM/CO/PK/SK/ST; FALSE for BT/WCT; NA for CT/DV/RB), per-species NOT EXISTS gate in `lnk_barrier_overrides()`, three new tests. 279 PASS. Amendment pushed to issue #44 documenting the species-scoped approach and biological rationale. -- Next: Phase 3 — `pak::local_install()`, `tar_make()`, compare rollup to bcfishpass; expected direction — BT/WCT/ST on ADMS/BABL recover to near pre-fix, CH/CM/CO/PK/SK slightly closer to bcfishpass. -- Phase 2a alone was insufficient — CH/CO/SK/ST on ADMS/BABL still drifted -15 to -22pp. Investigation traced the residual to my ctrl_filter also blocking the habitat-path INSERT in lnk_barrier_overrides. bcfishpass's `hab_upstr` CTE has no control join — habitat is higher-trust and bypasses the filter. -- Phase 2b (6f3bc46) — removed ctrl_where/ctrl_filter from habitat INSERT; flipped the "control applies to habitat" test to assert absence; docstring notes habitat bypass. 279 PASS. -- Post-Phase-2b rollup exactly matches pre-fix baseline on all 4 parity WSGs. Investigation showed all 6 TRUE control rows on ADMS/BULK/BABL are rescued by observation threshold or habitat path — filter correctly wired but inactive on these WSGs. -- Phase 2c: province-wide hunt for TRUE control rows with ≥ threshold obs AND zero habitat upstream produced CAMB (11 obs), DEAD (6), LFRA (16 but too large), SALM (7). Picked DEAD — single TRUE control row at FALLS (356361749, 45743) with exactly 6 CH-group obs and zero habitat. Added DEAD to `data-raw/_targets.R`, incremental tar_make builds only comparison_DEAD + rollup (42s). -- DEAD rollup: all species within 3% of bcfishpass reference. Direct inspection of `working_dead.barrier_overrides` at (356361749, 45743): BT only, confirming per-species gate (BT bypass + CH/CM/CO/PK/SK/ST blocked). Commit fb8a0db. -- Log files committed (1c683e3): 20260423_01_phase2, _02_phase2a, _03_phase2b, _04_repro, _05_dead, _06_repro_dead. -- 5-WSG rebuild reproducibility confirmed: two consecutive `tar_destroy + tar_make` produce rollup digest `210c3f8254c47ac88573a80d96a2701e`, 46 rows, identical. -- Phase 4 (f52dcbc): NEWS 0.6.0, DESCRIPTION 0.5.0→0.6.0, research doc (DEAD table + key-fixes row + three-part-fix subsection + DAG update), vignette (5-WSG narrative + pivot column), bcfishpass config README updated, vignette artifacts regenerated. -- Follow-up filed: #46 (migrate `.lnk_pipeline_prep_gradient()` + `.lnk_pipeline_prep_overrides()` probes to manifest-driven gating). -- Branch pushed. PR #47 opened. SRED tag `Relates to NewGraphEnvironment/sred-2025-2026#24` in body. -- Flagged in PR: commit 22ac1dd ("comms(→link): M1 verified as R-worker host") landed on this branch from a parallel session's branch-landing policy; orthogonal to #44 scope. +- PR #47 merged (link 0.6.0, squash `8eda3fb`). Local main rebased onto origin/main; b330672 dropped as empty (already in squash). +- Branched `48-user-barriers-definite-bypass` off main. +- Archived #44 PWF under `planning/archive/2026-04-23-barriers-definite-control/` with README. +- Pre-flight diagnostic (findings.md): ELKR has 4 user-definite override matches that need fixing; other 4 WSGs are empty for barriers_definite. ELKR is the active test case. +- Next: Phase 1 — modify `.lnk_pipeline_prep_natural()` and `.lnk_pipeline_prep_minimal()` per Shape A in findings.md. diff --git a/planning/active/task_plan.md b/planning/active/task_plan.md index 7f8ca92..8530d46 100644 --- a/planning/active/task_plan.md +++ b/planning/active/task_plan.md @@ -1,92 +1,48 @@ -# Task Plan: Wire barriers_definite_control into lnk_barrier_overrides (#44) +# Task Plan: user_barriers_definite should bypass override (#48) ## Goal -Honour `user_barriers_definite_control.csv`'s `barrier_ind = TRUE` rows at the observation-override step. Positions marked as non-overridable (known fish-blocking dams, long impassable falls, diversions) must never be re-opened by historical upstream observations. Matches bcfishpass's per-species access SQL. +Match bcfishpass's architecture for `user_barriers_definite` rows: they must be appended to each per-model barrier table **post-filter** — always blocking, never eligible for observation override. Same-family fix as #44 but different mechanism. -Bit-identical-across-reruns reproducibility preserved. Rollup direction expected: toward bcfishpass reference, not away. +Pre-fix defect on ELKR confirmed: 4 override rows at user-definite positions (Erickson Creek exclusion, 2× Spillway MISC) that bcfishpass would keep as blockers. -## Phase 1: lnk_barrier_overrides control filter fix +## Phase 1: Code change (simpler than initially planned) -- [x] Read `R/lnk_barrier_overrides.R` control block. Confirmed: current filter treated ANY control row as blocking; docstring said only `barrier_ind = TRUE` rows block. -- [x] Updated `ctrl_filter` to `"AND (c.blue_line_key IS NULL OR c.barrier_ind::boolean = false)"`. -- [x] Updated the inline comment to describe the fixed semantics. -- [x] New test file `tests/testthat/test-lnk_barrier_overrides.R` with mocked SQL assertions — 7 tests covering observation-path control filter, NULL-control path, habitat-path control filter. -- [x] `devtools::test()` green: 265 PASS. -- [x] lintr clean on changed R/test files (only pre-existing indentation style notes, consistent with the rest of the codebase). -- [ ] `/code-check` before commit +Investigation showed `barriers_definite` is already wired as a break source in `lnk_pipeline_break` (sequential `frs_break_apply`) and into `fresh.streams_breaks` in `lnk_pipeline_classify` (access-gating barrier table). User-definite positions already end up as segment boundaries AND as blocking barriers at classification — bcfishpass parity on those two surfaces. -## Phase 2: Wire control through .lnk_pipeline_prep_overrides +The only defect is `.lnk_pipeline_prep_natural()` UNIONing `barriers_definite` into `natural_barriers`. `natural_barriers` is passed to `lnk_barrier_overrides()` which generates per-species override (skip) rows for any barrier with threshold observations upstream. That's what lets user-definite positions be re-opened. -- [x] Updated `.lnk_pipeline_prep_overrides` with manifest-gated `control_arg` computation; passes `control = control_arg` to `lnk_barrier_overrides`. -- [x] Fixed asymmetric gating — `.lnk_pipeline_prep_load_aux` now always creates a schema-valid (possibly empty) `.barriers_definite_control` table when the manifest declares the key, even if the AOI has zero control rows. Mirrors the `barriers_definite` pattern above. Lets `.lnk_pipeline_prep_overrides` gate on the manifest without worrying about the per-AOI row count. -- [x] Two new `.lnk_pipeline_prep_overrides` tests in `test-lnk_pipeline_prepare.R` — manifest present → `control = ".barriers_definite_control"`; manifest absent → `control = NULL`. -- [x] `devtools::test()` green: 271 PASS. -- [x] `/code-check` surfaced the asymmetric-gating bug — fixed and re-verified before commit. +Only `natural_barriers` caller outside prep_natural is `.lnk_pipeline_prep_overrides()` (confirmed via grep). Safe to drop the definite UNION without touching prep_minimal. -## Phase 2a: Per-species control gate (observation_control_apply) +- [x] `.lnk_pipeline_prep_natural()` — drop the `INSERT INTO natural_barriers SELECT ... FROM barriers_definite` block. `natural_barriers` becomes gradient + falls only. Inline NOTE comment explains the bcfishpass parity reasoning. +- [ ] Update tests in `test-lnk_pipeline_prepare.R` — the existing `prep_natural unions gradient + falls + definite` assertion needs to drop the "definite" clause. -Post-Phase-2 `tar_make()` drifted 11–22pp *away* from bcfishpass on ADMS/BABL because bcfishpass applies the control filter per-species (CH/CM/CO/PK/SK and ST only), while my implementation applied it across all species. Residents (BT, WCT) inhabit reaches upstream of anadromous-blocking falls — their observations should still override. +## Phase 2: Verification -- [x] Add `observation_control_apply` column to `inst/extdata/configs/bcfishpass/parameters_fresh.csv`. TRUE for CH/CM/CO/PK/SK/ST; FALSE for BT/WCT; NA for CT/DV/RB. -- [x] `lnk_barrier_overrides()` gates the NOT EXISTS clause per-species on `params$observation_control_apply[i]`. Missing column or NA ⇒ no filter (resident default). -- [x] Updated `@param control` / `@param params` roxygen to document the gate. -- [x] Extended `.stub_params()` in `test-lnk_barrier_overrides.R` with optional `control_apply`. Three new tests: FALSE ⇒ no clause, NA ⇒ no clause, mixed-species params ⇒ per-species gating. -- [x] `devtools::test()`: 279 PASS. -- [x] Amend issue #44 body with Phase 2a scope and biological rationale. -- [x] `/code-check` before commit — two rounds, both Clean. +- [ ] `devtools::test()` — all green. +- [ ] `pak::local_install()` and `cd data-raw && tar_destroy + tar_make`. +- [ ] Query `working_elkr.barrier_overrides` joined to `working_elkr.barriers_definite` — should be empty (pre-fix: 4 matches). +- [ ] ELKR rollup should shift toward bcfishpass: link BT/WCT spawning currently +3.4% / +4.0%; expected to DECREASE (closer to 0) since upstream habitat at Erickson and Spillway positions now correctly blocked. +- [ ] Reproducibility: two consecutive `tar_destroy + tar_make` produce bit-identical 46-row rollups. -## Phase 2b: Ungate habitat override path from control +## Phase 3: Artifacts -Phase 2a species-gating fixed BT/WCT drift but CH/CM/CO/PK/SK/ST still dropped 11–22pp on ADMS/BABL. Root cause: my `ctrl_filter` was applied to BOTH the observation and habitat paths of `lnk_barrier_overrides()`. bcfishpass's `hab_upstr` CTE has no control join at all — expert-confirmed habitat is higher-trust than the control designation and bypasses the filter. +- [ ] Regenerate vignette artifacts via `data-raw/vignette_reproducing_bcfishpass.R`. +- [ ] Correct vignette text: user-definite barriers bullet no longer says "eligible for per-species override". Say they always block, are appended post-filter (bcfishpass parity). +- [ ] `research/bcfishpass_comparison.md` — new row in "Key fixes" table; update ELKR parity table with post-fix numbers; short paragraph describing the fix. +- [ ] `NEWS.md` 0.7.0 entry. +- [ ] `DESCRIPTION` 0.6.0 → 0.7.0. -- [x] Removed `ctrl_where` / `ctrl_filter` from the habitat INSERT in `lnk_barrier_overrides()`. Observation path unchanged. -- [x] Updated roxygen: control parameter now notes it applies only to observations; habitat bypasses. -- [x] Flipped the existing "control filter applies to habitat too" test to assert the opposite (bcfishpass parity). `devtools::test()` 279 PASS. -- [x] Committed (6f3bc46). -- [x] `tar_make()` — Phase 2b rollup numerically identical to pre-fix baseline on all 34 rows, all 4 WSGs within 5% of bcfishpass reference. +## Phase 4: Ship -## Phase 2c: Add DEAD as the filter's end-to-end test WSG - -Discovered post-Phase 2b: none of ADMS/BULK/BABL/ELKR actually exercises the new control filter end-to-end. All 6 TRUE control rows across these WSGs are rescued by either the observation threshold (obs < 5) or the habitat path (classification upstream). That's why post-fix == pre-fix — correct, but information-less. - -Province-wide hunt for TRUE control rows with ≥ threshold observations upstream AND zero habitat coverage turned up 4 candidates: CAMB (11 obs), DEAD (6), LFRA (16, but too large), SALM (7). Picked **DEAD** (Deadman River) — smallest runtime, 6 obs just above CH threshold, single TRUE control row at FALLS (356361749, 45743). bcfishpass reference keeps this fall in `barriers_ch_cm_co_pk_sk` (control worked); pre-fix link would have overridden via observations. - -- [x] Added DEAD to `data-raw/_targets.R` wsgs vector. -- [ ] `tar_make()` incremental — builds `comparison_DEAD` + new rollup (ADMS/BULK/BABL/ELKR cached from Phase 2b run). -- [ ] Verify DEAD's diff_pct on CH/CO/SK/ST is small (post-fix link ≈ bcfishpass — filter working). -- [ ] Verify the specific fall at (356361749, 45743) is NOT in `working_dead.barrier_overrides` for CH/CM/CO/PK/SK/ST (filter blocked the override). - -## Phase 3: End-to-end verification - -- [x] `pak::local_install()` to pick up pipeline changes. -- [x] First post-fix run: `20260423_02_tar_make_phase2a.txt`, `20260423_03_tar_make_phase2b.txt`. -- [x] Inspect rollup against pre-change baseline — matches exactly on 4 WSGs (filter moot on those; DEAD being added to exercise it). -- [ ] Reproducibility run (Phase 2b state): `20260423_04_tar_make_repro.txt` in progress. Rollup must be bit-identical to Phase 2b. -- [ ] `digest::digest()` on two Phase 2b rollup tibbles → same hash. -- [ ] Post-DEAD reproducibility: two consecutive `tar_make()` runs with DEAD present produce bit-identical 5-WSG rollups. - -## Phase 4: Artifact updates - -- [ ] Regenerate vignette data: `Rscript data-raw/vignette_reproducing_bcfishpass.R`. Produces new `rollup.rds` + `sub_ch.rds` + `sub_ch_bcfp.rds`. -- [ ] Render vignette locally to verify pivot tables + map update cleanly -- [ ] Update `research/bcfishpass_comparison.md`: - - Four per-WSG parity tables with new numbers - - Short paragraph under "Key fixes during comparison" documenting the control wiring + numeric direction -- [ ] `NEWS.md` 0.6.0 entry: "Honour `user_barriers_definite_control.csv` at the observation-override step. Previously controlled positions could be re-opened by upstream observations; now they can't." -- [ ] `DESCRIPTION` version bump 0.5.0 → 0.6.0 - -## Phase 5: Ship - -- [ ] `/code-check` on full staged diff -- [ ] Commit atomically per the plan's commit layout -- [ ] Push branch -- [ ] Open PR with SRED tag `Relates to NewGraphEnvironment/sred-2025-2026#24` -- [ ] **File follow-up issue** (before closing PR 44): "Migrate remaining pipeline probes to manifest-driven gating". See `/Users/airvine/.claude/plans/stateful-hopping-feather.md` for scope. +- [ ] `/code-check` on staged diff — 2 rounds minimum. +- [ ] Commit atomically (code + tests, then verification, then artifacts). +- [ ] Push branch. +- [ ] Open PR with SRED tag (`Relates to NewGraphEnvironment/sred-2025-2026#24`), link to #48, note closing of #48. ## Versions at start - fresh: 0.14.0 -- link: main (0.5.0, target 0.6.0) +- link: main (0.6.0, target 0.7.0) - bcfishpass: ea3c5d8 - fwapg: Docker (FWA 20240830) diff --git a/planning/archive/2026-04-23-barriers-definite-control/README.md b/planning/archive/2026-04-23-barriers-definite-control/README.md new file mode 100644 index 0000000..05cb83d --- /dev/null +++ b/planning/archive/2026-04-23-barriers-definite-control/README.md @@ -0,0 +1,10 @@ +# #44 — barriers_definite_control wiring (closed) + +PR #47 merged 2026-04-23 as link 0.6.0 (squash commit `8eda3fb`). + +Wired `user_barriers_definite_control.csv` through `lnk_barrier_overrides()` at the observation-override step: TRUE control rows block observation-based overrides; habitat-path bypasses the filter (bcfishpass parity); per-species gate via new `observation_control_apply` column in `parameters_fresh.csv` (TRUE for CH/CM/CO/PK/SK/ST, FALSE for BT/WCT). Added DEAD (Deadman River) as end-to-end test WSG — single TRUE control row at FALLS (356361749, 45743) with 6 anadromous obs upstream and zero habitat coverage. Reproducibility verified: two consecutive `tar_destroy + tar_make` produce bit-identical 46-row rollups (digest `210c3f8254c47ac88573a80d96a2701e`). + +## Follow-ups filed + +- #46 — Migrate remaining `information_schema` probes to manifest-driven gating +- #48 — `user_barriers_definite` should bypass override per bcfishpass (same family as #44) diff --git a/planning/archive/2026-04-23-barriers-definite-control/findings.md b/planning/archive/2026-04-23-barriers-definite-control/findings.md new file mode 100644 index 0000000..3ee943e --- /dev/null +++ b/planning/archive/2026-04-23-barriers-definite-control/findings.md @@ -0,0 +1,60 @@ +# Findings: Wire barriers_definite_control (#44) + +## Where the gap lives + +Three places where `"barriers_definite_control"` is referenced in link today: + +1. **`R/lnk_pipeline_prepare.R` → `.lnk_pipeline_prep_load_aux`** — loads the per-AOI filtered CSV rows into `.barriers_definite_control` when `cfg$overrides$barriers_definite_control` is non-NULL. Already correct. +2. **`R/lnk_pipeline_prepare.R` → `.lnk_pipeline_prep_gradient`** — `information_schema` probe for the table, then `DELETE FROM gradient_barriers_raw g USING barriers_definite_control c WHERE ... c.barrier_ind::boolean = false`. Already correct. Removes **passable** positions from gradient set before minimal reduction. +3. **`R/lnk_pipeline_prepare.R` → `.lnk_pipeline_prep_overrides`** — does NOT pass `control` to `lnk_barrier_overrides`. This is the gap. `lnk_barrier_overrides` accepts a `control` parameter but is called without one from this site. + +## Latent bug in lnk_barrier_overrides control filter + +Read `R/lnk_barrier_overrides.R` lines 140–153. The implementation: + +```r +ctrl_where <- sprintf("LEFT JOIN %s c ON b.blue_line_key = c.blue_line_key + AND abs(b.downstream_route_measure - c.downstream_route_measure) < 1", control) +ctrl_filter <- "AND c.blue_line_key IS NULL" +``` + +Filter treats ANY control row as blocking override — including `barrier_ind = FALSE` rows. Docstring (lines 27–30) says only `barrier_ind = TRUE` rows block. + +In practice on bcfishpass input this is masked because `.lnk_pipeline_prep_gradient`'s upstream DELETE removes `barrier_ind = FALSE` positions from `gradient_barriers_raw` before they reach the override step. But falls and user-definite positions are not pruned by control at load time — they stay in `natural_barriers`. If a control row with `barrier_ind = FALSE` exists for a fall or definite-barrier position, the current filter blocks observation overrides on it. Should not block. + +Fix: `"AND (c.blue_line_key IS NULL OR c.barrier_ind::boolean = false)"`. + +## Manifest-driven gating decision + +`.lnk_pipeline_prep_overrides` could probe `information_schema.tables` to discover whether the control table exists (same pattern used there for habitat). Decided against: the manifest key is the direct contract. If `cfg$overrides$barriers_definite_control` is non-NULL the load step wrote the table; if it's NULL no table exists. Manifest gate, not DB probe. + +Scope discipline: the existing `information_schema` probe for the habitat table in the same function, and the similar probe in `.lnk_pipeline_prep_gradient` for the control table, work correctly today. Leaving them alone in this PR; filing a follow-up issue for consistency. Using the manifest as the contract is well preferred across the package. + +## Tests that need to exist + +- `tests/testthat/test-lnk_barrier_overrides.R` does not exist. Creating new. +- `tests/testthat/test-lnk_pipeline_prepare.R` exists — extending with prep_overrides control pass-through tests. + +## No `information_schema` probe in the new code + +`.lnk_pipeline_prep_overrides`'s new control guard reads `cfg$overrides$barriers_definite_control` directly. That field is populated by `lnk_config()` when the manifest declares the key. No DB round-trip needed. + +## Expected rollup direction + +Running the pipeline pre-fix vs post-fix on bcfishpass config: + +- WSGs with `user_barriers_definite_control.csv` rows having `barrier_ind = TRUE` and upstream observations at those positions → rollup `link_km` shrinks for affected species (positions that were wrongly overridden are no longer overridden). Moves toward bcfishpass reference. +- WSGs with no such rows → rollup unchanged. + +Magnitude: unknown. Control-TRUE rows on the four validated WSGs are uncommon, so likely small. Direction matters more than magnitude. + +## Reproducibility + +The change is a deterministic additional filter clause on a `LEFT JOIN`. No new randomness, no schedule-dependent behaviour. Two back-to-back `tar_make()` runs must produce bit-identical rollups. Will verify with `digest::digest()`. + +## Cross-refs + +- Plan file: `/Users/airvine/.claude/plans/stateful-hopping-feather.md` +- Issue: link#44 +- Parallel cleanup issue (separate PR): link#45 (gradient classes) +- Follow-up to file at end of this PR: "Migrate remaining pipeline probes to manifest-driven gating" diff --git a/planning/archive/2026-04-23-barriers-definite-control/progress.md b/planning/archive/2026-04-23-barriers-definite-control/progress.md new file mode 100644 index 0000000..2b5960d --- /dev/null +++ b/planning/archive/2026-04-23-barriers-definite-control/progress.md @@ -0,0 +1,25 @@ +# Progress + +## Session 2026-04-23 + +- Archived `2026-04-23-targets-pipeline/` — link#38 closed via PRs #41/#42/#43. Three consecutive `tar_make()` runs produced bit-identical rollups. All species within 5% of bcfishpass reference on all four WSGs. +- Branched `44-barriers-definite-control` off main. +- Plan approved. PWF initialized for #44. +- Pre-flight complete: identified the `ctrl_filter` bug in `lnk_barrier_overrides` (all rows block, not just `barrier_ind = TRUE`), and confirmed `.lnk_pipeline_prep_overrides` doesn't pass `control`. Same PR fixes both — filter semantics + missing pass-through. +- Next: Phase 1 — fix `R/lnk_barrier_overrides.R` `ctrl_filter` and add `tests/testthat/test-lnk_barrier_overrides.R`. +- Phase 1 committed (d1a7109) — `NOT EXISTS` control filter, 11 tests, 269 PASS. +- Phase 2 committed (53bedbd) — manifest-gated `control` pass-through in `.lnk_pipeline_prep_overrides`, fixed asymmetric load_aux (schema-valid empty table), 271 PASS. +- Post-Phase-2 `tar_make()` (log: `data-raw/logs/20260423_01_tar_make_post_44.txt`) showed 11–22pp drift AWAY from bcfishpass on ADMS/BABL; BULK/ELKR unchanged. Root cause: bcfishpass applies control filter only in CH/CM/CO/PK/SK and ST models (not BT/WCT/CT/DV/RB). My implementation applied it across all species in the `params` loop. +- Phase 2a: new `observation_control_apply` column in `parameters_fresh.csv` (TRUE for CH/CM/CO/PK/SK/ST; FALSE for BT/WCT; NA for CT/DV/RB), per-species NOT EXISTS gate in `lnk_barrier_overrides()`, three new tests. 279 PASS. Amendment pushed to issue #44 documenting the species-scoped approach and biological rationale. +- Next: Phase 3 — `pak::local_install()`, `tar_make()`, compare rollup to bcfishpass; expected direction — BT/WCT/ST on ADMS/BABL recover to near pre-fix, CH/CM/CO/PK/SK slightly closer to bcfishpass. +- Phase 2a alone was insufficient — CH/CO/SK/ST on ADMS/BABL still drifted -15 to -22pp. Investigation traced the residual to my ctrl_filter also blocking the habitat-path INSERT in lnk_barrier_overrides. bcfishpass's `hab_upstr` CTE has no control join — habitat is higher-trust and bypasses the filter. +- Phase 2b (6f3bc46) — removed ctrl_where/ctrl_filter from habitat INSERT; flipped the "control applies to habitat" test to assert absence; docstring notes habitat bypass. 279 PASS. +- Post-Phase-2b rollup exactly matches pre-fix baseline on all 4 parity WSGs. Investigation showed all 6 TRUE control rows on ADMS/BULK/BABL are rescued by observation threshold or habitat path — filter correctly wired but inactive on these WSGs. +- Phase 2c: province-wide hunt for TRUE control rows with ≥ threshold obs AND zero habitat upstream produced CAMB (11 obs), DEAD (6), LFRA (16 but too large), SALM (7). Picked DEAD — single TRUE control row at FALLS (356361749, 45743) with exactly 6 CH-group obs and zero habitat. Added DEAD to `data-raw/_targets.R`, incremental tar_make builds only comparison_DEAD + rollup (42s). +- DEAD rollup: all species within 3% of bcfishpass reference. Direct inspection of `working_dead.barrier_overrides` at (356361749, 45743): BT only, confirming per-species gate (BT bypass + CH/CM/CO/PK/SK/ST blocked). Commit fb8a0db. +- Log files committed (1c683e3): 20260423_01_phase2, _02_phase2a, _03_phase2b, _04_repro, _05_dead, _06_repro_dead. +- 5-WSG rebuild reproducibility confirmed: two consecutive `tar_destroy + tar_make` produce rollup digest `210c3f8254c47ac88573a80d96a2701e`, 46 rows, identical. +- Phase 4 (f52dcbc): NEWS 0.6.0, DESCRIPTION 0.5.0→0.6.0, research doc (DEAD table + key-fixes row + three-part-fix subsection + DAG update), vignette (5-WSG narrative + pivot column), bcfishpass config README updated, vignette artifacts regenerated. +- Follow-up filed: #46 (migrate `.lnk_pipeline_prep_gradient()` + `.lnk_pipeline_prep_overrides()` probes to manifest-driven gating). +- Branch pushed. PR #47 opened. SRED tag `Relates to NewGraphEnvironment/sred-2025-2026#24` in body. +- Flagged in PR: commit 22ac1dd ("comms(→link): M1 verified as R-worker host") landed on this branch from a parallel session's branch-landing policy; orthogonal to #44 scope. diff --git a/planning/archive/2026-04-23-barriers-definite-control/task_plan.md b/planning/archive/2026-04-23-barriers-definite-control/task_plan.md new file mode 100644 index 0000000..7f8ca92 --- /dev/null +++ b/planning/archive/2026-04-23-barriers-definite-control/task_plan.md @@ -0,0 +1,92 @@ +# Task Plan: Wire barriers_definite_control into lnk_barrier_overrides (#44) + +## Goal + +Honour `user_barriers_definite_control.csv`'s `barrier_ind = TRUE` rows at the observation-override step. Positions marked as non-overridable (known fish-blocking dams, long impassable falls, diversions) must never be re-opened by historical upstream observations. Matches bcfishpass's per-species access SQL. + +Bit-identical-across-reruns reproducibility preserved. Rollup direction expected: toward bcfishpass reference, not away. + +## Phase 1: lnk_barrier_overrides control filter fix + +- [x] Read `R/lnk_barrier_overrides.R` control block. Confirmed: current filter treated ANY control row as blocking; docstring said only `barrier_ind = TRUE` rows block. +- [x] Updated `ctrl_filter` to `"AND (c.blue_line_key IS NULL OR c.barrier_ind::boolean = false)"`. +- [x] Updated the inline comment to describe the fixed semantics. +- [x] New test file `tests/testthat/test-lnk_barrier_overrides.R` with mocked SQL assertions — 7 tests covering observation-path control filter, NULL-control path, habitat-path control filter. +- [x] `devtools::test()` green: 265 PASS. +- [x] lintr clean on changed R/test files (only pre-existing indentation style notes, consistent with the rest of the codebase). +- [ ] `/code-check` before commit + +## Phase 2: Wire control through .lnk_pipeline_prep_overrides + +- [x] Updated `.lnk_pipeline_prep_overrides` with manifest-gated `control_arg` computation; passes `control = control_arg` to `lnk_barrier_overrides`. +- [x] Fixed asymmetric gating — `.lnk_pipeline_prep_load_aux` now always creates a schema-valid (possibly empty) `.barriers_definite_control` table when the manifest declares the key, even if the AOI has zero control rows. Mirrors the `barriers_definite` pattern above. Lets `.lnk_pipeline_prep_overrides` gate on the manifest without worrying about the per-AOI row count. +- [x] Two new `.lnk_pipeline_prep_overrides` tests in `test-lnk_pipeline_prepare.R` — manifest present → `control = ".barriers_definite_control"`; manifest absent → `control = NULL`. +- [x] `devtools::test()` green: 271 PASS. +- [x] `/code-check` surfaced the asymmetric-gating bug — fixed and re-verified before commit. + +## Phase 2a: Per-species control gate (observation_control_apply) + +Post-Phase-2 `tar_make()` drifted 11–22pp *away* from bcfishpass on ADMS/BABL because bcfishpass applies the control filter per-species (CH/CM/CO/PK/SK and ST only), while my implementation applied it across all species. Residents (BT, WCT) inhabit reaches upstream of anadromous-blocking falls — their observations should still override. + +- [x] Add `observation_control_apply` column to `inst/extdata/configs/bcfishpass/parameters_fresh.csv`. TRUE for CH/CM/CO/PK/SK/ST; FALSE for BT/WCT; NA for CT/DV/RB. +- [x] `lnk_barrier_overrides()` gates the NOT EXISTS clause per-species on `params$observation_control_apply[i]`. Missing column or NA ⇒ no filter (resident default). +- [x] Updated `@param control` / `@param params` roxygen to document the gate. +- [x] Extended `.stub_params()` in `test-lnk_barrier_overrides.R` with optional `control_apply`. Three new tests: FALSE ⇒ no clause, NA ⇒ no clause, mixed-species params ⇒ per-species gating. +- [x] `devtools::test()`: 279 PASS. +- [x] Amend issue #44 body with Phase 2a scope and biological rationale. +- [x] `/code-check` before commit — two rounds, both Clean. + +## Phase 2b: Ungate habitat override path from control + +Phase 2a species-gating fixed BT/WCT drift but CH/CM/CO/PK/SK/ST still dropped 11–22pp on ADMS/BABL. Root cause: my `ctrl_filter` was applied to BOTH the observation and habitat paths of `lnk_barrier_overrides()`. bcfishpass's `hab_upstr` CTE has no control join at all — expert-confirmed habitat is higher-trust than the control designation and bypasses the filter. + +- [x] Removed `ctrl_where` / `ctrl_filter` from the habitat INSERT in `lnk_barrier_overrides()`. Observation path unchanged. +- [x] Updated roxygen: control parameter now notes it applies only to observations; habitat bypasses. +- [x] Flipped the existing "control filter applies to habitat too" test to assert the opposite (bcfishpass parity). `devtools::test()` 279 PASS. +- [x] Committed (6f3bc46). +- [x] `tar_make()` — Phase 2b rollup numerically identical to pre-fix baseline on all 34 rows, all 4 WSGs within 5% of bcfishpass reference. + +## Phase 2c: Add DEAD as the filter's end-to-end test WSG + +Discovered post-Phase 2b: none of ADMS/BULK/BABL/ELKR actually exercises the new control filter end-to-end. All 6 TRUE control rows across these WSGs are rescued by either the observation threshold (obs < 5) or the habitat path (classification upstream). That's why post-fix == pre-fix — correct, but information-less. + +Province-wide hunt for TRUE control rows with ≥ threshold observations upstream AND zero habitat coverage turned up 4 candidates: CAMB (11 obs), DEAD (6), LFRA (16, but too large), SALM (7). Picked **DEAD** (Deadman River) — smallest runtime, 6 obs just above CH threshold, single TRUE control row at FALLS (356361749, 45743). bcfishpass reference keeps this fall in `barriers_ch_cm_co_pk_sk` (control worked); pre-fix link would have overridden via observations. + +- [x] Added DEAD to `data-raw/_targets.R` wsgs vector. +- [ ] `tar_make()` incremental — builds `comparison_DEAD` + new rollup (ADMS/BULK/BABL/ELKR cached from Phase 2b run). +- [ ] Verify DEAD's diff_pct on CH/CO/SK/ST is small (post-fix link ≈ bcfishpass — filter working). +- [ ] Verify the specific fall at (356361749, 45743) is NOT in `working_dead.barrier_overrides` for CH/CM/CO/PK/SK/ST (filter blocked the override). + +## Phase 3: End-to-end verification + +- [x] `pak::local_install()` to pick up pipeline changes. +- [x] First post-fix run: `20260423_02_tar_make_phase2a.txt`, `20260423_03_tar_make_phase2b.txt`. +- [x] Inspect rollup against pre-change baseline — matches exactly on 4 WSGs (filter moot on those; DEAD being added to exercise it). +- [ ] Reproducibility run (Phase 2b state): `20260423_04_tar_make_repro.txt` in progress. Rollup must be bit-identical to Phase 2b. +- [ ] `digest::digest()` on two Phase 2b rollup tibbles → same hash. +- [ ] Post-DEAD reproducibility: two consecutive `tar_make()` runs with DEAD present produce bit-identical 5-WSG rollups. + +## Phase 4: Artifact updates + +- [ ] Regenerate vignette data: `Rscript data-raw/vignette_reproducing_bcfishpass.R`. Produces new `rollup.rds` + `sub_ch.rds` + `sub_ch_bcfp.rds`. +- [ ] Render vignette locally to verify pivot tables + map update cleanly +- [ ] Update `research/bcfishpass_comparison.md`: + - Four per-WSG parity tables with new numbers + - Short paragraph under "Key fixes during comparison" documenting the control wiring + numeric direction +- [ ] `NEWS.md` 0.6.0 entry: "Honour `user_barriers_definite_control.csv` at the observation-override step. Previously controlled positions could be re-opened by upstream observations; now they can't." +- [ ] `DESCRIPTION` version bump 0.5.0 → 0.6.0 + +## Phase 5: Ship + +- [ ] `/code-check` on full staged diff +- [ ] Commit atomically per the plan's commit layout +- [ ] Push branch +- [ ] Open PR with SRED tag `Relates to NewGraphEnvironment/sred-2025-2026#24` +- [ ] **File follow-up issue** (before closing PR 44): "Migrate remaining pipeline probes to manifest-driven gating". See `/Users/airvine/.claude/plans/stateful-hopping-feather.md` for scope. + +## Versions at start + +- fresh: 0.14.0 +- link: main (0.5.0, target 0.6.0) +- bcfishpass: ea3c5d8 +- fwapg: Docker (FWA 20240830) diff --git a/research/bcfishpass_comparison.md b/research/bcfishpass_comparison.md index 595060a..c931ab5 100644 --- a/research/bcfishpass_comparison.md +++ b/research/bcfishpass_comparison.md @@ -44,8 +44,10 @@ All species within 5% of bcfishpass reference. Pipeline runs serially in ~8.5 mi | Species | Spawning | Rearing | |---------|----------|---------| -| BT | +3.4% | -0.7% | -| WCT | +4.0% | +1.6% | +| BT | +2.8% | -1.2% | +| WCT | +2.6% | +0.3% | + +Updated 2026-04-23 (#48) — #48 removed `barriers_definite` from `natural_barriers`, which stopped observation-based override of user-definite positions (Erickson Creek exclusion + 2 Spillway MISC entries). Those four positions now correctly block upstream habitat, bringing link toward bcfishpass. Pre-#48 numbers: BT +3.4% / -0.7%, WCT +4.0% / +1.6%. ### DEAD @@ -146,6 +148,7 @@ Composite steps in the DAG that aren't a single function call: | Three-phase cluster | CH +6% → +2.6% | fresh code (0.13.8) | | Index input tables | 228s → 6.6s classification | fresh code (0.13.4) | | Wire `barriers_definite_control` into override step, per-species, observation-path only | DEAD CH/CO/PK/ST +1.1 to +1.4% (moot on ADMS/BULK/BABL/ELKR) | link code (0.6.0) | +| Drop `barriers_definite` from `natural_barriers` (not eligible for observation override) | ELKR BT spawn +3.4% → +2.8%, WCT spawn +4.0% → +2.6%, WCT rear +1.6% → +0.3% | link code (0.7.0) | ### barriers_definite_control wiring (#44) @@ -157,6 +160,14 @@ bcfishpass pairs `user_barriers_definite.csv` with a control table that flags po End-to-end validation on DEAD (added specifically for this reason — see section above). Numerical impact on the four original WSGs is zero because every TRUE control row is already rescued by the observation threshold or the habitat path; the filter is correctly wired but inactive on those WSGs. +### user_barriers_definite bypass (#48) + +Same-family fix as #44, different mechanism. bcfishpass's `model_access_*.sql` builds its barriers CTE from gradient + falls + subsurfaceflow only; `barriers_user_definite` is appended post-filter via `UNION ALL`, so upstream observations and habitat confirmations never re-open user-definite positions. link's `.lnk_pipeline_prep_natural()` was unioning `barriers_definite` into `natural_barriers`, which `lnk_barrier_overrides()` iterates over — the 227 reviewer-added user-definite rows (EXCLUSION zones, MISC-type barriers) became eligible for observation override. + +Active defect on ELKR pre-fix: 4 rows in `working_elkr.barrier_overrides` matched `working_elkr.barriers_definite` positions — Erickson Creek exclusion (mining impacts) and two Spillway MISC entries. Post-fix: 0 matches on all 5 WSGs, and ELKR rollup shifts toward bcfishpass on BT/WCT spawning and rearing (see per-WSG table above). Other four WSGs unchanged: ADMS/BABL/DEAD have empty `barriers_definite`; BULK has 87 rows but none have observation counts clearing threshold. + +Fix is a single edit to `.lnk_pipeline_prep_natural()` — drop the `INSERT INTO natural_barriers SELECT ... FROM barriers_definite` block. `barriers_definite` stays consumed separately: `lnk_pipeline_break()` applies it as a sequential break source (segmentation boundary); `lnk_pipeline_classify()` UNION ALLs it directly into `fresh.streams_breaks` (access-gating barrier set). Both surfaces are unchanged. + ## Remaining gaps ### BT rearing -2.2% (BULK) diff --git a/tests/testthat/test-lnk_pipeline_prepare.R b/tests/testthat/test-lnk_pipeline_prepare.R index 49f42ed..3d08904 100644 --- a/tests/testthat/test-lnk_pipeline_prepare.R +++ b/tests/testthat/test-lnk_pipeline_prepare.R @@ -105,7 +105,12 @@ test_that(".lnk_pipeline_prep_gradient prunes by control when control table exis # -- prep_natural SQL shape ------------------------------------------------- -test_that(".lnk_pipeline_prep_natural unions gradient + falls + definite", { +test_that(".lnk_pipeline_prep_natural unions gradient + falls only (no definite)", { + # barriers_definite is intentionally NOT unioned into natural_barriers — + # bcfishpass appends user-definite post-filter in each model_access_*.sql, + # so observations/habitat never override them. lnk_pipeline_break handles + # them as a separate break source; lnk_pipeline_classify emits them into + # fresh.streams_breaks directly. captured <- character(0) local_mocked_bindings( .lnk_db_execute = function(conn, sql) { @@ -120,7 +125,7 @@ test_that(".lnk_pipeline_prep_natural unions gradient + falls + definite", { expect_match(joined, "CREATE TABLE w_bulk.natural_barriers") expect_match(joined, "FROM w_bulk.gradient_barriers_raw g") expect_match(joined, "FROM w_bulk.falls f") - expect_match(joined, "FROM w_bulk.barriers_definite d") + expect_no_match(joined, "FROM w_bulk\\.barriers_definite\\b") expect_match(joined, "'blocked'") }) diff --git a/vignettes/reproducing-bcfishpass.Rmd b/vignettes/reproducing-bcfishpass.Rmd index 5a51507..ceedd7b 100644 --- a/vignettes/reproducing-bcfishpass.Rmd +++ b/vignettes/reproducing-bcfishpass.Rmd @@ -134,10 +134,13 @@ where the decision can change: (mirrored at [`inst/extdata/configs/bcfishpass/overrides/user_barriers_definite.csv`](https://github.com/NewGraphEnvironment/link/blob/main/inst/extdata/configs/bcfishpass/overrides/user_barriers_definite.csv)). Each row specifies `blue_line_key` and `downstream_route_measure` - for a barrier that always blocks access. Treated the same as falls - — always-blocking, always a break position, eligible for - per-species override via `lnk_barrier_overrides` when enough - upstream observations clear the threshold. + for a barrier that always blocks access — reviewer-added positions + covering EXCLUSION zones and MISC barriers the model doesn't detect + from gradient / falls / subsurface detection. These are always- + blocking, always a break position, and **never eligible for + observation-based override** — matches bcfishpass's `model_access_*.sql`, + which appends `barriers_user_definite` post-filter via `UNION ALL` so + upstream observations and habitat confirmations never re-open them. - **Habitat classification endpoints** — manual spawning / rearing delineations from bcfishpass's