diff --git a/NEWS.md b/NEWS.md index d5e5ec34..e03e2d46 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,8 @@ * `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. * 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 diff --git a/R/args.R b/R/args.R index b04004f2..fa673f57 100644 --- a/R/args.R +++ b/R/args.R @@ -43,7 +43,6 @@ CmdStanArgs <- R6::R6Class( sig_figs = NULL, opencl_ids = NULL, model_variables = NULL, - num_threads = NULL, save_cmdstan_config = NULL) { self$model_name <- model_name @@ -81,7 +80,6 @@ CmdStanArgs <- R6::R6Class( init <- process_init(init, num_inits, model_variables) self$init <- init self$opencl_ids <- opencl_ids - self$num_threads <- NULL self$method_args$validate(num_procs = length(self$proc_ids)) if (is.logical(self$save_cmdstan_config)) { self$save_cmdstan_config <- as.integer(self$save_cmdstan_config) @@ -185,9 +183,6 @@ CmdStanArgs <- R6::R6Class( if (!is.null(self$opencl_ids)) { args$opencl <- c("opencl", paste0("platform=", self$opencl_ids[1]), paste0("device=", self$opencl_ids[2])) } - if (!is.null(self$num_threads)) { - num_threads <- c(args$output, paste0("num_threads=", self$num_threads)) - } args <- do.call(c, append(args, list(use.names = FALSE))) self$method_args$compose(idx, args) }, diff --git a/R/csv.R b/R/csv.R index 8a31022c..d0785152 100644 --- a/R/csv.R +++ b/R/csv.R @@ -815,7 +815,7 @@ read_csv_metadata <- function(csv_file) { csv_file_info$step_size <- csv_file_info$stepsize csv_file_info$iter_warmup <- csv_file_info$num_warmup csv_file_info$iter_sampling <- csv_file_info$num_samples - if (csv_file_info$method %in% c("variational", "optimize", "laplace")) { + if (csv_file_info$method %in% c("variational", "optimize", "laplace", "pathfinder")) { csv_file_info$threads <- csv_file_info$num_threads } else { csv_file_info$threads_per_chain <- csv_file_info$num_threads diff --git a/R/model.R b/R/model.R index 747ceb6d..d807c36a 100644 --- a/R/model.R +++ b/R/model.R @@ -1837,10 +1837,11 @@ CmdStanModel$set("public", name = "variational", value = variational) #' installed version of CmdStan #' #' @template model-common-args -#' @param num_threads (positive integer) If the model was +#' @param threads (positive integer) If the model was #' [compiled][model-method-compile] with threading support, the number of #' threads to use in parallelized sections (e.g., for multi-path pathfinder #' as well as `reduce_sum`). +#' @param num_threads Deprecated. Please use `threads` instead. #' @param init_alpha (positive real) The initial step size parameter. #' @param tol_obj (positive real) Convergence tolerance on changes in objective function value. #' @param tol_rel_obj (positive real) Convergence tolerance on relative changes in objective function value. @@ -1897,6 +1898,7 @@ pathfinder <- function(data = NULL, output_dir = getOption("cmdstanr_output_dir"), output_basename = NULL, sig_figs = NULL, + threads = NULL, opencl_ids = NULL, num_threads = NULL, init_alpha = NULL, @@ -1917,11 +1919,18 @@ pathfinder <- function(data = NULL, show_messages = TRUE, show_exceptions = TRUE, save_cmdstan_config = getOption("cmdstanr_save_config", FALSE)) { + if (!is.null(num_threads)) { + if (!is.null(threads)) { + stop("Cannot specify both 'threads' and deprecated 'num_threads'.", call. = FALSE) + } + warning("'num_threads' is deprecated. Please use 'threads' instead.", call. = FALSE) + threads <- num_threads + } procs <- CmdStanProcs$new( num_procs = 1, show_stderr_messages = show_exceptions, show_stdout_messages = show_messages, - threads_per_proc = assert_valid_threads(num_threads, self$cpp_options()) + threads_per_proc = assert_valid_threads(threads, self$cpp_options()) ) model_variables <- NULL if (is_variables_method_supported(self)) { @@ -1963,7 +1972,6 @@ pathfinder <- function(data = NULL, sig_figs = sig_figs, opencl_ids = assert_valid_opencl(opencl_ids, self$cpp_options()), model_variables = model_variables, - num_threads = num_threads, save_cmdstan_config = save_cmdstan_config ) runset <- CmdStanRun$new(args, procs) diff --git a/man/model-method-pathfinder.Rd b/man/model-method-pathfinder.Rd index 1f7f5fc3..9fd6f08d 100644 --- a/man/model-method-pathfinder.Rd +++ b/man/model-method-pathfinder.Rd @@ -14,6 +14,7 @@ pathfinder( output_dir = getOption("cmdstanr_output_dir"), output_basename = NULL, sig_figs = NULL, + threads = NULL, opencl_ids = NULL, num_threads = NULL, init_alpha = NULL, @@ -150,15 +151,17 @@ values with 6 significant figures. The upper limit for \code{sig_figs} is 18. Increasing this value will result in larger output CSV files and thus an increased usage of disk space.} +\item{threads}{(positive integer) If the model was +\link[=model-method-compile]{compiled} with threading support, the number of +threads to use in parallelized sections (e.g., for multi-path pathfinder +as well as \code{reduce_sum}).} + \item{opencl_ids}{(integer vector of length 2) The platform and device IDs of the OpenCL device to use for fitting. The model must be compiled with \code{cpp_options = list(stan_opencl = TRUE)} for this argument to have an effect.} -\item{num_threads}{(positive integer) If the model was -\link[=model-method-compile]{compiled} with threading support, the number of -threads to use in parallelized sections (e.g., for multi-path pathfinder -as well as \code{reduce_sum}).} +\item{num_threads}{Deprecated. Please use \code{threads} instead.} \item{init_alpha}{(positive real) The initial step size parameter.} diff --git a/tests/testthat/test-model-pathfinder.R b/tests/testthat/test-model-pathfinder.R index ce74f6dd..472f6c85 100644 --- a/tests/testthat/test-model-pathfinder.R +++ b/tests/testthat/test-model-pathfinder.R @@ -40,6 +40,7 @@ bad_arg_values <- list( refresh = -1, init = "maybe :P", seed = -80, + threads = "NOT_THREADS", init_alpha = "cat.jpeg", init_alpha = -3, tol_obj = -1, diff --git a/tests/testthat/test-threads.R b/tests/testthat/test-threads.R index 5e9f04f7..ea9c7140 100644 --- a/tests/testthat/test-threads.R +++ b/tests/testthat/test-threads.R @@ -118,6 +118,48 @@ test_that("threading works with variational()", { expect_equal(f$metadata()$threads, 4) }) +test_that("threading works with pathfinder()", { + mod <- cmdstan_model(stan_program, cpp_options = list(stan_threads = TRUE), + force_recompile = TRUE) + pathfinder_args <- list( + data = data_file_json, + seed = 123, + refresh = 0, + draws = 10, + single_path_draws = 10, + num_paths = 1, + num_elbo_draws = 10, + max_lbfgs_iters = 10 + ) + + expect_error( + do.call(mod$pathfinder, pathfinder_args), + "The model was compiled with 'cpp_options = list(stan_threads = TRUE)' but 'threads' was not set!", + fixed = TRUE + ) + + pathfinder_args$threads <- 2 + expect_output( + f <- do.call(mod$pathfinder, pathfinder_args), + "Finished in", + fixed = TRUE + ) + expect_equal(as.integer(Sys.getenv("STAN_NUM_THREADS")), 2) + expect_equal(f$metadata()$threads, 2) + + pathfinder_args$num_threads <- 2 + expect_error( + do.call(mod$pathfinder, pathfinder_args), + "Cannot specify both 'threads' and deprecated 'num_threads'" + ) + pathfinder_args$threads <- NULL + pathfinder_args$show_messages <- FALSE + expect_warning( + do.call(mod$pathfinder, pathfinder_args), + "'num_threads' is deprecated. Please use 'threads' instead" + ) +}) + test_that("threading works with generate_quantities()", { mod <- cmdstan_model(stan_program, cpp_options = list(stan_threads = TRUE), force_recompile = TRUE) mod_gq <- cmdstan_model(stan_gq_program, cpp_options = list(stan_threads = TRUE), force_recompile = TRUE)