mrc-ide / covid-sim

This is the COVID-19 CovidSim microsimulation model developed by the MRC Centre for Global Infectious Disease Analysis hosted at Imperial College, London.
GNU General Public License v3.0
1.23k stars 257 forks source link

Mac OS builds failing re OpenMP #493

Closed weshinsley closed 1 year ago

weshinsley commented 1 year ago

Something has changed in github CI for Mac OS -

-- The C compiler identification is AppleClang 14.0.0.14000029
-- The CXX compiler identification is AppleClang 14.0.0.14000029
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /Applications/Xcode_14.2.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_14.2.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Setting build type to 'RelWithDebInfo' as none was specified.
CMake Error at /usr/local/Cellar/cmake/3.25.1/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:230 (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.25.1/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:600 (_FPHSA_FAILURE_MESSAGE)
  /usr/local/Cellar/cmake/3.25.1/share/cmake/Modules/FindOpenMP.cmake:580 (find_package_handle_standard_args)
  CMakeLists.txt:35 (find_package)

Adding this section in the root CMakeLists.txt:

  # Find OpenMP
if(APPLE AND USE_OPENMP)
    if(CMAKE_C_COMPILER_ID MATCHES "Clang")
        set(OpenMP_C "${CMAKE_C_COMPILER}")
        set(OpenMP_C_FLAGS "-fopenmp=libomp -Wno-unused-command-line-argument")
        set(OpenMP_C_LIB_NAMES "libomp" "libgomp" "libiomp5")
        set(OpenMP_libomp_LIBRARY ${OpenMP_C_LIB_NAMES})
        set(OpenMP_libgomp_LIBRARY ${OpenMP_C_LIB_NAMES})
        set(OpenMP_libiomp5_LIBRARY ${OpenMP_C_LIB_NAMES})
    endif()
    if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
      set(OpenMP_CXX "${CMAKE_CXX_COMPILER}")
      set(OpenMP_CXX_FLAGS "-fopenmp=libomp -Wno-unused-command-line-argument")
      set(OpenMP_CXX_LIB_NAMES "libomp" "libgomp" "libiomp5")
      set(OpenMP_libomp_LIBRARY ${OpenMP_CXX_LIB_NAMES})
      set(OpenMP_libgomp_LIBRARY ${OpenMP_CXX_LIB_NAMES})
      set(OpenMP_libiomp5_LIBRARY ${OpenMP_CXX_LIB_NAMES})
    endif()
endif()

seems to be part of the answer - we get passed the configure stage, but fail when building with:

 [  2%] Building CXX object src/Geometry/CMakeFiles/geometrylib.dir/Vector2.cpp.o
[  4%] Linking CXX static library libgeometrylib.a
[  4%] Built target geometrylib
[  6%] Building CXX object src/CMakeFiles/CovidSim.dir/CovidSim.cpp.o
clang: error: unsupported argument 'libomp' to option 'fopenmp='
make[2]: *** [src/CMakeFiles/CovidSim.dir/CovidSim.cpp.o] Error 1
make[1]: *** [src/CMakeFiles/CovidSim.dir/all] Error 2
make: *** [all] Error 2

and removing the libomp from the fopenmp= fails with

 [  6%] Building CXX object src/CMakeFiles/CovidSim.dir/CovidSim.cpp.o
clang: error: unsupported option '-fopenmp'
make[2]: *** [src/CMakeFiles/CovidSim.dir/CovidSim.cpp.o] Error 1
make[1]: *** [src/CMakeFiles/CovidSim.dir/all] Error 2
make: *** [all] Error 2
weshinsley commented 1 year ago

So - something has changed in the Mac OS images github uses, and I think there is some sort of conflict between different versions of Clang/C++ that seem to coexist. Logging into the CI node and running clang --help shows that -fopenmp should be possible, but at the build it seems to be an invalid option. I've worked around this for now by disabling OpenMP support and only running the single-threaded tests on Mac OS. Not an ideal solution, but Apple seem to be making this hard, and I'm out of ideas for now.

Having fixed that, google-tests 1.10 would also no longer compile on the Mac OS image, because of some deprecations that are now viewed by Apple's compiler as fatal errors. A bit of a balancing act:-

So I've moved Google Tests to the 1.12.1 release and if CentOS is detected, fall back to 1.10.

weshinsley commented 1 year ago

If anyone should read this and know how to fix OpenMP on the Mac OS build, I would be very grateful for advice!

weshinsley commented 1 year ago

Bit more progress -

if(APPLE)
  set(CMAKE_C_COMPILER "/usr/local/opt/llvm/bin/clang")
  set(CMAKE_CXX_COMPILER "/usr/local/opt/llvm/bin/clang++")
  add_compile_options("-I/usr/local/opt/libomp/include -L/usr/local/opt/libomp/lib")
  link_libraries("-I/usr/local/opt/libomp/include -L/usr/local/opt/libomp/lib")
endif()

Pasting this before the project line successfully forces the compiler to be the right one. We then get an error later on (this is verbose from cmake --build . -v)

[  6%] Linking CXX executable CovidSim
cd /Users/runner/work/covid-sim/covid-sim/build/src && /usr/local/Cellar/cmake/3.25.2/bin/cmake -E cmake_link_script CMakeFiles/CovidSim.dir/link.txt --verbose=1
/usr/local/opt/llvm/bin/clang++ -O2 -g -DNDEBUG -isysroot /Applications/Xcode_14.2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.1.sdk -mmacosx-version-min=12.6 -Wl,-search_paths_first -Wl,-headerpad_max_install_names CMakeFiles/CovidSim.dir/CovidSim.cpp.o CMakeFiles/CovidSim.dir/Rand.cpp.o CMakeFiles/CovidSim.dir/Error.cpp.o CMakeFiles/CovidSim.dir/Dist.cpp.o CMakeFiles/CovidSim.dir/Kernels.cpp.o CMakeFiles/CovidSim.dir/Bitmap.cpp.o CMakeFiles/CovidSim.dir/SetupModel.cpp.o CMakeFiles/CovidSim.dir/CalcInfSusc.cpp.o CMakeFiles/CovidSim.dir/Sweep.cpp.o CMakeFiles/CovidSim.dir/Update.cpp.o CMakeFiles/CovidSim.dir/Param.cpp.o CMakeFiles/CovidSim.dir/Person.cpp.o CMakeFiles/CovidSim.dir/Direction.cpp.o CMakeFiles/CovidSim.dir/InverseCdf.cpp.o CMakeFiles/CovidSim.dir/Memory.cpp.o CMakeFiles/CovidSim.dir/CLI.cpp.o CMakeFiles/CovidSim.dir/Files.cpp.o CMakeFiles/CovidSim.dir/ReadParams.cpp.o -o CovidSim  -I/usr/local/opt/libomp/include -L/usr/local/opt/libomp/lib Geometry/libgeometrylib.a -I/usr/local/opt/libomp/include -L/usr/local/opt/libomp/lib -llibomp -llibgomp -llibiomp5 -llibomp -llibgomp -llibiomp5 
ld: library not found for -llibomp

So the -L and -I are there (twice as it happens), libomp.a and libomp.dylib are present in /usr/local/opt/libomp/lib (and similarly the include folder exists with .h files). So this looks promising, but still isn't right.

weshinsley commented 1 year ago

Ok - it turns out this is sufficient

if(APPLE)
  set(CMAKE_C_COMPILER "/usr/local/opt/llvm/bin/clang")
  set(CMAKE_CXX_COMPILER "/usr/local/opt/llvm/bin/clang++")
endif()

before the project line - don't specify the libraries - and everything will be picked up. The Google Test update (and backward-compatibility for CentOS) is still required with the compiler above.