From 450d0607ff716c47c6bed863b6f2c003f39ce552 Mon Sep 17 00:00:00 2001 From: jgabry Date: Wed, 23 Sep 2026 11:32:14 -0600 Subject: [PATCH 1/6] Build model methods against RcppParallel's TBB when it is installed CmdStan bundles TBB 2020.3 and RcppParallel ships oneTBB under the same library file name on every platform. Once one copy is in the R process, code linked against the other fails to load, so the model methods failed after rstan or brms had loaded RcppParallel's copy, and RcppParallel failed to load after the methods had brought in CmdStan's. rcpp_source_stan() now asks make for its flags with TBB_INC, TBB_LIB and TBB_INTERFACE_NEW pointing at RcppParallel's install whenever RcppParallel is there with its oneTBB. Stan Math supports oneTBB through TBB_INTERFACE_NEW and CmdStan's makefiles already handle the substitution. RcppParallel goes in Suggests for the new test, which also gets CI to install it on every platform. Closes #1270. --- DESCRIPTION | 3 +- NEWS.md | 3 ++ R/expose.R | 57 +++++++++++++++++++++++++++-- tests/testthat/test-model-methods.R | 6 +++ 4 files changed, 64 insertions(+), 5 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 59da5a10a..e9b30dffd 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -58,7 +58,8 @@ Suggests: qs2, rmarkdown, testthat (>= 3.3.0), - Rcpp + Rcpp, + RcppParallel VignetteBuilder: knitr Config/testthat/edition: 3 Config/roxygen2/version: 8.0.0 diff --git a/NEWS.md b/NEWS.md index 98fd61c26..e18d2eadb 100644 --- a/NEWS.md +++ b/NEWS.md @@ -251,6 +251,9 @@ and saying how to rebuild it. Previously the fitting methods and * On Windows a model executable is now launched with the TBB it was built against. Previously the selected CmdStan installation's TBB was used, which was wrong once `set_cmdstan_path()` had selected a different one. (#1261) +* `$init_model_methods()` and `$expose_functions()` now work in a session that +has loaded rstan or brms, by building against RcppParallel's TBB when +RcppParallel is installed. (#1270) ## Removed and deprecated diff --git a/R/expose.R b/R/expose.R index 83d682aca..c63f6eda7 100644 --- a/R/expose.R +++ b/R/expose.R @@ -25,18 +25,67 @@ check_sundials_fpic <- function(verbose) { } } +#' RcppParallel's TBB, to build R-loaded code against +#' +#' CmdStan bundles TBB 2020.3 and RcppParallel ships oneTBB under the +#' same library name (`libtbb.dylib`, `tbb.dll`, `libtbb.so.2`). Once +#' one copy is in the process, code linked against the other fails to +#' load: model methods built after rstan or brms loaded RcppParallel's, +#' or RcppParallel itself after the methods loaded CmdStan's. Building +#' the methods against RcppParallel's copy whenever it is installed +#' avoids both orders. Stan Math supports oneTBB through +#' `TBB_INTERFACE_NEW`. +#' +#' @return A list with RcppParallel's `include` and `lib` directories, +#' or `NULL` when RcppParallel is not installed with its oneTBB. +#' @noRd +rcppparallel_tbb <- function() { + arch <- .Platform$r_arch + lib <- system.file(paste(c("lib", arch[nzchar(arch)]), collapse = "/"), + package = "RcppParallel") + version_h <- system.file("include/tbb/version.h", package = "RcppParallel") + if (!nzchar(lib) || !nzchar(version_h)) { + return(NULL) + } + list(include = dirname(dirname(version_h)), lib = lib) +} + +#' Compile C++ that uses the Stan Math library and load it into R +#' +#' Wraps `Rcpp::sourceCpp()` with the include paths, defines and link +#' flags CmdStan's make would use for a model, so the model methods and +#' standalone functions build against the same Stan Math, SUNDIALS and +#' TBB as the executable. When RcppParallel is installed we build against +#' its TBB instead, see `rcppparallel_tbb()`. +#' +#' @param code Character string with the C++ source. +#' @param env Environment the compiled functions are assigned into. +#' @param verbose Logical. Print compiler output and, on Linux, the +#' SUNDIALS rebuild output? +#' @param ... Passed to `Rcpp::sourceCpp()`. +#' @return `NULL`, invisibly. +#' @noRd rcpp_source_stan <- function(code, env, verbose = FALSE, ...) { check_sundials_fpic(verbose) - cxxflags <- get_cmdstan_flags("CXXFLAGS") - cppflags <- get_cmdstan_flags("CPPFLAGS") + tbb <- rcppparallel_tbb() + make_args <- character() + tbb_dir <- tbb_path() + if (!is.null(tbb)) { + make_args <- c(paste0("TBB_INC=", tbb$include), + paste0("TBB_LIB=", tbb$lib), "TBB_INTERFACE_NEW=1") + tbb_dir <- tbb$lib + } + cxxflags <- get_cmdstan_flags("CXXFLAGS", make_args) + cppflags <- get_cmdstan_flags("CPPFLAGS", make_args) cmdstanr_includes <- system.file("include", package = "cmdstanr", mustWork = TRUE) cmdstanr_includes <- paste0(" -I\"", cmdstanr_includes,"\"") libs <- c("LDLIBS", "LIBSUNDIALS", "TBB_TARGETS", "LDFLAGS_TBB", "SUNDIALS_TARGETS") - libs <- paste(sapply(libs, get_cmdstan_flags), collapse = " ") + libs <- paste(sapply(libs, get_cmdstan_flags, make_args = make_args), + collapse = " ") if (.Platform$OS.type == "windows") { libs <- paste(libs, "-fopenmp") } - withr::with_path(repair_path(file.path(cmdstan_path(),"stan/lib/stan_math/lib/tbb")), + withr::with_path(tbb_dir, withr::with_makevars( c( USE_CXX14 = 1, diff --git a/tests/testthat/test-model-methods.R b/tests/testthat/test-model-methods.R index 50736bb19..5bc837719 100644 --- a/tests/testthat/test-model-methods.R +++ b/tests/testthat/test-model-methods.R @@ -7,6 +7,12 @@ utils::capture.output( fit <- mod$sample(data = data_list, chains = 1, refresh = 0) ) +test_that("model methods load with RcppParallel's TBB in the session", { + skip_if_not_installed("RcppParallel") + loadNamespace("RcppParallel") + expect_no_error(fit$init_model_methods()) +}) + test_that("Model methods automatically initialise when needed", { expect_no_error(fit$log_prob(unconstrained_variables=c(0.1))) }) From df6ba9f1e0c065bbfa8fee6bb59b5f1845f87873 Mon Sep 17 00:00:00 2001 From: jgabry Date: Wed, 23 Sep 2026 12:13:44 -0600 Subject: [PATCH 2/6] Quote RcppParallel's paths in the model-method build flags Make's print rule echoes through the shell, which drops quotes, so an R library path with a space came back as two arguments and the build failed. Quote the two paths after they come back, the way get_cmdstan_flags() already quotes CmdStan's own. The roxygen for rcpp_source_stan() now says the flags come from the selected installation rather than the one that built the executable. Part of #1270. --- R/expose.R | 12 ++++++++---- tests/testthat/test-model-methods.R | 19 +++++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/R/expose.R b/R/expose.R index c63f6eda7..6b3b9d79e 100644 --- a/R/expose.R +++ b/R/expose.R @@ -53,10 +53,9 @@ rcppparallel_tbb <- function() { #' Compile C++ that uses the Stan Math library and load it into R #' #' Wraps `Rcpp::sourceCpp()` with the include paths, defines and link -#' flags CmdStan's make would use for a model, so the model methods and -#' standalone functions build against the same Stan Math, SUNDIALS and -#' TBB as the executable. When RcppParallel is installed we build against -#' its TBB instead, see `rcppparallel_tbb()`. +#' flags the selected CmdStan installation's make would use for a model, +#' with RcppParallel's TBB substituted when it is installed, see +#' `rcppparallel_tbb()`. #' #' @param code Character string with the C++ source. #' @param env Environment the compiled functions are assigned into. @@ -82,6 +81,11 @@ rcpp_source_stan <- function(code, env, verbose = FALSE, ...) { libs <- c("LDLIBS", "LIBSUNDIALS", "TBB_TARGETS", "LDFLAGS_TBB", "SUNDIALS_TARGETS") libs <- paste(sapply(libs, get_cmdstan_flags, make_args = make_args), collapse = " ") + if (!is.null(tbb)) { + # make's print rule drops the quotes, so quote the paths here + cxxflags <- gsub(tbb$include, shQuote(tbb$include), cxxflags, fixed = TRUE) + libs <- gsub(tbb$lib, shQuote(tbb$lib), libs, fixed = TRUE) + } if (.Platform$OS.type == "windows") { libs <- paste(libs, "-fopenmp") } diff --git a/tests/testthat/test-model-methods.R b/tests/testthat/test-model-methods.R index 5bc837719..78b50ee69 100644 --- a/tests/testthat/test-model-methods.R +++ b/tests/testthat/test-model-methods.R @@ -7,6 +7,25 @@ utils::capture.output( fit <- mod$sample(data = data_list, chains = 1, refresh = 0) ) +test_that("RcppParallel's TBB works from a library path with a space", { + skip_if_not_installed("RcppParallel") + installed <- find.package("RcppParallel") + lib <- withr::local_tempdir("R library") + file.copy(installed, lib, recursive = TRUE) + copy <- lapply(rcppparallel_tbb(), function(dir) { + sub(installed, file.path(lib, "RcppParallel"), dir, fixed = TRUE) + }) + local_mocked_bindings(rcppparallel_tbb = function() copy) + code <- paste( + "#include ", + "#include ", + "// [[Rcpp::export]]", + "int tbb_interface_version() { return TBB_INTERFACE_VERSION; }", + sep = "\n" + ) + expect_no_error(rcpp_source_stan(code, new.env())) +}) + test_that("model methods load with RcppParallel's TBB in the session", { skip_if_not_installed("RcppParallel") loadNamespace("RcppParallel") From 64d2918b620bd721ef42a0e0c044115ebec67002 Mon Sep 17 00:00:00 2001 From: jgabry Date: Wed, 23 Sep 2026 12:24:31 -0600 Subject: [PATCH 3/6] Skip the spaced-path test when RcppParallel uses a system TBB rcppparallel_tbb() returns NULL for such a build, and lapply() turned that into an empty list that sent the mocked helper down the RcppParallel branch without paths. Part of #1270. --- tests/testthat/test-model-methods.R | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/testthat/test-model-methods.R b/tests/testthat/test-model-methods.R index 78b50ee69..6d4d7fc7f 100644 --- a/tests/testthat/test-model-methods.R +++ b/tests/testthat/test-model-methods.R @@ -9,10 +9,12 @@ utils::capture.output( test_that("RcppParallel's TBB works from a library path with a space", { skip_if_not_installed("RcppParallel") + tbb <- rcppparallel_tbb() + skip_if(is.null(tbb), "RcppParallel is built against a system TBB") installed <- find.package("RcppParallel") lib <- withr::local_tempdir("R library") file.copy(installed, lib, recursive = TRUE) - copy <- lapply(rcppparallel_tbb(), function(dir) { + copy <- lapply(tbb, function(dir) { sub(installed, file.path(lib, "RcppParallel"), dir, fixed = TRUE) }) local_mocked_bindings(rcppparallel_tbb = function() copy) From 4dddec5fafd41eaa6e3741d75ecc259419d3f151 Mon Sep 17 00:00:00 2001 From: jgabry Date: Wed, 23 Sep 2026 16:14:25 -0600 Subject: [PATCH 4/6] Use forward slashes in RcppParallel's paths on Windows In R CMD check's temporary library system.file() returns the RcppParallel paths with backslashes. The flags make prints go through sh in the Makevars, which strips the backslashes, so the compiler saw -isystem C:UsersRUNNER~1... and every model-method build failed on the Windows CI jobs. repair_path() replaces them with forward slashes. Part of #1270. --- R/expose.R | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/R/expose.R b/R/expose.R index 6b3b9d79e..4c2f14adf 100644 --- a/R/expose.R +++ b/R/expose.R @@ -47,7 +47,9 @@ rcppparallel_tbb <- function() { if (!nzchar(lib) || !nzchar(version_h)) { return(NULL) } - list(include = dirname(dirname(version_h)), lib = lib) + # R CMD check's temporary library returns backslash paths on Windows + list(include = repair_path(dirname(dirname(version_h))), + lib = repair_path(lib)) } #' Compile C++ that uses the Stan Math library and load it into R From 1163a15efa09d7e2fe14e70ebc1a7b3b4c104213 Mon Sep 17 00:00:00 2001 From: jgabry Date: Wed, 23 Sep 2026 16:15:15 -0600 Subject: [PATCH 5/6] Accept stanc's new diagnostic format in two tests stanc3 changed its error reports on 2026-09-23 (stanc3 PR #1724): the "Semantic error in '', line N" heading is gone and the location is now a ":N:M" frame under the message. The CmdStan 2.40.0 tarball ships a placeholder for the macOS stanc binary, so its makefile downloads the nightly and every macOS CI job started seeing the new format. The two tests that pin the location now match either format. Part of #1270. --- tests/testthat/test-model-compile.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-model-compile.R b/tests/testthat/test-model-compile.R index c66f870a9..3f375fc3c 100644 --- a/tests/testthat/test-model-compile.R +++ b/tests/testthat/test-model-compile.R @@ -138,7 +138,7 @@ test_that("a program stanc rejects errors with stanc's message", { stan_file <- testing_stan_file("fail") expect_error( cmdstan_model(stan_file), - "Semantic error in '.*fail.stan', line 7" + "fail.stan(', line 7|:7:)" ) }) @@ -539,7 +539,7 @@ test_that("building errors on removed syntax", { } " stan_file <- write_stan_file(model_code) - expect_error(cmdstan_model(stan_file), "Syntax error in '.*', line 4") + expect_error(cmdstan_model(stan_file), "Syntax error.*(line 4|:4:)") }) test_that("compilation errors if folder with the model name exists", { From 63fa0c640cc37a8d520c7af01db91b9f52adc09c Mon Sep 17 00:00:00 2001 From: jgabry Date: Wed, 23 Sep 2026 22:23:59 -0600 Subject: [PATCH 6/6] Drop the ELF-only linker flag on Windows and repair the test's path When TBB_LIB is set CmdStan's make adds -Wl,--disable-new-dtags on every OS except macOS. The flag only exists for ELF binaries, so Rtools' ld and lld both refuse it and every model-methods build on Windows failed at link time. Passing LDFLAGS_TBB_DTAGS= on the make command line leaves the flag out; CmdStan's own Windows builds never set TBB_LIB, which is why nobody hit this before. The test that builds against a library path with a space handed the mocked rcppparallel_tbb() a backslash path from tempdir(), which make's print rule mangled so the quoting in rcpp_source_stan() no longer matched. The real function repairs its paths, so the mock now does too. Part of #1270. --- R/expose.R | 4 ++++ tests/testthat/test-model-methods.R | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/R/expose.R b/R/expose.R index 4c2f14adf..21b4d4aad 100644 --- a/R/expose.R +++ b/R/expose.R @@ -74,6 +74,10 @@ rcpp_source_stan <- function(code, env, verbose = FALSE, ...) { if (!is.null(tbb)) { make_args <- c(paste0("TBB_INC=", tbb$include), paste0("TBB_LIB=", tbb$lib), "TBB_INTERFACE_NEW=1") + if (.Platform$OS.type == "windows") { + # Rtools' linkers reject the ELF-only flag make adds for a system TBB + make_args <- c(make_args, "LDFLAGS_TBB_DTAGS=") + } tbb_dir <- tbb$lib } cxxflags <- get_cmdstan_flags("CXXFLAGS", make_args) diff --git a/tests/testthat/test-model-methods.R b/tests/testthat/test-model-methods.R index 6d4d7fc7f..dc13b5b07 100644 --- a/tests/testthat/test-model-methods.R +++ b/tests/testthat/test-model-methods.R @@ -12,7 +12,7 @@ test_that("RcppParallel's TBB works from a library path with a space", { tbb <- rcppparallel_tbb() skip_if(is.null(tbb), "RcppParallel is built against a system TBB") installed <- find.package("RcppParallel") - lib <- withr::local_tempdir("R library") + lib <- repair_path(withr::local_tempdir("R library")) file.copy(installed, lib, recursive = TRUE) copy <- lapply(tbb, function(dir) { sub(installed, file.path(lib, "RcppParallel"), dir, fixed = TRUE)