Skip to content

Commit a3e08ee

Browse files
committed
Record what the instantiate survey found
instantiate adopts an existing executable through cmdstan_model(exe_file =, compile = FALSE), so dropping the argument is the whole fix; adoption never compiles. Its other branch has no successor, but that state means a package was installed without its binary, and erroring there is defensible. stan_package_compile() maps onto compile_stan_file() directly. Says why these two packages are the priorities. They are chokepoints rather than merely important: instantiate's dependents call stan_package_model() rather than cmdstanr, so fixing instantiate carries its dependency tree. Everyone else has the candidate period. Also notes that instantiate calls us by string name through eval(parse(...)), so no static check will find a break in it. Both packages' test suites should be run against the candidate rather than relying on a search.
1 parent 1e247e0 commit a3e08ee

1 file changed

Lines changed: 25 additions & 5 deletions

File tree

dev-notes/compilation-state.md

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -951,13 +951,33 @@ A 1.0 candidate ships after Stage 4, so downstream packages have something to
951951
migrate against rather than a release note. That is what makes §8's breaking change
952952
affordable.
953953

954-
Two are known to be affected. `instantiate` is built around precompiled models.
955-
**brms** uses `cmdstan_model(compile = FALSE)` in `.parse_model_cmdstanr()`
954+
**brms** and **instantiate** are the priorities, and they are chokepoints rather
955+
than merely important packages: instantiate's own dependents call
956+
`instantiate::stan_package_model()` rather than cmdstanr directly, so fixing
957+
instantiate carries its dependency tree with it. Everyone else has the candidate
958+
period to adapt on their own.
959+
960+
brms uses `cmdstan_model(compile = FALSE)` in `.parse_model_cmdstanr()`
956961
(`brms/R/backends.R:23-34`) to build a throwaway object solely for `$check_syntax()`
957962
and `$code()` — precisely the case §8's standalone family replaces, and the clearest
958-
confirmation so far that the family is the right shape. brms also sets
959-
`cpp_options$stan_threads` only when threading is requested, so its users meet §1's
960-
threading policy as a rebuild when they toggle it off.
963+
confirmation so far that the family is the right shape; the replacement removes
964+
lines rather than adding them. `.compile_model_cmdstanr()` needs no change at all,
965+
since it already supplies options on every construction, which is what §2 asks of
966+
every caller. brms does set `cpp_options$stan_threads` only when threading is
967+
requested, so its users meet §1's threading policy as a rebuild when they toggle it
968+
off.
969+
970+
instantiate is smaller still. `stan_package_model()` adopts an existing executable
971+
with `cmdstan_model(exe_file = , compile = FALSE)`, and dropping the argument is the
972+
whole fix, since adoption never compiles. Its other branch — `stan_file = ` with
973+
`compile = FALSE` and the executable *missing* — has no successor, but that state
974+
means a package was installed without its binary, so erroring is defensible.
975+
`stan_package_compile()` maps onto `compile_stan_file()` directly.
976+
977+
Neither survey proves there is no third caller, and instantiate reaches us through
978+
`eval(parse(text = paste0("cmdstanr::", name)))`, so no static check will find a
979+
break in it — ours or theirs. Run both packages' test suites against the candidate
980+
rather than trusting a search.
961981

962982
**These are signals, not constraints.** brms internals are ours to change, and the
963983
survey above is worth having because it shows what real callers need to express

0 commit comments

Comments
 (0)