v1.0 stage 5: add stan_build_info() and tidy up the code around the build - #1278
Merged
Merged
Conversation
stan_build_info(exe_file) reads the build record beside an executable and reports it as a public result: the record's status and, when it is unavailable, why; the features the executable reports about itself, with unknown kept apart from false; and, with a usable record, the configuration the build was asked for, each file it read with whether that path still exists, the CmdStan installation, and the dependencies cmdstanr cannot track. The result is narrower than the record so the record's format can change underneath it. A print method shows the same in prose. $build_info() on CmdStanModel calls it on the model's executable. adopt_executable() and the new function share inspect_executable(), which reads the record or queries the executable. The ?CmdStanModel methods tables are regrouped around the program and the executable now that $compile() is gone, and gain $variables(), which was missing. The cmdstan_model() docs for cpp_options and stanc_options now say how an entry is written and point at stan_build_info() for an example, instead of at make/local and an Examples section that had none. Also removes tests/testthat/testthat-problems.rds, committed by accident in #1277, and ignores testthat's problem files from now on. Part of #1258.
Every internal roxygen block and comment the v1.0 stages wrote gets a title, its fields, and wording a reader can follow without the design doc. A few names changed with them: the record checkers are now assert_record_*(), assess_build() takes `expected` rather than `wanted`, constructor_message() is build_message(), and test-model-guard.R is test-model-stale-executable.R. install_executable() moves from utils.R to build.R beside its only caller, with its tests and snapshots. The guard test also learns about $build_info(), which Stage 5 never added to its member list. Part of #1258.
Everything that talks to stanc moves from model.R and utils.R into stanc.R: the checks on stanc_options, the flag conversions, and running stanc for its C++ or its --info output. The Rcpp code behind $init_model_methods() and $expose_functions() moves from utils.R into expose.R. run_info_cli() and parse_exe_info_string() join their only caller in build_record.R, tbb_path() moves from run.R to path.R, and cpp_opts.R is now cpp_options.R after the argument it handles. The executable check in args.R no longer tells the user to run a compile() method that no longer exists. Part of #1258.
The four stanc helpers tested inside test-model-compile.R and the two get_standalone_hpp() tests in test-utils.R now live in test-stanc.R, with their snapshots, since the functions live in R/stanc.R. The parse_exe_info_string() test moves from test-cpp-options.R to test-build-record.R for the same reason. The tests README now says the build tests also cover writing the record and stan_build_info(). Part of #1258.
The error for an executable with no record that prints no Stan version from its `info` command now leads with that fact and names both explanations, that it is not a CmdStan executable or cannot be run, instead of saying it "did not identify itself". The design note keeps its reason for the weak claim: a script that exits 0 reaches the same error, so "it did not run" alone would be wrong. Part of #1258.
Member
Author
|
@andrjohns just wanted to give you a heads up that this also does some file reorganization that includes moving your code to expose Stan functions and model methods into a dedicated |
A relative executable name was handed to processx unresolved, so a file of the same name on PATH could be inspected instead. The path is now resolved first, as the constructor does. The optional dependencies were read with `$`, which could partial-match a longer name in a record with extra members. Untracked dependencies were sorted and deduplicated as written, so extra fields or a different member order kept duplicates apart; they are now projected onto kind and detected_in first. The tests that assert a valid record never runs the executable used a mock that only stopped, which reported_features_from_exe() swallowed. They now count launches. A new test compiles a model for real and reads it back with and without its record, which nothing did before. The docs say stan_version can be NA, where format_version lives, what stanc_options and include_paths hold, and that make/local may have changed since the build. The §5 table gains a $build_info() row. Part of #1258.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v1.0 #1278 +/- ##
=======================================
Coverage ? 93.37%
=======================================
Files ? 19
Lines ? 7200
Branches ? 0
=======================================
Hits ? 6723
Misses ? 477
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Submission Checklist
Summary
This PR and the summary below were assisted by AI. I have reviewed all code and doc.
Part of #1258. This is Stage 5, the last of the feature stages: the public function that reads a build record back, plus three cleanup commits over the code the earlier stages wrote.
stan_build_info(exe_file)reads the record beside an executable and returns astan_build_infoobject. Two fields are always there:record, withstatusand, when the record cannot be used, areason(missing,unreadable,executable_mismatch, orunsupported_formatwith theformat_versionfound); andreported_features, what the executable says about its own build, withNAkept apart fromFALSEfor a feature the executable did not report. With a usable record there are four more:configuration(thecpp_options,stanc_optionsandinclude_pathsthe build was asked for),dependencies(each file the build read, with the path it had then and whether it exists now),cmdstan(the installation's path and version, and whether the path exists), anduntracked_dependencies(files the build depended on that cmdstanr cannot follow). A field that is absent means there was no record to read it from; an empty list is a recorded empty value. The result is narrower than the record on purpose, so the record's format can change underneath it. There is a print method, and$build_info()on aCmdStanModelcalls the function on the model's executable. It reads the executable and record as they are on disk now, which is the point of it, so it does not go through the same check as the methods that run the executable.The
?CmdStanModelmethods tables are regrouped around the program and the executable now that$compile()is gone, and gain$variables(), which was missing. Thecmdstan_model()docs forcpp_optionsandstanc_optionsnow say how an entry is written and point atstan_build_info()for an example.The second commit goes over every internal roxygen block and comment the v1.0 stages wrote, so each has a title, its fields, and wording that stands without the design doc. A few names changed with it: the record checkers are
assert_record_*(),assess_build()takesexpectedrather thanwanted,constructor_message()isbuild_message(), andtest-model-guard.Ristest-model-stale-executable.R.install_executable()moved fromutils.Rtobuild.Rbeside its only caller, with its tests.The third commit gives two groups of functions files of their own. Everything that talks to stanc, from
model.Randutils.R, is nowR/stanc.R; the Rcpp code behind$init_model_methods()and$expose_functions(), fromutils.R, is nowR/expose.R. Two exe-info helpers join their only caller inbuild_record.R,tbb_path()moves fromrun.Rtopath.R, andcpp_opts.Ris renamedcpp_options.Rafter the argument it handles. Every function's body is unchanged; the one non-move edit is an error inargs.Rthat still told the user to run$compile(). The fourth commit moves the unit tests of those functions into test-stanc.R and test-build-record.R to match.Also removes
tests/testthat/testthat-problems.rds, committed by accident in #1277, and ignores testthat's problem files from now on.Not in this PR: the development-version NEWS entries that still describe
$compile(), which the pre-RC NEWS pass rewrites.Targeted test files run locally: test-build-info, test-build-record, test-build-assess, test-build-record-compile, test-build-install, test-model-stale-executable, test-model-rebuild-rules, test-model-compile, test-model-expose-functions, test-model-methods, test-cpp-options, test-utils, test-stanc, all passing.
Copyright and Licensing
Please list the copyright holder for the work you are submitting
(this will be you or your assignee, such as a university or company):
Jonah Gabry
By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the following licenses: