diff --git a/CLAUDE.md b/CLAUDE.md index 4b20c011..46b6e88e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -11,6 +11,25 @@ Experimental package — breaking all the time and loving the learning curve. St **Prefix:** `lnk_` **Branch:** `main` (current version: `DESCRIPTION` / [`NEWS.md`](NEWS.md)) +## Status (2026-10-02) — the validator scores each group on its own model (#299) + +**`lnk_habitat_validate()` re-tests a `mad` group on discharge, not width.** +- **One rule for the model:** fresh's `.frs_habitat_models()`, through `.lnk_wsg_model()`, + for classify and validator both. The method table is the one in the `cfg` you pass, so + swap it on that object; `lnk_pipeline_classify(method_csv =)` is invisible to it. +- **Labels are shared** (`fails_width` means the group's size); `model` / `mad_m3s` split + them. **`no_mad_threshold`** is a `mad` miss a MAD range alone would admit. +- Evidence: `data-raw/logs/habitat_validate_299/`; archive + `planning/archive/2026-10-issue-299-validate-mad/`. + +**Facts not worth re-deriving:** +- **"Persisted TRUE, predicate FALSE" cannot catch a wrong model.** A cw predicate on a + `mad` run is looser, so the error shows as `post_predicate` (27 BT rearing misses on + ADMS), never as a disagreement. +- **`no_mad_threshold` is a floor, not a count, and cannot score a fix**: same-cause + misses also read `width_null` / `fails_gradient_and_width`, and once a range exists the + label cannot fire. Score MAD thresholds with capture, cost and the band score. + ## Status (2026-10-01) — per-WSG `cw`/`mad` habitat model threaded (#286) **Each bundle's `parameters_habitat_method.csv` reaches fresh as `params_method`.** @@ -32,7 +51,8 @@ Experimental package — breaking all the time and loving the learning curve. St - **fresh 0.36.0 reversed `frs_db_conn()`'s precedence** (`PG*` first). `lnk_db_conn()` still reads `PG_*_SHARE` first; on a machine with both groups set they connect to different databases. -- **`lnk_habitat_validate()` is cw-only** (#299). +- **`lnk_habitat_validate()` scores each group on its own model** (#299), from the + `cfg` it is given; `no_mad_threshold` names a species with no MAD range. ## Status (2026-09-29) — #284 step 5: BT `rear_gradient_max` 0.1349 scored and held diff --git a/R/lnk_config.R b/R/lnk_config.R index 1350a5ee..727755da 100644 --- a/R/lnk_config.R +++ b/R/lnk_config.R @@ -356,6 +356,23 @@ print.lnk_config <- function(x, ...) { fresh_path } +# A habitat method table read as classify hands it to fresh: every column +# character, and no `na.strings`, so a group coded "NA" stays a code rather +# than becoming a missing value that fresh would then resolve to cw. +.lnk_habitat_method_read <- function(path) { + utils::read.csv(path, stringsAsFactors = FALSE, colClasses = "character", + na.strings = character(0)) +} + +# The model (`cw` or `mad`) each watershed group classifies on, resolved by +# fresh's own rule so classify and the validator cannot disagree with it: +# an unlisted group is cw, and a model other than cw/mad or a duplicated +# group code is an error. Unnamed, one per `wsg`. +.lnk_wsg_model <- function(params_method, wsg) { + resolve <- utils::getFromNamespace(".frs_habitat_models", "fresh") + unname(resolve(wsg, params_method)) +} + # Absolute path of a provenance entry. Keys are relative to the bundle that # declared them: the leaf for its own entries, `.dir` for inherited ones. .lnk_provenance_path <- function(cfg, rel) { diff --git a/R/lnk_habitat_validate.R b/R/lnk_habitat_validate.R index 3977d9a9..7d3ab5e9 100644 --- a/R/lnk_habitat_validate.R +++ b/R/lnk_habitat_validate.R @@ -93,9 +93,15 @@ #' the rear reason (compare them with `n_rearing_any`, not `n_rearing`). #' Both re-evaluate the bundle's own #' habitat predicates ([fresh::frs_habitat_predicates()] over `cfg$rules` -#' and its thresholds CSV, channel-width model) on the segment, then again -#' with the gradient and the channel width each moved to the stage's -#' minimum: +#' and its thresholds CSV) on the segment, then again with the gradient and +#' the size each moved to the stage's minimum. The size is the one the group +#' classified on: the model `cfg`'s `parameters_habitat_method.csv` gives +#' the WSG, resolved as [lnk_pipeline_classify()] resolves it (an unlisted +#' group is `cw`). On `cw` it is the channel width; on `mad` it is the mean +#' annual discharge `mad_m3s`, joined from +#' `whse_basemapping.fwa_stream_networks_discharge` on `linear_feature_id` +#' because the persist does not carry it. The `width` labels below mean +#' that size on either model; `model` and `mad_m3s` split them: #' - `NA` — captured; `no_segment` — the location attaches to no segment; #' - `not_accessible` — the segment's `access_` is not 1 or 2; #' - `fails_gradient` / `fails_width` / `width_null` — passes once that one @@ -105,10 +111,21 @@ #' - `fails_gradient_or_width` — either relaxation alone passes (through #' different branches of the rule); #' - `fails_gradient_and_width` — passes only with both relaxed; +#' - `no_mad_threshold` — a `mad` group where the species has no MAD range +#' for the stage (fresh then fails every inheriting stream rule outright) +#' and supplying one would admit the segment; a segment with no discharge +#' reads `width_null` instead, and one whose gradient also fails reads +#' `fails_gradient_and_width`; #' - `rule_excludes` — fails even then: edge type, waterbody or lake size; #' - `post_predicate` — passes the predicate but is not habitat: removed by #' clustering (connectivity to spawning) or access gating. #' +#' The method table is the one in `cfg`. A run classified with another +#' (a swapped bundle file, or `lnk_pipeline_classify(method_csv =)`) is not +#' detected, so swap it on the `cfg` passed here too. A rule-level size +#' window in `rules.yaml` with a floor above the stage minimum would turn +#' size misses into `rule_excludes`; no bundled rules set one. +#' #' @param conn A [DBI::DBIConnection-class] object (from [lnk_db_conn()]). #' @param aoi Character vector of watershed group codes. Each must be #' persisted in `schema`, with `streams_access` built (fails loud @@ -177,12 +194,15 @@ #' `accessible_km`, `spawning_km`, `rearing_km` from [lnk_rollup_wsg()]. #' With `absences`, also `n_absence`, `n_absence_accessible`, #' `n_absence_spawning`, `n_absence_rearing` (stream) and -#' `n_absence_rearing_any` (the same on every stage). +#' `n_absence_rearing_any` (the same on every stage). `model` is the +#' WSG's habitat model (`cw` or `mad`). #' - `observations`: one row per retained location, with its segment's -#' `gradient`, `channel_width`, `channel_width_source`, `edge_type`, -#' `stream_order`, `waterbody_type`, `access`, the capture flags, -#' `in_uhc_spawn`, `in_uhc_rear`, the predicate results, and the two miss -#' reasons. +#' `gradient`, `channel_width`, `channel_width_source`, `mad_m3s` (on +#' `mad` groups only, else `NA`), `edge_type`, `stream_order`, +#' `waterbody_type`, `access`, `model`, the capture flags, +#' `in_uhc_spawn`, `in_uhc_rear`, the predicate results (`pred_`, +#' relaxed `_g`, `_w`, `_gw`, and on `mad` groups for a species with no +#' MAD range `_nomad`, `_nomad_g`), and the two miss reasons. #' #' @examples #' \dontrun{ @@ -252,13 +272,22 @@ lnk_habitat_validate <- function(conn, aoi, cfg, loaded, species, schema, .lnk_hv_check_schema(conn, schema, aoi, species) logged <- .lnk_hv_check_log(conn, schema, aoi, cfg) + # The model each group classified on, from the bundle's method table and + # by classify's own rule. + method_csv <- .lnk_habitat_method_csv(cfg) + if (!nzchar(method_csv) || !file.exists(method_csv)) { + stop("method table not found: ", method_csv, call. = FALSE) + } + models <- stats::setNames( + .lnk_wsg_model(.lnk_habitat_method_read(method_csv), aoi), aoi) + if (any(models == "mad")) .lnk_hv_check_mad(conn, schema, models) spec <- .lnk_hv_spec(loaded$wsg_species_presence, aoi, species, species_obs) obs <- .lnk_hv_obs(conn, schema, observations, spec, loaded, species, - match_types, source_exclude, buffer_m) + match_types, source_exclude, buffer_m, models) obs <- .lnk_hv_dedup(obs) - obs <- .lnk_hv_predicates(conn, schema, obs, cfg, loaded, species) + obs <- .lnk_hv_predicates(conn, schema, obs, cfg, loaded, species, models) obs <- .lnk_hv_reasons(obs) cost <- do.call(rbind, lapply(aoi, function(w) { @@ -275,6 +304,7 @@ lnk_habitat_validate <- function(conn, aoi, cfg, loaded, species, schema, summary <- .lnk_hv_summary(obs, cost, abs_sum, aoi, species) summary$run_logged <- summary$watershed_group_code %in% logged + summary$model <- unname(models[summary$watershed_group_code]) summary <- cbind( data.frame( schema = schema, config_name = cfg$name %||% NA_character_, @@ -327,6 +357,27 @@ lnk_habitat_validate <- function(conn, aoi, cfg, loaded, species, schema, invisible(NULL) } +#' Fail loud when a mad group cannot be scored: its discharge is joined on +#' `linear_feature_id` from a table the persist does not carry. +#' @noRd +.lnk_hv_check_mad <- function(conn, schema, models) { + mad <- names(models)[models == "mad"] + tbl <- strsplit(.lnk_hv_discharge_tbl(), ".", fixed = TRUE)[[1]] + if (!DBI::dbExistsTable(conn, DBI::Id(schema = tbl[1], table = tbl[2]))) { + stop("the method table puts ", paste(mad, collapse = ", "), + " on mad, but ", .lnk_hv_discharge_tbl(), " does not exist", + call. = FALSE) + } + cols <- names(DBI::dbGetQuery(conn, sprintf( + "SELECT * FROM %s.streams LIMIT 0", schema))) + if (!"linear_feature_id" %in% cols) { + stop("the method table puts ", paste(mad, collapse = ", "), + " on mad, but ", schema, ".streams has no linear_feature_id to ", + "join discharge on", call. = FALSE) + } + invisible(NULL) +} + #' WSGs whose latest logged run is recorded, after checking the config. #' #' A schema is scored with `cfg`'s rules, so a WSG logged as built by a @@ -520,7 +571,7 @@ lnk_habitat_validate <- function(conn, aoi, cfg, loaded, species, schema, #' Filter observations, attach each to its segment, and flag capture. #' @noRd .lnk_hv_obs <- function(conn, schema, observations, spec, loaded, species, - match_types, source_exclude, buffer_m) { + match_types, source_exclude, buffer_m, models) { s <- .lnk_hv_source(conn, observations) has <- function(cl) cl %in% s$cols opt <- function(cl, type) { @@ -622,19 +673,29 @@ lnk_habitat_validate <- function(conn, aoi, cfg, loaded, species, schema, # The segment the model tests: the one STARTING within 1 m (upstream), # else the one containing the point. n_cand > 1 is the expected case of # a point just below a break. + # Discharge only when a group is on mad: the persist does not carry it, + # and a cw-only run should not depend on the discharge table. + mad_sql <- if (any(models == "mad")) { + sprintf("(SELECT d.mad_m3s FROM %s d + WHERE d.linear_feature_id = s.linear_feature_id)", + .lnk_hv_discharge_tbl()) + } else { + "NULL::double precision" + } .lnk_hv_drop_temp(conn, "lnk_vd_att") .lnk_db_execute(conn, sprintf( "CREATE TEMP TABLE lnk_vd_att AS SELECT o.*, s.id_segment, s.n_cand, s.seg_drm, s.gradient, - s.channel_width, s.channel_width_source, s.edge_type, + s.channel_width, s.channel_width_source, s.mad_m3s, s.edge_type, s.stream_order, s.waterbody_key FROM pg_temp.lnk_vd_obs o LEFT JOIN LATERAL ( SELECT s.id_segment, s.downstream_route_measure AS seg_drm, s.gradient, s.channel_width, s.channel_width_source, + %2$s AS mad_m3s, s.edge_type, s.stream_order, s.waterbody_key, count(*) OVER ()::int AS n_cand - FROM %s.streams s + FROM %1$s.streams s WHERE s.blue_line_key = o.blue_line_key AND s.watershed_group_code = o.watershed_group_code AND (abs(s.downstream_route_measure - o.m) < 1 @@ -642,7 +703,7 @@ lnk_habitat_validate <- function(conn, aoi, cfg, loaded, species, schema, AND o.m < s.upstream_route_measure)) ORDER BY (abs(s.downstream_route_measure - o.m) < 1) DESC, s.downstream_route_measure DESC - LIMIT 1) s ON true", schema)) + LIMIT 1) s ON true", schema, mad_sql)) buf <- format(buffer_m, scientific = FALSE) per_species <- paste(vapply(species, function(sp) { @@ -697,6 +758,9 @@ lnk_habitat_validate <- function(conn, aoi, cfg, loaded, species, schema, out$spawning[none] <- NA out$rearing[none] <- NA out$rearing_any[none] <- NA + out$model <- unname(models[out$watershed_group_code]) + # The size a cw group was not classified on is not reported for it. + out$mad_m3s[out$model != "mad"] <- NA_real_ out } @@ -751,49 +815,138 @@ lnk_habitat_validate <- function(conn, aoi, cfg, loaded, species, schema, params_sp = ps) } +#' Mean annual discharge per FWA line, which the persist does not carry +#' (#286); prepare joins the same table onto the working streams. +#' @noRd +.lnk_hv_discharge_tbl <- function() { + "whse_basemapping.fwa_stream_networks_discharge" +} + +#' The size column a habitat model tests: channel width (`cw`) or mean +#' annual discharge (`mad`). +#' @noRd +.lnk_hv_size_col <- function(model) { + if (identical(model, "mad")) "mad_m3s" else "channel_width" +} + #' The stage minimums the predicates test against, from the same inputs. #' #' Spawning's gradient floor is `spawn_gradient_min` (parameters_fresh), #' which fresh's spawn predicate uses directly; rearing's is the literal 0 -#' fresh writes into the rear predicate (`c(0, rear_g[2])`). Widths are the -#' `ranges$$channel_width` minimums both predicates inherit. No -#' bundled rules.yaml sets a rule-level gradient, and the rule-level -#' `channel_width: [0, 9999]` on river polygons contains every minimum. +#' fresh writes into the rear predicate (`c(0, rear_g[2])`). Sizes are the +#' `ranges$$channel_width` (cw) or `ranges$$mad_m3s` (mad) +#' minimums both predicates inherit; a species with no MAD range gets 0, +#' which relaxes nothing because its mad predicate has no size test to +#' relax. No bundled rules.yaml sets a rule-level gradient or `mad`, and the +#' rule-level `channel_width: [0, 9999]` on river polygons (dropped under +#' mad) contains every minimum. #' @noRd -.lnk_hv_stage_min <- function(spp) { +.lnk_hv_stage_min <- function(spp, model = "cw") { rng <- spp$params_sp$ranges + col <- .lnk_hv_size_col(model) list( spawn = c(gradient = spp$spawn_gradient_min, - width = rng$spawn$channel_width[1] %||% 0), + size = rng$spawn[[col]][1] %||% 0), rear = c(gradient = 0, - width = rng$rear$channel_width[1] %||% 0)) + size = rng$rear[[col]][1] %||% 0)) } -#' A predicate with s.gradient and/or s.channel_width fixed to a value. +#' A predicate with s.gradient and/or the size column fixed to a value. #' @noRd -.lnk_hv_relax <- function(pred, gradient = NULL, width = NULL) { +.lnk_hv_relax <- function(pred, gradient = NULL, size = NULL, + size_col = "channel_width") { if (!is.null(gradient)) { pred <- gsub("\\bs\\.gradient\\b", sprintf("(%s::double precision)", format(gradient, scientific = FALSE)), pred, perl = TRUE) } - if (!is.null(width)) { - pred <- gsub("\\bs\\.channel_width\\b", - sprintf("(%s::double precision)", format(width, scientific = FALSE)), + if (!is.null(size)) { + pred <- gsub(sprintf("\\bs\\.%s\\b", size_col), + sprintf("(%s::double precision)", format(size, scientific = FALSE)), pred, perl = TRUE) } pred } +#' Whether fresh writes FALSE for a stage's size test under `mad` because +#' the species has no MAD range there, following +#' `frs_habitat_predicates()`'s own branches: a stage with rules gets FALSE +#' on every rule that inherits thresholds (`rear: []` compiles to FALSE +#' either way); without rules, the CSV path gives spawning a size test +#' always and rearing one only when the stage has ranges. +#' @noRd +.lnk_hv_mad_missing <- function(params_sp, st) { + rng <- params_sp$ranges[[st]] + if (!is.null(rng[["mad_m3s"]])) return(FALSE) + !is.null(params_sp[["rules"]][[st]]) || st == "spawn" || !is.null(rng) +} + +#' The predicate select expressions for one species on one habitat model. +#' +#' `pred_`, then with the gradient (`_g`), the size (`_w`) and both +#' (`_gw`) moved to the stage minimum. The rear stage ORs stream, lake and +#' wetland rearing, as `rearing_any` does. `pred__nomad` is the +#' predicate a `mad` group would have if the species had a MAD range for +#' that stage, with the size relaxed (`_nomad_g`: gradient too): fresh +#' writes FALSE in place of the size test of a species without one, which +#' no relaxation reaches. NULL where the model is cw, the species has a +#' MAD range, or fresh writes no size test for the stage at all (see +#' `.lnk_hv_mad_missing()`). +#' @noRd +.lnk_hv_stage_exprs <- function(spp, model = "cw") { + size_col <- .lnk_hv_size_col(model) + stage_pred <- function(pr) { + list(spawn = pr$spawn, + rear = sprintf("(%s) OR (%s) OR (%s)", pr$rear, pr$lake_rear, + pr$wetland_rear)) + } + sp_pred <- stage_pred(fresh::frs_habitat_predicates(spp, model = model)) + mins <- .lnk_hv_stage_min(spp, model) + no_range <- vapply(c("spawn", "rear"), function(st) { + model == "mad" && .lnk_hv_mad_missing(spp$params_sp, st) + }, logical(1)) + open_pred <- NULL + if (any(no_range)) { + # Any range will do: the size test is relaxed away below. + open <- spp + for (st in names(no_range)[no_range]) { + open$params_sp$ranges[[st]][["mad_m3s"]] <- c(0, 0) + } + open_pred <- stage_pred(fresh::frs_habitat_predicates(open, model = model)) + } + unlist(lapply(c("spawn", "rear"), function(st) { + g <- mins[[st]][["gradient"]] + w <- mins[[st]][["size"]] + p <- sp_pred[[st]] + v <- c(p, + .lnk_hv_relax(p, gradient = g), + .lnk_hv_relax(p, size = w, size_col = size_col), + .lnk_hv_relax(p, gradient = g, size = w, size_col = size_col)) + nomad <- if (no_range[[st]]) { + o <- open_pred[[st]] + sprintf("coalesce((%s), false)", + c(.lnk_hv_relax(o, size = 0, size_col = size_col), + .lnk_hv_relax(o, gradient = g, size = 0, size_col = size_col))) + } else { + rep("NULL::boolean", 2L) + } + c(sprintf("coalesce((%s), false) AS pred_%s%s", v, st, + c("", "_g", "_w", "_gw")), + sprintf("%s AS pred_%s%s", nomad, st, c("_nomad", "_nomad_g"))) + })) +} + #' Evaluate the bundle's habitat predicates on each observation's segment. #' -#' Adds pred_, pred__g (gradient relaxed), pred__w -#' (width relaxed) and pred__gw. The rear stage ORs stream, lake and -#' wetland rearing, as `rearing_any` does. +#' Each segment is tested on its group's model (`models`, named by WSG), so +#' a `mad` group reads `mad_m3s`, joined here from the discharge table +#' because the persist does not carry it. Adds the columns of +#' `.lnk_hv_stage_exprs()`. #' @noRd -.lnk_hv_predicates <- function(conn, schema, obs, cfg, loaded, species) { - cols <- paste0("pred_", rep(c("spawn", "rear"), each = 4L), - c("", "_g", "_w", "_gw")) +.lnk_hv_predicates <- function(conn, schema, obs, cfg, loaded, species, + models) { + cols <- paste0("pred_", rep(c("spawn", "rear"), each = 6L), + c("", "_g", "_w", "_gw", "_nomad", "_nomad_g")) for (cl in cols) obs[[cl]] <- rep(NA, nrow(obs)) obs$gradient_min_spawn <- rep(NA_real_, nrow(obs)) obs$gradient_min_rear <- rep(NA_real_, nrow(obs)) @@ -804,6 +957,7 @@ lnk_habitat_validate <- function(conn, aoi, cfg, loaded, species, schema, rules_yaml = cfg$rules) seg <- unique(att[, c("species_code", "id_segment", "watershed_group_code")]) + seg$model <- unname(models[seg$watershed_group_code]) DBI::dbWriteTable(conn, "lnk_vd_seg", seg, temporary = TRUE, overwrite = TRUE) @@ -815,39 +969,34 @@ lnk_habitat_validate <- function(conn, aoi, cfg, loaded, species, schema, obs$gradient_min_rear[is_sp] <- mins$rear[["gradient"]] } - res <- lapply(species, function(sp) { - if (!any(seg$species_code == sp)) return(NULL) + combos <- unique(seg[, c("species_code", "model")]) + res <- lapply(seq_len(nrow(combos)), function(j) { + sp <- combos$species_code[j] + model <- combos$model[j] spp <- .lnk_hv_sp_params(params, loaded$parameters_fresh, sp) - # Channel-width model only. fresh >= 0.35.0 takes `model = "mad"`, but the - # miss-reason relaxation below rewrites s.channel_width, so a bundle that - # puts a group on mad is scored here as if it were cw (#286 follow-up). - pr <- fresh::frs_habitat_predicates(spp) - mins <- .lnk_hv_stage_min(spp) - stage_pred <- list( - spawn = pr$spawn, - rear = sprintf("(%s) OR (%s) OR (%s)", pr$rear, pr$lake_rear, - pr$wetland_rear)) - exprs <- unlist(lapply(c("spawn", "rear"), function(st) { - g <- mins[[st]][["gradient"]] - w <- mins[[st]][["width"]] - p <- stage_pred[[st]] - v <- c(p, - .lnk_hv_relax(p, gradient = g), - .lnk_hv_relax(p, width = w), - .lnk_hv_relax(p, gradient = g, width = w)) - sprintf("coalesce((%s), false) AS pred_%s%s", v, st, - c("", "_g", "_w", "_gw")) - })) + exprs <- .lnk_hv_stage_exprs(spp, model) + # A rule-level `mad:` reaches s.mad_m3s on a cw group too. + src <- if (any(grepl("\\bs\\.mad_m3s\\b", exprs, perl = TRUE))) { + # Every piece of a broken FWA line carries its line's discharge, as on + # the working table classify read. + sprintf("(SELECT s.*, d.mad_m3s FROM %s.streams s + LEFT JOIN %s d + ON d.linear_feature_id = s.linear_feature_id)", + schema, .lnk_hv_discharge_tbl()) + } else { + paste0(schema, ".streams") + } DBI::dbGetQuery(conn, sprintf( "SELECT %s AS species_code, s.id_segment, s.watershed_group_code, %s - FROM %s.streams s + FROM %s s JOIN pg_temp.lnk_vd_seg k ON k.id_segment = s.id_segment AND k.watershed_group_code = s.watershed_group_code - AND k.species_code = %s", + AND k.species_code = %s + AND k.model = %s", .lnk_quote_literal(sp), paste(exprs, collapse = ",\n "), - schema, .lnk_quote_literal(sp))) + src, .lnk_quote_literal(sp), .lnk_quote_literal(model))) }) res <- do.call(rbind, res) key_o <- paste(obs$species_code, obs$id_segment, obs$watershed_group_code) @@ -860,10 +1009,11 @@ lnk_habitat_validate <- function(conn, aoi, cfg, loaded, species, schema, #' Miss reason for one stage from its capture flag and predicate results. #' @noRd -.lnk_habitat_miss_reason <- function(captured, accessible, channel_width, +.lnk_habitat_miss_reason <- function(captured, accessible, size, p, p_g, p_w, p_gw, gradient = NA_real_, - gradient_min = NA_real_) { + gradient_min = NA_real_, + p_nomad = NA, p_nomad_g = NA) { below <- gradient < gradient_min out <- rep(NA_character_, length(captured)) out[is.na(captured)] <- "no_segment" @@ -876,11 +1026,17 @@ lnk_habitat_validate <- function(conn, aoi, cfg, loaded, species, schema, set(p, "post_predicate") set(p_g & !p_w & below, "gradient_below_min") set(p_g & !p_w, "fails_gradient") - set(p_w & !p_g & is.na(channel_width), "width_null") + set(p_w & !p_g & is.na(size), "width_null") set(p_w & !p_g, "fails_width") # Either relaxation alone passes (different OR-branches of the rule). set(p_g & p_w, "fails_gradient_or_width") set(p_gw, "fails_gradient_and_width") + # A mad group, and the species has no MAD range for the stage: passes + # with one, or with one and the gradient relaxed. A range would not admit + # a segment with no discharge, so that is the missing value, as on cw. + set(p_nomad & is.na(size), "width_null") + set(p_nomad, "no_mad_threshold") + set(p_nomad_g, "fails_gradient_and_width") set(rep(TRUE, length(out)), "rule_excludes") out } @@ -888,14 +1044,18 @@ lnk_habitat_validate <- function(conn, aoi, cfg, loaded, species, schema, #' Add miss_reason_spawn / miss_reason_rear. #' @noRd .lnk_hv_reasons <- function(obs) { + # The size the group was classified on. + size <- ifelse(obs$model %in% "mad", obs$mad_m3s, obs$channel_width) obs$miss_reason_spawn <- .lnk_habitat_miss_reason( - obs$spawning, obs$accessible, obs$channel_width, obs$pred_spawn, + obs$spawning, obs$accessible, size, obs$pred_spawn, obs$pred_spawn_g, obs$pred_spawn_w, obs$pred_spawn_gw, - obs$gradient, obs$gradient_min_spawn) + obs$gradient, obs$gradient_min_spawn, obs$pred_spawn_nomad, + obs$pred_spawn_nomad_g) obs$miss_reason_rear <- .lnk_habitat_miss_reason( - obs$rearing_any, obs$accessible, obs$channel_width, obs$pred_rear, + obs$rearing_any, obs$accessible, size, obs$pred_rear, obs$pred_rear_g, obs$pred_rear_w, obs$pred_rear_gw, - obs$gradient, obs$gradient_min_rear) + obs$gradient, obs$gradient_min_rear, obs$pred_rear_nomad, + obs$pred_rear_nomad_g) obs } diff --git a/R/lnk_pipeline_classify.R b/R/lnk_pipeline_classify.R index 0e71bb1a..a427c2cb 100644 --- a/R/lnk_pipeline_classify.R +++ b/R/lnk_pipeline_classify.R @@ -91,9 +91,7 @@ lnk_pipeline_classify <- function(conn, aoi, cfg, loaded, schema, if (!nzchar(method_csv) || !file.exists(method_csv)) { stop("method_csv not found: ", method_csv, call. = FALSE) } - params_method <- utils::read.csv(method_csv, stringsAsFactors = FALSE, - colClasses = "character", - na.strings = character(0)) + params_method <- .lnk_habitat_method_read(method_csv) species <- species %||% lnk_pipeline_species(cfg, loaded, aoi) if (length(species) == 0L) { @@ -164,7 +162,7 @@ lnk_pipeline_classify <- function(conn, aoi, cfg, loaded, schema, # # Channel-width model only (#286): the bypass stands in for a width test, # and bcfp applies it inside its cw branch alone. A `mad` group skips it. - aoi_model <- params_method$model[match(aoi, params_method$watershed_group_code)] + aoi_model <- .lnk_wsg_model(params_method, aoi) species_bypass <- if (identical(aoi_model, "mad")) character(0) else species for (sp in species_bypass) { rear_rules <- params[[sp]][["rules"]][["rear"]] diff --git a/R/lnk_preflight_fresh.R b/R/lnk_preflight_fresh.R index ffb7ffbe..2407e534 100644 --- a/R/lnk_preflight_fresh.R +++ b/R/lnk_preflight_fresh.R @@ -120,9 +120,12 @@ lnk_preflight_fresh <- function(required = .lnk_fresh_required(), # Arguments link passes that older fresh releases do not accept. The call # would fail with "unused argument" only once a WSG reached that phase. -# R/lnk_pipeline_classify.R: params_method arrived in fresh 0.35.0 (#286). +# R/lnk_pipeline_classify.R: params_method arrived in fresh 0.35.0 (#286); +# R/lnk_habitat_validate.R: frs_habitat_predicates(model) in the same +# release (#299). .lnk_fresh_required_formals <- function() { - list(frs_habitat_classify = "params_method") + list(frs_habitat_classify = "params_method", + frs_habitat_predicates = "model") } # "fn(arg)" for each required formal the namespace does not provide. A @@ -149,10 +152,11 @@ lnk_preflight_fresh <- function(required = .lnk_fresh_required(), out } -# Non-exported fresh objects link reaches via getFromNamespace(). -# R/lnk_pipeline_connect.R:101. +# Non-exported fresh objects link reaches via getFromNamespace(): +# .frs_run_connectivity in lnk_pipeline_connect(), .frs_habitat_models in +# .lnk_wsg_model() (R/lnk_config.R, #299). .lnk_fresh_required_internal <- function() { - ".frs_run_connectivity" + c(".frs_run_connectivity", ".frs_habitat_models") } # Every `fresh::sym` / `fresh:::sym` reached from link's own namespace, diff --git a/RUNBOOK.md b/RUNBOOK.md index eea6019b..a37aa121 100644 --- a/RUNBOOK.md +++ b/RUNBOOK.md @@ -714,8 +714,14 @@ next dispatch.** test, so a `mad` group without coverage loses all of its stream habitat with no error. BULK has none in the local fwapg. Check `count(mad_m3s)` before moving a group. -- **`lnk_habitat_validate()` is still cw-only**: its miss-reason relaxation - rewrites `s.channel_width`, so it scores a `mad` group as if it were `cw`. +- **`lnk_habitat_validate()` scores each group on its own model** (#299), from the + `cfg` it is handed, resolved by fresh's `.frs_habitat_models()` (one rule for classify + and validator). A `mad` group's predicates and size relaxation read `mad_m3s`, joined + from the discharge table on `linear_feature_id`; reason labels are shared with `cw` + and the `model` / `mad_m3s` columns split them, plus `no_mad_threshold` for a species + with no MAD range. To score a run made with a swapped method table, swap it on the + same `cfg` object (`cfg$files$parameters_habitat_method$path`) before validating: + `lnk_pipeline_classify(method_csv =)` is invisible to the validator. - **Connectivity reads no size model.** `.frs_run_connectivity` takes no `params_method`; its one width test (`.frs_connected_waterbody`, `spawn_connected_cw_min`) is 0 for SK/KO in every bundle, so it is inert. diff --git a/data-raw/habitat_validate.R b/data-raw/habitat_validate.R index f5c9d108..0bb23965 100644 --- a/data-raw/habitat_validate.R +++ b/data-raw/habitat_validate.R @@ -200,7 +200,8 @@ if (nrow(bundles) == 2L) { # totals' n_habitat; its reason is then the rear reason. stage_rows <- function(obs) { base <- data.frame( - species_code = obs$species_code, gradient = obs$gradient, + species_code = obs$species_code, model = obs$model, + gradient = obs$gradient, channel_width = obs$channel_width, edge_type = obs$edge_type, is_spawn = obs$is_spawn, is_rear = obs$is_rear, reason_any = ifelse(obs$spawning %in% TRUE, NA_character_, @@ -209,7 +210,7 @@ stage_rows <- function(obs) { reason_rear = obs$miss_reason_rear, stringsAsFactors = FALSE) pick <- function(d, stage, col) { cbind(data.frame(stage = rep(stage, nrow(d))), d[, c("species_code", - "gradient", "channel_width", "edge_type")], reason = d[[col]]) + "model", "gradient", "channel_width", "edge_type")], reason = d[[col]]) } rbind(pick(base, "any", "reason_any"), pick(base[base$is_spawn, , drop = FALSE], "spawn", "reason_spawn"), @@ -223,18 +224,24 @@ binned <- list() for (r in runs[vapply(runs, function(r) r$summary$buffer_m[1] == 0, TRUE)]) { d <- stage_rows(r$observations) d$reason[is.na(d$reason)] <- "captured" + # model: a mad group's width reasons are about discharge (#299). tab <- as.data.frame(table(stage = d$stage, species_code = d$species_code, - reason = d$reason), stringsAsFactors = FALSE) + model = d$model, reason = d$reason), + stringsAsFactors = FALSE) tab <- tab[tab$Freq > 0, ] names(tab)[names(tab) == "Freq"] <- "n" misses[[length(misses) + 1L]] <- cbind( data.frame(bundle = rep(r$bundle, nrow(tab))), tab) m <- d[!d$reason %in% c("captured", "no_segment"), ] m$gradient_bin <- as.character(cut(m$gradient, breaks_g, right = TRUE)) - m$width_bin <- ifelse(is.na(m$channel_width), "NULL", + # Width bins describe the cw size dimension only; a mad row keeps its + # count under "mad" so the binned table still sums to misses.csv. + m$width_bin <- ifelse(m$model %in% "mad", "mad", + ifelse(is.na(m$channel_width), "NULL", as.character(cut(m$channel_width, breaks_w, - right = FALSE))) + right = FALSE)))) b <- as.data.frame(table(stage = m$stage, species_code = m$species_code, + model = m$model, reason = m$reason, gradient_bin = m$gradient_bin, width_bin = m$width_bin, useNA = "ifany"), stringsAsFactors = FALSE) diff --git a/data-raw/logs/habitat_validate_299/20261002_adms_compare.txt b/data-raw/logs/habitat_validate_299/20261002_adms_compare.txt new file mode 100644 index 00000000..4c708707 --- /dev/null +++ b/data-raw/logs/habitat_validate_299/20261002_adms_compare.txt @@ -0,0 +1,76 @@ +## 1. cw no-change (fresh_default, ADMS) +observations identical apart from new columns: TRUE +summary identical apart from `model`: TRUE +branch model column: cw +observations: 94 + +## 2. persisted TRUE, re-built predicate FALSE (zz299_mad, ADMS on mad) +branch: + species_code n spawn_true_pred_false rear_true_pred_false + BT 37 0 0 + CH 11 0 0 + CO 46 0 0 +main (cw predicates on a mad run): + species_code n spawn_true_pred_false rear_true_pred_false + BT 37 0 0 + CH 11 0 0 + CO 46 0 0 + +## 3. miss reasons on the mad run (locations) +spawn_all + species reason Freq_branch Freq_main + BT fails_gradient 0 6 + BT fails_gradient_and_width 8 2 + BT fails_width 0 2 + BT no_mad_threshold 27 0 + BT post_predicate 0 24 + BT rule_excludes 2 2 + BT width_null 0 1 + CH captured 8 8 + CH fails_gradient 1 1 + CH fails_width 1 0 + CH post_predicate 0 1 + CH rule_excludes 1 1 + CO captured 41 41 + CO fails_gradient 1 1 + CO fails_width 2 0 + CO not_accessible 1 1 + CO rule_excludes 1 1 + CO width_null 0 2 +spawn_staged + species reason Freq_branch Freq_main + BT no_mad_threshold 1 0 + BT post_predicate 0 1 + CH captured 5 5 + CO captured 22 22 +rear_all + species reason Freq_branch Freq_main + BT captured 2 2 + BT fails_gradient 0 3 + BT fails_gradient_and_width 4 1 + BT fails_width 0 3 + BT no_mad_threshold 31 0 + BT post_predicate 0 27 + BT width_null 0 1 + CH captured 9 9 + CH fails_gradient 1 1 + CH post_predicate 1 1 + CO captured 38 38 + CO fails_gradient 3 3 + CO fails_width 4 0 + CO not_accessible 1 1 + CO post_predicate 0 2 + CO width_null 0 2 +rear_staged + species reason Freq_branch Freq_main + BT fails_width 0 2 + BT no_mad_threshold 6 0 + BT post_predicate 0 4 + CH captured 1 1 + CH post_predicate 1 1 + CO captured 12 12 + CO fails_width 1 0 + CO width_null 0 1 + +mad_m3s present on 94 of 94 mad-group locations +validate seconds: branch cw 12 main cw 15 branch mad 11 main mad 15 diff --git a/data-raw/logs/habitat_validate_299/20261002_adms_mad_km.txt b/data-raw/logs/habitat_validate_299/20261002_adms_mad_km.txt new file mode 100644 index 00000000..730b3a21 --- /dev/null +++ b/data-raw/logs/habitat_validate_299/20261002_adms_mad_km.txt @@ -0,0 +1,7 @@ + species_code | spawning_km | rearing_km | lake_wetland_rearing_km +--------------+-------------+------------+------------------------- + BT | 0.0 | 0.0 | 318.2 + CH | 256.4 | 317.5 | 98.7 + CO | 292.4 | 317.8 | 113.9 +(3 rows) + diff --git a/data-raw/logs/habitat_validate_299/20261002_adms_model_mad.txt b/data-raw/logs/habitat_validate_299/20261002_adms_model_mad.txt new file mode 100644 index 00000000..a01ac528 --- /dev/null +++ b/data-raw/logs/habitat_validate_299/20261002_adms_model_mad.txt @@ -0,0 +1,24 @@ +Warning messages: +1: In st_read.DBIObject(conn, query = query) : + Could not find a simple features geometry column. Will return a `data.frame`. +2: In st_read.DBIObject(conn, query = query) : + Could not find a simple features geometry column. Will return a `data.frame`. +3: In st_read.DBIObject(conn, query = query) : + Could not find a simple features geometry column. Will return a `data.frame`. +4: In st_read.DBIObject(conn, query = query) : + Could not find a simple features geometry column. Will return a `data.frame`. +5: In st_read.DBIObject(conn, query = query) : + Could not find a simple features geometry column. Will return a `data.frame`. +6: In st_read.DBIObject(conn, query = query) : + Could not find a simple features geometry column. Will return a `data.frame`. +7: In st_read.DBIObject(conn, query = query) : + Could not find a simple features geometry column. Will return a `data.frame`. +8: In st_read.DBIObject(conn, query = query) : + Could not find a simple features geometry column. Will return a `data.frame`. +9: In st_read.DBIObject(conn, query = query) : + Could not find a simple features geometry column. Will return a `data.frame`. +10: In st_read.DBIObject(conn, query = query) : + Could not find a simple features geometry column. Will return a `data.frame`. +## model_mad ADMS -> zz299_mad method=method_adms_mad.csv link=0.55.0 fresh=0.36.2 2.8 min + n_segments +1 39422 diff --git a/data-raw/logs/habitat_validate_299/20261002_adms_validate_branch_cw.txt b/data-raw/logs/habitat_validate_299/20261002_adms_validate_branch_cw.txt new file mode 100644 index 00000000..59bedef0 --- /dev/null +++ b/data-raw/logs/habitat_validate_299/20261002_adms_validate_branch_cw.txt @@ -0,0 +1 @@ +## validate fresh_default CH,CO,BT method=- sha=be3be30 fresh=0.36.2 12 s, 94 locations diff --git a/data-raw/logs/habitat_validate_299/20261002_adms_validate_branch_mad.txt b/data-raw/logs/habitat_validate_299/20261002_adms_validate_branch_mad.txt new file mode 100644 index 00000000..dab4abd7 --- /dev/null +++ b/data-raw/logs/habitat_validate_299/20261002_adms_validate_branch_mad.txt @@ -0,0 +1 @@ +## validate zz299_mad CH,CO,BT method=method_adms_mad.csv sha=be3be30 fresh=0.36.2 11 s, 94 locations diff --git a/data-raw/logs/habitat_validate_299/20261002_adms_validate_main_cw.txt b/data-raw/logs/habitat_validate_299/20261002_adms_validate_main_cw.txt new file mode 100644 index 00000000..2c6d0e20 --- /dev/null +++ b/data-raw/logs/habitat_validate_299/20261002_adms_validate_main_cw.txt @@ -0,0 +1 @@ +## validate fresh_default CH,CO,BT method=- sha=4df1ffc fresh=0.36.2 15 s, 94 locations diff --git a/data-raw/logs/habitat_validate_299/20261002_adms_validate_main_mad.txt b/data-raw/logs/habitat_validate_299/20261002_adms_validate_main_mad.txt new file mode 100644 index 00000000..d47d3b5e --- /dev/null +++ b/data-raw/logs/habitat_validate_299/20261002_adms_validate_main_mad.txt @@ -0,0 +1 @@ +## validate zz299_mad CH,CO,BT method=- sha=4df1ffc fresh=0.36.2 15 s, 94 locations diff --git a/data-raw/logs/habitat_validate_299/README.md b/data-raw/logs/habitat_validate_299/README.md new file mode 100644 index 00000000..e6899b11 --- /dev/null +++ b/data-raw/logs/habitat_validate_299/README.md @@ -0,0 +1,52 @@ +# #299 live verification — validator scores each group on its own model + +Local docker fwapg (:5432), 2026-10-02 UTC. Config `default`, ADMS, species CH, CO, BT +(DV pooled per `species_pooling.csv`), buffer 0. fresh 0.36.2. Branch runs are the +working tree on top of `be3be30`, before the Phase 2–3 commit (the `sha=` in their +headers is HEAD, not the tree that ran). Main runs are a detached worktree at `4df1ffc`. + +- `model_mad.R` models ADMS with the `default` bundle into the scratch persist schema + `zz299_mad`, with its method table swapped for `method_adms_mad.csv` (ADMS on `mad`), + `mapping_code = TRUE`. 39,422 segments, 2.8 min. `km.sql` → + `20261002_adms_mad_km.txt`: stream spawning / rearing km BT 0 / 0, CH 256.4 / 317.5, + CO 292.4 / 317.8. BT keeps 318.2 km of lake and wetland rearing. +- `validate_run.R` runs `lnk_habitat_validate()` from a given repo, swapping the method + table on the same `cfg` object it validates with. +- `compare.R` compares the four runs: `20261002_adms_compare.txt`. + +## Result + +| check | result | +|---|---| +| cw no-change: `fresh_default` ADMS, branch vs main | `observations` identical apart from the new columns; `summary` identical apart from `model` (94 locations) | +| `mad_m3s` on mad-group locations | 94 of 94 | +| persisted TRUE, re-built predicate FALSE (outside UHC) | 0 on branch, **and 0 on main**: this metric does not discriminate (see below) | + +**The planned metric was the wrong direction.** On a `mad` run, main re-builds the *cw* +predicate. For BT it is looser than the `mad` one that classified the run, so it passes +where classify failed. That is persisted FALSE with predicate TRUE, which the validator +reports as `post_predicate` ("removed by clustering or access gating"), not as a +disagreement. The discriminating numbers are the reasons, in locations +(`20261002_adms_compare.txt` §3). "All" tests every location against that stage's +predicate; "staged" keeps only locations whose records carry the stage, as the summary's +stage rows and the driver's `misses.csv` count them: + +| species | reason column | main (cw predicates) | branch (mad predicates) | +|---|---|---|---| +| BT, 37 locations | spawn, all | 24 `post_predicate`, 6 `fails_gradient`, 2 `fails_width`, 1 `width_null`, 2 `fails_gradient_and_width`, 2 `rule_excludes` | 27 `no_mad_threshold`, 8 `fails_gradient_and_width`, 2 `rule_excludes` | +| BT | spawn, staged (1) | 1 `post_predicate` | 1 `no_mad_threshold` | +| BT | rear, all | 27 `post_predicate`, 3 `fails_gradient`, 3 `fails_width`, 1 `width_null`, 1 `fails_gradient_and_width`, 2 captured | 31 `no_mad_threshold`, 4 `fails_gradient_and_width`, 2 captured | +| BT | rear, staged (6) | 4 `post_predicate`, 2 `fails_width` | 6 `no_mad_threshold` | +| CO, 46 locations | spawn, all | 2 `width_null` | 2 `fails_width` | +| CO | rear, all | 2 `width_null`, 2 `post_predicate` | 4 `fails_width` | +| CO | rear, staged (13) | 1 `width_null` | 1 `fails_width` | +| CH, 11 locations | spawn, all | 1 `post_predicate` | 1 `fails_width` | + +On the 27 BT locations main calls `post_predicate` against rearing, it tells a reader +the predicate passed and clustering or gating removed them. On a `mad` group they were +never in the predicate, because BT has no MAD range. Main's CO `width_null` are NULL +channel widths the `mad` run never tested; the branch tests their discharge (0.0127 and +0.018 m³/s on the two spawn misses), which is below CO's minimum. + +Scratch schemas `zz299_mad` and `zz299_w_adms` were dropped after the runs; the RDS +files were not kept. diff --git a/data-raw/logs/habitat_validate_299/compare.R b/data-raw/logs/habitat_validate_299/compare.R new file mode 100644 index 00000000..7152e1ad --- /dev/null +++ b/data-raw/logs/habitat_validate_299/compare.R @@ -0,0 +1,76 @@ +# Usage: Rscript compare.R +# 1. cw no-change: branch vs main on fresh_default, new columns dropped. +# 2. mad: where the persisted flag is TRUE but the re-built predicate is +# FALSE, outside the UHC overlay. Classify's own predicate cannot +# disagree that way, so a non-zero count means the validator re-built a +# different predicate. Branch vs main on the same mad-classified schema. +# 3. Reasons on the mad run, branch vs main. +d <- commandArgs(trailingOnly = TRUE)[1] +rd <- function(f) readRDS(file.path(d, f)) +new_obs <- c("model", "mad_m3s", "pred_spawn_nomad", "pred_spawn_nomad_g", + "pred_rear_nomad", "pred_rear_nomad_g") +strip <- function(x, drop) { + x <- x[, setdiff(names(x), drop), drop = FALSE] + rownames(x) <- NULL + x +} +bc <- rd("branch_cw.rds"); mc <- rd("main_cw.rds") +cat("## 1. cw no-change (fresh_default, ADMS)\n") +cat("observations identical apart from new columns:", + isTRUE(all.equal(strip(bc$observations, new_obs), + strip(mc$observations, character(0)))), "\n") +cat("summary identical apart from `model`:", + isTRUE(all.equal(strip(bc$summary, "model"), + strip(mc$summary, character(0)))), "\n") +cat("branch model column:", unique(bc$observations$model), "\n") +cat("observations:", nrow(bc$observations), "\n\n") + +disagree <- function(o) { + data.frame( + species_code = sort(unique(o$species_code)), + n = as.vector(table(o$species_code)[sort(unique(o$species_code))]), + spawn_true_pred_false = vapply(sort(unique(o$species_code)), function(sp) { + k <- o$species_code == sp + sum(o$spawning[k] %in% TRUE & o$pred_spawn[k] %in% FALSE & + !o$in_uhc_spawn[k]) + }, integer(1)), + rear_true_pred_false = vapply(sort(unique(o$species_code)), function(sp) { + k <- o$species_code == sp + sum(o$rearing_any[k] %in% TRUE & o$pred_rear[k] %in% FALSE & + !o$in_uhc_rear[k]) + }, integer(1)), row.names = NULL) +} +bm <- rd("branch_mad.rds"); mm <- rd("main_mad.rds") +cat("## 2. persisted TRUE, re-built predicate FALSE (zz299_mad, ADMS on mad)\n") +cat("branch:\n"); print(disagree(bm$observations), row.names = FALSE) +cat("main (cw predicates on a mad run):\n") +print(disagree(mm$observations), row.names = FALSE) +cat("\n## 3. miss reasons on the mad run (locations)\n") +# `all`: every location, against that stage's predicate. `staged`: only +# locations whose records carry the stage, as the summary's stage rows and +# the driver's misses.csv count them. +tab <- function(o, col, keep) { + o <- o[keep(o), , drop = FALSE] + as.data.frame(table(species = o$species_code, + reason = ifelse(is.na(o[[col]]), "captured", o[[col]])), + stringsAsFactors = FALSE) +} +sets <- list( + spawn_all = list("miss_reason_spawn", function(o) rep(TRUE, nrow(o))), + spawn_staged = list("miss_reason_spawn", function(o) o$is_spawn %in% TRUE), + rear_all = list("miss_reason_rear", function(o) rep(TRUE, nrow(o))), + rear_staged = list("miss_reason_rear", function(o) o$is_rear %in% TRUE)) +for (nm in names(sets)) { + col <- sets[[nm]][[1]]; keep <- sets[[nm]][[2]] + m <- merge(tab(bm$observations, col, keep), tab(mm$observations, col, keep), + by = c("species", "reason"), all = TRUE, + suffixes = c("_branch", "_main")) + m[is.na(m)] <- 0L + m <- m[m$Freq_branch > 0 | m$Freq_main > 0, ] + cat(nm, "\n"); print(m, row.names = FALSE) +} +cat("\nmad_m3s present on", sum(!is.na(bm$observations$mad_m3s)), "of", + nrow(bm$observations), "mad-group locations\n") +cat("validate seconds: branch cw", round(bc$stamp$secs), "main cw", + round(mc$stamp$secs), "branch mad", round(bm$stamp$secs), "main mad", + round(mm$stamp$secs), "\n") diff --git a/data-raw/logs/habitat_validate_299/km.sql b/data-raw/logs/habitat_validate_299/km.sql new file mode 100644 index 00000000..43609a9e --- /dev/null +++ b/data-raw/logs/habitat_validate_299/km.sql @@ -0,0 +1,10 @@ +-- Stream km per species in the scratch mad run (ADMS on mad, config default). +SELECT h.species_code, + round(sum(CASE WHEN h.spawning THEN s.length_metre ELSE 0 END)::numeric / 1000, 1) AS spawning_km, + round(sum(CASE WHEN h.rearing THEN s.length_metre ELSE 0 END)::numeric / 1000, 1) AS rearing_km, + round(sum(CASE WHEN h.wetland_rearing OR h.lake_rearing THEN s.length_metre ELSE 0 END)::numeric / 1000, 1) AS lake_wetland_rearing_km + FROM (SELECT 'BT' AS species_code, * FROM zz299_mad.streams_habitat_bt + UNION ALL SELECT 'CH', * FROM zz299_mad.streams_habitat_ch + UNION ALL SELECT 'CO', * FROM zz299_mad.streams_habitat_co) h + JOIN zz299_mad.streams s USING (id_segment, watershed_group_code) + GROUP BY 1 ORDER BY 1; diff --git a/data-raw/logs/habitat_validate_299/method_adms_mad.csv b/data-raw/logs/habitat_validate_299/method_adms_mad.csv new file mode 100644 index 00000000..5bc4d723 --- /dev/null +++ b/data-raw/logs/habitat_validate_299/method_adms_mad.csv @@ -0,0 +1,2 @@ +watershed_group_code,model +ADMS,mad diff --git a/data-raw/logs/habitat_validate_299/model_mad.R b/data-raw/logs/habitat_validate_299/model_mad.R new file mode 100644 index 00000000..3780e737 --- /dev/null +++ b/data-raw/logs/habitat_validate_299/model_mad.R @@ -0,0 +1,26 @@ +# Usage: Rscript model_mad.R +# Models one WSG with the `default` bundle into a scratch persist schema, +# with the bundle's method table swapped for , and builds +# streams_access (mapping_code) so lnk_habitat_validate() can score it. +args <- commandArgs(trailingOnly = TRUE) +repo <- args[1]; persist <- args[2]; method_csv <- args[3]; aoi <- args[4] +suppressMessages(devtools::load_all(repo, quiet = TRUE)) +conn <- lnk_db_conn(dbname = "fwapg", host = "localhost", port = 5432L, + user = "postgres", password = "postgres") +cfg <- lnk_config("default") +cfg$pipeline$schema <- persist +cfg$files$parameters_habitat_method <- list(path = normalizePath(method_csv)) +loaded <- suppressWarnings(lnk_load_overrides(cfg)) +t0 <- Sys.time() +# Own working schema: the default working_ may hold someone's work. +lnk_pipeline_run(conn, aoi = aoi, cfg = cfg, loaded = loaded, + schema = paste0("zz299_w_", tolower(aoi)), + mapping_code = TRUE, run_label = "habitat_validate_299") +cat(sprintf("## model_mad %s -> %s method=%s link=%s fresh=%s %.1f min\n", + aoi, persist, basename(method_csv), utils::packageVersion("link"), + utils::packageVersion("fresh"), + as.numeric(difftime(Sys.time(), t0, units = "mins")))) +print(DBI::dbGetQuery(conn, sprintf( + "SELECT count(*) AS n_segments FROM %s.streams WHERE watershed_group_code = %s", + persist, DBI::dbQuoteString(conn, aoi)))) +DBI::dbDisconnect(conn) diff --git a/data-raw/logs/habitat_validate_299/validate_run.R b/data-raw/logs/habitat_validate_299/validate_run.R new file mode 100644 index 00000000..e4b4876d --- /dev/null +++ b/data-raw/logs/habitat_validate_299/validate_run.R @@ -0,0 +1,32 @@ +# Usage: Rscript validate_run.R +# Runs lnk_habitat_validate() on ADMS with the `default` bundle from +# (branch or a main worktree), optionally with the method table swapped on +# the same cfg object, and saves the result. +args <- commandArgs(trailingOnly = TRUE) +repo <- args[1]; schema <- args[2]; method_csv <- args[3] +species <- strsplit(args[4], ",")[[1]]; out <- args[5] +suppressMessages(devtools::load_all(repo, quiet = TRUE)) +conn <- lnk_db_conn(dbname = "fwapg", host = "localhost", port = 5432L, + user = "postgres", password = "postgres") +cfg <- lnk_config("default") +if (method_csv != "-") { + cfg$files$parameters_habitat_method <- list(path = normalizePath(method_csv)) +} +loaded <- suppressWarnings(lnk_load_overrides(cfg)) +pool <- lnk_species_pooling(loaded, aoi = "ADMS", species = species) +t0 <- Sys.time() +v <- lnk_habitat_validate(conn, aoi = "ADMS", cfg = cfg, loaded = loaded, + species = species, schema = schema, + species_obs = pool) +v$stamp <- list(repo = repo, sha = system2("git", c("-C", repo, "rev-parse", + "--short", "HEAD"), + stdout = TRUE), + link = as.character(utils::packageVersion("link")), + fresh = as.character(utils::packageVersion("fresh")), + schema = schema, method_csv = method_csv, + secs = as.numeric(difftime(Sys.time(), t0, units = "secs"))) +saveRDS(v, out) +cat(sprintf("## validate %s %s method=%s sha=%s fresh=%s %.0f s, %d locations\n", + schema, paste(species, collapse = ","), basename(method_csv), + v$stamp$sha, v$stamp$fresh, v$stamp$secs, nrow(v$observations))) +DBI::dbDisconnect(conn) diff --git a/man/lnk_habitat_validate.Rd b/man/lnk_habitat_validate.Rd index 31df438c..f34df484 100644 --- a/man/lnk_habitat_validate.Rd +++ b/man/lnk_habitat_validate.Rd @@ -101,12 +101,15 @@ records the WSG), \code{n_obs} (locations on a segment), \code{n_unattached}, \code{accessible_km}, \code{spawning_km}, \code{rearing_km} from \code{\link[=lnk_rollup_wsg]{lnk_rollup_wsg()}}. With \code{absences}, also \code{n_absence}, \code{n_absence_accessible}, \code{n_absence_spawning}, \code{n_absence_rearing} (stream) and -\code{n_absence_rearing_any} (the same on every stage). +\code{n_absence_rearing_any} (the same on every stage). \code{model} is the +WSG's habitat model (\code{cw} or \code{mad}). \item \code{observations}: one row per retained location, with its segment's -\code{gradient}, \code{channel_width}, \code{channel_width_source}, \code{edge_type}, -\code{stream_order}, \code{waterbody_type}, \code{access}, the capture flags, -\code{in_uhc_spawn}, \code{in_uhc_rear}, the predicate results, and the two miss -reasons. +\code{gradient}, \code{channel_width}, \code{channel_width_source}, \code{mad_m3s} (on +\code{mad} groups only, else \code{NA}), \code{edge_type}, \code{stream_order}, +\code{waterbody_type}, \code{access}, \code{model}, the capture flags, +\code{in_uhc_spawn}, \code{in_uhc_rear}, the predicate results (\verb{pred_}, +relaxed \verb{_g}, \verb{_w}, \verb{_gw}, and on \code{mad} groups for a species with no +MAD range \verb{_nomad}, \verb{_nomad_g}), and the two miss reasons. } } \description{ @@ -215,9 +218,15 @@ observed gradients are cut off at the access limit. the rear reason (compare them with \code{n_rearing_any}, not \code{n_rearing}). Both re-evaluate the bundle's own habitat predicates (\code{\link[fresh:frs_habitat_predicates]{fresh::frs_habitat_predicates()}} over \code{cfg$rules} -and its thresholds CSV, channel-width model) on the segment, then again -with the gradient and the channel width each moved to the stage's -minimum: +and its thresholds CSV) on the segment, then again with the gradient and +the size each moved to the stage's minimum. The size is the one the group +classified on: the model \code{cfg}'s \code{parameters_habitat_method.csv} gives +the WSG, resolved as \code{\link[=lnk_pipeline_classify]{lnk_pipeline_classify()}} resolves it (an unlisted +group is \code{cw}). On \code{cw} it is the channel width; on \code{mad} it is the mean +annual discharge \code{mad_m3s}, joined from +\code{whse_basemapping.fwa_stream_networks_discharge} on \code{linear_feature_id} +because the persist does not carry it. The \code{width} labels below mean +that size on either model; \code{model} and \code{mad_m3s} split them: \itemize{ \item \code{NA} — captured; \code{no_segment} — the location attaches to no segment; \item \code{not_accessible} — the segment's \verb{access_} is not 1 or 2; @@ -228,10 +237,21 @@ window (a negative gradient), not above it; \item \code{fails_gradient_or_width} — either relaxation alone passes (through different branches of the rule); \item \code{fails_gradient_and_width} — passes only with both relaxed; +\item \code{no_mad_threshold} — a \code{mad} group where the species has no MAD range +for the stage (fresh then fails every inheriting stream rule outright) +and supplying one would admit the segment; a segment with no discharge +reads \code{width_null} instead, and one whose gradient also fails reads +\code{fails_gradient_and_width}; \item \code{rule_excludes} — fails even then: edge type, waterbody or lake size; \item \code{post_predicate} — passes the predicate but is not habitat: removed by clustering (connectivity to spawning) or access gating. } + +The method table is the one in \code{cfg}. A run classified with another +(a swapped bundle file, or \code{lnk_pipeline_classify(method_csv =)}) is not +detected, so swap it on the \code{cfg} passed here too. A rule-level size +window in \code{rules.yaml} with a floor above the stage minimum would turn +size misses into \code{rule_excludes}; no bundled rules set one. } \examples{ diff --git a/planning/archive/2026-10-issue-299-validate-mad/README.md b/planning/archive/2026-10-issue-299-validate-mad/README.md new file mode 100644 index 00000000..ea8fe48b --- /dev/null +++ b/planning/archive/2026-10-issue-299-validate-mad/README.md @@ -0,0 +1,39 @@ +## Outcome + +`lnk_habitat_validate()` now re-tests each observation's segment on the habitat model its +watershed group classified on. The model comes from the method table of the `cfg` it is +given, resolved by fresh's own `.frs_habitat_models()`, which classify now shares. A +`mad` group's predicates use `frs_habitat_predicates(model = "mad")`, its size relaxation +moves `mad_m3s` (joined from `fwa_stream_networks_discharge` on `linear_feature_id`), and +`width_null` tests discharge. Reason labels are shared across models; `model` and +`mad_m3s` columns split them. New reason `no_mad_threshold`: a `mad` miss that a MAD +range alone would admit, for BT, GR, KO and RB, which have none. Two of the three review +defects came from the `no_mad_threshold` branch being a hand-written copy of the cw +relaxation ladder (it relaxed gradient too, and missed the NULL-size arm); the third, +`.lnk_hv_mad_missing()`, was restating fresh's own rule for when a size test is written. +The loop ended on two enumerations, not on a reviewer going quiet: every miss arm × model × +NULL pattern, evaluated in Postgres (review round 3), and a case-by-case test of the +missing-range rule against fresh. The state of knowledge is in +[`research/habitat_validation.md`](../../../research/habitat_validation.md) (Method, +"Size model per group"). A follow-up issue for tuning MAD thresholds is drafted in +`draft-issue-mad-thresholds.md`, not filed; #300 covers scoring the mad model. + +## Measurement + +- **cw output did not move.** ADMS on `fresh_default`, CH/CO/BT, 94 locations: branch and + main `observations` identical apart from the new columns, `summary` identical apart from + `model`. +- **The planned acceptance metric did not discriminate.** On ADMS modelled on `mad`, + "persisted TRUE, re-built predicate FALSE" is 0 on main as well as the branch. The error + runs the other way: main re-builds the looser cw predicate and calls 27 BT rearing misses + `post_predicate` (removed by clustering or gating). The branch calls them + `no_mad_threshold`, and CO's 2 `width_null` (NULL width, never tested on `mad`) become + `fails_width` on discharge of 0.0127 and 0.018 m³/s. +- Full suite 2,394 tests, 0 failed (16 warnings, the baseline). Validator suite 114 s vs + main's 90 s, from the two new DB tests. + +## Evidence + +`data-raw/logs/habitat_validate_299/` — README, scripts and `20261002_*` outputs. + +Closed by: PR (this branch, `299-lnk-habitat-validate-scores-mad-watersh`) diff --git a/planning/archive/2026-10-issue-299-validate-mad/draft-issue-mad-thresholds.md b/planning/archive/2026-10-issue-299-validate-mad/draft-issue-mad-thresholds.md new file mode 100644 index 00000000..73848916 --- /dev/null +++ b/planning/archive/2026-10-issue-299-validate-mad/draft-issue-mad-thresholds.md @@ -0,0 +1,43 @@ + + +# Title + +Tune MAD (discharge) thresholds for BT, GR, KO and RB rather than leave them missing + +# Body + +**If done:** a `mad` watershed group keeps stream habitat for every modelled species, on +discharge windows set from evidence. **If never:** any group moved to `mad` loses all +stream spawning and rearing for BT, GR, KO and RB by construction. That is not a finding +about the fish. Only waterbody rules (wetlands, lakes, the 1050/1150 edges) keep them. + +## Problem + +`parameters_habitat_thresholds.csv` carries `spawn_mad_*` for CH, CM, CO, PK, SK, ST and +WCT, and `rear_mad_*` for CH, CO, ST and WCT only (`default`, `default_tuned` and +`bcfishpass` alike). The gap was inherited from bcfishpass `example_newgraph`. Under +the `mad` model, fresh writes `FALSE` in place of the size test for a species with no +range, as bcfishpass does. Since #299, `lnk_habitat_validate()` names a miss the missing +range alone explains `no_mad_threshold`. Misses from the same cause also read +`width_null` (no discharge on the line) or `fails_gradient_and_width` (gradient fails +too), so that label is a floor on the loss, not a count of it. On ADMS on `mad`, 27 BT +locations read it against spawning and 8 more read `fails_gradient_and_width` +(`data-raw/logs/habitat_validate_299/`). The operator's direction (2026-10-02, +#299 plan gate): tune and add the thresholds, rather than leave them missing in the long +run. + +## Proposed Solution + +- **Measure first.** Use #300's held-out, discharge-covered groups (Peace, Fraser, + Columbia). Take the distribution of `mad_m3s` at BT / GR / KO / RB observation locations + by stage (pooled DV per `species_pooling.csv`), alongside accessible availability. This + is the #284 method with discharge in place of width. +- **Literature and FISS** for each species' discharge range, recorded in + `research/habitat_thresholds.md` as one verdict per threshold. +- **Land the values in `default_tuned`** (a thin bundle) first, scored out-of-sample with + `lnk_habitat_validate_band()`. `default` stays untouched until a score supports a move. +- **Score before/after with capture and cost** (`share_*`, `*_km`) and + `lnk_habitat_validate_band()`, not with `no_mad_threshold`. Once a range exists, the + label cannot fire, so its "after" is 0 whatever the value. + +Depends on #299 and #300. Relates to #284, #286. diff --git a/planning/archive/2026-10-issue-299-validate-mad/findings.md b/planning/archive/2026-10-issue-299-validate-mad/findings.md new file mode 100644 index 00000000..feae90a7 --- /dev/null +++ b/planning/archive/2026-10-issue-299-validate-mad/findings.md @@ -0,0 +1,55 @@ +# Findings — lnk_habitat_validate() scores mad watershed groups as if they were cw (#299) + +## Issue context + +**If done:** observation validation reports the right miss reasons for groups a bundle +puts on discharge. **If never:** a `mad` group's "missed for width" counts are computed +against channel width the classification never used. Nothing is affected until a +bundle actually sets a group to `mad`, and none does today. + +Since #286, `lnk_pipeline_classify()` passes the bundle's +`parameters_habitat_method.csv` to fresh, and a `mad` group classifies on `mad_m3s`. +`lnk_habitat_validate()` still builds its predicates with +`fresh::frs_habitat_predicates(spp)`, which is the cw model +(`R/lnk_habitat_validate.R:823`). It then relaxes width by rewriting `s.channel_width` +(`.lnk_hv_relax()`, `:772-786`), with minimums taken from +`ranges$$channel_width` (`.lnk_hv_stage_min()`, `:763`). On a `mad` group: +- the predicate is not the one that classified the segment, so `pred_*` can disagree + with the persisted `spawning`/`rearing`; +- width relaxation rewrites a column the mad predicate does not reference; +- `width_null` (`.lnk_hv_miss_reason()`) tests `channel_width`, where it should test + `mad_m3s`. The persist does not carry `mad_m3s` (decided in #286), so the validator + would join it from `whse_basemapping.fwa_stream_networks_discharge` on + `linear_feature_id`. +- The stream-order rearing bypass is skipped for `mad` groups (#286), so a miss + reason must not credit it there. + +fresh >= 0.35.0 has `frs_habitat_predicates(model = "mad")`. Resolve each scored +group's model from the bundle's method table, build the predicates per model, and relax +`s.mad_m3s` for mad groups. `test-lnk_habitat_validate.R` "the predicate call stays on +the channel-width model" pins today's behaviour and should change with it. + + +## Facts from exploration that shape this + +- fresh 0.36.2 `frs_habitat_predicates(spp, model = "mad")`: size column `mad_m3s`; an + absent MAD range inherits as `FALSE` (no `s.mad_m3s` reference, so relaxation can't + rescue it); rule-level `channel_width` (river-polygon bypass) dropped; rule-level + `mad:` emits `s.mad_m3s BETWEEN`. Lake/wetland rules inherit nothing under either + model, so BT keeps wetland rearing on mad. +- Persisted `.streams` carries `linear_feature_id` but not `mad_m3s` (#286 + decision); prepare joins it from `whse_basemapping.fwa_stream_networks_discharge` on + `linear_feature_id` (`R/lnk_pipeline_prepare.R:697`). +- Classify resolves the group model with `params_method$model[match(aoi, ...)]` + (`R/lnk_pipeline_classify.R:165`), unlisted → cw. Validator will share that, not + re-derive it. +- The stream-order rearing bypass is never modelled by the validator (no reference to + it), so it cannot be credited on a mad group; a test pins that a mad order-1 child + miss gets a predicate reason, not `post_predicate`. +- Test fixture (`local_validate_fixture`) has no `linear_feature_id`; BT only. + + +## Errors Encountered + +| Error | Resolution | +|-------|------------| diff --git a/planning/archive/2026-10-issue-299-validate-mad/progress.md b/planning/archive/2026-10-issue-299-validate-mad/progress.md new file mode 100644 index 00000000..467f0a37 --- /dev/null +++ b/planning/archive/2026-10-issue-299-validate-mad/progress.md @@ -0,0 +1,17 @@ +# Progress — lnk_habitat_validate() scores mad watershed groups as if they were cw (#299) + +## Session 2026-10-02 + +- Plan-mode exploration — phases approved by user (labels kept + `model`/`mad_m3s` columns; new `no_mad_threshold` reason) +- Created branch `299-lnk-habitat-validate-scores-mad-watersh` off main +- Scaffolded PWF baseline from issue #299 with approved phases +- Next: start Phase 1 + +- Phase 1: `.lnk_habitat_method_read()` + `.lnk_wsg_model()` (fresh's `.frs_habitat_models()` via getFromNamespace); classify routed through them. 232 tests pass across config/classify/preflight. +- Code-check Phase 1: round 1 Clean (`review-p1-round1.md`). Rounds 2-3 deliberately folded into the Phase 2-3 review, which reviews the branch diff including this one. +- Plan review (Plan agent): 1 blocker (nomad also relaxed gradient), several latent gaps; triage in `review-plan.md`. +- Phases 2-3 landed. Code-check on the branch diff: round 2 (`review-p23-round2.md`) found NULL discharge mislabelled `no_mad_threshold` (inside the nomad fix) -> `width_null`; round 3 (`review-p23-round3.md`) named the mechanism (nomad ladder is a hand copy of the cw relaxation ladder), enumerated every arm x model x NULL pattern against Postgres (all right), and found `no_range` restating fresh's rule imperfectly -> `.lnk_hv_mad_missing()` plus a case-by-case test cross-checked against fresh. Loop ended by those two enumerations. Validator file: 195 pass. +- Reviewer spend for the task so far: 1 plan review + 3 code-check rounds. +- Phase 4: roxygen (Miss reasons, @return), driver `model` column + `width_bin = "mad"`, RUNBOOK §7, research/habitat_validation.md, CLAUDE.md; follow-up issue drafted (not filed) in `draft-issue-mad-thresholds.md` — #300 exists for scoring mad; this one is for adding thresholds. +- Phase 5: ADMS on mad into `zz299_mad` (2.8 min), validator branch vs main on both schemas. cw identical apart from new columns. Planned disagreement metric is 0 on main too, so it does not discriminate; reasons do (README). Scratch schemas dropped. +- Code-check Phases 4-5: one round (`review-p45-round1.md`), 5 false claims in prose (stage-unfiltered reason table, schema "dropped" before it was, draft issue before/after and species list, uncommitted km) — all fixed. Reviewer spend: 1 plan review + 4 code-check rounds = 5 agents. diff --git a/planning/archive/2026-10-issue-299-validate-mad/review-p1-round1.md b/planning/archive/2026-10-issue-299-validate-mad/review-p1-round1.md new file mode 100644 index 00000000..853e15d4 --- /dev/null +++ b/planning/archive/2026-10-issue-299-validate-mad/review-p1-round1.md @@ -0,0 +1,13 @@ +# Review — #299 phase 1, round 1 + +## Clean +No issues found. + +What was checked: +- fresh floor: `.frs_habitat_models(wsg_codes, params_method)` was introduced in fresh c56961c (#220) and is byte-identical at tag v0.35.0 (R/utils.R:875) and in the installed 0.36.2, so the DESCRIPTION floor (>= 0.35.0) has it with the same signature and argument order the wrapper uses. +- Classify behaviour for valid inputs: `aoi` is validated as a single non-empty string, so the old `params_method$model[match(aoi, ...)]` returned the model or NA, and the new resolver returns the model or "cw"; `identical(x, "mad")` is TRUE for exactly the same inputs. The resolver's new stops (bad model, duplicate code, missing columns) fire only on tables `fresh::frs_habitat_classify(params_method = ...)` already rejects earlier in the same function via the same helper, so no previously-succeeding run now fails. `unname()` keeps `identical()` from failing on the names attribute. +- "NA" group code: `.lnk_habitat_method_read` keeps `na.strings = character(0)`, so "NA" stays a string; fresh's `incomparables = NA` only excludes real NA, so "NA" resolves (test confirms). +- Preflight: `lnk_preflight_fresh()` already vapply()s `exists` over `required_internal`, so the length-2 vector is handled; the stopifnot has no length-1 constraint on it. +- Tests run (NOT_CRAN=true, load_all): test-lnk_config 158 pass, test-lnk_preflight_fresh 39, test-lnk_pipeline_classify 35, test-dictionaries 99, test-lnk_pipeline_connect 10, test-lnk_log 279, test-lnk_pipeline_run 22 — 0 fail, 0 skip. + +Non-blocking note (not a defect in the diff): planning/active/findings.md:44 cites `R/lnk_pipeline_classify.R:167`; the diff removes two lines above it, so the call now sits at :165. diff --git a/planning/archive/2026-10-issue-299-validate-mad/review-p23-round2.md b/planning/archive/2026-10-issue-299-validate-mad/review-p23-round2.md new file mode 100644 index 00000000..11228191 --- /dev/null +++ b/planning/archive/2026-10-issue-299-validate-mad/review-p23-round2.md @@ -0,0 +1,77 @@ +# Review: #299 Phases 1-3, round 2 (branch vs main, R/ + tests/) + +Scope: `git diff main -- R tests` (p23.diff, 859 lines), plus full reads of the changed +functions and fresh 0.36.2's `frs_habitat_predicates()`, `.frs_rule_to_sql()`, +`.frs_habitat_models()`, `.frs_preds_by_model()` and `frs_habitat_classify()`'s model +resolution. + +Tests run in a copy of the tree (`NOT_CRAN=true`, local docker fwapg): +test-lnk_habitat_validate.R 180 pass, test-lnk_config.R 158 pass, +test-lnk_preflight_fresh.R 39 pass, 0 fail / 0 skip. + +## Findings + +- **[fragile]** R/lnk_habitat_validate.R `.lnk_habitat_miss_reason()` (the new + `set(p_nomad, "no_mad_threshold")` line), with `.lnk_hv_stage_exprs()` building + `_nomad` with the size relaxed to 0. A miss on a mad group, for a species with no + MAD range, on a segment whose `mad_m3s` is NULL reads `no_mad_threshold`. The + decision recorded for #299 says this label means the missing range is the ONLY + thing in the way. Here it is not: supply a range and the segment still fails, + because the discharge is NULL. `_nomad` substitutes a constant for `s.mad_m3s`, so + it cannot see a NULL. On cw the same situation, size relaxed passes and the value + is NULL, reads `width_null`. The `width_null` arm never fires for these rows, + because `p_w` still carries fresh's literal `FALSE` and so is always FALSE. + The test fixture pins the current behaviour without saying so: `o2` sits on AAAA + segment 1, which `local_mad_fixture()` gives the `mad_m3s IS NULL` line, and the + test expects `miss_reason_spawn == "no_mad_threshold"`. The comment there gives only + the persisted-spawning reason. + Impact: in real mad groups with discharge gaps, NULL-discharge misses for BT, GR, + KO and RB are counted as missing thresholds rather than missing data. That + overstates what adding a MAD range would recover, which is the question this + label exists to answer. + Fix, if the stated meaning is kept: gate it as + `set(p_nomad & !is.na(size), "no_mad_threshold")` and route + `p_nomad & is.na(size)` to `width_null`, mirroring the cw order. Then update the + `o2` expectation. If the current behaviour is deliberate, put that in the test + comment and in the roxygen. + +## Checked and clean + +- cw output: `.lnk_hv_stage_exprs(spp, "cw")` equals the pre-#299 construction apart + from the two `NULL::boolean` `_nomad` columns (test-pinned). `size` is + `channel_width` for cw, and `p_nomad` is NA so it cannot set a reason. The only + cw-visible changes are the added `model`, `mad_m3s` and `pred_*_nomad*` columns. + `mad_m3s` is inserted mid-frame, between `channel_width_source` and `edge_type`. + Nothing in R/ or data-raw/ reads by position. +- Discharge join: `fwa_stream_networks_discharge` has a unique PK on + `linear_feature_id` (2,716,652 rows, 2,716,652 distinct). So the scalar subquery in + `.lnk_hv_obs` cannot raise "more than one row" and the LEFT JOIN in + `.lnk_hv_predicates` cannot fan out. The persist's `cols_streams` carries + `linear_feature_id`, and prepare joins the same table on the same key, so the + validator reads what classify read. +- `.lnk_wsg_model()` delegates to fresh's resolver, so it matches classify. fresh + 0.35.0, the DESCRIPTION floor, has both `.frs_habitat_models` and + `frs_habitat_predicates(model=)`. The preflight declares both. The `exists()` loop + fix is correct: `exists()` is not vectorised. +- `lnk_pipeline_classify`: swapping `match()` for `.lnk_wsg_model()` keeps the bypass + decision identical, because an unlisted group or NA is cw and was never `"mad"` + either way. It errors on a bad table earlier, which fresh would have done anyway. +- `.lnk_hv_relax(size_col = "mad_m3s")` leaves `s.mad_m3s_x` alone (`\b`). MAD + minimums in every bundled thresholds CSV have at most 5 significant digits, so + `format()` cannot round a relaxed value outside its window. fresh's `frs_params()` + leaves `ranges$$mad_m3s` NULL, never `c(NA, NA)`, when the CSV is NA, so the + `is.null()` test in `no_range` is the right one. +- The `_nomad` construction substitutes `c(0, 0)` and relaxes the size to 0. That + makes the inherited `BETWEEN 0 AND 0` or `>= 0 AND <= 0` true, and keeps the lake + and wetland branches polygon-equivalent. +- `.lnk_hv_predicates` per (species, model) combos: each (species, id_segment, WSG) + key falls in exactly one combo, because the model is per WSG. So the `match()` + merge is 1:1. The empty-`seg` path still returns `NULL` from `rbind`, as before. +- `out$mad_m3s[out$model != "mad"] <- NA_real_` is safe if `model` were ever NA + (length-1 value), and every observation's group is in `aoi` via `lnk_vd_spec`. + +## Note (not a finding) + +The roxygen `@section Miss reasons:` (R/lnk_habitat_validate.R ~L95-110) still says +"channel-width model" and does not list `no_mad_threshold`, so the label is absent +from the man page that presents the list as complete. diff --git a/planning/archive/2026-10-issue-299-validate-mad/review-p23-round3.md b/planning/archive/2026-10-issue-299-validate-mad/review-p23-round3.md new file mode 100644 index 00000000..70f3ebcf --- /dev/null +++ b/planning/archive/2026-10-issue-299-validate-mad/review-p23-round3.md @@ -0,0 +1,76 @@ +# Review — #299 phase 2-3, round 3 + +Diff: `p23r3.diff` (branch vs main, R/ tests/ man/). fresh 0.36.2 (installed and checkout agree). +Tests: `test-lnk_habitat_validate.R` — FAIL 0 | WARN 0 | SKIP 0 | PASS 181 (local docker fwapg). + +## Mechanism + +Every miss label is read off *which relaxed predicate passes*, and the label assumes the +relaxed predicate differs from the real one in exactly the dimension the label names. A +relaxation is a textual substitution of a constant for a column, and a constant has no NULL +and carries no other condition. The cw ladder (`p_g`/`p_w`/`p_gw`) already encodes the +consequences as extra arms (`width_null` = `p_w & !p_g & is.na(size)`; `p_gw` -> +`fails_gradient_and_width` whatever the NULLs). The nomad ladder (`p_nomad`/`p_nomad_g`) is a +second, hand-written copy of that ladder: `p_nomad` is the analogue of `p_w` (the missing +range plays "width fails") and `p_nomad_g` the analogue of `p_gw`. Rounds 1 and 2 were both +places where the copy had fewer arms, or a wider substitution, than the original: round 1 +substituted gradient too (`p_nomad` stood in for `p_gw` while labelled like `p_w`), round 2 +lacked `p_w`'s `is.na(size)` companion arm. The second list of the same shape is +`no_range` in `.lnk_hv_stage_exprs()`, which is link's restatement of fresh's own condition +for writing `FALSE` in place of a size test (`size_inherit()`: `is.null(rng) && model == "mad"`); +the two agree only because every bundled stage that has rules also has a channel-width range. + +## Enumeration + +Measured, not reasoned: real predicates from the `default` bundle (`.lnk_hv_stage_exprs()`), +evaluated in Postgres over a synthetic grid — gradient {NULL, -0.02, 0.01, 0.20} x channel +width {NULL, 0.5, 5} x mad {NULL, 0.01, 1, 200} x edge {1000, 1050} x waterbody {none, a real +river polygon} — for BT, KO (no MAD range), SK (spawn range, no rear range), CO, CH (ranges), +each on cw and mad, both stages, then `.lnk_habitat_miss_reason()`. The nomad labels were +compared row by row with the cw label of the same species on the same segment with width +mapped to "fails" (0.5) where discharge is present and NULL where it is NULL. Probe: +scratchpad `r3/probe.R`, `r3/cmp.R`. + +| arm | cw | mad, range (CO/CH, SK spawn) | mad, no range (BT, KO spawn, SK/KO rear) | +|---|---|---|---| +| `no_segment` | unchanged | same | same | +| `not_accessible` | unchanged | same | same | +| `post_predicate` (p) | unchanged | right | right: 1050/1150 `thresholds: false` and L/W polygon branches still pass (BT keeps wetland/lake rear) | +| `gradient_below_min` (p_g & !p_w & below) | unchanged | right (size present, gradient < 0) | unreachable: p_g == p (no non-size branch carries a gradient in any bundle); below-min + missing range reads `fails_gradient_and_width`, = cw's `p_gw` for below-min + failing width | +| `fails_gradient` (p_g & !p_w) | unchanged (NULL gradient lands here, pre-existing) | right, incl. NULL gradient, as cw | unreachable (same reason) | +| `width_null` (p_w & !p_g & NA size) | unchanged | right: size = `mad_m3s`; discharge NULL + gradient ok | unreachable via this arm (p_w == p) | +| `fails_width` (p_w & !p_g) | unchanged | right (below min and above max, e.g. CO rear mad 200 > 40) | unreachable | +| `fails_gradient_or_width` (p_g & p_w) | unchanged | right (no case in grid; same SQL shape as cw) | unreachable | +| `fails_gradient_and_width` (p_gw) | unchanged (NULL width + failing gradient lands here, pre-existing) | right, incl. NULL size/NULL gradient combos, as cw | unreachable | +| `width_null` (p_nomad & NA size) | NA on cw | NA (nomad cols NULL) | right: gradient ok + discharge NULL; matches cw `width_null` analogue on every row | +| `no_mad_threshold` (p_nomad) | NA | NA | right: gradient ok, discharge present (any value 0.01-200); analogue `fails_width` on every row; river polygons with discharge also read it (the R bypass is dropped under mad, so the range is the only obstacle) — correct | +| `fails_gradient_and_width` (p_nomad_g) | NA | NA | right: gradient NULL / negative / too steep, discharge NULL or present; analogue `fails_gradient_and_width` on every non-polygon row | +| `rule_excludes` | unchanged | right | right (KO/SK rear off-lake; 1050 off-polygon spawn) | + +cw unchanged: the cw exprs are byte-identical to the pre-#299 construction (pinned by test), +`size` is `channel_width` for cw rows, nomad columns are NULL on cw, and classify's +`aoi_model` moves from `NA` to `"cw"` for an unlisted group, which `identical(, "mad")` reads +the same. The only cw-visible change is that classify now errors on a duplicate or non-cw/mad +method row, which fresh's classify raises on the same table anyway. + +Other reaches checked: the `s.mad_m3s` join in `.lnk_hv_predicates` and the scalar subquery in +attach read the same table on the same key, and `linear_feature_id` is its unique PK (2,716,652 +rows, 0 NULL), so neither fans out nor errors; `_nomad`'s open range `[0, 0]` adds a size gate +to lake/wetland rearing that the size-0 relaxation removes again, so SK/KO rear `p_nomad == p`. + +## Findings + +- **[severity: fragile]** R/lnk_habitat_validate.R:893-896 — `no_range` requires + `!is.null(rng[["channel_width"]])`, a condition fresh does not have: fresh writes `FALSE` for + any stage with no MAD range under mad (`size_inherit()`), channel-width range or not. For a + stage with rules and a gradient but no channel-width range, cw classifies stream habitat on + gradient alone and mad classifies none, yet the validator gives the nomad columns NULL and the + miss reads `rule_excludes` instead of `no_mad_threshold`. The docstring's "(a stage with no + stream habitat at all)" is only true when the rules are empty too. **Unreachable in every + shipped bundle**: the only stages without a channel-width range are CM and PK rear, and their + `rear:` is `[]` in all five `rules.yaml`. Matters only for a custom bundle; dropping the + `channel_width` clause would make it follow fresh's rule (setting a `[0, 0]` range on CM/PK + rear is harmless — `rear: []` compiles to `FALSE` either way). + +No bugs found in the mad-path labels: every arm x model x NULL/negative/present combination +reads either the intended label or the cw analogue's label. diff --git a/planning/archive/2026-10-issue-299-validate-mad/review-p45-round1.md b/planning/archive/2026-10-issue-299-validate-mad/review-p45-round1.md new file mode 100644 index 00000000..693597b8 --- /dev/null +++ b/planning/archive/2026-10-issue-299-validate-mad/review-p45-round1.md @@ -0,0 +1,88 @@ +# Review — #299 phase 4-5 (docs, driver, evidence), round 1 + +Diff: `p45.diff` (staged). Code checked against HEAD `52c7eca` `R/lnk_habitat_validate.R`. + +## Probes run (scratchpad, nothing in the repo edited) + +- **cw no-change at HEAD.** Exported `4df1ffc` with `git archive` and ran `lnk_habitat_validate()` + on `fresh_default` ADMS (CH, CO, BT, pooled) from it and from HEAD. `compare.R`'s two + `all.equal` checks are TRUE at HEAD and 94 locations, as the log says. The extra columns + are exactly the six in `new_obs`. +- **mad numbers at HEAD.** `zz299_mad` still exists in local fwapg (log row 2026-10-02 + 08:15 UTC, config `default`). I re-ran the validator from HEAD with the method table + swapped. Every count in `20261002_adms_compare.txt` §3 "branch" reproduces, as does + `mad_m3s` 94/94. The README's stream km for CH (256.41 / 317.50) and CO (292.43 / 317.75) + match `summary`. So the `R/` edits made after the runs (file mtime 01:33, README 01:27) + did not move these numbers. +- **README tables against `compare.txt`.** Every cell matches. +- **Driver `misses` / `misses_binned`.** I extracted the block and ran it on the cw run, the + mad run and a mixed cw+mad run. The binned totals equal the `misses.csv` non-captured, + non-`no_segment` totals in all three (16/16, 50/50, 66/66). No downstream code in the + script reads these frames by position. +- **Docs against the code.** These all match the code: the label semantics, the reason order + (`width_null` before `no_mad_threshold` before the `p_nomad_g` arm), `mad_m3s` coming from + `whse_basemapping.fwa_stream_networks_discharge` on `linear_feature_id`, the + `.frs_habitat_models()` resolution, "a swapped `method_csv` is not detected", and the MAD + maxima (CH 100, CO 40, ST 60, WCT 40, against the default CSV). +- **CO `width_null` → `fails_width` in the README.** The two locations have `mad_m3s` + 0.0127 and 0.0182, below CO's minima (spawn 0.164, rear 0.03). The claim is true. + +## Findings + +- **[severity: false-claim]** `data-raw/logs/habitat_validate_299/README.md:43`, "Scratch + schema `zz299_mad` and the RDS files were not kept." The schema is still in local fwapg + (`SELECT nspname FROM pg_namespace WHERE nspname LIKE 'zz299%'` → `zz299_mad`). Either + drop it before committing, or reword the sentence. (The working schema `zz299_w_adms` is + gone.) + +- **[severity: false-claim]** `data-raw/logs/habitat_validate_299/README.md:30-38`. The + reasons table is labelled by `stage`, but its counts are `miss_reason_spawn` / + `miss_reason_rear` over **all** 37 BT locations, not over spawn- or rear-staged ones. + `compare.R`'s `tab()` does not filter on `is_spawn` / `is_rear`. Only 1 BT location is + spawn-staged and 6 are rear-staged (measured at HEAD). So "BT | spawn | 27 + `no_mad_threshold`" reads as 27 spawning observations when there is 1. That conflicts + with the driver's own `misses.csv`, whose `stage` column *is* stage-filtered. + + The prose has the same problem: "Main tells a reader that 51 BT misses were removed by + clustering". 51 is 24 spawn-reason plus 27 rear-reason over 37 locations, so locations + are counted twice. `post_predicate` also covers access gating, not only clustering + (roxygen: "removed by clustering ... or access gating"). Fix: relabel the column "reason + column (all locations)", or tabulate by stage. Then state the figure per reason column, + not as 51 misses. + +- **[severity: false-claim]** `planning/active/draft-issue-mad-thresholds.md:35`, "`no_mad_threshold` + counts per WSG × species are the before/after for each value added". After a value is + added the species has a MAD range, so `.lnk_hv_mad_missing()` is FALSE, the `_nomad` + columns are NULL, and the label cannot occur. The "after" is 0 by construction, + whatever the value is. The count also understates the "before": `no_mad_threshold` + covers only misses with discharge present and passing gradient. Missing-range misses + read `width_null` (NULL discharge) or `fails_gradient_and_width` (gradient also fails); + for BT spawn on ADMS that is 8 more beside the 27. The meaningful before/after is + capture and cost (`share_spawning` / `share_rearing_any`, `*_km`), or the band score + the draft already names. The same issue touches line 19-20: "so the loss can be counted" + is only partly true, for the same reason. + +- **[severity: false-claim, minor]** `planning/active/draft-issue-mad-thresholds.md:17-18`, + "carries `spawn_mad_*` / `rear_mad_*` for CH, CM, CO, PK, SK, ST and WCT only". In all + bundles (`default`, `default_tuned`, `bcfishpass`), CM, PK and SK carry + `spawn_mad_*` only and their `rear_mad_*` is empty. `rear_mad_*` exists for CH, CO, ST + and WCT. It is harmless in practice, since CM and PK have `rear: []` and SK rears in + lakes only, but as written it misstates the CSV in a public issue. + +- **[severity: fragile, minor]** `data-raw/logs/habitat_validate_299/README.md:9-10` + gives stream km (CH 256.4 / 317.5, CO 292.4 / 317.8, "BT 0 / 0") that appear in no + committed log. `model_mad.R` prints only `n_segments`. The values are correct (I + reproduced them from `summary` at HEAD). BT's rollup gives `NA` (no rows), not 0. The + rule is never a number without its producer: either print them in `model_mad.R` / + `compare.R`, or say where they came from. + +## Not findings (checked) + +- `model_mad.R` / `validate_run.R` load the source tree (`devtools::load_all(repo)`) and set + the persist schema through `cfg$pipeline$schema`, which persist reads. They use their own + working schema and no top-level `on.exit`. The only credentials are the local-docker + defaults. No host addresses or personal paths in any committed file. +- The `habitat_validate.R` header comment (line 42, "per bundle x species x stage") does + not mention the new `model` dimension. It is stale-ish, but not false. +- `totals.csv` / `diff.csv` sum across WSGs whatever their model. That is unchanged by + this diff and reasonable. diff --git a/planning/archive/2026-10-issue-299-validate-mad/review-plan.md b/planning/archive/2026-10-issue-299-validate-mad/review-plan.md new file mode 100644 index 00000000..33a67c09 --- /dev/null +++ b/planning/archive/2026-10-issue-299-validate-mad/review-plan.md @@ -0,0 +1,22 @@ +# Plan review (Plan agent, 2026-10-02) — triage + +Full findings were returned in the agent reply; triaged here. + +| Finding | Verdict | Action | +|---|---|---| +| `no_mad_threshold` relaxes gradient too, contradicting "only that stands in the way" (o9 at 20 % would read it) | **Real** | nomad relaxes size only; a miss passing only with gradient also relaxed reads `fails_gradient_and_width` (cw's "both" label) via `pred__nomad_g` | +| CSV path (no rules YAML): CM/PK rear would read `no_mad_threshold` | Real, latent | no-range = cw range present and MAD range absent | +| Rule-level `mad:` on a cw group → `s.mad_m3s` missing → SQL error | Real, latent | predicate query joins discharge whenever an expression references `s.mad_m3s` | +| Rule-level window can't be reached by relaxing to the stage min | Real, latent (same as cw) | comment extended; test asserts no bundled rules.yaml sets rule-level `gradient`/`mad` | +| Discharge join written twice | Real | one helper for the source table | +| Fail loud when a mad group's prerequisites are missing | Real | check discharge table + `streams.linear_feature_id` up front | +| Preflight: `frs_habitat_predicates(model)` not asserted | Real | added to required formals | +| Existing tests break on renames | Already handled in working tree | — | +| Parity test that mad exprs wrap `frs_habitat_predicates(spp, "mad")` | Real | added | +| CO on a mad group never exercised in SQL | Real | fixture adds CO (c1/c2) | +| Live discharge values make fixture data-dependent | Accepted | ids picked by `mad_m3s` range, deterministic order; skip if table absent | +| Driver: mad misses unbinned | Real | `width_bin = "mad"` on mad rows, `model` column carried | +| `size_above_max` label | Not taken | documented: `mad_m3s` lets a reader split; maxima bind for CH/CO/ST/WCT rear | +| Warn on `config_hash` mismatch | Scope | documented caveat only | +| Phase 5 must mutate `cfg` and pass the same object to the validator | Noted | Phase 5 does exactly that | +| Phase 5 metric not valid for SK/KO (`spawn_connected`) | Noted | Phase 5 scores CH/CO/BT only | diff --git a/planning/archive/2026-10-issue-299-validate-mad/task_plan.md b/planning/archive/2026-10-issue-299-validate-mad/task_plan.md new file mode 100644 index 00000000..f6615db6 --- /dev/null +++ b/planning/archive/2026-10-issue-299-validate-mad/task_plan.md @@ -0,0 +1,85 @@ +# Task: lnk_habitat_validate() scores mad watershed groups as if they were cw (#299) + +**If done:** observation validation reports the right miss reasons for groups a bundle +puts on discharge. **If never:** a `mad` group's "missed for width" counts are computed +against channel width the classification never used. Nothing is affected until a +bundle actually sets a group to `mad`, and none does today. + +Since #286, `lnk_pipeline_classify()` passes the bundle's +`parameters_habitat_method.csv` to fresh, and a `mad` group classifies on `mad_m3s`. +`lnk_habitat_validate()` still builds its predicates with +`fresh::frs_habitat_predicates(spp)`, which is the cw model +(`R/lnk_habitat_validate.R:823`). It then relaxes width by rewriting `s.channel_width` +(`.lnk_hv_relax()`, `:772-786`), with minimums taken from +`ranges$$channel_width` (`.lnk_hv_stage_min()`, `:763`). On a `mad` group: +- the predicate is not the one that classified the segment, so `pred_*` can disagree + with the persisted `spawning`/`rearing`; +- width relaxation rewrites a column the mad predicate does not reference; +- `width_null` (`.lnk_hv_miss_reason()`) tests `channel_width`, where it should test + `mad_m3s`. The persist does not carry `mad_m3s` (decided in #286), so the validator + would join it from `whse_basemapping.fwa_stream_networks_discharge` on + `linear_feature_id`. +- The stream-order rearing bypass is skipped for `mad` groups (#286), so a miss + reason must not credit it there. + +fresh >= 0.35.0 has `frs_habitat_predicates(model = "mad")`. Resolve each scored +group's model from the bundle's method table, build the predicates per model, and relax +`s.mad_m3s` for mad groups. `test-lnk_habitat_validate.R` "the predicate call stays on +the channel-width model" pins today's behaviour and should change with it. + + +**Decisions taken (user, plan gate):** +- Size miss labels **keep their names** (`fails_width`, `width_null`, + `fails_gradient_or_width`, `fails_gradient_and_width`) and mean "the group's size + dimension"; observations gain `model` and `mad_m3s` so a reader can split them. +- **New reason `no_mad_threshold`**: a miss on a `mad` group where the species has no MAD + range for that stage (BT, GR, KO, RB today) and only that stands in the way. Longer + term the user wants those thresholds tuned and added rather than left missing; a + follow-up issue body is drafted for review (not filed). + +## Phase 1: Shared model resolution + +Resolver is fresh's own `.frs_habitat_models()` via `getFromNamespace()` (added to the preflight's required internals), not a link copy. + +- [x] Add `.lnk_habitat_method_read(path)` and `.lnk_wsg_model(params_method, wsg)` beside `.lnk_habitat_method_csv()` in `R/lnk_config.R` (character read as classify does; unlisted → `"cw"`; error on a model other than cw/mad) +- [x] `lnk_pipeline_classify()` uses both for its read and its `aoi_model` (behaviour-preserving) +- [x] Unit tests: unlisted group is cw, bad model value errors, classify still skips the bypass only for mad + +## Phase 2: Model-aware predicates and relaxation + +As landed (review-driven): `_nomad` relaxes size only and `_nomad_g` size + gradient (→ `fails_gradient_and_width`); a `_nomad` miss with no discharge reads `width_null`; `.lnk_hv_mad_missing()` mirrors fresh's rules/CSV branches; the predicate query joins discharge whenever an expression references `s.mad_m3s`; `.lnk_hv_check_mad()` fails loud; preflight asserts `frs_habitat_predicates(model)`. + +- [x] Test first: replace "the predicate call stays on the channel-width model" with a test that the call passes `model =`; unit tests for a new pure helper `.lnk_hv_stage_exprs(spp, model)` — mad exprs reference `s.mad_m3s` not `s.channel_width`, relaxed variants replace `s.mad_m3s`, cw exprs byte-identical to today's +- [x] `.lnk_hv_stage_min(spp, model)` reads `ranges$$mad_m3s` on mad (absent → 0) +- [x] `.lnk_hv_relax(pred, gradient, size, size_col)` rewrites the model's size column +- [x] `.lnk_hv_predicates()`: resolve each scored WSG's model, put it on `lnk_vd_seg`, run one query per species × model present; mad queries read `streams` LEFT JOINed to the discharge table on `linear_feature_id` (aliased so predicates still see `s.mad_m3s`) +- [x] `pred__nomad`: on mad groups whose species lacks that stage's MAD range, the predicate rebuilt with the range filled and gradient + size relaxed; `NA` elsewhere + +## Phase 3: Miss reasons and output columns +- [x] `.lnk_hv_obs()` attaches `model` per WSG and `mad_m3s` (joined only when some scored group is mad, so a cw-only call issues no new query; `NA` otherwise) +- [x] `.lnk_habitat_miss_reason()` takes the size value (`channel_width` on cw, `mad_m3s` on mad) for `width_null`, and a `p_nomad` arm placed after `fails_gradient_and_width`, before `rule_excludes` +- [x] `summary` gains `model` +- [x] DB tests on the fixture: add `linear_feature_id` drawn from the live discharge table (skip if absent), a method table putting AAAA on mad; assert BT stream misses read `no_mad_threshold`, `mad_m3s` is populated, `model` columns, and every existing cw assertion unchanged +- [x] Unit test: `width_null` keys on the size value passed, so a mad row with NULL discharge and a width reads `width_null` + +## Phase 4: Docs and driver +- [x] Roxygen: Miss reasons section (size dimension per model, `no_mad_threshold`, model from the bundle's method table, caveat that a schema built under a different method table is not detected); `@return` columns; `devtools::document()` +- [x] `data-raw/habitat_validate.R`: carry `model` into `misses.csv` / `misses_binned.csv`; bin width on cw rows only +- [x] RUNBOOK §7 bullet, `research/habitat_validation.md`, CLAUDE.md "cw-only (#299)" fact +- [x] Draft (not file) follow-up issue: tune and add MAD thresholds for BT/GR/KO/RB + +## Phase 5: Live verification (local docker fwapg) +Run decisions: config `default` with a temp method table putting **ADMS** on `mad`; +persist schema **`zz299_mad`** (scratch, dropped after); WSG ADMS only (no closure); +species from config × presence; `mapping_code = TRUE` (validator needs `streams_access`); +no `--refresh-primitives`. +- [x] cw no-change: validate ADMS on `fresh_default` with branch vs main — `observations` and `summary` identical apart from the new columns +- [x] mad: model ADMS into `zz299_mad`, validate CH/CO/BT; count locations where persisted `spawning`/`rearing` is TRUE but `pred_*` FALSE outside UHC — expect ~0 on branch, show main's count for contrast; tabulate reasons (BT → `no_mad_threshold`). **As measured:** that count is 0 on main too (does not discriminate); the discriminating evidence is main's 24 + 27 BT `post_predicate` vs the branch's `no_mad_threshold` — README +- [x] Log under `data-raw/logs/habitat_validate_299/` with env stamp + +## Validation + +- [x] Tests pass (full suite 2394 / 0 failed / 0 skipped, 16 warnings = baseline) +- [ ] `/code-check` clean on each commit — **partly**: Phase 1 round 1 Clean; Phases 1-3 rounds 2-3 (found + fixed 2, ended by enumeration); Phases 4-5 docs/evidence one round only (5 false claims, all fixed, rounds 2-3 not run) +- [x] PWF checkboxes match landed work +- [x] `/planning-archive` on completion diff --git a/research/habitat_validation.md b/research/habitat_validation.md index a7a87eb0..6f4d4f2b 100644 --- a/research/habitat_validation.md +++ b/research/habitat_validation.md @@ -1,6 +1,6 @@ # Habitat validation against fish observations -**Verified:** 2026-09-27 · **Issues:** #283 (this), #284 (step 5 scores with it), #290 (pooling as data), #203 (full-key joins), fresh#218 · **Produced by:** `lnk_habitat_validate()` via `data-raw/habitat_validate.R` → `data-raw/logs/habitat_validate_283/` (link @ `0c19e0a`) · **Status:** baseline; #284 step 5 scored BT `rear_gradient_max` with it (see below) +**Verified:** 2026-09-27; size model per group 2026-10-02 · **Issues:** #283 (this), #284 (step 5 scores with it), #290 (pooling as data), #203 (full-key joins), #299 (cw / mad per group), fresh#218 · **Produced by:** `lnk_habitat_validate()` via `data-raw/habitat_validate.R` → `data-raw/logs/habitat_validate_283/` (link @ `0c19e0a`) · **Status:** baseline; #284 step 5 scored BT `rear_gradient_max` with it (see below) ## What it measures @@ -46,6 +46,20 @@ that both retained the same observations. uses. The habitat flags are gated by `streams_habitat.accessible`, which differs slightly, so spawning capture is not strictly a subset of accessible capture. #284 used the latter. +- **Size model per group (#299):** each WSG is re-tested on the model the bundle's + `parameters_habitat_method.csv` gives it, resolved by classify's own rule (fresh's + `.frs_habitat_models()`). A `mad` group's predicates test `mad_m3s`, joined from + `fwa_stream_networks_discharge` on `linear_feature_id` (the persist does not carry + it), and its size relaxation moves discharge, not width. The reason labels are + unchanged: `fails_width` / `width_null` mean "the group's size dimension", and the + `model` and `mad_m3s` columns split them. `no_mad_threshold` is a `mad` miss the + species' missing MAD range alone explains (BT, GR, KO, RB have none: fresh writes + `FALSE` for the size test, which no relaxation reaches); one that needs the gradient + relaxed too reads `fails_gradient_and_width`. MAD maxima bind (CH 100, CO 40, ST 60, + WCT 40 m³/s rearing), so on `mad` a big-river miss also reads `fails_width`; read + `mad_m3s` to tell it from a small one. The validator uses the method table of the + `cfg` it is given: a schema built under another table, or with + `lnk_pipeline_classify(method_csv =)`, is not detected. - **Buffer:** `buffer_m` credits habitat that starts within that distance *upstream* on the same stream, because points often mark the downstream end of a site. The UHC flags test the same window. The driver reports 0 and 100 m. diff --git a/tests/testthat/test-lnk_config.R b/tests/testthat/test-lnk_config.R index 29a3154a..dcdd8317 100644 --- a/tests/testthat/test-lnk_config.R +++ b/tests/testthat/test-lnk_config.R @@ -284,6 +284,31 @@ test_that(".lnk_habitat_thresholds_csv falls back to fresh's copy, loudly", { package = "fresh")) }) +# -- habitat model per watershed group (#299) --------------------------------- + +test_that(".lnk_wsg_model resolves each group, unlisted groups on cw", { + skip_if_not_installed("fresh") + pm <- data.frame(watershed_group_code = c("ADMS", "BULK"), + model = c("mad", "cw"), stringsAsFactors = FALSE) + expect_identical(.lnk_wsg_model(pm, c("BULK", "ADMS", "ZZZZ")), + c("cw", "mad", "cw")) +}) + +test_that(".lnk_wsg_model rejects a model other than cw or mad", { + skip_if_not_installed("fresh") + pm <- data.frame(watershed_group_code = "ADMS", model = "MAD") + expect_error(.lnk_wsg_model(pm, "ADMS"), "must be \"cw\" or \"mad\"") +}) + +test_that(".lnk_habitat_method_read keeps a group coded NA as a code", { + csv <- withr::local_tempfile(fileext = ".csv") + writeLines("watershed_group_code,model\nNA,mad", csv) + pm <- .lnk_habitat_method_read(csv) + expect_identical(pm$watershed_group_code, "NA") + skip_if_not_installed("fresh") + expect_identical(.lnk_wsg_model(pm, "NA"), "mad") +}) + # -- default_tuned: the first shipped thin bundle (#282) ---------------------- test_that("default_tuned extends default and overrides only its thresholds", { diff --git a/tests/testthat/test-lnk_habitat_validate.R b/tests/testthat/test-lnk_habitat_validate.R index d51ff259..69dbc2ae 100644 --- a/tests/testthat/test-lnk_habitat_validate.R +++ b/tests/testthat/test-lnk_habitat_validate.R @@ -21,7 +21,7 @@ test_that(".lnk_habitat_miss_reason attributes each miss once, in order", { r <- link:::.lnk_habitat_miss_reason( captured = c(TRUE, NA, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE), accessible = c(TRUE, NA, FALSE, TRUE, TRUE, TRUE, TRUE, TRUE, TRUE, TRUE), - channel_width = c(5, NA, 5, 5, 5, NA, 5, 5, 5, 5), + size = c(5, NA, 5, 5, 5, NA, 5, 5, 5, 5), p = c(TRUE, NA, TRUE, TRUE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE), p_g = c(TRUE, NA, TRUE, TRUE, TRUE, FALSE, FALSE, TRUE, FALSE, FALSE), p_w = c(TRUE, NA, TRUE, TRUE, FALSE, TRUE, TRUE, TRUE, FALSE, FALSE), @@ -35,21 +35,44 @@ test_that(".lnk_habitat_miss_reason attributes each miss once, in order", { test_that(".lnk_habitat_miss_reason separates a gradient below the window from one above it", { r <- link:::.lnk_habitat_miss_reason( captured = c(FALSE, FALSE), accessible = c(TRUE, TRUE), - channel_width = c(5, 5), p = c(FALSE, FALSE), p_g = c(TRUE, TRUE), + size = c(5, 5), p = c(FALSE, FALSE), p_g = c(TRUE, TRUE), p_w = c(FALSE, FALSE), p_gw = c(TRUE, TRUE), gradient = c(-0.02, 0.2), gradient_min = c(0, 0)) expect_identical(r, c("gradient_below_min", "fails_gradient")) }) +test_that(".lnk_habitat_miss_reason names a missing MAD range before rule_excludes (#299)", { + r <- link:::.lnk_habitat_miss_reason( + captured = c(FALSE, FALSE, FALSE, FALSE), + accessible = c(TRUE, TRUE, TRUE, TRUE), + size = c(1, 1, 1, NA), p = c(FALSE, FALSE, FALSE, FALSE), + p_g = c(FALSE, FALSE, FALSE, FALSE), p_w = c(FALSE, FALSE, FALSE, FALSE), + p_gw = c(FALSE, FALSE, FALSE, FALSE), p_nomad = c(TRUE, FALSE, NA, TRUE), + p_nomad_g = c(TRUE, TRUE, NA, TRUE)) + # Passing with a range alone is the missing range; needing the gradient + # relaxed as well is cw's "both" reason; with no discharge to test, a + # range would not help and the value is what is missing. + expect_identical(r, c("no_mad_threshold", "fails_gradient_and_width", + "rule_excludes", "width_null")) +}) + test_that(".lnk_hv_relax fixes gradient and width without touching lookalike columns", { p <- "s.gradient BETWEEN 0 AND 0.05 AND s.channel_width >= 2 AND s.gradient_x > 1" - r <- link:::.lnk_hv_relax(p, gradient = 0, width = 2) + r <- link:::.lnk_hv_relax(p, gradient = 0, size = 2) expect_false(grepl("s\\.gradient ", r)) expect_false(grepl("s\\.channel_width", r)) expect_true(grepl("s.gradient_x", r, fixed = TRUE)) expect_identical(link:::.lnk_hv_relax(p), p) }) +test_that(".lnk_hv_relax rewrites the size column it is given", { + p <- "s.mad_m3s BETWEEN 0.1 AND 40 AND s.channel_width >= 2 AND s.mad_m3s_x > 1" + r <- link:::.lnk_hv_relax(p, size = 0.1, size_col = "mad_m3s") + expect_false(grepl("s.mad_m3s ", r, fixed = TRUE)) + expect_true(grepl("s.channel_width", r, fixed = TRUE)) + expect_true(grepl("s.mad_m3s_x", r, fixed = TRUE)) +}) + test_that(".lnk_hv_stage_min takes the spawn gradient floor from parameters_fresh", { spp <- list(spawn_gradient_min = 0.0025, params_sp = list(ranges = list( @@ -57,28 +80,157 @@ test_that(".lnk_hv_stage_min takes the spawn gradient floor from parameters_fres rear = list(gradient = c(0, 0.1), channel_width = c(1.5, 9999))))) m <- link:::.lnk_hv_stage_min(spp) expect_identical(m$spawn[["gradient"]], 0.0025) - expect_identical(m$spawn[["width"]], 4) + expect_identical(m$spawn[["size"]], 4) expect_identical(m$rear[["gradient"]], 0) - expect_identical(m$rear[["width"]], 1.5) -}) - -test_that("the predicate call stays on the channel-width model", { - # The validator relaxes s.channel_width, so it is cw-only. fresh >= 0.35.0 - # accepts `model =`; passing it here needs the relaxation reworked first - # (#286 follow-up), and this pins that it has not happened by accident. - calls <- list() - walk <- function(e) { - if (is.call(e)) { - if (identical(e[[1]], quote(fresh::frs_habitat_predicates))) { - calls[[length(calls) + 1L]] <<- e - } - for (a in as.list(e)[-1]) if (!missing(a)) walk(a) - } + expect_identical(m$rear[["size"]], 1.5) +}) + +test_that(".lnk_hv_stage_min takes MAD minimums on mad, 0 where a species has none (#299)", { + spp <- list(spawn_gradient_min = 0, + params_sp = list(ranges = list( + spawn = list(channel_width = c(4, 9999), mad_m3s = c(0.164, 9999)), + rear = list(channel_width = c(1.5, 9999))))) + m <- link:::.lnk_hv_stage_min(spp, "mad") + expect_identical(m$spawn[["size"]], 0.164) + expect_identical(m$rear[["size"]], 0) +}) + +# One species' predicate inputs from the default bundle, as the validator +# assembles them. +default_spp <- function(sp) { + cfg <- lnk_config("default") + params <- fresh::frs_params(csv = link:::.lnk_habitat_thresholds_csv(cfg), + rules_yaml = cfg$rules) + link:::.lnk_hv_sp_params( + params, utils::read.csv(cfg$files$parameters_fresh$path), sp) +} + +test_that("cw predicate expressions are the ones the validator built before #299", { + # The pre-#299 construction, inline: cw output must not move. + for (sp in c("BT", "CO")) { + spp <- default_spp(sp) + pr <- fresh::frs_habitat_predicates(spp) + mins <- link:::.lnk_hv_stage_min(spp) + stage_pred <- list( + spawn = pr$spawn, + rear = sprintf("(%s) OR (%s) OR (%s)", pr$rear, pr$lake_rear, + pr$wetland_rear)) + old <- unlist(lapply(c("spawn", "rear"), function(st) { + g <- mins[[st]][["gradient"]] + w <- mins[[st]][["size"]] + p <- stage_pred[[st]] + v <- c(p, link:::.lnk_hv_relax(p, gradient = g), + link:::.lnk_hv_relax(p, size = w), + link:::.lnk_hv_relax(p, gradient = g, size = w)) + c(sprintf("coalesce((%s), false) AS pred_%s%s", v, st, + c("", "_g", "_w", "_gw")), + sprintf("NULL::boolean AS pred_%s%s", st, c("_nomad", "_nomad_g"))) + })) + expect_identical(link:::.lnk_hv_stage_exprs(spp, "cw"), old, info = sp) } - walk(body(link:::.lnk_hv_predicates)) - expect_length(calls, 1L) - expect_length(as.list(calls[[1]])[-1], 1L) - expect_null(names(as.list(calls[[1]])[-1])) +}) + +test_that("mad predicate expressions test and relax discharge, not width (#299)", { + e <- link:::.lnk_hv_stage_exprs(default_spp("CO"), "mad") + pick <- function(k) e[grepl(paste0(" AS ", k, "$"), e)] + expect_true(grepl("s.mad_m3s", pick("pred_spawn"), fixed = TRUE)) + expect_false(any(grepl("s.channel_width", e, fixed = TRUE))) + # Relaxing the size leaves no discharge test behind. + expect_false(grepl("s.mad_m3s", pick("pred_spawn_w"), fixed = TRUE)) + expect_false(grepl("s.mad_m3s", pick("pred_rear_gw"), fixed = TRUE)) + expect_true(grepl("s.mad_m3s", pick("pred_rear_g"), fixed = TRUE)) + # CO has MAD ranges for both stages: no missing-threshold test. + expect_identical(pick("pred_spawn_nomad"), "NULL::boolean AS pred_spawn_nomad") + expect_identical(pick("pred_rear_nomad_g"), "NULL::boolean AS pred_rear_nomad_g") +}) + +test_that(".lnk_hv_mad_missing follows fresh's branches, case by case (#299)", { + rule <- list(list(edge_types_explicit = 1000L)) + ps <- function(rules, ranges) list(rules = rules, ranges = ranges) + g <- list(gradient = c(0, 0.1)) + cw <- list(channel_width = c(1, 9999)) + mad <- list(mad_m3s = c(0.1, 40)) + f <- link:::.lnk_hv_mad_missing + # A MAD range: never missing. + expect_false(f(ps(list(rear = rule), list(rear = c(g, mad))), "rear")) + expect_false(f(ps(NULL, list(rear = c(g, mad))), "rear")) + # Rules path: missing, with or without a channel-width range. + expect_true(f(ps(list(rear = rule), list(rear = c(g, cw))), "rear")) + expect_true(f(ps(list(rear = rule), list(rear = g)), "rear")) + expect_true(f(ps(list(spawn = rule), list()), "spawn")) + # CSV path: spawning always tests size; rearing only with ranges. + expect_true(f(ps(NULL, list()), "spawn")) + expect_true(f(ps(NULL, list(rear = cw)), "rear")) + expect_false(f(ps(NULL, list()), "rear")) + # And against fresh: missing exactly where supplying a range changes the + # mad predicate AND the stage has habitat on cw. A stage FALSE on both + # models (CSV path, no rear ranges) has no threshold to miss; filling a + # range there would invent a test fresh never writes. + sp <- function(params_sp) list(species_code = "XX", spawn_gradient_min = 0, + spawn_gradient_max = 0.05, + params_sp = params_sp) + cases <- list(ps(list(rear = rule, spawn = rule), list(rear = g, spawn = g)), + ps(NULL, list(rear = cw)), ps(NULL, list())) + for (x in cases) for (st in c("spawn", "rear")) { + open <- x + open$ranges[[st]][["mad_m3s"]] <- c(0, 0) + a <- fresh::frs_habitat_predicates(sp(x), model = "mad")[[st]] + b <- fresh::frs_habitat_predicates(sp(open), model = "mad")[[st]] + cw_pred <- fresh::frs_habitat_predicates(sp(x), model = "cw")[[st]] + expect_identical(f(x, st), !identical(a, b) && !identical(cw_pred, "FALSE"), + info = paste(st, a)) + } +}) + +test_that("mad predicate expressions wrap exactly what classify embeds (#299)", { + for (sp in c("BT", "CO", "SK")) { + spp <- default_spp(sp) + pr <- fresh::frs_habitat_predicates(spp, model = "mad") + e <- link:::.lnk_hv_stage_exprs(spp, "mad") + expect_identical(e[1], sprintf("coalesce((%s), false) AS pred_spawn", pr$spawn), + info = sp) + expect_identical(e[7], sprintf( + "coalesce((%s), false) AS pred_rear", + sprintf("(%s) OR (%s) OR (%s)", pr$rear, pr$lake_rear, pr$wetland_rear)), + info = sp) + } +}) + +test_that("no bundled rules set a rule-level gradient or mad window", { + # Relaxation moves the size to the stage's CSV minimum; a rule-level + # window with a higher floor would turn size misses into rule_excludes. + files <- list.files(system.file("extdata", "configs", package = "link"), + pattern = "^rules\\.yaml$", recursive = TRUE, + full.names = TRUE) + expect_gt(length(files), 0L) + key <- function(k) sprintf("^\\s*(-\\s*)?(%s):", k) + for (f in files) { + y <- readLines(f) + # The pattern can match: rule-level channel_width is the same shape. + expect_true(any(grepl(key("channel_width"), y)), info = f) + expect_false(any(grepl(key("gradient|mad"), y)), info = f) + } +}) + +test_that("a species with no MAD range gets a missing-threshold test on mad only (#299)", { + spp <- default_spp("BT") + e <- link:::.lnk_hv_stage_exprs(spp, "mad") + pick <- function(k) e[grepl(paste0(" AS ", k, "$"), e)] + for (st in c("spawn", "rear")) { + nm <- pick(paste0("pred_", st, "_nomad")) + expect_false(grepl("NULL::boolean", nm, fixed = TRUE), info = st) + expect_false(grepl("s.mad_m3s", nm, fixed = TRUE), info = st) + # The gradient is still tested; only _nomad_g relaxes it. + expect_true(grepl("s.gradient ", nm, fixed = TRUE), info = st) + expect_false(grepl("s.gradient ", + pick(paste0("pred_", st, "_nomad_g")), fixed = TRUE), + info = st) + } + # fresh writes FALSE for the absent size test, so no relaxation reaches it. + expect_true(grepl("AND FALSE", pick("pred_spawn_w"), fixed = TRUE)) + expect_true(all(grepl("NULL::boolean", + link:::.lnk_hv_stage_exprs(spp, "cw")[c(5, 6, 11, 12)], + fixed = TRUE))) }) test_that(".lnk_hv_spec admits observation species only where the model species is present", { @@ -554,4 +706,113 @@ test_that("lnk_habitat_validate refuses a schema logged as another config's", { unique(v$summary$run_logged[v$summary$watershed_group_code == "BBBB"]), FALSE) }) +# -- mad groups (#299) -------------------------------------------------------- + +# The fixture with AAAA's segments tied to real FWA lines, so the discharge +# join has rows to find: seg 3 low discharge (inside CO rearing's MAD window, +# below spawning's), seg 1 no discharge (but a channel width), the rest +# high. Adds CO, with no modelled habitat, and two CO locations on AAAA +# (seg 3, seg 1). +local_mad_fixture <- function(conn, env = parent.frame()) { + s <- local_validate_fixture(conn, env) + if (!DBI::dbExistsTable(conn, DBI::Id(schema = "whse_basemapping", + table = "fwa_stream_networks_discharge"))) { + testthat::skip("no fwa_stream_networks_discharge in this database") + } + lf <- DBI::dbGetQuery(conn, + "SELECT + (SELECT linear_feature_id FROM whse_basemapping.fwa_stream_networks_discharge + WHERE mad_m3s BETWEEN 0.05 AND 0.1 ORDER BY linear_feature_id LIMIT 1) AS low, + (SELECT linear_feature_id FROM whse_basemapping.fwa_stream_networks_discharge + WHERE mad_m3s IS NULL ORDER BY linear_feature_id LIMIT 1) AS none, + (SELECT linear_feature_id FROM whse_basemapping.fwa_stream_networks_discharge + WHERE mad_m3s > 2 ORDER BY linear_feature_id LIMIT 1) AS high") + stmts <- strsplit(sprintf( + "ALTER TABLE %1$s.streams ADD COLUMN linear_feature_id bigint; + UPDATE %1$s.streams SET linear_feature_id = CASE + WHEN watershed_group_code = 'AAAA' AND id_segment = 3 THEN %2$s + WHEN watershed_group_code = 'AAAA' AND id_segment = 1 THEN %3$s + ELSE %4$s END; + ALTER TABLE %1$s.streams_access ADD COLUMN access_co integer DEFAULT 1; + CREATE TABLE %1$s.streams_habitat_co AS + SELECT id_segment, watershed_group_code, true AS accessible, + false AS spawning, false AS rearing, false AS lake_rearing, + false AS wetland_rearing + FROM %1$s.streams_habitat_bt; + INSERT INTO %1$s.observations + (observation_key, species_code, watershed_group_code, blue_line_key, + downstream_route_measure, match_type, source) + VALUES ('c1', 'CO', 'AAAA', 1, 200, 'A.', 'FISS'), + ('c2', 'CO', 'AAAA', 1, 20, 'A.', 'FISS')", + s, format(lf$low, scientific = FALSE), format(lf$none, scientific = FALSE), + format(lf$high, scientific = FALSE)), ";", fixed = TRUE)[[1]] + for (st in stmts) DBI::dbExecute(conn, st) + s +} + +run_validate_mad <- function(conn, s, method = "watershed_group_code,model\nAAAA,mad") { + csv <- withr::local_tempfile(fileext = ".csv", .local_envir = parent.frame()) + writeLines(method, csv) + cfg <- lnk_config("default") + cfg$files$parameters_habitat_method <- list(path = csv) + loaded <- validate_loaded() + loaded$wsg_species_presence$co <- c("t", "") + lnk_habitat_validate(conn, aoi = c("AAAA", "BBBB"), cfg = cfg, + loaded = loaded, species = c("BT", "CO"), schema = s, + observations = paste0(s, ".observations")) +} + +test_that("a mad group is scored on discharge (#299)", { + conn <- validate_conn() + s <- local_mad_fixture(conn) + v <- run_validate_mad(conn, s) + obs <- v$observations + o <- function(k) obs[obs$observation_key == k, ] + + expect_true(all(obs$model[obs$watershed_group_code == "AAAA"] == "mad")) + sm <- v$summary + expect_true(all(sm$model[sm$watershed_group_code == "AAAA"] == "mad")) + expect_true(all(sm$model[sm$watershed_group_code == "BBBB"] == "cw")) + + # c1: discharge below CO spawning's minimum, inside rearing's window. + expect_true(o("c1")$mad_m3s >= 0.05 && o("c1")$mad_m3s <= 0.1) + expect_identical(o("c1")$miss_reason_spawn, "fails_width") + expect_identical(o("c1")$miss_reason_rear, "post_predicate") + # c2: a channel width but no discharge; width_null keys on discharge. + expect_true(is.na(o("c2")$mad_m3s)) + expect_false(is.na(o("c2")$channel_width)) + expect_identical(o("c2")$miss_reason_spawn, "width_null") + expect_identical(o("c2")$miss_reason_rear, "width_null") + + # BT has no MAD thresholds. o10 fails on its NULL width under cw; under + # mad nothing but the missing range keeps it out. o9's 20 % gradient + # still fails, so it needs the gradient relaxed as well. + expect_identical(o("o10")$miss_reason_rear, "no_mad_threshold") + expect_identical(o("o9")$miss_reason_spawn, "fails_gradient_and_width") + expect_identical(o("o9")$miss_reason_rear, "fails_gradient_and_width") + # o2's segment is persisted spawning-free; under cw it passed the spawn + # predicate (post_predicate), under mad it cannot. Its line has no + # discharge, so a MAD range would not admit it either: the value is + # missing, not the threshold. + expect_false(o("o2")$pred_spawn) + expect_true(is.na(o("o2")$mad_m3s)) + expect_identical(o("o2")$miss_reason_spawn, "width_null") + expect_identical(o("o1")$miss_reason_rear, "not_accessible") +}) + +test_that("the same fixture on cw keeps the cw reasons and reports no discharge (#299)", { + conn <- validate_conn() + s <- local_mad_fixture(conn) + v <- run_validate_mad(conn, s, method = "watershed_group_code,model\nAAAA,cw") + obs <- v$observations + o <- function(k) obs[obs$observation_key == k, ] + expect_true(all(obs$model == "cw")) + expect_true(all(is.na(obs$mad_m3s))) + expect_true(all(is.na(obs$pred_spawn_nomad))) + expect_identical(o("o9")$miss_reason_spawn, "fails_gradient") + expect_identical(o("o10")$miss_reason_rear, "width_null") + expect_identical(o("o2")$miss_reason_spawn, "post_predicate") + expect_false(any(c(obs$miss_reason_spawn, obs$miss_reason_rear) %in% + "no_mad_threshold")) +}) # nolint end: indentation_linter diff --git a/tests/testthat/test-lnk_preflight_fresh.R b/tests/testthat/test-lnk_preflight_fresh.R index c438cecb..8ef415e7 100644 --- a/tests/testthat/test-lnk_preflight_fresh.R +++ b/tests/testthat/test-lnk_preflight_fresh.R @@ -54,13 +54,14 @@ test_that("a pre-0.35.0 frs_habitat_classify signature is caught (#286)", { verbose) { NULL } + ns$frs_habitat_predicates <- function(sp_params) NULL expect_identical( .lnk_fresh_missing_formals(ns, .lnk_fresh_required_formals()), - "frs_habitat_classify(params_method)") + c("frs_habitat_classify(params_method)", "frs_habitat_predicates(model)")) # And a function that is absent altogether reports its arguments too. expect_identical( .lnk_fresh_missing_formals(new.env(), .lnk_fresh_required_formals()), - "frs_habitat_classify(params_method)") + c("frs_habitat_classify(params_method)", "frs_habitat_predicates(model)")) }) test_that("required formals name functions in the required export set", { @@ -99,8 +100,11 @@ test_that("the required set names only symbols fresh actually exports", { expect_length( setdiff(.lnk_fresh_required(), getNamespaceExports(asNamespace("fresh"))), 0L) - expect_true(exists(.lnk_fresh_required_internal(), - envir = asNamespace("fresh"), inherits = FALSE)) + # exists() reads only the first name, so test each one. + for (nm in .lnk_fresh_required_internal()) { + expect_true(exists(nm, envir = asNamespace("fresh"), inherits = FALSE), + info = nm) + } }) test_that("every fresh:: call site in link is declared as required", {