From fb734eaf59efd63c907bba057e91e9b7c651fc93 Mon Sep 17 00:00:00 2001 From: Brian Ward Date: Tue, 7 May 2024 14:17:56 -0400 Subject: [PATCH] Support for USER_HEADER in build system (#233) * Support for USER_HEADER in build system * Skip in Rust loading tests * Document in 'Customizing BridgeStan' * Fix 'make clean' deleting test file --- .gitignore | 2 + Makefile | 64 +++++++++++++++++++----------- docs/getting-started.rst | 13 ++++++ python/test/test_compile.py | 19 +++++++++ rust/tests/loading.rs | 10 +++-- test_models/external/external.stan | 18 +++++++++ test_models/external/make_odds.hpp | 5 +++ 7 files changed, 103 insertions(+), 28 deletions(-) create mode 100644 test_models/external/external.stan create mode 100644 test_models/external/make_odds.hpp diff --git a/.gitignore b/.gitignore index ac21f96d..464f4d7f 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,8 @@ c-example/example c-example/example_static *.exe +make/local + # Rust rust/target/ diff --git a/Makefile b/Makefile index d729c373..09b6788a 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -## include paths +# include paths BS_ROOT ?= . # user config @@ -10,21 +10,21 @@ STANC ?= $(BS_ROOT)/bin/stanc$(EXE) MATH ?= $(STAN)lib/stan_math/ RAPIDJSON ?= $(STAN)lib/rapidjson_1.1.0/ -## required C++ includes +# required C++ includes INC_FIRST ?= -I $(STAN)src -I $(RAPIDJSON) -## makefiles needed for math library +# makefiles needed for math library include $(MATH)make/compiler_flags include $(MATH)make/libraries -## Set -fPIC globally since we're always building a shared library -CXXFLAGS += -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -CXXFLAGS_SUNDIALS += -fPIC -CPPFLAGS += -DBRIDGESTAN_EXPORT +# Set -fPIC globally since we're always building a shared library +override CXXFLAGS += -fPIC -fvisibility=hidden -fvisibility-inlines-hidden +override CXXFLAGS_SUNDIALS += -fPIC +override CPPFLAGS += -DBRIDGESTAN_EXPORT -## set flags for stanc compiler (math calls MIGHT? set STAN_OPENCL) +# set flags for stanc compiler (math calls MIGHT? set STAN_OPENCL) ifdef STAN_OPENCL - STANCFLAGS += --use-opencl + override STANCFLAGS += --use-opencl STAN_FLAG_OPENCL=_opencl else STAN_FLAG_OPENCL= @@ -35,7 +35,7 @@ else STAN_FLAG_THREADS= endif ifdef BRIDGESTAN_AD_HESSIAN - CPPFLAGS += -DSTAN_MODEL_FVAR_VAR -DBRIDGESTAN_AD_HESSIAN + override CPPFLAGS += -DSTAN_MODEL_FVAR_VAR -DBRIDGESTAN_AD_HESSIAN STAN_FLAG_HESS=_adhessian else STAN_FLAG_HESS= @@ -51,16 +51,31 @@ $(BRIDGE_O) : $(BRIDGE_DEPS) @mkdir -p $(dir $@) $(COMPILE.cpp) $(OUTPUT_OPTION) $(LDLIBS) $< -## generate .hpp file from .stan file using stanc + +ifneq ($(findstring allow-undefined,$(STANCFLAGS)),) +USER_HEADER ?= $(dir $(MAKECMDGOALS))user_header.hpp +USER_INCLUDE = -include $(USER_HEADER) +# Give a better error message if the USER_HEADER is not found +$(USER_HEADER): + @echo 'ERROR: Missing user header.' + @echo 'Because --allow-undefined is set, we need a C++ header file to include.' + @echo 'We tried to find the user header at:' + @echo ' $(USER_HEADER)' + @echo '' + @echo 'You can also set the USER_HEADER variable to the path of your C++ file.' + @exit 1 +endif + +# generate .hpp file from .stan file using stanc %.hpp : %.stan $(STANC) @echo '' @echo '--- Translating Stan model to C++ code ---' $(STANC) $(STANCFLAGS) --o=$(subst \,/,$@) $(subst \,/,$<) -%.o : %.hpp +%.o : %.hpp $(USER_HEADER) @echo '' @echo '--- Compiling C++ code ---' - $(COMPILE.cpp) -x c++ -o $(subst \,/,$*).o $(subst \,/,$<) + $(COMPILE.cpp) $(USER_INCLUDE) -x c++ -o $(subst \,/,$*).o $(subst \,/,$<) %_model.so : %.o $(BRIDGE_O) $(SUNDIALS_TARGETS) $(MPI_TARGETS) $(TBB_TARGETS) @echo '' @@ -71,22 +86,23 @@ $(BRIDGE_O) : $(BRIDGE_DEPS) docs: $(MAKE) -C docs/ html +# build all test models at once +ALL_TEST_MODEL_NAMES = $(patsubst $(BS_ROOT)/test_models/%/, %, $(sort $(dir $(wildcard $(BS_ROOT)/test_models/*/)))) +# these are for compilation testing in the interfaces +SKIPPED_TEST_MODEL_NAMES = syntax_error external +TEST_MODEL_NAMES := $(filter-out $(SKIPPED_TEST_MODEL_NAMES), $(ALL_TEST_MODEL_NAMES)) +TEST_MODEL_LIBS = $(join $(addprefix $(BS_ROOT)/test_models/, $(TEST_MODEL_NAMES)), $(addsuffix _model.so, $(addprefix /, $(TEST_MODEL_NAMES)))) + +.PHONY: test_models +test_models: $(TEST_MODEL_LIBS) + .PHONY: clean clean: $(RM) $(SRC)/*.o $(RM) test_models/**/*.so - $(RM) test_models/**/*.hpp + $(RM) $(join $(addprefix $(BS_ROOT)/test_models/, $(TEST_MODEL_NAMES)), $(addsuffix .hpp, $(addprefix /, $(TEST_MODEL_NAMES)))) $(RM) bin/stanc$(EXE) - -# build all test models at once -TEST_MODEL_NAMES = $(patsubst $(BS_ROOT)/test_models/%/, %, $(sort $(dir $(wildcard $(BS_ROOT)/test_models/*/)))) -TEST_MODEL_NAMES := $(filter-out syntax_error, $(TEST_MODEL_NAMES)) -TEST_MODEL_LIBS = $(join $(addprefix test_models/, $(TEST_MODEL_NAMES)), $(addsuffix _model.so, $(addprefix /, $(TEST_MODEL_NAMES)))) - -.PHONY: test_models -test_models: $(TEST_MODEL_LIBS) - .PHONY: stan-update stan-update-version stan-update: git submodule update --init --recursive @@ -112,7 +128,7 @@ format: # R seems to have no good formatter that doesn't choke on doc comments # Rscript -e 'formatR::tidy_dir("R/", recursive=TRUE)' || $(BS_FORMAT_IGNOREABLE) -## print value of makefile variable (e.g., make print-TBB_TARGETS) +# print value of makefile variable (e.g., make print-TBB_TARGETS) .PHONY: print-% print-% : ; @echo $* = $($*) ; diff --git a/docs/getting-started.rst b/docs/getting-started.rst index 24ac4b33..b01ea0fc 100644 --- a/docs/getting-started.rst +++ b/docs/getting-started.rst @@ -165,6 +165,19 @@ to set :makevar:`CPPFLAGS` in :file:`make/local`: CPPFLAGS+=-DSTAN_MATH_CONSTRAINT_TOLERANCE=1e-5 + +Using External C++ Code +_______________________ + +BridgeStan supports the same `capability to plug in external C++ code as CmdStan `_. + +Namely, you can declare a function in your Stan model and then define it in a separate C++ file. +This requires passing the ``--allow-undefined`` flag to the Stan compiler when building your model. +The :makevar:`USER_HEADER` variable must point to the C++ file containing the function definition. +By default, this will be the file :file:`user_header.hpp` in the same directory as the Stan model. + +For a more complete example, consult the `CmdStan documentation `_. + Using Older Stan Versions __________________________ diff --git a/python/test/test_compile.py b/python/test/test_compile.py index 870b5b84..10e7d12e 100644 --- a/python/test/test_compile.py +++ b/python/test/test_compile.py @@ -24,6 +24,25 @@ def test_compile_good(): assert "STAN_THREADS=true" in model.model_info() +def test_compile_user_header(): + stanfile = STAN_FOLDER / "external" / "external.stan" + lib = bs.compile.generate_so_name(stanfile) + lib.unlink(missing_ok=True) + + with pytest.raises(RuntimeError, match=r"declared without specifying a definition"): + bs.compile_model(stanfile) + + with pytest.raises(RuntimeError, match=r"USER_HEADER"): + bs.compile_model(stanfile, stanc_args=["--allow-undefined"]) + + header = stanfile.parent / "make_odds.hpp" + res = bs.compile_model( + stanfile, stanc_args=["--allow-undefined"], make_args=[f"USER_HEADER={header}"] + ) + assert lib.samefile(res) + assert lib.exists() + + def test_compile_bad_ext(): not_stanfile = STAN_FOLDER / "multi" / "multi.data.json" with pytest.raises(ValueError, match=r".stan"): diff --git a/rust/tests/loading.rs b/rust/tests/loading.rs index e6b8c593..03728063 100644 --- a/rust/tests/loading.rs +++ b/rust/tests/loading.rs @@ -8,6 +8,8 @@ use std::{ use bridgestan::Model; +const EXCLUDED_MODELS: [&str; 4] = ["logistic", "regression", "syntax_error", "external"]; + #[test] fn create_all_serial() { let base = model_dir(); @@ -15,7 +17,7 @@ fn create_all_serial() { let path = path.unwrap().path(); let name = path.file_name().unwrap().to_str().unwrap(); - if (name == "logistic") | (name == "regression") | (name == "syntax_error") { + if EXCLUDED_MODELS.contains(&name) { continue; } @@ -44,7 +46,7 @@ fn create_all_late_drop_fwd() { let handles: Vec<_> = names .into_iter() - .filter(|name| (name != "logistic") & (name != "regression") & (name != "syntax_error")) + .filter(|name| !EXCLUDED_MODELS.contains(&name.as_str())) .map(|name| { let (lib, data) = get_model(&name); let Ok(model) = Model::new(&lib, data.as_ref(), 42) else { @@ -74,7 +76,7 @@ fn create_all_thread_serial() { names.into_iter().for_each(|name| { spawn(move || { - if (&name == "logistic") | (&name == "regression") | (&name == "syntax_error") { + if EXCLUDED_MODELS.contains(&name.as_str()) { return; } @@ -108,7 +110,7 @@ fn create_all_parallel() { .into_iter() .map(|name| { spawn(move || { - if (&name == "logistic") | (&name == "regression") | (&name == "syntax_error") { + if EXCLUDED_MODELS.contains(&name.as_str()) { return; } diff --git a/test_models/external/external.stan b/test_models/external/external.stan new file mode 100644 index 00000000..5281c411 --- /dev/null +++ b/test_models/external/external.stan @@ -0,0 +1,18 @@ +functions { + real make_odds(data real theta); +} +data { + int N; + array[N] int y; +} +parameters { + real theta; +} +model { + theta ~ beta(1, 1); // uniform prior on interval 0, 1 + y ~ bernoulli(theta); +} +generated quantities { + real odds; + odds = make_odds(theta); +} diff --git a/test_models/external/make_odds.hpp b/test_models/external/make_odds.hpp new file mode 100644 index 00000000..00a49417 --- /dev/null +++ b/test_models/external/make_odds.hpp @@ -0,0 +1,5 @@ +#include + +double make_odds(const double& theta, std::ostream *pstream__) { + return theta / (1 - theta); +}