envelope-project / laik

Other
9 stars 8 forks source link

CMAKE options must be optional (Openmp is not manatory). #86

Closed twittidai closed 6 years ago

twittidai commented 6 years ago

-- The C compiler identification is AppleClang 9.0.0.9000039 -- The CXX compiler identification is AppleClang 9.0.0.9000039 -- Check for working C compiler: /Library/Developer/CommandLineTools/usr/bin/cc -- Check for working C compiler: /Library/Developer/CommandLineTools/usr/bin/cc -- works -- Detecting C compiler ABI info -- Detecting C compiler ABI info - done -- Detecting C compile features -- Detecting C compile features - done -- Check for working CXX compiler: /Library/Developer/CommandLineTools/usr/bin/c++ -- Check for working CXX compiler: /Library/Developer/CommandLineTools/usr/bin/c++ -- works -- Detecting CXX compiler ABI info -- Detecting CXX compiler ABI info - done -- Detecting CXX compile features -- Detecting CXX compile features - done CMake Error at /usr/local/Cellar/cmake/3.10.2/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:137 (message): Could NOT find OpenMP_C (missing: OpenMP_C_FLAGS OpenMP_C_LIB_NAMES) Call Stack (most recent call first): /usr/local/Cellar/cmake/3.10.2/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:378 (_FPHSA_FAILURE_MESSAGE) /usr/local/Cellar/cmake/3.10.2/share/cmake/Modules/FindOpenMP.cmake:447 (find_package_handle_standard_args) examples/CMakeLists.txt:45 (find_package)

twittidai commented 6 years ago

not mandatory is everything... e.g. mpi protobuf mosquito etc.

AlexanderKurtz commented 6 years ago

OpenMP support is optional, it is just turned on by default. To build with OpenMP disabled use:

cmake -D enable-openmp-examples=off ..

We can of course turn this option off by default (it is just a matter of flipping a switch), but then everybody (including Travis) would build without testing this. Maybe instead have a look at brew install llvm [0,1] or brew install clang-omp [2]?

[0] http://antonmenshov.com/2017/09/09/clang-openmp-setup-in-xcode/ [1] https://stackoverflow.com/questions/40095958/apple-clang-fopenmp-not-working [2]http://stechschulte.net/2016/03/20/openmp-osx-cmake.html

AlexanderKurtz commented 6 years ago

Oh, and have you noticed https://github.com/envelope-project/laik/blob/master/doc/cmake-readme.md? It may be a little hidden, but it should detail the whole CMake build process including turning features on/off!

weidendo commented 6 years ago

As far as I remember cmake can check for features at first run?

twittidai commented 6 years ago

Indeed @weidendo I also prefer to check what is available. e.g. I have gcc on my Mac, but it's name is gcc-7 instead of gcc. If we change to a advance build system such as cmake I think this should be supported.

Can you fix this @AlexanderKurtz to search&enable?

weidendo commented 6 years ago

I know this requires to write CMake configure checks in this strange CMake syntax I am not comfortable with...

Anyway, for now, I like to have both regular Makefiles and CMake next to each other at least for some time.

AlexanderKurtz commented 6 years ago

So you both would like CMake to automatically enable/disable features depending on what is available on the host system, correct? I can certainly add this, but I personally think this is fundamentally the wrong way around:

If I ask a tool to do X (e.g. "build LAIK in default configuration") it should either do this or fail, not do Y (i.e. "build LAIK but with some features disabled") instead. IMHO the latter approach tends to hide bugs, for example if we ever added a feature and forgot to add its package dependencies to the Dockerfile, Travis would still succeed, but quietly ignore (and therefore not test) the new feature.

So, before I change CMake this way, please consider simply configuring CMake explicitly, e.g. like this:

CC=gcc-7 CXX=g++-7 cmake -D enable-openmp-examples=off ..

Remember, you only have to do this once! Future CMake or make calls will pick up these settings! Of course, we could also flip the defaults on some options here, but that would mean that new contributors could accidentally break these features without noticing (since they'll probably build with the default settings).

AlexanderKurtz commented 6 years ago

So, essentially we have to choose between:

  1. Build with everything on per default, the user has to explicitly disable features on the first CMake call.
  2. Build with some features off per default, the user has to explicitly enable and/or disable features on first CMake call.
  3. Build with everything that the host system supports at the time of the first CMake call, disable everything else.

I prefer 1 > 2 > 3, what do you think?

twittidai commented 6 years ago

I indeed prefers 3>2>1 for production use and 1>3>2 for testing use. I must admit that typically HPC user knows what they are doing and what is available, in that case, option 2 would be the best. Generally one should call "Configure" then "cmake/make", which will find out what is accessible on the system, and potentially use the best backend for the current system.

weidendo commented 6 years ago

CMake Warning at src/CMakeLists.txt:42 (message): Dependency check for option 'mpi-backend' failed, skipping!

But: $ mpicc clang: error: no input files

So MPI is obviously installed.

$ which mpicc /usr/local/bin/mpicc

This is brew installing in /usr/local. But shouldn't this be detected, as it is in $PATH?

AlexanderKurtz commented 6 years ago

Hmmm, CMake treats MPI like any other library and just uses pkg-config to find it. What does

pkg-config --cflags mpi

and

pkg-config --cflags ompi

return on your system?

weidendo commented 6 years ago

Oh, ok. Then brew somehow should have added its installation directory to the pgk-config PATH - I can do that, too. I cannot check currently, my Mac currently is in repair - some capacitor blow up internally, being almost new :-(

AlexanderKurtz commented 6 years ago

Ok, this can certainly wait, so no hurry. However, please note that the CMake logic will skip the MPI backend if any of these is true:

  1. pkg-config is not installed
  2. No MPI implementation providing a pkg-config file is installed.
  3. The installed MPI implementation does not provide the mpi name in pkg-config (e.g. only ompi or mpich).

I am currently wondering if 1) should be turned into a hard failure and 3) into a configuration option (e.g. "mpi-implementation" defaulting to mpi). What do you think?

weidendo commented 6 years ago

Is using pkg-config the standard way of how CMake detects MPI? I would have expected that it checks if mpicc is in the path...

Anyway, it makes no sence to work around standard cmake procedures.

AlexanderKurtz commented 6 years ago

I don't think there's any standard way. There is a dedicated FindMPI module [0], but it's almost 2k lines of code [1] and pkg-config does essentially the same thing (and more, e.g. search overrides via environment variables). This essentially comes down to whether we want to support systems where MPI is not available via pkg-config. Do we?

[0] https://cmake.org/cmake/help/v3.5/module/FindMPI.html [1] https://github.com/Kitware/CMake/blob/master/Modules/FindMPI.cmake

weidendo commented 6 years ago

I think we should do the best we can for people who want to compile LAIK themselves. If there is a dedicated and officially supported FindMPI module, we should use it.

I yesterday tried to install some HPC tool, and there was no INSTALL, and even after the configure from autotools went through, it did throw compile errors due to missing packages. If this is the first contact with a library, that's just bad. LAIK should do its if somebody wants to compile it in arbitrary HPC environments.

weidendo commented 6 years ago

Ok, my Mac is back. Just installed homebrew and openmpi:

$ cmake ..
-- The C compiler identification is AppleClang 9.0.0.9000039
-- The CXX compiler identification is AppleClang 9.0.0.9000039
-- Check for working C compiler: /Library/Developer/CommandLineTools/usr/bin/cc
-- Check for working C compiler: /Library/Developer/CommandLineTools/usr/bin/cc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /Library/Developer/CommandLineTools/usr/bin/c++
-- Check for working CXX compiler: /Library/Developer/CommandLineTools/usr/bin/c++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
CMake Warning at doc/CMakeLists.txt:22 (message):
  Dependency check for option 'documentation' failed, skipping!

-- Could NOT find OpenMP_C (missing: OpenMP_C_FLAGS OpenMP_C_LIB_NAMES) 
-- Could NOT find OpenMP_CXX (missing: OpenMP_CXX_FLAGS OpenMP_CXX_LIB_NAMES) 
-- Could NOT find OpenMP (missing: OpenMP_C_FOUND OpenMP_CXX_FOUND) 
CMake Warning at examples/CMakeLists.txt:60 (message):
  Dependency check for option 'openmp-examples' failed, skipping!

-- Could NOT find PkgConfig (missing: PKG_CONFIG_EXECUTABLE) 
CMake Warning at external/FT-STIM/CMakeLists.txt:21 (message):
  Dependency check for option 'failure-simulator' failed, skipping!

CMake Warning at external/MQTT/CMakeLists.txt:19 (message):
  Dependency check for option 'mosquitto-agent' failed, skipping!

-- Could NOT find PkgConfig (missing: PKG_CONFIG_EXECUTABLE) 
CMake Warning at external/profiling/CMakeLists.txt:18 (message):
  Dependency check for option 'profiling-agent' failed, skipping!

-- Could NOT find PkgConfig (missing: PKG_CONFIG_EXECUTABLE) 
-- Found MPI_C: /usr/local/Cellar/open-mpi/3.0.0_2/lib/libmpi.dylib (found version "3.1") 
-- Found MPI_CXX: /usr/local/Cellar/open-mpi/3.0.0_2/lib/libmpi.dylib (found version "3.1") 
-- Found MPI: TRUE (found version "3.1")  
-- Configuring done
-- Generating done
-- Build files have been written to: /Users/di57cib/SW/laik/b

So MPI detection works, and PkgConfig is not installed. The Apple-provided gcc does not support OpenMP. So this looks fine.

Why does the non-availability of a feature trigger a warning? Just an information (as other cmake output) would be enough. I also have no idea why there always are two empty lines after a "warning"?

AlexanderKurtz commented 6 years ago

So this looks fine.

Good, thanks!

Why does the non-availability of a feature trigger a warning? Just an information (as other cmake output) would be enough.

Well, I felt that auto-disabling a feature deserves a warning since it means that parts of LAIK aren't built (and therefore not tested). However, this can be trivially changed from WARNING to STATUS [0]. Would you prefer this?

I also have no idea why there always are two empty lines after a "warning"?

IIRC this is just CMake's way to make warnings stand out, so this should be gone too if we change this to a simple status message!

[0] https://cmake.org/cmake/help/v3.5/command/message.html

weidendo commented 6 years ago

I would prefer a status message being printed about the outcome of each environment check done? E.g. also something like "C++ detected: compiling C++ examples" when C++ was detected.

Instead of having the status messages inline with the rest of cmake output, it perhaps may be nice to have the outcome of the checks printed as summary at the end, if that is possible?