search

stan-dev / rstan

RStan, the R interface to Stan
https://mc-stan.org
1.03k stars 264 forks source link

CFLAGS from R_MAKEVARS_USER no longer respected when compiling stan model #621

Open aaronjg opened 5 years ago

aaronjg commented 5 years ago

Summary:

It should be possible to set a custom Makevars file using the environment variable R_MAKEVARS_USER. This worked up until 2.18.1 (GitRev 398abb20baf6) , however it seems to be broken in 2.18.2

Description:

I use the R_MAKEVARS_USER file to set custom CFLAGS when compiling my stan models. In particular, I like to use -O0 when I am debugging the nuts and bolts of my model, and then use -O2 when I am actually running my model. In both cases, I leave out the '-g' flag since the debugging symbols slow down compiling, and are not particularly useful to me.

Reproducible Steps:

If I create a Makevar file:

CFLAGS =              -O0 -pipe  -std=gnu99 
CXXFLAGS =            -O0 -pipe -Wno-unused -std=c++11

CXX=g++

And set the appropriate environment variable, then look at the process as it is compiling, I see that my CFLAGS are ignored:

$ps aux|grep cc1
aaronjg  22382  0.0  1.0 135924 85424 pts/9    R+   22:31   0:00 /usr/lib/gcc/x86_64-linux-gnu/5/cc1plus -quiet -I /usr/share/R/include -I /home/aaronjg/R/x86_64-pc-linux-gnu-library/3.5/Rcpp/include/ -I /home/aaronjg/R/x86_64-pc-linux-gnu-library/3.5/RcppEigen/include/ -I /home/aaronjg/R/x86_64-pc-linux-gnu-library/3.5/RcppEigen/include/unsupported -I /home/aaronjg/R/x86_64-pc-linux-gnu-library/3.5/BH/include -I /home/aaronjg/R/x86_64-pc-linux-gnu-library/3.5/StanHeaders/include/src/ -I /home/aaronjg/R/x86_64-pc-linux-gnu-library/3.5/StanHeaders/include/ -I /home/aaronjg/R/x86_64-pc-linux-gnu-library/3.5/rstan/include -imultiarch x86_64-linux-gnu -D_GNU_SOURCE -D NDEBUG -D EIGEN_NO_DEBUG -D BOOST_DISABLE_ASSERTS -D _FORTIFY_SOURCE=2 file573053c287bf.cpp -quiet -dumpbase file573053c287bf.cpp -mtune=generic -march=x86-64 -auxbase-strip file573053c287bf.o -g -g -O2 -Wformat=1 -Werror=format-security -Wdate-time -std=gnu++14 -fpic -fstack-protector-strong -fstack-protector-strong -Wformat -Wformat-security -o /tmp/ccJvwHtD.s

This had worked fine in rstan 2.18.1 GitRev 398abb20baf6 (I have not yet tested it in the 2.18.1 final release)

If I set them as PKG_CFLAGS, 

CFLAGS =              -O0 -pipe  -std=gnu99 
CXXFLAGS =            -O0 -pipe -Wno-unused -std=c++11

CXX=g++

PKG_CFLAGS =              -O0 -pipe  -std=gnu99 
PKG_CXXFLAGS =            -O0 -pipe -Wno-unused -std=c++11

they are included, but overridden by other CFLAGS that are coming from somewhere else.

aaronjg  22222  162  2.8 335968 227412 pts/9   R+   22:30   0:01 /usr/lib/gcc/x86_64-linux- gnu/5/cc1plus -quiet -I /usr/share/R/include -I /home/aaronjg/R/x86_64-pc-linux-gnu-library /3.5/Rcpp/include/ -I /home/aaronjg/R/x86_64-pc-linux-gnu-library/3.5/RcppEigen/include/ -I /home/aaronjg/R/x86_64-pc-linux-gnu-library/3.5/RcppEigen/include/unsupported -I /home/aaronjg/R/x86_64-pc-linux-gnu-library/3.5/BH/include -I /home/aaronjg/R/x86_64-pc-linux-gnu-library/3.5/StanHeaders/include/src/ -I /home/aaronjg/R/x86_64-pc-linux-gnu-library/3.5/StanHeaders/include/ -I /home/aaronjg/R/x86_64-pc-linux-gnu-library/3.5/rstan/include -imultiarch x86_64-linux-gnu -D_GNU_SOURCE -D NDEBUG -D EIGEN_NO_DEBUG -D BOOST_DISABLE_ASSERTS -D _FORTIFY_SOURCE=2 file567c1917208.cpp -quiet -dumpbase file567c1917208.cpp -mtune=generic -march=x86-64 -auxbase-strip file567c1917208.o -g -g -O0 -O2 -Wno-unused -Wformat=1 -Werror=format-security -Wdate-time -std=gnu++14 -std=c++11 -fpic -fstack-protector-strong -fstack-protector-strong -Wformat -Wformat-security -o -

RStan Version:

2.18.2

R Version:

R version 3.5.3 (2019-03-11)

Operating System:

Ubuntu 16.04

bgoodri commented 5 years ago

This is strange. I am not aware of us doing anything with CFLAGS and nothing comes up when I grep for it in the rstan repo. There is some stuff in makefile_op.R but I don't think anything is calling the functions in it, except for get_makefile_txt which just stores some things after the Stan model has been compiled. Bisecting would be a mess. I'll think about it.

aaronjg commented 5 years ago

Hmm. There may be a way to bisect it with -frecord-gcc-switches, however I am currently having some trouble compiling the latest version of rstan due to some (likely unrelated) issues with the vignettes. I looked through diffs though, and it looks like there were some changes at 41c085f35 that impacted the compiler flags. Maybe that was it?

bgoodri commented 5 years ago

Can you try it after commenting out the make_makevars(DIR = tempdir()) line of cxxfunplus.R ? That was put in when we were transitioning on CRAN to C++14 and might not be necessary anymore now that we are faking as if we are using C++11 on Windows and then adding -std=c++1y to override that.

On Sat, Apr 6, 2019 at 11:26 AM aaronjg notifications@github.com wrote:

Hmm. There may be a way to bisect it with -frecord-gcc-switches, however I am currently having some trouble compiling the latest version of rstan due to some (likely unrelated) issues with the vignettes. I looked through diffs though, and it looks like there were some changes at 41c085f https://github.com/stan-dev/rstan/commit/41c085f35a235d9059a69ad33017b0bd170e1a5e that impacted the compiler flags. Maybe that was it?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/stan-dev/rstan/issues/621#issuecomment-480512807, or mute the thread https://github.com/notifications/unsubscribe-auth/ADOrqrGOk_Pss-pQJi_pd-5y1dTvs8prks5veLyMgaJpZM4cf-Bw .

aaronjg commented 5 years ago

Interesting. It looks like that line has already been removed in the latest git version. The make_makevars function could also probably be removed. When I tried with the latest git, and change my CXXFLAGS to CXX14FLAGS, it seems to be working as before.

bgoodri commented 5 years ago

OK, we're just sorting out the monitor PR and then rstan 2.19 will be ready to release.

On Sat, Apr 6, 2019 at 1:07 PM aaronjg notifications@github.com wrote:

Interesting. It looks like that line has already been removed in the latest git version. The make_makevars function could also probably be removed. When I tried with the latest git, and change my CXXFLAGS to CXX14FLAGS, it seems to be working as before.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/stan-dev/rstan/issues/621#issuecomment-480520567, or mute the thread https://github.com/notifications/unsubscribe-auth/ADOrqsNm68lZRU42YPAZEAABzKABaGsbks5veNRfgaJpZM4cf-Bw .

aaronjg commented 5 years ago

Sounds good. Can the make_makevars function be eliminated at this point? It looks like it isn't called anywhere in the code as of now.

bgoodri commented 5 years ago

Yeah. I'll do it.

On Sat, Apr 6, 2019 at 5:24 PM aaronjg notifications@github.com wrote:

Sounds good. Can the make_makevars function be eliminated at this point? It looks like it isn't called anywhere in the code as of now.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/stan-dev/rstan/issues/621#issuecomment-480538872, or mute the thread https://github.com/notifications/unsubscribe-auth/ADOrqgrQ9n9m6PsmJSzXO-8mx9X_F8xyks5veRCMgaJpZM4cf-Bw .