build error related to C++11 flag on Mac + conda
I have been trying to build IMP from source on Conda on my mac. It mostly goes well except in one file it gets flustered by a for-range loop (see *Build output *below). If I build using the same cmd-line just with *--std=c++11*, or *std=c++14*, or *std=c++17*, it works just fine. Moreover, cmake does detect that it should work with C++11 (see *CMake output* below). Any ideas? Thanks! Barak
*Build output:*
(IMP_BUILD_py310) /Users/Shared/imp/fast_checks$ $ /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ -DIMPATOM_COMPILATION -DIMP_EXECUTABLE -Iinclude -I/Users/Shared/imp/repository/modules/core/dependency/python-ihm/src -isystem /opt/local/include/eigen3 -w -O3 -DNDEBUG -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.1.sdk -MD -MT modules/atom/benchmark/CMakeFiles/IMP.atom-benchmark_md_charmm.cpp.dir/benchmark_md_charmm.cpp.o -MF modules/atom/benchmark/CMakeFiles/IMP.atom-benchmark_md_charmm.cpp.dir/benchmark_md_charmm.cpp.o.d -o modules/atom/benchmark/CMakeFiles/IMP.atom-benchmark_md_charmm.cpp.dir/benchmark_md_charmm.cpp.o -c /Users/Shared/imp/repository/modules/atom/benchmark/benchmark_md_charmm.cpp
/Users/Shared/imp/repository/modules/atom/benchmark/benchmark_md_charmm.cpp:69:30: error: unexpected ':' in nested name specifier; did you mean '::'?
for(atom::Hierarchy atom : atoms) {
^
::
/Users/Shared/imp/repository/modules/atom/benchmark/benchmark_md_charmm.cpp:69:32: error: definition or redeclaration of 'atoms' not allowed inside a function
for(atom::Hierarchy atom : atoms) {
~~~~~~ ^
/Users/Shared/imp/repository/modules/atom/benchmark/benchmark_md_charmm.cpp:69:32: error: no member named 'atoms' in namespace 'IMP::atom'
for(atom::Hierarchy atom : atoms) {
~~~~~~ ^
/Users/Shared/imp/repository/modules/atom/benchmark/benchmark_md_charmm.cpp:69:37: error: expected ';' in 'for' statement specifier
for(atom::Hierarchy atom : atoms) {
^
/Users/Shared/imp/repository/modules/atom/benchmark/benchmark_md_charmm.cpp:69:37: error: expected ';' in 'for' statement specifier
/Users/Shared/imp/repository/modules/atom/benchmark/benchmark_md_charmm.cpp:70:17: error: unexpected namespace name 'atom': expected expression
core::XYZ(atom).set_coordinates_are_optimized(true);
^
6 errors generated.
*CMake output:* -- The C compiler identification is AppleClang 13.0.0.13000029
-- The CXX compiler identification is AppleClang 13.0.0.13000029
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found Python3: /Users/Shared/Miniconda/envs/IMP_BUILD_py310/bin/python3.1 (found version "3.10.8") found components: Interpreter Development NumPy Development.Module Development.Embed
-- Running check_common_problems
-- Running clean_build_dir
-- Running setup_cmake
-- clang version: 13.0.0
-- Using clang C++11 (or later) support
On 12/13/22 3:48 AM, Barak Raveh wrote: > I have been trying to build IMP from source on Conda on my mac. It > mostly goes well except in one file it gets flustered by a for-range > loop (see *Build output *below). If I build using the same cmd-line just > with /--std=c++11/, or /std=c++14/, or /std=c++17/, it works just fine. > Moreover, cmake does detect that it should work with C++11 (see *CMake > output* below). Any ideas?
If you're in a conda environment, you should probably use the conda compilers so you're building IMP with the same compiler as its dependencies - looks like you're using the system-provided compiler here. This is done by "conda install gxx_linux-64" on Linux followed by "source ${CONDA_PREFIX}/etc/conda/activate.d/activate-gxx_linux-64.sh" (it'll be a little different on a Mac though).
Otherwise, I would just force a C++ standard by passing -DCMAKE_CXX_FLAGS="-std=c++17" or similar to your CMake invocation. Modern clang is supposed to compile for recent C++ (I think C++14) by default, but maybe it is getting confused by one of your other compile flags somewhere.
Ben
Thanks! At least on my Mac installation, conda's gcc package seems to be broken. Let me report that I was able to build IMP with conda & clang using the following workaround: * adding -DCMAKE_CXX_FLAGS="-std=c++17" as you suggested * switching IMP's random number generator from boost to std in IMP/kernel/include/random.h due to some weird constexpr issue, due to a conflict between conda's boost::mt19337::min()/max() functions and llvm's std::shuffle(), which requires them to return a constexpr (this was fixed in the latest boost versions)
I tested for speed, and std::mt19337 is as fast or even faster on my mac. This is the change, l did not dare to check it in. Let me know if you think we should stick with the boost version or try to add this change - it doesn't break anything but could affect speed on some systems, perhaps for better but not necessarily. It required C11.
// For some constexpr related reason, clang + C11 don't work
// well with the boost random number generator
#if defined(__clang__) && __cplusplus >= 201103
#include <random>
#else
#include <boost/random/mersenne_twister.hpp>
#endif
IMPKERNEL_BEGIN_NAMESPACE
#ifndef SWIG // the RNG is defined explicitly in pyext/IMP_kernel.random.i
#if defined(__clang__) && __cplusplus >= 201103
class RandomNumberGenerator : public std::mt19937 {
typedef std::mt19937 T;
#else
class RandomNumberGenerator : public ::boost::mt19937 {
typedef ::boost::mt19937 T;
#endif
On Tue, Dec 13, 2022 at 8:39 PM Ben Webb ben@salilab.org wrote:
> On 12/13/22 3:48 AM, Barak Raveh wrote: > > I have been trying to build IMP from source on Conda on my mac. It > > mostly goes well except in one file it gets flustered by a for-range > > loop (see *Build output *below). If I build using the same cmd-line just > > with /--std=c++11/, or /std=c++14/, or /std=c++17/, it works just fine. > > Moreover, cmake does detect that it should work with C++11 (see *CMake > > output* below). Any ideas? > > If you're in a conda environment, you should probably use the conda > compilers so you're building IMP with the same compiler as its > dependencies - looks like you're using the system-provided compiler > here. This is done by "conda install gxx_linux-64" on Linux followed by > "source ${CONDA_PREFIX}/etc/conda/activate.d/activate-gxx_linux-64.sh" > (it'll be a little different on a Mac though). > > Otherwise, I would just force a C++ standard by passing > -DCMAKE_CXX_FLAGS="-std=c++17" or similar to your CMake invocation. > Modern clang is supposed to compile for recent C++ (I think C++14) by > default, but maybe it is getting confused by one of your other compile > flags somewhere. > > Ben > -- > ben@salilab.org https://salilab.org/~ben/ > "It is a capital mistake to theorize before one has data." > - Sir Arthur Conan Doyle >
On 12/19/22 11:35 AM, Barak Raveh wrote: > At least on my Mac installation, conda's gcc package seems to be > broken.
Are you sure you have the latest versions of everything? IMP built just fine on Mac with conda-forge just 4 days ago, e.g. https://dev.azure.com/conda-forge/84710dde-1620-425b-80d0-4cf5baca359d/_apis...
> a conflict between conda's boost::mt19337::min()/max() functions and > llvm's std::shuffle(), which requires them to return a constexpr (this > was fixed in the latest boost versions)
Yes, we had the same issue with Ubuntu, which has an older Boost: https://github.com/salilab/imp/commit/f1b873faeee
But this issue was fixed in Boost 1.75 and if you're using conda-forge you should have 1.78 as the IMP conda-forge package is pinned: https://github.com/conda-forge/conda-forge-pinning-feedstock/blob/main/recip...
> I tested for speed, and std::mt19337 is as fast or even faster on my > mac. This is the change, l did not dare to check it in. Let me know if > you think we should stick with the boost version or try to add this > change
Looks like your proposed change only affects clang builds. If we're going to switch to std::mt19337 we should just do so on all platforms, since IMP requires C++11 anyway. If there are some weird platforms where it doesn't work, we can always #ifdef it there and fall back to Boost.
Ben
Ok, so I will run all tests and push the switch from boost::mt19337 to std::mt19337 later this week, since this seems to make sense regardless. I think it should not affect or even improve speed, I will test it on a unix system tomorrow.
P. S. for for some reason, conda-forge chose version 1.74 from a completely fresh build. Not sue why - I will see what's up with that...
Thanks! Barak
On Mon, Dec 19, 2022, 9:55 PM Ben Webb ben@salilab.org wrote:
> On 12/19/22 11:35 AM, Barak Raveh wrote: > > At least on my Mac installation, conda's gcc package seems to be > > broken. > > Are you sure you have the latest versions of everything? IMP built just > fine on Mac with conda-forge just 4 days ago, e.g. > > https://urldefense.com/v3/__https://dev.azure.com/conda-forge/84710dde-1620-... > > > a conflict between conda's boost::mt19337::min()/max() functions and > > llvm's std::shuffle(), which requires them to return a constexpr (this > > was fixed in the latest boost versions) > > Yes, we had the same issue with Ubuntu, which has an older Boost: > https://urldefense.com/v3/__https://github.com/salilab/imp/commit/f1b873faee... > > But this issue was fixed in Boost 1.75 and if you're using conda-forge > you should have 1.78 as the IMP conda-forge package is pinned: > > https://urldefense.com/v3/__https://github.com/conda-forge/conda-forge-pinni... > > > I tested for speed, and std::mt19337 is as fast or even faster on my > > mac. This is the change, l did not dare to check it in. Let me know if > > you think we should stick with the boost version or try to add this > > change > > Looks like your proposed change only affects clang builds. If we're > going to switch to std::mt19337 we should just do so on all platforms, > since IMP requires C++11 anyway. If there are some weird platforms where > it doesn't work, we can always #ifdef it there and fall back to Boost. > > Ben > -- > ben@salilab.org https://salilab.org/~ben/ > "It is a capital mistake to theorize before one has data." > - Sir Arthur Conan Doyle >
participants (2)
-
Barak Raveh -
Ben Webb