diff --git a/.gitignore b/.gitignore index ba2ec833b..9e1aaeed3 100644 --- a/.gitignore +++ b/.gitignore @@ -15,4 +15,4 @@ inst/shiny/log/* .DS_Store # {shinytest2}: Ignore new debug snapshots for `$expect_values()` *_.new.png -desktop.ini \ No newline at end of file +desktop.ini diff --git a/DESCRIPTION b/DESCRIPTION index eb8388c36..a457258b1 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: aNCA Title: (Pre-)Clinical NCA in a Dynamic Shiny App -Version: 0.1.0.9184 +Version: 0.1.0.9187 Authors@R: c( person("Ercan", "Suekuer", email = "ercan.suekuer@roche.com", role = "aut", comment = c(ORCID = "0009-0001-1626-1526")), diff --git a/NEWS.md b/NEWS.md index 49d3d4af1..bf1f273ed 100644 --- a/NEWS.md +++ b/NEWS.md @@ -25,6 +25,9 @@ * Partial interval parameters section supports calculations beyond `AUCINT`: `RCAMINT`, `AUCINTD`, `CAVGINT`, and others. Table starts empty by default with a Remove Row button (#524, #1249) * "Min. Points for Half-life" setting added (range 2–10, default 3) (#1155) * BLQ imputation rules via `NCA Setup > Data Imputation` (#139) + +### Bug Fixes +* Half-life (LAMZHL) and lambda.z (LAMZ) now correctly retain BLQ imputation when half-life-dependent parameters (AUCIFO, AUCIFP) are requested. Previously `rm_impute_obs_params()` removed imputation from `half.life` because its dependency check was limited to one level — missing the transitive chain `half.life -> lambda.z -> aucinf.obs`. The dependency resolution now recursively traverses upstream dependencies to include all parameters in the AUC calculation chain, and also walks downstream so AUC consumers (e.g., `vss.obs`, `vz.obs`) keep the same imputed data (#1057). * General Exclusions section for in-app NCA exclusions, with "Excl. TLG" checkbox per entry (#851, #1018) * Parameter Exclusions tab: exclude individual PK parameter rows from descriptive statistics and ADPP export via PPSUMFL/PPSUMRSN flags (#1040) * NCA flag rules (NCAwXRS) from ADNCA standards — flagged records are excluded from NCA (#752) diff --git a/R/intervals.R b/R/intervals.R index 0db736fa0..20422f1ac 100644 --- a/R/intervals.R +++ b/R/intervals.R @@ -295,6 +295,12 @@ update_main_intervals <- function( # and apply it only for non-observational parameters if (!is.null(blq_imputation_rule)) { + # Ensure impute column exists so dplyr mutate below references the + # column rather than the function parameter (which could be FALSE + # from YAML settings, causing "PKNCA_impute_method_FALSE" error). + if (!"impute" %in% names(data$intervals)) { + data$intervals$impute <- NA_character_ + } data$intervals <- data$intervals %>% mutate( impute = ifelse( @@ -324,16 +330,26 @@ update_main_intervals <- function( #' @import dplyr #' rm_impute_obs_params <- function(data, metadata_nca_parameters = metadata_nca_parameters) { - # Don't impute parameters that are not AUC dependent + # Don't impute parameters that are not AUC dependent. + # Parameters in the AUC calculation chain (half.life feeds lambda.z, + # which feeds aucinf.obs) must all use the same BLQ-imputed data (#1057). params_auc_dep <- metadata_nca_parameters %>% filter(grepl("auc|aumc", PKNCA) | grepl("auc", Depends)) %>% pull(PKNCA) + # Build consumer map ("who consumes X") and walk upstream from AUC params + # to find all parameters in their dependency chain (#1057). + consumer_map <- .build_consumer_map(metadata_nca_parameters) + needs_impute <- .find_upstream_deps(params_auc_dep, consumer_map) + + # Walk downstream too: AUC consumers (e.g., vss.obs = f(cl.obs, mrt.obs), + # vz.obs = f(cl.obs, lambda.z)) compute AUC-derived quantities internally, + # so they must use the same imputed data for consistency (#1057). + dependency_map <- .build_dependency_map(metadata_nca_parameters) + needs_impute <- .find_downstream_consumers(needs_impute, dependency_map) + params_not_to_impute <- metadata_nca_parameters %>% - filter( - !grepl("auc|aumc", PKNCA), - !grepl(paste0(params_auc_dep, collapse = "|"), Depends) - ) %>% + filter(!PKNCA %in% needs_impute) %>% pull(PKNCA) %>% intersect(names(PKNCA::get.interval.cols())) @@ -367,3 +383,95 @@ rm_impute_obs_params <- function(data, metadata_nca_parameters = metadata_nca_pa data } + +#' Build a consumer map from the Depends column. +#' For each parameter A, returns which parameters consume A (list A as a dependency). +#' e.g., lambda.z lists half.life in Depends, so consumer_map$half.life = "lambda.z". +#' @noRd +.build_consumer_map <- function(metadata) { + rev <- list() + for (i in seq_len(nrow(metadata))) { + pkg_name <- metadata$PKNCA[i] + dep_str <- metadata$Depends[i] + if (is.na(dep_str) || dep_str == "") next + dep_list <- trimws(strsplit(dep_str, ",")[[1]]) + for (d in dep_list) { + rev[[d]] <- unique(c(rev[[d]], pkg_name)) + } + } + rev +} + +#' Build a dependency map from the Depends column. +#' Mirror image of `.build_consumer_map()`: for each parameter A, returns the +#' parameters A depends on. +#' e.g., vss.obs lists "cl.obs, mrt.obs" in Depends, so +#' dependency_map$vss.obs = c("cl.obs", "mrt.obs"). +#' @noRd +.build_dependency_map <- function(metadata) { + fwd <- list() + for (i in seq_len(nrow(metadata))) { + pkg_name <- metadata$PKNCA[i] + dep_str <- metadata$Depends[i] + if (is.na(dep_str) || dep_str == "") next + fwd[[pkg_name]] <- trimws(strsplit(dep_str, ",")[[1]]) + } + fwd +} + +#' Check if a parameter is a new upstream consumer of the current chain. +#' @noRd +.is_new_upstream_consumer <- function(pkg, consumer_map, needs, obs_params) { + !pkg %in% needs && !pkg %in% obs_params && any(consumer_map[[pkg]] %in% needs) +} + +#' Find all upstream dependencies transitively from `start_set`. +#' Walks the consumer map to collect params that feed into the current chain, +#' stopping at purely observational leaf params (cmax, tmax, tlast). +#' +#' @param start_set Character vector of starting PKNCA parameter names. +#' @param consumer_map Named list from `.build_consumer_map()`. +#' @param obs_params Character vector of observational params to exclude. +#' Must be kept in sync with metadata_nca_parameters when new leaf params +#' are added. Default: cmax, tmax, tlast. +#' @param max_iter Maximum iterations to guard against infinite loops from +#' circular dependencies. 50 is generous (~40 params in real metadata). +#' @noRd +.find_upstream_deps <- function(start_set, consumer_map, + obs_params = c("cmax", "tmax", "tlast"), + max_iter = 50L) { + needs <- start_set + for (iter in seq_len(max_iter)) { + newly_found <- character() + for (pkg in names(consumer_map)) { + if (.is_new_upstream_consumer(pkg, consumer_map, needs, obs_params)) { + newly_found <- c(newly_found, pkg) + } + } + if (length(newly_found) == 0) break + needs <- c(needs, newly_found) + } + needs +} + +#' Find all downstream consumers transitively from `start_set`. +#' Params whose Depends list intersects the current chain consume AUC-derived +#' quantities (e.g., vss.obs consumes cl.obs and mrt.obs) and therefore compute +#' AUC internally — they must use the same BLQ-imputed data (#1057). +#' +#' The fixpoint walk is direction-agnostic: `.find_upstream_deps()` follows +#' "who consumes X" edges, so passing the reverse (dependency) map from +#' `.build_dependency_map()` walks the graph downstream. Purely observational +#' leaf params (cmax, tmax, tlast) are excluded as in the upstream walk. +#' +#' @param start_set Character vector of starting PKNCA parameter names. +#' @param dependency_map Named list from `.build_dependency_map()`. +#' @param obs_params Character vector of observational params to exclude. +#' @param max_iter Maximum iterations to guard against infinite loops from +#' circular dependencies. +#' @noRd +.find_downstream_consumers <- function(start_set, dependency_map, + obs_params = c("cmax", "tmax", "tlast"), + max_iter = 50L) { + .find_upstream_deps(start_set, dependency_map, obs_params, max_iter) +} diff --git a/inst/WORDLIST b/inst/WORDLIST index 4d990a028..deb0c4880 100644 --- a/inst/WORDLIST +++ b/inst/WORDLIST @@ -7,7 +7,9 @@ AEFRLT AFRLT ARRLT ATPTREF +AUCIFO AUCIFOD +AUCIFP AUCINT AUCLSTD AUCPEO @@ -58,6 +60,7 @@ INTRAVASCULAR Kezia Kobana LAMZ +LAMZHL LAMZLL LAMZNPT LAMZSPN diff --git a/tests/testthat/data/test-blq-ADNCA.csv b/tests/testthat/data/test-blq-ADNCA.csv new file mode 100644 index 000000000..f49390beb --- /dev/null +++ b/tests/testthat/data/test-blq-ADNCA.csv @@ -0,0 +1,13 @@ +"STUDYID","USUBJID","PCSPEC","PARAM","DOSETRT","DOSEA","DOSEU","ROUTE","ADOSEDUR","AVAL","AVALU","AFRLT","NFRLT","ARRLT","NRRLT","RRLTU","ATPTREF","TRT01A","METABFL" +"S1","BLQ-TEST-01","SERUM","DrugA","DrugA",10,"mg","INTRAVENOUS BOLUS",0,100,"ng/mL",0.5,0.5,0.5,0.5,"hr","DOSE 1","DrugA 10 mg","N" +"S1","BLQ-TEST-01","SERUM","DrugA","DrugA",10,"mg","INTRAVENOUS BOLUS",0,80,"ng/mL",1,1,1,1,"hr","DOSE 1","DrugA 10 mg","N" +"S1","BLQ-TEST-01","SERUM","DrugA","DrugA",10,"mg","INTRAVENOUS BOLUS",0,65,"ng/mL",1.5,1.5,1.5,1.5,"hr","DOSE 1","DrugA 10 mg","N" +"S1","BLQ-TEST-01","SERUM","DrugA","DrugA",10,"mg","INTRAVENOUS BOLUS",0,52,"ng/mL",2,2,2,2,"hr","DOSE 1","DrugA 10 mg","N" +"S1","BLQ-TEST-01","SERUM","DrugA","DrugA",10,"mg","INTRAVENOUS BOLUS",0,35,"ng/mL",3,3,3,3,"hr","DOSE 1","DrugA 10 mg","N" +"S1","BLQ-TEST-01","SERUM","DrugA","DrugA",10,"mg","INTRAVENOUS BOLUS",0,25,"ng/mL",4,4,4,4,"hr","DOSE 1","DrugA 10 mg","N" +"S1","BLQ-TEST-01","SERUM","DrugA","DrugA",10,"mg","INTRAVENOUS BOLUS",0,12,"ng/mL",6,6,6,6,"hr","DOSE 1","DrugA 10 mg","N" +"S1","BLQ-TEST-01","SERUM","DrugA","DrugA",10,"mg","INTRAVENOUS BOLUS",0,6,"ng/mL",8,8,8,8,"hr","DOSE 1","DrugA 10 mg","N" +"S1","BLQ-TEST-01","SERUM","DrugA","DrugA",10,"mg","INTRAVENOUS BOLUS",0,2.5,"ng/mL",12,12,12,12,"hr","DOSE 1","DrugA 10 mg","N" +"S1","BLQ-TEST-01","SERUM","DrugA","DrugA",10,"mg","INTRAVENOUS BOLUS",0,0,"ng/mL",24,24,24,24,"hr","DOSE 1","DrugA 10 mg","N" +"S1","BLQ-TEST-01","SERUM","DrugA","DrugA",10,"mg","INTRAVENOUS BOLUS",0,0,"ng/mL",36,36,36,36,"hr","DOSE 1","DrugA 10 mg","N" +"S1","BLQ-TEST-01","SERUM","DrugA","DrugA",10,"mg","INTRAVENOUS BOLUS",0,0,"ng/mL",48,48,48,48,"hr","DOSE 1","DrugA 10 mg","N" diff --git a/tests/testthat/test-intervals.R b/tests/testthat/test-intervals.R index 82a73b7e0..0d44c63f9 100644 --- a/tests/testthat/test-intervals.R +++ b/tests/testthat/test-intervals.R @@ -449,3 +449,254 @@ describe("rm_impute_obs_params", { expect_equal(result$intervals, intervals_before) }) }) + +describe(".build_consumer_map", { + it("builds consumer map from Depends column", { + metadata <- data.frame( + PKNCA = c("half.life", "lambda.z", "aucinf.obs", "cmax"), + Depends = c("tmax, tlast", "half.life", "lambda.z", NA), + stringsAsFactors = FALSE + ) + result <- .build_consumer_map(metadata) + expect_type(result, "list") + expect_true("lambda.z" %in% result[["half.life"]]) + expect_true("aucinf.obs" %in% result[["lambda.z"]]) + expect_null(result[["cmax"]]) + }) + + it("handles empty Depends strings", { + metadata <- data.frame( + PKNCA = c("param1", "param2"), + Depends = c("", NA), + stringsAsFactors = FALSE + ) + result <- .build_consumer_map(metadata) + expect_type(result, "list") + }) +}) + +describe(".find_upstream_deps", { + it("finds half.life via transitive chain: half.life -> lambda.z -> aucinf.obs", { + metadata <- data.frame( + PKNCA = c("half.life", "lambda.z", "aucinf.obs", "cmax", "tmax"), + Depends = c("tmax, tlast", "half.life", "lambda.z", NA, NA), + stringsAsFactors = FALSE + ) + consumer_map <- .build_consumer_map(metadata) + start_set <- c("aucinf.obs") + result <- .find_upstream_deps(start_set, consumer_map) + expect_true("aucinf.obs" %in% result) + expect_true("lambda.z" %in% result) + expect_true("half.life" %in% result) + expect_false("cmax" %in% result) + expect_false("tmax" %in% result) + }) + + it("excludes observational leaf params (cmax, tmax, tlast) regardless of depth", { + metadata <- data.frame( + PKNCA = c("half.life", "lambda.z", "aucinf.obs"), + Depends = c("tmax", "half.life", "lambda.z"), + stringsAsFactors = FALSE + ) + consumer_map <- .build_consumer_map(metadata) + result <- .find_upstream_deps(c("aucinf.obs"), consumer_map) + expect_true("lambda.z" %in% result) + expect_true("half.life" %in% result) + # tmax is in default obs_params → excluded regardless of depth + expect_false("tmax" %in% result) + }) +}) + +describe(".find_downstream_consumers", { + it("finds AUC consumers like vss.obs/vz.obs via the dependency map (#1057)", { + metadata <- data.frame( + PKNCA = c("aucinf.obs", "cl.obs", "mrt.obs", "vss.obs", "vz.obs", + "cmax", "cmax.dn"), + Depends = c("auclast, clast.obs, lambda.z", "aucinf.obs", + "aucinf.obs, aumcinf.obs", "cl.obs, mrt.obs", + "cl.obs, lambda.z", NA, "cmax"), + stringsAsFactors = FALSE + ) + dependency_map <- .build_dependency_map(metadata) + start_set <- c("aucinf.obs", "cl.obs", "mrt.obs", "lambda.z") + result <- .find_downstream_consumers(start_set, dependency_map) + expect_true("vss.obs" %in% result) + expect_true("vz.obs" %in% result) + # cmax.dn only depends on cmax, which is not in the chain → not added + expect_false("cmax" %in% result) + expect_false("cmax.dn" %in% result) + }) + + it("excludes observational leaf params even if they depend on the chain", { + metadata <- data.frame( + PKNCA = c("auclast", "cmax"), + Depends = c(NA, "auclast"), + stringsAsFactors = FALSE + ) + dependency_map <- .build_dependency_map(metadata) + result <- .find_downstream_consumers(c("auclast"), dependency_map) + expect_false("cmax" %in% result) + }) + + it("walks transitive downstream chains", { + metadata <- data.frame( + PKNCA = c("cl.obs", "vss.obs", "vss.dn"), + Depends = c("aucinf.obs", "cl.obs", "vss.obs"), + stringsAsFactors = FALSE + ) + dependency_map <- .build_dependency_map(metadata) + result <- .find_downstream_consumers(c("cl.obs"), dependency_map) + expect_true("vss.obs" %in% result) + expect_true("vss.dn" %in% result) + }) +}) + +describe("rm_impute_obs_params integration", { + it("retains imputation for half-life chain and removes it for observational params (#1057)", { + data <- FIXTURE_PKNCA_DATA + # Mark all intervals as having imputation + if (!"impute" %in% names(data$intervals)) { + data$intervals$impute <- "blq" + } else { + data$intervals$impute[is.na(data$intervals$impute)] <- "blq" + } + + # Request AUC-chain params and observational params + requested <- c("aucinf.obs", "half.life", "cmax", "tmax", "tlast") + for (col in names(data$intervals)) { + if (col %in% c("start", "end", "impute", "type_interval")) next + data$intervals[[col]] <- col %in% requested + } + + result <- rm_impute_obs_params(data, metadata_nca_parameters) + + # AUC-chain params (half.life, lambda.z, aucinf.obs) should RETAIN imputation + auc_chain_rows <- result$intervals[ + result$intervals[["aucinf.obs"]] | + result$intervals[["half.life"]] | + result$intervals[["lambda.z"]], + ] + expect_true("blq" %in% auc_chain_rows$impute) + + # Observational params (cmax, tmax, tlast) should LOSE imputation + obs_rows <- result$intervals[ + result$intervals[["cmax"]] | + result$intervals[["tmax"]] | + result$intervals[["tlast"]], + ] + expect_false("blq" %in% obs_rows$impute) + expect_true(all(is.na(obs_rows$impute) | obs_rows$impute == "")) + }) +}) + +describe("BLQ imputation end-to-end on real bug dataset (#1057)", { + # Dataset attached to issue #1057: IV bolus profile with trailing BLQ (0) + # concentrations at 24/36/48 hr. Before the fix, rm_impute_obs_params() + # removed BLQ imputation from half.life (dependency check was one level + # deep), so lambda.z was calculated on non-imputed data while aucinf.obs + # used imputed data, breaking the AUCIFO = AUCLST + CLST/LAMZ identity. + blq_adnca <- read.csv(testthat::test_path("data", "test-blq-ADNCA.csv"), + na.strings = c("", "NA")) + blq_rule <- list(first = "keep", middle = "drop", last = 0) + auc_chain_params <- c("auclast", "aucinf.obs", "aucinf.pred", + "half.life", "lambda.z", "clast.obs", "clast.pred", + "cmax", "tmax", "tlast") + + pknca_data <- PKNCA_create_data_object(blq_adnca) + study_types <- unique(.derive_study_types(pknca_data)$type) + parameter_selections <- setNames( + lapply(study_types, function(x) auc_chain_params), + study_types + ) + + updated_data <- PKNCA_update_data_object( + adnca_data = pknca_data, + method = "lin up/log down", + selected_analytes = "DrugA", + selected_profile = "DOSE 1", + selected_pcspec = "SERUM", + start_impute = FALSE, + parameter_selections = parameter_selections, + blq_imputation_rule = blq_rule + ) + nca_results <- PKNCA_calculate_nca(updated_data, blq_rule = blq_rule) + results_df <- as.data.frame(nca_results$result) + get_pp <- function(pp) results_df$PPORRES[results_df$PPTESTCD == pp] + + it("keeps BLQ imputation on the half-life chain and removes it from observational params", { + auc_chain_rows <- updated_data$intervals[updated_data$intervals$aucinf.obs, ] + expect_true("blq" %in% auc_chain_rows$impute) + + obs_rows <- updated_data$intervals[updated_data$intervals$cmax, ] + expect_true(all(is.na(obs_rows$impute) | obs_rows$impute == "")) + }) + + it("produces internally consistent AUCIFO = AUCLST + CLST/LAMZ (obs and pred)", { + auclst <- get_pp("auclast") + lambda_z <- get_pp("lambda.z") + + expect_false(any(is.na(c(auclst, lambda_z, get_pp("aucinf.obs"))))) + expect_equal(get_pp("aucinf.obs"), auclst + get_pp("clast.obs") / lambda_z, + tolerance = 1e-6) + expect_equal(get_pp("aucinf.pred"), auclst + get_pp("clast.pred") / lambda_z, + tolerance = 1e-6) + }) + + it("matches the parameter values obtained in the app for the issue dataset", { + expect_equal(get_pp("auclast"), 251.4521, tolerance = 1e-4) + expect_equal(get_pp("aucinf.obs"), 259.2045, tolerance = 1e-4) + expect_equal(get_pp("lambda.z"), 0.32248, tolerance = 1e-4) + expect_equal(get_pp("half.life"), 2.1494, tolerance = 1e-4) + }) + + it("keeps imputation for downstream AUC consumers (vss.obs, vz.obs) and stays warning-free", { + # vss.obs/vz.obs have no "auc" in name or Depends, but compute AUC + # internally via cl.obs/mrt.obs/lambda.z. Stripping their imputation + # makes PKNCA warn "AUC range starting before the first measurement" + # and computes them on non-imputed data (#1057 downstream regression). + default_like_params <- c( + "aucinf.obs", "auclast", "cmax", "clast.obs", "tlast", "tmax", + "half.life", "cl.obs", "vss.obs", "vz.obs", "mrt.last", "mrt.obs", + "lambda.z", "r.squared", "span.ratio", "adj.r.squared" + ) + default_selections <- setNames( + lapply(study_types, function(x) default_like_params), + study_types + ) + default_data <- PKNCA_update_data_object( + adnca_data = PKNCA_create_data_object(blq_adnca), + method = "lin up/log down", + selected_analytes = "DrugA", + selected_profile = "DOSE 1", + selected_pcspec = "SERUM", + start_impute = TRUE, + parameter_selections = default_selections, + blq_imputation_rule = blq_rule + ) + + # Downstream consumers stay in the imputed row, cmax stays stripped + vss_rows <- default_data$intervals[default_data$intervals$vss.obs, ] + expect_true(all(grepl("blq", vss_rows$impute))) + cmax_rows <- default_data$intervals[default_data$intervals$cmax, ] + expect_true(all(is.na(cmax_rows$impute) | cmax_rows$impute == "")) + + # No "AUC range starting before the first measurement" warning + run_warnings <- character() + default_results <- withCallingHandlers( + PKNCA_calculate_nca(default_data, blq_rule = blq_rule), + warning = function(w) { + run_warnings <<- c(run_warnings, conditionMessage(w)) + invokeRestart("muffleWarning") + } + ) + expect_false(any(grepl("before the first measurement", run_warnings))) + + # Consumers are consistent with the imputed AUC chain values + default_df <- as.data.frame(default_results$result) + get_dp <- function(pp) default_df$PPORRES[default_df$PPTESTCD == pp] + expect_equal(get_dp("vz.obs"), get_dp("cl.obs") / get_dp("lambda.z"), + tolerance = 1e-6) + expect_equal(get_dp("vss.obs"), get_dp("cl.obs") * get_dp("mrt.obs"), + tolerance = 1e-6) + }) +})