Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for USER_HEADER in build system #233

Merged
merged 4 commits into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
c-example/example
c-example/example_static
*.exe
make/local


# Rust
rust/target/
Expand Down
64 changes: 40 additions & 24 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
## include paths
# include paths
BS_ROOT ?= .

# user config
Expand All @@ -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=
Expand All @@ -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=
Expand All @@ -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 ''
Expand All @@ -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
Expand All @@ -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 $* = $($*) ;

Expand Down
13 changes: 13 additions & 0 deletions docs/getting-started.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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 <https://mc-stan.org/docs/cmdstan-guide/external_code.html>`_.

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.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we say explicitly that it should be a C++ header file? We only reference it being a .hpp file in the makevar variable name and the example file name.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It technically does not need to be a header (or even a .hpp, in theory a .txt would also work, since the compiler is just pasting it in), as long as it's code that can be included into the model's translation unit it would be fine.

Personally I think the name USER_HEADER is evocative enough

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 <https://mc-stan.org/docs/cmdstan-guide/external_code.html>`_.

Using Older Stan Versions
__________________________

Expand Down
19 changes: 19 additions & 0 deletions python/test/test_compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"):
Expand Down
10 changes: 6 additions & 4 deletions rust/tests/loading.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,16 @@ use std::{

use bridgestan::Model;

const EXCLUDED_MODELS: [&str; 4] = ["logistic", "regression", "syntax_error", "external"];

#[test]
fn create_all_serial() {
let base = model_dir();
for path in base.read_dir().unwrap() {
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;
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down
18 changes: 18 additions & 0 deletions test_models/external/external.stan
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
functions {
real make_odds(data real theta);
}
data {
int<lower=0> N;
array[N] int<lower=0, upper=1> y;
}
parameters {
real<lower=0, upper=1> theta;
}
model {
theta ~ beta(1, 1); // uniform prior on interval 0, 1
y ~ bernoulli(theta);
}
generated quantities {
real odds;
odds = make_odds(theta);
}
5 changes: 5 additions & 0 deletions test_models/external/make_odds.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#include <ostream>

double make_odds(const double& theta, std::ostream *pstream__) {
return theta / (1 - theta);
}
Loading