diff --git a/DESCRIPTION b/DESCRIPTION index a2038162..7409c41d 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: neuralsbi Title: Neural Simulation-Based Inference -Version: 0.6.18 +Version: 0.6.19 Authors@R: person("Pedro", "Nascimento de Lima", email = "plima@rand.org", role = c("aut", "cre"), comment = c(ORCID = "0000-0001-9057-198X")) diff --git a/NEWS.md b/NEWS.md index 16ebbb82..c6e2cd36 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,7 @@ +# neuralsbi 0.6.19 + +* **`nre()` no longer silently trains on zero gradient when the training split, not just the validation split, drops to one row.** `check_train_controls()` (`R/train.R`) enforced `min_val_rows` (2 for `nre()`'s atomic contrastive objective, per #188) against `n_val` only, with no matching floor on `n_tr = n - n_val`. A large `validation_fraction` can clear the validation-side floor while leaving `n_tr` below it -- `n_simulations = 4`, `validation_fraction = 0.75` gives `n_val = 3` (passes) and `n_tr = 1` (was never checked). `train_restarts()` then trained on that single row, and `nre_atomic_log_prob()`'s `k < 2L` guard -- the same branch #188 fixed for the validation side -- returned a constant zero loss every step: no gradient, no error, training ran to `patience` epochs and reported a `best_val_loss` as if it had actually trained. `check_train_controls()` now also requires `n - n_val >= min_val_rows`, the same floor already applied to the validation side (#239) (#242). + # neuralsbi 0.6.18 * **`map_estimate()` on a 1-D posterior no longer errors on a `prior_custom()` with one infinite bound.** Its `fit$dim_theta == 1L` branch chose `stats::optim(method = "Brent")` whenever `prior$lower` and `prior$upper` were both non-`NULL`, on the assumption that a `NULL` bound is the only way a prior can be one-sided. `prior_custom(dim = 1, lower = -Inf, upper = 5)` has both fields set -- one is just non-finite -- so it took the Brent branch anyway, and `stats::optim(method = "Brent", lower = -Inf, ...)` errored immediately with `"'lower' and 'upper' must be finite values"` before the search ran at all. The Brent branch now also requires `is.finite(prior$lower) && is.finite(prior$upper)`; a bound that is present but infinite falls through to the existing `L-BFGS-B` branch, which already handles `Inf` on the missing side (#238) (#241). diff --git a/R/nre.R b/R/nre.R index e1e7831b..3bf0ab50 100644 --- a/R/nre.R +++ b/R/nre.R @@ -473,9 +473,12 @@ nre_atomic_log_prob <- function(num_atoms) { #' #' Passes `min_val_rows = 2L` down to [fit_torch_de()], unlike the MDN/MAF/NSF #' callers. The atomic objective ([nre_atomic_log_prob()]) needs a second row -#' to contrast the true parameter against; with a validation split of one row -#' it silently returns a constant zero loss every epoch instead of a real -#' signal, which breaks early stopping (GitHub #188). +#' to contrast the true parameter against; with a split of one row on either +#' side it silently returns a constant zero loss every epoch instead of a real +#' signal -- breaking early stopping when it is the validation side (GitHub +#' #188), and training on zero gradient with no error when it is the training +#' side (GitHub #239). `check_train_controls()` enforces `min_val_rows` on +#' both sides of the split for exactly this reason. #' @keywords internal fit_nre_net <- function(theta, x, classifier = "resnet", hidden = 50L, n_blocks = 2L, num_atoms = 10L, diff --git a/R/train.R b/R/train.R index e7463a05..7ae92b23 100644 --- a/R/train.R +++ b/R/train.R @@ -24,12 +24,13 @@ #' are created there, and the net is moved there right after `build_net()`, #' so the two never disagree the way they do under a bare #' `torch::with_device()`. -#' @param min_val_rows Smallest validation split `check_train_controls()` will -#' accept. Every estimator here can score a real, if noisy, log-density on a -#' single validation row, so the default of `1L` is unchanged for MDN, MAF -#' and NSF. [fit_nre_net()] passes `2L`: its atomic contrastive objective -#' needs a second row to contrast against, and with only one it silently -#' returns a constant zero loss instead of training. +#' @param min_val_rows Smallest training *or* validation split +#' `check_train_controls()` will accept. Every estimator here can score a +#' real, if noisy, log-density on a single row, so the default of `1L` is +#' unchanged for MDN, MAF and NSF. [fit_nre_net()] passes `2L`: its atomic +#' contrastive objective needs a second row to contrast against, and with +#' only one it silently returns a constant zero loss instead of training -- +#' on whichever side of the split falls that low. #' @return `list(net, best_val_loss, history, device)`, where `history` is a #' data frame of per-epoch train/validation losses for the winning restart #' and `device` is the resolved device (`"cpu"`, `"cuda"` or `"mps"`) @@ -71,13 +72,21 @@ train_conditional_de <- function(build_net, log_prob_fn, theta, x, #' is known, `validation_fraction` is checked against it: the requirement is #' that both sides of the split come out non-empty, which the fraction alone #' cannot decide. `min_val_rows` raises that floor for callers whose objective -#' needs more than one validation row to mean anything -- see [fit_nre_net()]. +#' needs more than one row to mean anything -- see [fit_nre_net()] -- and +#' applies to *both* sides of the split. Without a matching floor on the +#' training side, a large `validation_fraction` can leave `n - n_val` rows +#' below what the objective needs while the validation side alone still +#' clears it: the same "constant zero loss, no signal" failure NRE's +#' `min_val_rows` was added for (see the note below), just on the training +#' side, and unlike a genuine simulation-budget error it trains silently to +#' `patience` epochs instead of raising one. #' #' @inheritParams npe #' @param n Number of training rows, or `NULL` when they do not exist yet. -#' @param min_val_rows Smallest validation split this call will accept -#' (default `1L`, i.e. only require it non-empty). Raise it for an objective -#' that needs more than one validation row to produce a real signal. +#' @param min_val_rows Smallest split -- training or validation -- this call +#' will accept (default `1L`, i.e. only require each side non-empty). Raise +#' it for an objective that needs more than one row to produce a real +#' signal. #' @keywords internal check_train_controls <- function(max_epochs, batch_size, lr, validation_fraction, patience, n_restarts, @@ -112,6 +121,18 @@ check_train_controls <- function(max_epochs, batch_size, lr, n_things(as.integer(need), "row")), call. = FALSE) } + n_tr <- n - n_val + if (n_tr < min_val_rows) { + need <- max(2L, ceiling(min_val_rows / (1 - validation_fraction))) + stop(sprintf(paste0("`validation_fraction` of %s leaves only %s of ", + "%d for training, but this estimator needs at ", + "least %s to score its objective on. At this ", + "fraction it needs at least %s."), + format(validation_fraction), n_things(n_tr, "row"), n, + n_things(as.integer(min_val_rows), "row"), + n_things(as.integer(need), "row")), + call. = FALSE) + } } invisible(TRUE) } diff --git a/tests/testthat/test-nre.R b/tests/testthat/test-nre.R index 24000a19..9acee969 100644 --- a/tests/testthat/test-nre.R +++ b/tests/testthat/test-nre.R @@ -295,6 +295,43 @@ test_that("fit_nre_net() rejects a validation split too small for the atomic los "needs at least .* to score its objective on") }) +# GitHub #239: check_train_controls() enforced min_val_rows on the validation +# side only. A large validation_fraction can hold out enough rows to clear +# that floor while leaving fewer than min_val_rows for training -- e.g. +# n_simulations = 4, validation_fraction = 0.75 gives n_val = 3 (clears the +# floor of 2) and n_tr = 1 (does not). Before the fix this slipped through: +# every training step scored nre_atomic_log_prob()'s k < 2L branch, a +# constant zero loss with no gradient, and training ran silently to +# `patience` epochs reporting a best_val_loss as if it had actually trained. +test_that("nre() fails before simulating rather than train on a 1-row training split", { + calls <- 0L + counting_simulator <- function(mu, nu) { + calls <<- calls + 1L + gauss_sim(mu, nu) + } + expect_error( + nre(gauss_prior(), counting_simulator, n_simulations = 4, + validation_fraction = 0.75, classifier = "resnet"), + "needs at least .* to score its objective on") + expect_identical(calls, 0L) + + # The closed-form logistic classifier never splits off a validation set + # (min_val_rows = 1L), so the same split must not trip this floor for it. + expect_no_error( + nre(gauss_prior(), counting_simulator, n_simulations = 4, + validation_fraction = 0.75, classifier = "logistic")) + expect_identical(calls, 4L) +}) + +test_that("fit_nre_net() rejects a training split too small for the atomic loss", { + theta <- matrix(stats::rnorm(4), ncol = 1) + x <- matrix(stats::rnorm(4), ncol = 1) + + expect_error( + fit_nre_net(theta, x, classifier = "resnet", validation_fraction = 0.75), + "needs at least .* to score its objective on") +}) + test_that("nre() defers to prepare_simulations() when n_simulations can't hint a row count", { # An invalid n_simulations (not >= 1) and no pre-computed theta/x means the # early min_val_rows check has nothing to check against yet, so it must not diff --git a/tests/testthat/test-train.R b/tests/testthat/test-train.R index 6f642b51..0e7d8b42 100644 --- a/tests/testthat/test-train.R +++ b/tests/testthat/test-train.R @@ -77,6 +77,28 @@ test_that("min_val_rows raises check_train_controls()'s validation-split floor", "needs at least .* to score its objective on") }) +test_that("min_val_rows also raises check_train_controls()'s training-split floor", { + # GitHub #239: min_val_rows was only ever checked against n_val, so a large + # validation_fraction could hold out enough rows to clear that floor while + # leaving n_tr = n - n_val below it. n = 4, validation_fraction = 0.75 gives + # n_val = 3 (clears min_val_rows = 2) and n_tr = 1 (does not) -- the exact + # scenario from the issue. + expect_no_error( + check_train_controls(2000L, 200L, 5e-4, 0.75, 20L, 1L, 5, n = 4)) + expect_error( + check_train_controls(2000L, 200L, 5e-4, 0.75, 20L, 1L, 5, n = 4, + min_val_rows = 2L), + "leaves only 1 row of 4 for training.*needs at least .* to score its objective on") + + four <- matrix(stats::rnorm(4), ncol = 1) + expect_error( + train_conditional_de(build_net = function() stop("not reached"), + log_prob_fn = function(...) stop("not reached"), + theta = four, x = four, validation_fraction = 0.75, + min_val_rows = 2L), + "needs at least .* to score its objective on") +}) + test_that("minibatches() covers every row and never leaves one on its own", { order <- seq_len(21L)