Merge branch 'develop' into automated-reporting-and-visualization

Author: dlebauer

base/workflow/R/runModule.run.write.configs.R

@@ -14,12 +14,12 @@
 #' @param overwrite logical: Replace config files if they already exist?
 #' @param input_design Optional. The parameter/input design for the runs,
 #'   normally the full result of \code{generate_joint_ensemble_design()}: a list
-#'   with \code{X} (a data.frame whose \code{param} column selects rows of
+#'   with \code{design_matrix} (a data.frame whose \code{param} column selects rows of
 #'   \code{trait.samples}/\code{ensemble.samples}, plus optional columns named
 #'   for \code{settings$run$inputs} tags such as \code{met} or \code{soil}) and
 #'   \code{samples} (the parameter bundle those indices point into). Can be:
 #'   \itemize{
-#'     \item The \code{list(X, samples)} returned by
+#'     \item The \code{list(design_matrix, samples)} returned by
 #'           \code{generate_joint_ensemble_design()}
 #'     \item \code{NULL} to generate the design and samples internally from
 #'           \code{settings}
@@ -161,9 +161,10 @@ runModule.run.write.configs <- function(settings,
 #'   \item If \code{input_design} is already a list with
 #'         \code{ensemble}/\code{sensitivity} keys (e.g. threaded from a
 #'         MultiSettings parent), return as-is.
-#'   \item If \code{input_design} is the \code{list(X, samples)} from
-#'         \code{generate_joint_ensemble_design()}, use \code{X} as the ensemble
-#'         design and \code{samples} as the bundle (no resampling).
+#'   \item If \code{input_design} is the \code{list(design_matrix, samples)}
+#'         from \code{generate_joint_ensemble_design()}, use the design matrix
+#'         as the ensemble design and \code{samples} as the bundle (no
+#'         resampling). \code{X} is accepted as the older name for it.
 #'   \item If \code{input_design} is a bare data.frame (a design without its
 #'         samples), raise an error: the design's \code{param} indices only match
 #'         the samples they were drawn with.
@@ -191,24 +192,32 @@ runModule.run.write.configs <- function(settings,
   # into the samples it was drawn with, so a design must arrive together with
   # those samples; otherwise it would be silently paired with a fresh, mismatched
   # resample. We therefore accept the full generate_joint_ensemble_design()
-  # result (a list with X and samples) and reject a bare design.
+  # result (a list with design_matrix and samples) and reject a bare design.
   if (!is.null(input_design)) {
-    if (is.list(input_design) && !is.data.frame(input_design) &&
-        all(c("X", "samples") %in% names(input_design)) &&
-        !is.null(input_design$samples)) {
-      designs$ensemble <- input_design$X
+    # Generators return the design as `design_matrix`. `X` is the older name for
+    # the same matrix, kept so existing callers keep working, and is what
+    # sensitivity sets on a sobol object.
+    supplied_design <- if (is.list(input_design) && !is.data.frame(input_design)) {
+      input_design[["design_matrix"]] %||% input_design[["X"]]
+    } else {
+      NULL
+    }
+
+    if (!is.null(supplied_design) && !is.null(input_design$samples)) {
+      designs$ensemble <- supplied_design
       supplied_samples <- input_design$samples
     } else if (is.data.frame(input_design)) {
       PEcAn.logger::logger.severe(
         "input_design was supplied without its parameter samples.",
         "Pass the full generate_joint_ensemble_design() result",
-        "(a list with `X` and `samples`) so the design's `param` indices match",
-        "the samples, or leave input_design = NULL to generate both together."
+        "(a list with `design_matrix` and `samples`) so the design's `param`",
+        "indices match the samples, or leave input_design = NULL to generate",
+        "both together."
       )
     } else {
       PEcAn.logger::logger.severe(
-        "Unrecognized input_design format. Expected NULL or the list(X, samples)",
-        "returned by generate_joint_ensemble_design()."
+        "Unrecognized input_design format. Expected NULL or the",
+        "list(design_matrix, samples) returned by generate_joint_ensemble_design()."
       )
     }
   }
@@ -252,6 +261,20 @@ runModule.run.write.configs <- function(settings,
     designs$samples <- samples
   }
 
+  # Deprecation: internal design generation is going away. Passing input_design
+  # explicitly (the generate_joint_ensemble_design() result) will become the
+  # required path. Warn only when we are actually about to auto-generate.
+  auto_generating <-
+    (is.null(designs$ensemble) && need_ensemble) ||
+    (is.null(designs$sensitivity) && need_sa)
+  if (auto_generating) {
+    PEcAn.logger::logger.warn(
+      "Internal input design generation is deprecated and will be removed.",
+      "Pass input_design explicitly as the list(design_matrix, samples) returned",
+      "by generate_joint_ensemble_design(); this will become required."
+    )
+  }
+
   # Generate the ensemble design only when the caller did not supply one,
   # handing over the resolved samples so the generator does not resample.
   if (is.null(designs$ensemble) && need_ensemble) {
@@ -260,17 +283,17 @@ runModule.run.write.configs <- function(settings,
       ensemble_size = ensemble_size,
       samples       = designs$samples
     )
-    designs$ensemble <- design_result$X
+    designs$ensemble <- design_result$design_matrix %||% design_result$X
   }
 
   # Generate the SA design if needed, threading the SA samples so the generator
   # uses them directly instead of re-reading samples.Rdata via the deprecated path.
   if (is.null(designs$sensitivity) && need_sa) {
     design_result <- PEcAn.uncertainty::generate_OAT_SA_design(
       settings,
-      sa_samples = designs$samples$sa.samples
+      samples = designs$samples
     )
-    designs$sensitivity <- design_result$X
+    designs$sensitivity <- design_result$design_matrix %||% design_result$X
   }
 
   return(designs)

base/workflow/man/dot-prepare_input_designs.Rd

@@ -182,9 +182,13 @@ test_that(".prepare_input_designs threads SA samples into the OAT generator", {
   gps_args <- mockery::mock_args(gps)[[1]]
   expect_false(gps_args$do_ensemble)
 
-  # the OAT generator receives the SA samples directly (no disk re-read).
+  # the OAT generator receives the full sample bundle directly (no disk
+  # re-read), and uses sa.samples from it
   oat_args <- mockery::mock_args(oat)[[1]]
-  expect_identical(oat_args$sa_samples, sa_bundle$sa.samples)
+  # generate_OAT_SA_design(settings, samples = designs$samples): settings is
+  # positional (first), the bundle comes through named `samples`
+  # OAT is handed the resolved bundle itself (so it never re-reads samples.Rdata)
+  expect_equal(oat_args$samples, designs$samples)
 
   expect_equal(designs$sensitivity, data.frame(param = 1:4))
   expect_true(file.exists(file.path(tmp, "samples.Rdata")))
@@ -213,4 +217,73 @@ test_that(".prepare_input_designs rejects a design whose samples are NULL", {
   mockery::expect_called(loader, 0)
   mockery::expect_called(gps, 0)
   mockery::expect_called(gen, 0)
+})
+
+test_that(".prepare_input_designs warns when it auto-generates a design", {
+  tmp <- withr::local_tempdir()
+  settings <- make_prep_settings(tmp)
+
+  mockery::stub(.prepare_input_designs,
+                "PEcAn.uncertainty::load_pft_posteriors",
+                function(...) fake_loaded())
+  mockery::stub(.prepare_input_designs,
+                "PEcAn.uncertainty::get_parameter_samples",
+                function(...) fake_bundle())
+  mockery::stub(.prepare_input_designs,
+                "PEcAn.uncertainty::generate_joint_ensemble_design",
+                function(...) list(X = data.frame(param = 1:3)))
+
+  msgs <- capture.output(
+    invisible(.prepare_input_designs(settings, input_design = NULL)),
+    type = "message"
+  )
+  expect_match(paste(msgs, collapse = "\n"), "deprecated", all = FALSE)
+})
+
+test_that(".prepare_input_designs does not warn when a design is supplied", {
+  tmp <- withr::local_tempdir()
+  settings <- make_prep_settings(tmp)
+
+  supplied <- list(X = data.frame(param = 1:3), samples = fake_bundle())
+
+  msgs <- capture.output(
+    invisible(.prepare_input_designs(settings, input_design = supplied)),
+    type = "message"
+  )
+  expect_false(any(grepl("deprecated", msgs)))
+})
+
+test_that(".prepare_input_designs accepts a design supplied as design_matrix", {
+  tmp <- withr::local_tempdir()
+  settings <- make_prep_settings(tmp)
+
+  bundle   <- fake_bundle()
+  supplied <- list(design_matrix = data.frame(param = 1:3), samples = bundle)
+
+  gen <- mockery::mock()
+  mockery::stub(.prepare_input_designs,
+                "PEcAn.uncertainty::generate_joint_ensemble_design", gen)
+
+  designs <- .prepare_input_designs(settings, input_design = supplied)
+
+  mockery::expect_called(gen, 0)
+  expect_identical(designs$ensemble, supplied$design_matrix)
+  expect_identical(designs$samples, bundle)
+})
+
+test_that(".prepare_input_designs still accepts a design supplied as X", {
+  tmp <- withr::local_tempdir()
+  settings <- make_prep_settings(tmp)
+
+  bundle   <- fake_bundle()
+  supplied <- list(X = data.frame(param = 1:3), samples = bundle)
+
+  gen <- mockery::mock()
+  mockery::stub(.prepare_input_designs,
+                "PEcAn.uncertainty::generate_joint_ensemble_design", gen)
+
+  designs <- .prepare_input_designs(settings, input_design = supplied)
+
+  mockery::expect_called(gen, 0)
+  expect_identical(designs$ensemble, supplied$X)
 })