Skip to content

Commit b5389a1

Browse files
committed
Resolve include paths when they are stored (#1229)
Include paths were stored exactly as supplied and resolved to absolute paths only when stanc was called, so resolution depended on the working directory at that later point. A model created from a relative stan_file stored "." as its inferred include path, and assert_dir_exists(".") can never fail, so a change of working directory either produced a confusing stanc error or silently resolved #include directives against a same-named file in the new directory. Paths are now resolved where they are stored, using a shared resolve_path() helper that also replaces the equivalent inline expressions for stan_file_ and exe_file_. $include_paths() returns absolute paths.
1 parent 143ab16 commit b5389a1

6 files changed

Lines changed: 48 additions & 12 deletions

File tree

‎NEWS.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,11 @@
44
as of CmdStanR 1.0.0; use the lowercase `cmdstanr_no_ver_check` forms instead.
55
* CmdStanModel methods now correctly handle `#include` directories with spaces
66
in their paths. (#820)
7+
* `$include_paths()` now returns absolute paths, and relative include paths are
8+
resolved when the model object is created or `$compile()` is called rather than
9+
on each `stanc` call. Previously a model created from a relative path could
10+
resolve `#include` directives against the wrong directory if the working
11+
directory changed. (#1229)
712
* `$cpp_options()` no longer includes a `STAN_VERSION` entry read from the model
813
executable's metadata. It was never a C++ option; use `$cmdstan_version()` instead. (#1215)
914
* CmdStanModel methods now use executable metadata regardless of the

‎R/model.R‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ CmdStanModel <- R6::R6Class(
259259
if (!is.null(stan_file)) {
260260
assert_file_exists(stan_file, access = "r", extension = c("stan", "stanfunctions"))
261261
checkmate::assert_flag(compile)
262-
private$stan_file_ <- absolute_path(stan_file)
262+
private$stan_file_ <- resolve_path(stan_file)
263263
private$stan_code_ <- readLines(stan_file)
264264
private$model_name_ <- gsub(" ", "_", strip_ext(basename(private$stan_file_)))
265265
private$precompile_cpp_options_ <- args$cpp_options %||% list()
@@ -269,20 +269,20 @@ CmdStanModel <- R6::R6Class(
269269
private$using_user_header_ <- TRUE
270270
}
271271
if (is.null(args$include_paths) && any(grepl("#include" , private$stan_code_))) {
272-
private$precompile_include_paths_ <- dirname(stan_file)
272+
private$precompile_include_paths_ <- dirname(private$stan_file_)
273273
} else {
274-
private$precompile_include_paths_ <- args$include_paths
274+
private$precompile_include_paths_ <- resolve_path(args$include_paths)
275275
}
276276
}
277277
if (!is.null(exe_file)) {
278278
ext <- if (os_is_windows() && !os_is_wsl()) "exe" else ""
279-
private$exe_file_ <- repair_path(absolute_path(exe_file))
279+
private$exe_file_ <- resolve_path(exe_file)
280280
if (is.null(stan_file)) {
281281
assert_file_exists(private$exe_file_, access = "r", extension = ext)
282282
private$model_name_ <- gsub(" ", "_", strip_ext(basename(private$exe_file_)))
283283
}
284284
private$include_paths_ <-
285-
private$precompile_include_paths_ %||% args$include_paths
285+
private$precompile_include_paths_ %||% resolve_path(args$include_paths)
286286
}
287287
if (!is.null(stan_file) && compile) {
288288
self$compile(...)
@@ -422,7 +422,7 @@ CmdStanModel <- R6::R6Class(
422422
#' * `$model_name()` returns the model name as a string.
423423
#' * `$exe_file()` returns a path as a string, or `character(0)` if no
424424
#' executable path is set.
425-
#' * `$include_paths()` returns a character vector of paths or `NULL`.
425+
#' * `$include_paths()` returns a character vector of absolute paths or `NULL`.
426426
#' * `$cmdstan_version()` returns a CmdStan version as a string.
427427
#' * `$cpp_options()` returns a named list of C++ options.
428428
#' * `$hpp_file()` returns the path to the `.hpp` file as a string when C++ code
@@ -480,7 +480,10 @@ NULL
480480
#' [`$check_syntax()`][model-method-check_syntax] method can be used instead.
481481
#' @param include_paths (character vector) Paths to directories where Stan
482482
#' should look for files specified in `#include` directives in the Stan
483-
#' program.
483+
#' program. Relative paths are resolved against the working directory when
484+
#' the model object is created (or when `$compile()` is called) and stored as
485+
#' absolute paths, so subsequent changes to the working directory do not
486+
#' affect them.
484487
#' @param user_header (string) The path to a C++ file (with a .hpp extension)
485488
#' to compile with the Stan model.
486489
#' @param cpp_options (list) Any makefile options to be used when compiling the
@@ -594,7 +597,7 @@ compile <- function(quiet = TRUE,
594597
if (is.null(include_paths) && !is.null(private$precompile_include_paths_)) {
595598
include_paths <- private$precompile_include_paths_
596599
}
597-
private$include_paths_ <- include_paths
600+
private$include_paths_ <- resolve_path(include_paths)
598601
if (is.null(dir) && !is.null(private$dir_)) {
599602
dir <- absolute_path(private$dir_)
600603
} else if (!is.null(dir)) {

‎R/utils.R‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,17 @@ strip_ext <- function(file) {
194194
}
195195
absolute_path <- Vectorize(.absolute_path, USE.NAMES = FALSE)
196196

197+
# Resolve paths when they are stored on a model object rather than when they are
198+
# used, so that later use doesn't depend on the working directory. Empty input
199+
# becomes NULL, the value callers treat as "not set" (absolute_path() would
200+
# otherwise return list()).
201+
resolve_path <- function(path) {
202+
if (!length(path)) {
203+
return(NULL)
204+
}
205+
repair_path(absolute_path(path))
206+
}
207+
197208
# read, write, and copy files --------------------------------------------
198209

199210
#' Copy temporary files (e.g., output, data) to a different location

‎man/model-method-compile.Rd‎

Lines changed: 4 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎man/model-method-model-info.Rd‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎tests/testthat/test-model-compile.R‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,18 +155,32 @@ test_that("precompiled models retain include paths", {
155155
compile = FALSE,
156156
include_paths = model_dir
157157
)
158-
expect_equal(model_with_explicit_path$include_paths(), model_dir)
158+
expect_equal(model_with_explicit_path$include_paths(), repair_path(model_dir))
159159
expect_no_error(model_with_explicit_path$variables())
160160

161161
model_with_automatic_path <- cmdstan_model(
162162
stan_file,
163163
exe_file = compiled_model$exe_file(),
164164
compile = FALSE
165165
)
166-
expect_equal(model_with_automatic_path$include_paths(), dirname(stan_file))
166+
expect_equal(model_with_automatic_path$include_paths(), repair_path(dirname(stan_file)))
167167
expect_no_error(model_with_automatic_path$variables())
168168
})
169169

170+
test_that("include paths are resolved when the model is created", {
171+
model_dir <- withr::local_tempdir()
172+
file.copy(
173+
c(testing_stan_file("bernoulli_include"), testing_stan_file("divide_real_by_two")),
174+
model_dir
175+
)
176+
mod <- withr::with_dir(
177+
model_dir,
178+
cmdstan_model("bernoulli_include.stan", compile = FALSE)
179+
)
180+
# the working directory no longer contains the included file
181+
expect_true(mod$check_syntax(quiet = TRUE))
182+
})
183+
170184
test_that("name in STANCFLAGS is set correctly", {
171185
local_reproducible_output()
172186
out <- utils::capture.output(mod$compile(quiet = FALSE, force_recompile = TRUE))

0 commit comments

Comments
 (0)