stan-dev / stan-dev/cmdstanr

cmdstan_model(exe_file = ) surfaces a raw processx error when the executable cannot be run

Open
#1,246 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
R
Stars
160
Forks
69
Avg merge
1d 19h
Merged PRs (30d)
15

Description

Another issue noticed while working on #1235. I asked Claude to write up the report below:


Status

The adoption half is done on the v1.0 branch. cmdstan_model(exe_file = ) and stan_build_info() share inspect_executable() (R/build.R, added in #1273), which reads the record beside the executable and, when there is no usable record, runs <exe> info. A launch that fails or reports no Stan version is an error naming the file. The fit and $cmdstan_defaults() launch sites below still surface the raw processx error, so the issue stays open for those.

The code references below describe cmdstanr 0.9; model_compile_info() and the three call sites in the table no longer exist on v1.0.

cmdstan_model(exe_file = ) reads the executable's build metadata at
R/model.R:324, and that call is not guarded. If the file cannot be launched as
a process the raw processx failure reaches the user:

exe <- tempfile()
writeLines("i am not a program", exe)
Sys.chmod(exe, "0644")
cmdstan_model(exe_file = exe)
#> Error: ! Native call to `processx_exec` failed
#> Caused by error in `chain_call(...)`:
#> ! cannot start processx process '/tmp/.../file1025b6854155e'
#>   (system error 13, Permission denied) @unix/processx.c:651 (processx_exec)

assert_file_exists(exe_file, access = "r") checks only that the file is
readable, so anything readable-but-not-runnable — wrong architecture, a text
file, a lost execute bit — arrives here and fails this way.

What the right behaviour probably is

Erroring is defensible: you were handed a path that is not a working executable,
and $sample() would fail moments later anyway. The problem is the message, not
the decision to stop. So this is not simply a matter of wrapping the call in
tryCatch() — it wants an error that names the file and says it could not be
run as a CmdStan executable.

Note the asymmetry with the other two call sites, which is worth resolving at the
same time. model_compile_info() is read in three places:

Site Binary cannot be launched Binary runs, info exits nonzero
initialize(), R/model.R:324 raw processx error silent; $cpp_options() empty
no-op compile, R/model.R:751 silent (tryCatchNULL) silent
post-commit merge, R/model.R:958 silent (tryCatchNULL) silent

The two compile paths are deliberately quiet: an executable that cannot report
its metadata will fail loudly at $sample(), and the only cost of staying quiet
is that $cpp_options() under-reports options inherited from make/local, which
in turn produces a misleading threads_per_chain "will have no effect" warning.
That is recorded as a consequence of having no build provenance beside the
executable (#1238).

The construction path is the one that is neither quiet nor clear.

Related: tests/testthat/test-model-recompile-logic.R has a skip()ped test,
"warning when no recompile and no info", expecting a "Recompiling is recommended." warning for an executable whose metadata cannot be read. It is
parked behind #1019, so there is an existing intent to warn here that should be
settled alongside this.

Found while reviewing #1235.


Fix it in a shared adoption helper

From the #1254 design review.

The fix should live in a shared adoption helper used by every entry point that
adopts an executable, rather than being patched at the constructor alone, with
tests covering each entry point separately.

The helper only reaches a subprocess launch when provenance is unknown. Once #1238
lands, adopting an executable whose build record matches its hash hydrates from
that record without launching anything, so the helper covers the fallback path,
which is where a non-CmdStan binary shows up.

Every site that launches the model binary needs it, not only adoption

An unrunnable executable with a valid record never reaches the helper. It adopts
cleanly, and the first launch is the fit, so the raw error arrives at $sample()
instead of at construction. Measured, on a model whose executable lost its execute
bit after it was built:

Error: ! Native call to `processx_exec` failed
Caused by error in `chain_call(...)`:
! cannot start processx process './bern' (system error 13, Permission denied)

Note the relative path: R/run.R runs the binary from its own directory, so the
message names neither the model nor the file it could not start.

This is not a corner case. R's own utils::unzip() extracts an executable without
the execute bit where untar() and file.copy() keep it, so a project folder shared
as a zip and unpacked from R arrives in exactly this state, record and .stan file
intact. A binary built for another platform on a shared drive arrives in it too, and
so does one whose recorded TBB directory is gone.

The design settles what cmdstanr does about it
(#1254 §6, "An executable that will not launch is an error, not a rebuild trigger")
and puts the remedy here rather than in a rebuild: the error names the executable, and for a model with a source it says that
force_recompile = TRUE rebuilds it. With only an executable there is nothing to
rebuild and the message says so instead.

cmdstanr launches the model binary at four sites: the fit's process launch in R/run.R, the help-all call in parse_cmdstan_args() (R/model.R), and run_info_cli() (R/build_record.R) behind adoption and stan_build_info(). The error belongs at all of them, and on v1.0 only the last has it.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by tracing executable launches in R/run.R, R/model.R, and R/build_record.R, along with the shared adoption path and run_info_cli(). Review tests/testthat/test-model-recompile-logic.R for the existing warning intent. Done means each launch reports the executable path and a useful CmdStan error, including the force_recompile guidance when source is available, with separate coverage for each entry point.

Written by the indexing model from the issue text.

Assessment

Tech stack
r
Domain
backend, tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.