SyneRBI / SIRF-SuperBuild

SIRF CMake SuperBuild
http://www.ccpsynerbi.ac.uk
Apache License 2.0
15 stars 17 forks source link

Use OpenMP_<lang>_INCLUDE_DIR as opposed to OPENMP_INCLUDE_DIRS #872

Closed KrisThielemans closed 3 months ago

KrisThielemans commented 4 months ago

Necessary on MacOS.

Fixes #870

KrisThielemans commented 4 months ago

@francescaleek, could you give this a go?

First find out where your omp.h is (not sure how to do that on a Mac), most likely /usr/local/include, then do something like this for an existing build

cd SIRF-SuperBuild
git remote add kris https://github.com/KrisThielemans/SIRF-SuperBuild.git
git fetch kris
git checkout OpenMP_fixes
cd ../build
cmake -DOpenMP_CXX_INCLUDE_DIR:PATH=/usr/local/include -DOpenMP_C_INCLUDE_DIR:PATH=/usr/local/include .
make -j4

you'd have to reenable parallelproj and NiftyReg OpenMP to see of course.

If you have a working build already, you could copy all of that first, such that you don't screw it up.

It's not urgent to test this, but if you can and it works, then we'll include it in the imminent release.

KrisThielemans commented 4 months ago

This seems to have worked mostly for @francescaleek, except for NiftyReg. I've created https://github.com/KCL-BMEIS/niftyreg/pull/110. We could update version_config.cmake to point to that branch, or presumably set in External_NIFTYREG.cmake

set(OpenMP_C_FLAGS "${OpenMP_C_FLAGS} -I${OpenMP_C_INCLUDE_DIRS")
set(OpenMP_CXX_FLAGS "${OpenMP_CXX_FLAGS} -I${OpenMP_CXX_INCLUDE_DIRS")

maybe pointing to a correct version is best. @paskino @casperdcl any opinion?

KrisThielemans commented 3 months ago

@francescaleek, the NiftyReg fix has been merged, so can you try again with

cmake -DOpenMP_CXX_INCLUDE_DIR:PATH=/usr/local/include -DOpenMP_C_INCLUDE_DIR:PATH=/usr/local/include -DIFTYREG_TAG:STRING=a328efb3a2f8ea4b47cf0f7b581d983a570a1ffd .
make -j4

Hopefully this does the trick...