-
-
Notifications
You must be signed in to change notification settings - Fork 190
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
Allow users to specify lib & include paths for all dependencies #2834
Changes from 29 commits
e0e1dfd
dd09c41
792bf0b
a4d8d87
10074f0
41f809e
5167814
b573397
f4573e3
7c89eb4
d155b07
faf23c5
01a560b
7477808
e609398
7b0d205
a33c785
64c620e
22eeb6c
64ac7bb
c5def60
b6aae98
2b69c06
079119b
a257548
4407ba4
2930adb
2e827b9
1cbd670
f9500bb
873d86f
87630b0
a3892ed
9c827fe
95839e4
c3b7b2a
6e0d75a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,103 @@ | ||
name: System Libraries | ||
|
||
on: | ||
pull_request: | ||
branches: [ develop, master ] | ||
push: | ||
branches: [ develop ] | ||
paths-ignore: | ||
- 'doygen/**' | ||
- 'hooks/**' | ||
- 'licenses/**' | ||
- 'LICENSE.md' | ||
- 'README.md' | ||
- 'RELEASE-NOTES.txt' | ||
permissions: | ||
contents: read | ||
|
||
jobs: | ||
system-libraries-fwd-mix: | ||
permissions: | ||
actions: write # for n1hility/cancel-previous-runs to create & stop workflow runs | ||
contents: read # for actions/checkout to fetch code | ||
name: System Library Linking - fwd and non-fun mix tests | ||
runs-on: ubuntu-22.04 | ||
|
||
steps: | ||
- uses: n1hility/cancel-previous-runs@v2 | ||
with: | ||
token: ${{ secrets.GITHUB_TOKEN }} | ||
if: "!startsWith(github.ref, 'refs/tags/') && github.ref != 'refs/heads/master' && github.ref != 'refs/heads/develop'" | ||
|
||
- uses: actions/checkout@v3 | ||
- uses: actions/setup-python@v4 | ||
- name: Install system libraries | ||
run: | | ||
# Eigen version in 22.04 repos (3.4) not yet compatible with Stan, | ||
# manually install 3.3.9 from Debian repositories | ||
wget http://ftp.de.debian.org/debian/pool/main/e/eigen3/libeigen3-dev_3.3.9-2_all.deb | ||
sudo dpkg -i ./libeigen3-dev_3.3.9-2_all.deb | ||
sudo apt-get install -y libboost-all-dev libtbb-dev libsundials-dev ocl-icd-libopencl1 ocl-icd-opencl-dev opencl-c-headers opencl-clhpp-headers | ||
echo "EIGEN_INC=/usr/include/eigen3/" >> make/local | ||
echo "BOOST_INC=/usr/include/" >> make/local | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [BOOST_INC]: this should just be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is missing the proper include directory. |
||
echo "TBB_INC=/usr/include/" >> make/local | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [TBB_INC]: this is the same as the |
||
echo "TBB_LIB=/usr/lib/86_64-linux-gnu" >> make/local | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this looks wrong for the TBB include library (By inspection). An example from doxygen:
I'd expect there to be a TBB somewhere. |
||
echo "SUNDIALS_INC=/usr/include" >> make/local | ||
echo "SUNDIALS_LIB=/usr/lib/x86_64-linux-gnu" >> make/local | ||
echo "OPENCL_INC=/usr/include/" >> make/local | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [OPENCL_INC]: this should just be |
||
echo "OPENCL_LIB=/usr/lib/x86_64-linux-gnu" >> make/local | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [OPENCL_LIB]: should we just use the existing LDLIBS_OPENCL variable? You can just specify the link libraries directly. |
||
echo "STAN_OPENCL=true" >> make/local | ||
- name: Run fwd and non-fun mix tests | ||
run: | | ||
python runTests.py -j2 test/unit/math/fwd | ||
python runTests.py -j2 test/unit/math/mix/core | ||
python runTests.py -j2 test/unit/math/mix/functor | ||
python runTests.py -j2 test/unit/math/mix/meta | ||
python runTests.py -j2 test/unit/math/mix/prob | ||
python runTests.py -j2 test/unit/math/mix/*_test.cpp | ||
- name: Upload gtest_output xml | ||
uses: actions/upload-artifact@v3 | ||
if: failure() | ||
with: | ||
name: gtest_outputs_xml | ||
path: '**/*_test.xml' | ||
system-libraries-rev: | ||
permissions: | ||
actions: write # for n1hility/cancel-previous-runs to create & stop workflow runs | ||
contents: read # for actions/checkout to fetch code | ||
name: System Library Linking - rev tests | ||
runs-on: ubuntu-22.04 | ||
|
||
steps: | ||
- uses: n1hility/cancel-previous-runs@v2 | ||
with: | ||
token: ${{ secrets.GITHUB_TOKEN }} | ||
if: "!startsWith(github.ref, 'refs/tags/') && github.ref != 'refs/heads/master' && github.ref != 'refs/heads/develop'" | ||
|
||
- uses: actions/checkout@v3 | ||
- uses: actions/setup-python@v4 | ||
- name: Install system libraries | ||
run: | | ||
# Eigen version in 22.04 repos (3.4) not yet compatible with Stan, | ||
# manually install 3.3.9 from Debian repositories | ||
wget http://ftp.de.debian.org/debian/pool/main/e/eigen3/libeigen3-dev_3.3.9-2_all.deb | ||
sudo dpkg -i ./libeigen3-dev_3.3.9-2_all.deb | ||
sudo apt-get install -y libboost-all-dev libtbb-dev libsundials-dev ocl-icd-libopencl1 ocl-icd-opencl-dev opencl-c-headers opencl-clhpp-headers | ||
echo "EIGEN_INC=/usr/include/eigen3/" >> make/local | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are these repeated from above? |
||
echo "BOOST_INC=/usr/include/" >> make/local | ||
echo "TBB_INC=/usr/include/" >> make/local | ||
echo "TBB_LIB=/usr/lib/86_64-linux-gnu" >> make/local | ||
echo "SUNDIALS_INC=/usr/include" >> make/local | ||
echo "SUNDIALS_LIB=/usr/lib/x86_64-linux-gnu" >> make/local | ||
echo "OPENCL_INC=/usr/include/" >> make/local | ||
echo "OPENCL_LIB=/usr/lib/x86_64-linux-gnu" >> make/local | ||
echo "STAN_OPENCL=true" >> make/local | ||
- name: Run rev tests | ||
run: | | ||
python runTests.py -j2 test/unit/math/rev | ||
- name: Upload gtest_output xml | ||
uses: actions/upload-artifact@v3 | ||
if: failure() | ||
with: | ||
name: gtest_outputs_xml | ||
path: '**/*_test.xml' |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -105,7 +105,7 @@ endif | |
O ?= 3 | ||
|
||
## setup includes | ||
INC ?= $(INC_FIRST) -I $(if $(MATH),$(MATH),.) -I $(EIGEN) -I $(BOOST) $(INC_SUNDIALS) | ||
INC ?= $(INC_FIRST) -I $(if $(MATH),$(MATH),.) -include stan/math/version.hpp | ||
INC_SUNDIALS ?= -I $(SUNDIALS)/include -I $(SUNDIALS)/src/sundials | ||
INC_GTEST ?= -I $(GTEST)/include -I $(GTEST) | ||
|
||
|
@@ -117,10 +117,33 @@ CPPFLAGS_SUNDIALS ?= -DNO_FPRINTF_OUTPUT $(CPPFLAGS_OPTIM_SUNDIALS) $(CXXFLAGS_F | |
|
||
## setup compiler flags | ||
CXXFLAGS_LANG ?= -std=c++1y | ||
#CXXFLAGS_BOOST ?= | ||
CXXFLAGS_SUNDIALS ?= -pipe $(CXXFLAGS_OPTIM_SUNDIALS) $(CPPFLAGS_FLTO_SUNDIALS) | ||
#CXXFLAGS_GTEST | ||
|
||
ifdef BOOST_INC | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should remove this variable in favor of just using the Let's chat if you need help understanding how make works. (The doc is really tough to understand, so happy to walk you through it.) There are two things that really stand out that we shouldn't do here:
|
||
CXXFLAGS_BOOST ?= -I $(BOOST_INC) | ||
else | ||
CXXFLAGS_BOOST ?= -I $(BOOST) | ||
endif | ||
|
||
ifdef SUNDIALS_LIB | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same comment as above |
||
LDFLAGS_SUNDIALS ?= -L $(SUNDIALS_LIB) -lsundials_cvodes -lsundials_idas -lsundials_kinsol -lsundials_nvecserial | ||
ifdef SUNDIALS_INC | ||
CXXFLAGS_SUNDIALS += -I $(SUNDIALS_INC) | ||
endif | ||
else | ||
CXXFLAGS_SUNDIALS += $(INC_SUNDIALS) | ||
endif | ||
|
||
ifdef SUNDIALS_INTERFACE_OLD | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ??? what's this looks like some testing that got included. |
||
CXXFLAGS_SUNDIALS += -DSUNDIALS_INTERFACE_OLD | ||
endif | ||
|
||
ifdef EIGEN_INC | ||
CXXFLAGS_EIGEN ?= -I $(EIGEN_INC) | ||
else | ||
CXXFLAGS_EIGEN ?= -I $(EIGEN) | ||
endif | ||
|
||
## These are the other compiler flags that can be set. | ||
## | ||
|
@@ -218,7 +241,15 @@ ifdef STAN_OPENCL | |
CPPFLAGS_OPENCL ?= -DSTAN_OPENCL -DOPENCL_DEVICE_ID=$(OPENCL_DEVICE_ID) -DOPENCL_PLATFORM_ID=$(OPENCL_PLATFORM_ID) | ||
CPPFLAGS_OPENCL += -DCL_HPP_TARGET_OPENCL_VERSION=120 -DCL_HPP_MINIMUM_OPENCL_VERSION=120 | ||
CPPFLAGS_OPENCL += -DCL_HPP_ENABLE_EXCEPTIONS -Wno-ignored-attributes | ||
|
||
ifdef OPENCL_LIB | ||
LDFLAGS_OPENCL ?= -L $(OPENCL_LIB) | ||
endif | ||
ifdef OPENCL_INC | ||
CXXFLAGS_OPENCL ?= -I $(OPENCL_INC) | ||
else | ||
CXXFLAGS_OPENCL ?= -I $(OPENCL) | ||
endif | ||
|
||
ifdef INTEGRATED_OPENCL | ||
CPPFLAGS_OPENCL += -DINTEGRATED_OPENCL=1 | ||
|
@@ -312,9 +343,9 @@ ifdef STAN_MPI | |
CXXFLAGS_MPI ?= -Wno-delete-non-virtual-dtor | ||
endif | ||
|
||
CXXFLAGS += $(CXXFLAGS_LANG) $(CXXFLAGS_OS) $(CXXFLAGS_WARNINGS) $(CXXFLAGS_BOOST) $(CXXFLAGS_EIGEN) $(CXXFLAGS_OPENCL) $(CXXFLAGS_MPI) $(CXXFLAGS_THREADS) $(CXXFLAGS_TBB) $(CXXFLAGS_FLTO) $(CXXFLAGS_OPTIM) $(CXXFLAGS_NO_RANGE_CHECKS) -O$(O) $(INC) | ||
CXXFLAGS += $(CXXFLAGS_LANG) $(CXXFLAGS_OS) $(CXXFLAGS_WARNINGS) $(CXXFLAGS_BOOST) $(CXXFLAGS_EIGEN) $(CXXFLAGS_OPENCL) $(CXXFLAGS_MPI) $(CXXFLAGS_THREADS) $(CXXFLAGS_TBB) $(CXXFLAGS_SUNDIALS) $(CXXFLAGS_FLTO) $(CXXFLAGS_OPTIM) $(CXXFLAGS_NO_RANGE_CHECKS) -O$(O) $(INC) | ||
CPPFLAGS += $(CPPFLAGS_LANG) $(CPPFLAGS_OS) $(CPPFLAGS_WARNINGS) $(CPPFLAGS_BOOST) $(CPPFLAGS_EIGEN) $(CPPFLAGS_OPENCL) $(CPPFLAGS_TBB) $(CPPFLAGS_MPI) $(CPPFLAGS_TBB) $(CPPFLAGS_FLTO) $(CPPFLAGS_OPTIM) $(CXXFLAGS_NO_RANGE_CHECKS) | ||
LDFLAGS += $(LDFLAGS_LANG) $(LDFLAGS_OS) $(LDFLAGS_WARNINGS) $(LDFLAGS_BOOST) $(LDFLAGS_EIGEN) $(LDFLAGS_OPENCL) $(LDFLAGS_MPI) $(LDFLAGS_TBB) $(LDFLAGS_FLTO) $(LDFLAGS_OPTIM) | ||
LDFLAGS += $(LDFLAGS_LANG) $(LDFLAGS_OS) $(LDFLAGS_WARNINGS) $(LDFLAGS_BOOST) $(LDFLAGS_EIGEN) $(LDFLAGS_OPENCL) $(LDFLAGS_MPI) $(LDFLAGS_TBB) $(LDFLAGS_SUNDIALS) $(LDFLAGS_FLTO) $(LDFLAGS_OPTIM) | ||
LDLIBS += $(LDLIBS_LANG) $(LDLIBS_OS) $(LDLIBS_WARNINGS) $(LDLIBS_BOOST) $(LDLIBS_EIGEN) $(LDLIBS_OPENCL) $(LDLIBS_MPI) $(LDLIBS_TBB) | ||
|
||
.PHONY: print-compiler-flags | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,7 +23,7 @@ CPPLINT ?= $(MATH)lib/cpplint_1.4.5 | |
# Fortran bindings which we do not need for stan-math. Thus these targets | ||
# are ignored here. This convention was introduced with 4.0. | ||
## | ||
|
||
ifndef SUNDIALS_LIB | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this shouldn't be necessary. |
||
SUNDIALS_CVODES := $(patsubst %.c,%.o,\ | ||
$(wildcard $(SUNDIALS)/src/cvodes/*.c) \ | ||
$(filter-out $(SUNDIALS)/src/sundials/sundials_profiler.c, $(wildcard $(SUNDIALS)/src/sundials/*.c)) \ | ||
|
@@ -87,7 +87,9 @@ $(STAN_SUNDIALS_HEADERS) : $(LIBSUNDIALS) | |
clean-sundials: | ||
@echo ' cleaning sundials targets' | ||
$(RM) $(wildcard $(sort $(SUNDIALS_CVODES) $(SUNDIALS_IDAS) $(SUNDIALS_KINSOL) $(SUNDIALS_NVECSERIAL) $(LIBSUNDIALS))) | ||
|
||
else | ||
LIBSUNDIALS := $(SUNDIALS_LIB)/libsundials_nvecserial.a $(SUNDIALS_LIB)/libsundials_cvodes.a $(SUNDIALS_LIB)/libsundials_idas.a $(SUNDIALS_LIB)/libsundials_kinsol.a | ||
WardBrian marked this conversation as resolved.
Show resolved
Hide resolved
|
||
endif | ||
|
||
############################################################ | ||
# TBB build rules | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[EIGEN_INC]: We don't need a new variable. This is literally the existing
EIGEN
make variable. Unless we want this to be renaming all the includes.