recognize thread and OpenCL support regardless of the capitalization

NEWS.md

@@ -1,16 +1,27 @@
 # cmdstanr (development version)
 
-* The `CMDSTANR_NO_VER_CHECK` R option and environment variable are deprecated as of CmdStanR 1.0.0; use the lowercase `cmdstanr_no_ver_check` forms instead.
-* `$cpp_options()` no longer includes a `STAN_VERSION` entry read from the model executable's metadata. It was never a C++ option; use `$cmdstan_version()` instead. (#1215)
-* Pathfinder fits used as initial values now use uniform weights when CmdStan already PSIS-resampled their draws, avoiding a second application of importance weights. (#1206)
-* Pathfinder fits used as initial values now correctly treat draws with different initialization parameter values as distinct even when their log weights are equal, and collapse duplicate resampled draws while retaining their selection frequency. (#1207)
-* `pathfinder()` now passes separately supplied initial values to every path instead of using only the first path's initial values. (#1206)
+* The `CMDSTANR_NO_VER_CHECK` R option and environment variable are deprecated 
+as of CmdStanR 1.0.0; use the lowercase `cmdstanr_no_ver_check` forms instead.
+* `$cpp_options()` no longer includes a `STAN_VERSION` entry read from the model 
+executable's metadata. It was never a C++ option; use `$cmdstan_version()` instead. (#1215)
+* CmdStanModel methods now use executable metadata regardless of the 
+capitalization of C++ option names. Any executable reporting threading enabled 
+requires the corresponding `threads` or `threads_per_chain` argument. (#765, #1100)
+* Pathfinder fits used as initial values now use uniform weights when CmdStan 
+already PSIS-resampled their draws, avoiding a second application of importance weights. (#1206)
+* Pathfinder fits used as initial values now correctly treat draws with different 
+initialization parameter values as distinct even when their log weights are equal, 
+and collapse duplicate resampled draws while retaining their selection frequency. (#1207)
+* `pathfinder()` now passes separately supplied initial values to every path 
+instead of using only the first path's initial values. (#1206)
 * `pathfinder()` now respects `save_single_paths = TRUE` instead of always
 passing `0` to CmdStan.
 * `pathfinder()` now uses `threads` argument (`num_threads` is deprecated),
 to be consistent with other methods.
-* The `save_latent_dynamics` argument is now limited to `$sample()`, `$sample_mpi()`, and `$variational()`, matching the CmdStan algorithms that support diagnostic CSV output.
-* Informative error when exposing functions using names that are reserved
+* The `save_latent_dynamics` argument is now limited to `$sample()`, 
+`$sample_mpi()`, and `$variational()`, matching the CmdStan algorithms 
+that support diagnostic CSV output.
+* Informative error when exposing functions using names that are reserved 
 keywords (@VisruthSK, #1154)
 * `save_cmdstan_config` and `save_metric` default to `FALSE` but can be
 set to `TRUE` for an entire R session via new global options. (#1159)

R/cpp_opts.R

@@ -130,11 +130,22 @@ validate_cpp_options <- function(cpp_options) {
 }
 
 # check specific options for validity ---------------------------------
+cpp_option_value <- function(cpp_options, option) {
+  # CmdStanR input and executable metadata can use different casing. Prefer
+  # the final match, even when it is NULL, because later executable metadata
+  # best describes the binary.
+  matches <- which(tolower(names(cpp_options)) == tolower(option))
+  if (length(matches) == 0) {
+    return(NULL)
+  }
+  cpp_options[[matches[[length(matches)]]]]
+}
+
 # no type checking for opencl_ids
 # cpp_options must be a list
 # opencl_ids returned unchanged
 assert_valid_opencl <- function(opencl_ids, cpp_options) {
-  if (is.null(cpp_options[["stan_opencl"]])
+  if (is.null(cpp_option_value(cpp_options, "stan_opencl"))
       && !is.null(opencl_ids)) {
     stop("'opencl_ids' is set but the model was not compiled for use with OpenCL.",
          "\nRecompile the model with 'cpp_options = list(stan_opencl = TRUE)'",
@@ -148,7 +159,8 @@ assert_valid_threads <- function(threads, cpp_options, multiple_chains = FALSE)
   threads_arg <- if (multiple_chains) "threads_per_chain" else "threads"
   checkmate::assert_integerish(threads, .var.name = threads_arg,
                                null.ok = TRUE, lower = 1, len = 1)
-  if (is.null(cpp_options[["stan_threads"]]) || !isTRUE(cpp_options[["stan_threads"]])) {
+  stan_threads <- cpp_option_value(cpp_options, "stan_threads")
+  if (is.null(stan_threads) || !isTRUE(stan_threads)) {
     if (!is.null(threads)) {
       warning(
         "'", threads_arg, "' is set but the model was not compiled with ",
@@ -158,10 +170,10 @@ assert_valid_threads <- function(threads, cpp_options, multiple_chains = FALSE)
       )
       threads <- NULL
     }
-  } else if (isTRUE(cpp_options[["stan_threads"]]) && is.null(threads)) {
+  } else if (isTRUE(stan_threads) && is.null(threads)) {
     stop(
-      "The model was compiled with 'cpp_options = list(stan_threads = TRUE)' ",
-      "but '", threads_arg, "' was not set!",
+      "The model executable was built with threading enabled but '",
+      threads_arg, "' was not set!",
       call. = FALSE
     )
   }

R/model.R

@@ -484,10 +484,15 @@ NULL
 #' @param user_header (string) The path to a C++ file (with a .hpp extension)
 #'   to compile with the Stan model.
 #' @param cpp_options (list) Any makefile options to be used when compiling the
-#'   model (`STAN_THREADS`, `STAN_MPI`, `STAN_OPENCL`, etc.). Anything you would
+#'   model (`stan_threads`, `stan_mpi`, `stan_opencl`, etc.). Anything you would
 #'   otherwise write in the `make/local` file. For an example of using threading
-#'   see the Stan case study
-#'   [Reduce Sum: A Minimal Example](https://mc-stan.org/users/documentation/case-studies/reduce_sum_tutorial.html).
+#'   see the Stan case study [Reduce Sum: A Minimal
+#'   Example](https://mc-stan.org/users/documentation/case-studies/reduce_sum_tutorial.html).
+#'   **Note:** For historical reasons, CmdStan treats some options as enabled
+#'   whenever their `Make` variable is non-empty. In particular, setting
+#'   `stan_threads` to `FALSE` passes `STAN_THREADS=FALSE` to `Make`, which
+#'   still enables threading! To leave threading disabled, simply omit
+#'   `stan_threads` entirely or set it to `NULL`.
 #' @param stanc_options (list) Any Stan-to-C++ transpiler options to be used
 #'   when compiling the model. See the **Examples** section below as well as the
 #'   [`stanc` chapter of the CmdStan User's
@@ -603,7 +608,7 @@ compile <- function(quiet = TRUE,
     stanc_options[["warn-pedantic"]] <- TRUE
   }
 
-  if (isTRUE(cpp_options$stan_opencl)) {
+  if (isTRUE(cpp_option_value(cpp_options, "stan_opencl"))) {
     stanc_options[["use-opencl"]] <- TRUE
   }
 
@@ -1329,12 +1334,12 @@ CmdStanModel$set("public", name = "sample", value = sample)
 #'
 #'   An example of compiling with MPI:
 #'   ```
-#'   mpi_options = list(STAN_MPI=TRUE, CXX="mpicxx", TBB_CXX_TYPE="gcc")
+#'   mpi_options = list(stan_mpi = TRUE, CXX = "mpicxx", TBB_CXX_TYPE = "gcc")
 #'   mod = cmdstan_model("model.stan", cpp_options = mpi_options)
 #'   ```
 #'   The C++ options that must be supplied to the
 #'   [compile][model-method-compile] call are:
-#'   - `STAN_MPI`: Enables the use of MPI with Stan if `TRUE`.
+#'   - `stan_mpi`: Enables the use of MPI with Stan if `TRUE`.
 #'   - `CXX`: The name of the MPI C++ compiler wrapper. Typically `"mpicxx"`.
 #'   - `TBB_CXX_TYPE`: The C++ compiler the MPI wrapper wraps. Typically `"gcc"`
 #'   on Linux and `"clang"` on macOS.
@@ -1374,7 +1379,7 @@ CmdStanModel$set("public", name = "sample", value = sample)
 #'
 #' @examples
 #' \dontrun{
-#' # mpi_options <- list(STAN_MPI=TRUE, CXX="mpicxx", TBB_CXX_TYPE="gcc")
+#' # mpi_options <- list(stan_mpi = TRUE, CXX = "mpicxx", TBB_CXX_TYPE = "gcc")
 #' # mod <- cmdstan_model("model.stan", cpp_options = mpi_options)
 #' # fit <- mod$sample_mpi(..., mpi_args = list("n" = 4))
 #' }

tests/testthat/_snaps/cpp_opts.md

@@ -0,0 +1,30 @@
+# lowercase stan_threads behavior remains unchanged
+
+    Code
+      assert_valid_threads(2L, list(stan_threads = FALSE))
+    Condition
+      Warning:
+      'threads' is set but the model was not compiled with 'cpp_options = list(stan_threads = TRUE)' so 'threads' will have no effect!
+    Code
+      assert_valid_threads(2L, list(stan_threads = "dummy string"))
+    Condition
+      Warning:
+      'threads' is set but the model was not compiled with 'cpp_options = list(stan_threads = TRUE)' so 'threads' will have no effect!
+
+# uppercase stan_threads requires a thread count
+
+    Code
+      assert_valid_threads(NULL, list(STAN_THREADS = TRUE))
+    Condition
+      Error:
+      ! The model executable was built with threading enabled but 'threads' was not set!
+
+# cpp option checks do not use partial matching
+
+    Code
+      assert_valid_opencl(c(0L, 0L), list(stan_opencl_x = TRUE))
+    Condition
+      Error:
+      ! 'opencl_ids' is set but the model was not compiled for use with OpenCL.
+      Recompile the model with 'cpp_options = list(stan_opencl = TRUE)'
+

tests/testthat/_snaps/threads.md

@@ -6,3 +6,11 @@
       Warning:
       'num_threads' is deprecated as of CmdStanR 1.0.0 and will be removed in a future release. Please use 'threads' instead.
 
+# executable metadata takes precedence over compile options
+
+    Code
+      mod$sample(data = data_file_json, chains = 1)
+    Condition
+      Error:
+      ! The model executable was built with threading enabled but 'threads_per_chain' was not set!
+

tests/testthat/test-cpp_opts.R

@@ -37,6 +37,92 @@ test_that("validate_cpp_options works", {
   expect_warning(validate_cpp_options(list(STAN_OPENCL = FALSE)))
 })
 
+test_that("cpp option lookup is exact and case-insensitive", {
+  cpp_options <- list(STAN_THREADS = TRUE)
+  expect_identical(cpp_option_value(cpp_options, "stan_threads"), TRUE)
+  expect_named(cpp_options, "STAN_THREADS")
+
+  expect_identical(
+    cpp_option_value(
+      list(stan_threads = FALSE, STAN_THREADS = TRUE),
+      "stan_threads"
+    ),
+    TRUE
+  )
+  expect_identical(
+    cpp_option_value(
+      list(STAN_THREADS = TRUE, stan_threads = FALSE),
+      "stan_threads"
+    ),
+    FALSE
+  )
+  expect_null(
+    cpp_option_value(
+      list(STAN_OPENCL = TRUE, stan_opencl = NULL),
+      "stan_opencl"
+    )
+  )
+  expect_null(cpp_option_value(list(stan_opencl_x = TRUE), "stan_opencl"))
+})
+
+test_that("cpp option checks are case-insensitive", {
+  expect_identical(
+    assert_valid_threads(2L, list(STAN_THREADS = TRUE)),
+    2L
+  )
+  expect_identical(
+    assert_valid_threads(2L, list(stan_threads = TRUE)),
+    2L
+  )
+  expect_identical(
+    assert_valid_opencl(c(0L, 0L), list(STAN_OPENCL = TRUE)),
+    c(0L, 0L)
+  )
+  expect_identical(
+    assert_valid_opencl(c(0L, 0L), list(stan_opencl = TRUE)),
+    c(0L, 0L)
+  )
+})
+
+test_that("lowercase stan_threads behavior remains unchanged", {
+  expect_null(assert_valid_threads(NULL, list(stan_threads = FALSE)))
+  expect_null(assert_valid_threads(NULL, list(stan_threads = "dummy string")))
+  expect_snapshot({
+    assert_valid_threads(2L, list(stan_threads = FALSE))
+    assert_valid_threads(2L, list(stan_threads = "dummy string"))
+  })
+})
+
+test_that("cpp option checks prefer the last case-insensitive match", {
+  expect_identical(
+    assert_valid_threads(
+      2L,
+      list(stan_threads = FALSE, STAN_THREADS = TRUE)
+    ),
+    2L
+  )
+  expect_identical(
+    assert_valid_opencl(
+      c(0L, 0L),
+      list(stan_opencl = NULL, STAN_OPENCL = TRUE)
+    ),
+    c(0L, 0L)
+  )
+})
+
+test_that("uppercase stan_threads requires a thread count", {
+  expect_snapshot(
+    error = TRUE,
+    assert_valid_threads(NULL, list(STAN_THREADS = TRUE))
+  )
+})
+
+test_that("cpp option checks do not use partial matching", {
+  expect_snapshot(
+    error = TRUE,
+    assert_valid_opencl(c(0L, 0L), list(stan_opencl_x = TRUE))
+  )
+})
 
 test_that("exe_info cpp_options comparison works", {
   exe_info_all_flags_off <- exe_info_style_cpp_options(list())

tests/testthat/test-model-compile.R

@@ -919,6 +919,50 @@ test_that("STANCFLAGS from get_cmdstan_flags() are included in compile output",
   expect_output(print(out), out_w_flags)
 })
 
+test_that("compile() detects stan_opencl without case or partial matching", {
+  stan_file <- testing_stan_file("bernoulli")
+  model <- cmdstan_model(stan_file, compile = FALSE)
+  received_stancflags <- list()
+  local_mocked_bindings(
+    get_cmdstan_flags = function(flag_name) character(),
+    get_standalone_hpp = function(stan_file, stancflags) {
+      received_stancflags <<- append(received_stancflags, list(stancflags))
+      ""
+    }
+  )
+
+  model$compile(
+    cpp_options = list(STAN_OPENCL = TRUE),
+    force_recompile = TRUE,
+    dry_run = TRUE
+  )
+  expect_length(received_stancflags, 2)
+  expect_equal(
+    vapply(
+      received_stancflags,
+      function(x) "--use-opencl" %in% x,
+      logical(1)
+    ),
+    rep(TRUE, length(received_stancflags))
+  )
+
+  received_stancflags <- list()
+  model$compile(
+    cpp_options = list(stan_opencl_x = TRUE),
+    force_recompile = TRUE,
+    dry_run = TRUE
+  )
+  expect_length(received_stancflags, 2)
+  expect_equal(
+    vapply(
+      received_stancflags,
+      function(x) "--use-opencl" %in% x,
+      logical(1)
+    ),
+    rep(FALSE, length(received_stancflags))
+  )
+})
+
 test_that("compile() ignores directory chatter from MAKEFLAGS when reading STANCFLAGS", {
   withr::local_envvar(MAKEFLAGS = "-w -j 4")
   expect_compilation(mod, quiet = TRUE, force_recompile = TRUE)