Require min_val_rows on the training split too

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"))

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).

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,

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)
 }

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

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)