Very little of this is about getting a vote, there was a discussion about how forcing libc++ use on Ubuntu can cause intractable build problems (because the Ubuntu clang packages are incomplete, I think).
The current build of R 3.5.1, on the Mac uses the following toolchain: this release uses Clang 6.0.0. I build tensorflow with the same compiler. Why not use the same toolchain that is used to compile R to compile Stan?
Clang version 6.0.0 and not the earlier version 5 that you suggested. 6.0.0 is used for compiling the current version of the “R” language (v. 3.5.1) for the Mac.
https://cran.r-project.org/bin/macosx/
R for Mac OS X
Lastest release (3.5.1)
“…this release uses Clang 6.0.0…”
It make the compile of Stan compatible with the compiles of R which is a large compile and if it is stable enough to compile R it should presumably be stable enough for stan.
To use the same version of Clang for R package source compiles and also for tensorflow compiles.
Thank you.
My mistake, it was the original poster, seantalts that suggested clang 5 for Mac.
I’m glad you agree.
The CRAN R people only using clang version 6 for their compiles for Mac and apparently don’t support other compilers for Mac so that is a compelling reason to support this single compiler for Mac for Stan.
I’m assuming code supported by clang 5 will be supported by clang 6 but not necessarily the reverse. But I’m happy to limit our support to the more recent one if that’s what two people are clamoring for.
I think I like @ChrisChiasson’s proposal, @syclik do you like it? I think it fits @bgoodri’s criteria though in some sense we had already resolved this in a different way and moved on to the organizational issues over who is responsible for fixing bugs for which compilers :P And I’m sticking with gcc 5 for now until someone shows me gcc 7 compiling or the RTools people actually successfully complete the change to gcc 7 in the next couple of years.
To flesh out the proposal a little:
Math repo
PRs:
- mac / clang 6 / libc++ Threading
- linux / gcc 5 / stdlibc++ OpenMPI
- mac clang OR linux clang distribution tests (without row vectors)
Additional tests on merge to develop [0]
- linux / gcc 5 / stdlibc++ Threading
- mac / clang OR linux / gcc GPU
- mac clang OR linux clang distribution tests (with row vectors)
I don’t think we have any Windows Math repo tests right now - I think they have been broken for a while? I think the only place the math library gets implicitly tested on Windows is during the CmdStan integration tests. But we’ll be sticking with the RTools compiler (gcc 4.9.3) for those tests.
[0] we can add an interstitial branch if this causes develop to become unstable.
1 Like
That last proposal sounds great. I’d like to request that we write out our policy somewhere.
g++-5 is not used by much of anyone and is not being updated by the gcc developers anymore. Again, I don’t like saying "g++-7 is crashing on one test, so let’s not use it". If it is a g++-7 bug, we should report it. If it is a Stan bug, we should fix it. If it is somewhere in between, we should work around it. I believe stan::math::lgamma actually works fine with g++-7 but the test for it is uncompilable for some reason. But with C++14, there is no reason for stan::math::lgamma to be calling boost::math::lgamma anyway, since there is a std::lgamma.
Earlier you and others claimed it was important to test the default compiler for Ubuntu. gcc+±5 is the default compiler for your Xenial Ubuntu server.
It sounds like you know how to fix it. I retain my position - once someone shows me gcc-7 working on our Math unit tests, I’ll be happy to switch it over in the CI config. Our tests also have to work with gcc-5 (as far as I understand it) because of the way MPI should be compiled with the default system compiler.
That is true, although Xenial is not the current LTS. I’ll look into the bug more.
1 Like
clang > 3.8 also seems broken on Xenial unless I add -stdlib=libc++… any Ubuntu people know if I need to do something besides echo CC=clang++-6.0 > make/local for that to work? Or I guess whenever we use clang on linux it’s okay to use libc++?
I don’t know what’s going on, but -stdlib=libc++ always broke my builds (and the builds of folks who used the models I was working on) on 16.04 and 18.04. I only used whatever the default clang compilers were. Are these new VMs? Or could this possibly be a weird config issue with a computer?
What’s the bug without it? Hard to help when I don’t get this error
Oh, sorry, should have put the error message (here is one of many like it):
In file included from test/unit/math/fwd/mat/fun/to_fvar_test.cpp:1:
In file included from ./stan/math/fwd/mat.hpp:4:
In file included from ./stan/math/fwd/core.hpp:4:
In file included from ./stan/math/fwd/core/fvar.hpp:5:
In file included from ./stan/math/prim/scal/fun/is_nan.hpp:4:
In file included from lib/boost_1.66.0/boost/math/special_functions/fpclassify.hpp:15:
lib/boost_1.66.0/boost/config/no_tr1/cmath.hpp:21:12: fatal error: 'cmath' file not found
# include <cmath>
^~~~~~~
1 error generated.
clang++-6.0 -Wall -I . -isystem lib/eigen_3.3.3 -isystem lib/boost_1.66.0 -isystem lib/sundials_3.1.0/include -std=c++1y -DBOOST_RESULT_OF_USE_TR1 -DBOOST_NO_DECLTYPE -DBOOST_DISABLE_ASSERTS -DBOOST_PHOENIX_NO_VARIADIC_EXPRESSION -Wno-unused-function -Wno-uninitialized -DGTEST_USE_OWN_TR1_TUPLE -DGTEST_HAS_PTHREAD=0 -isystem lib/gtest_1.7.0/include -isystem lib/gtest_1.7.0 -O3 -DGTEST_USE_OWN_TR1_TUPLE -DGTEST_HAS_PTHREAD=0 -isystem lib/gtest_1.7.0/include -isystem lib/gtest_1.7.0 -O3 -DNO_FPRINTF_OUTPUT -pipe -c -o lib/gtest_1.7.0/src/gtest-all.o lib/gtest_1.7.0/src/gtest-all.cc
In file included from lib/gtest_1.7.0/src/gtest-all.cc:39:
lib/gtest_1.7.0/include/gtest/gtest.h:54:10: fatal error: 'limits' file not found
#include <limits>
^~~~~~~~
1 error generated.
<builtin>: recipe for target 'lib/gtest_1.7.0/src/gtest-all.o' failed
make: *** [lib/gtest_1.7.0/src/gtest-all.o] Error 1
This is on Ben Goodrich’s Xenial server, same one as always (but we were always using clang 3.8 in the past).
Those are the errors I get with -stdlib=libc++ hahaha
1 Like
(╯°□°)╯/ ┻━┻)
1 Like
does that server have g++ installed?
Yes, a few of them.
I clone a fresh math repo, here’s my Ubuntu:
krzysztof@sph-3s9bjl2linu:~/packages/math$ cat /etc/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=16.04
DISTRIB_CODENAME=xenial
DISTRIB_DESCRIPTION="Ubuntu 16.04.4 LTS"
I tried the specific clang+±6.0 you have failing and it works:
krzysztof@sph-3s9bjl2linu:~/packages/math$ clang++-6.0 -Wall -I . -isystem lib/eigen_3.3.3 -isystem lib/boost_1.66.0 -isystem lib/sundials_3.1.0/include -std=c++1y -DBOOST_RESULT_OF_USE_TR1 -DBOOST_NO_DECLTYPE -DBOOST_DISABLE_ASSERTS -DBOOST_PHOENIX_NO_VARIADIC_EXPRESSION -Wno-unused-function -Wno-uninitialized -DGTEST_USE_OWN_TR1_TUPLE -DGTEST_HAS_PTHREAD=0 -isystem lib/gtest_1.7.0/include -isystem lib/gtest_1.7.0 -O3 -DGTEST_USE_OWN_TR1_TUPLE -DGTEST_HAS_PTHREAD=0 -isystem lib/gtest_1.7.0/include -isystem lib/gtest_1.7.0 -O3 -DNO_FPRINTF_OUTPUT -pipe -c -o lib/gtest_1.7.0/src/gtest-all.o lib/gtest_1.7.0/src/gtest-all.cc
If I add --stdlib=libc++ I get:
lib/gtest_1.7.0/include/gtest/gtest.h:54:10: fatal error: 'limits' file not
found
#include <limits>
^~~~~~~~
1 error generated.
If I add --stdlib=libstdc++ it works again. If I run with the default and -H I get that the limits file is available from:
/usr/bin/../lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/limits
/usr/include/clang/6.0.0/include/limits.h
If I run with --stdlib=libstdc++ I get the above plus /usr/include/limits.h.
If I run with --stdlib=libc++ and -H I get:
/usr/include/clang/6.0.0/include/limits.h
/usr/include/limits.h
So… it should find the file… (which does in fact exist on my system and contain the right thing…) This is an include-ordering problem because if I add -isystem /usr/include (which has limits.h all of a sudden it compiles… so… does anybody remember why we have all these -isystem options? :/
Hmm, will have to look into this further. The ‘-isystem’ stuff doesn’t emit warnings for files included in that fashion, maybe there are other reasons too.
It’s not so much the -isystem itself as the fact that we include a lot of stuff that way so apparently we’re fighting with the system headers for some reason.