SyneRBI / SIRF-SuperBuild

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

parallelproj build failed on OSX due to omp.h not found #870

Closed KrisThielemans closed 3 months ago

KrisThielemans commented 4 months ago

@francescaleek has a failure funding omp.h when building parallelproj, but not any other project (whcih is a bit weird, but could be because STIR does not require OpenMP). The parallelproj CMake settings look like this: image

We currently do https://github.com/SyneRBI/SIRF-SuperBuild/blob/e43323735471174437666bf4134721e1413529ad/SuperBuild/External_parallelproj.cmake#L43-L48 similarly for STIR https://github.com/SyneRBI/SIRF-SuperBuild/blob/e43323735471174437666bf4134721e1413529ad/SuperBuild/External_STIR.cmake#L105-L106 and overall (as mark_as_superbuild should transport those to the subprojects) https://github.com/SyneRBI/SIRF-SuperBuild/blob/e43323735471174437666bf4134721e1413529ad/SuperBuild.cmake#L225-L231

I note we set OPENMP_INCLUDES, but https://cmake.org/cmake/help/latest/module/FindOpenMP.html says we should use OpenMP_CXX_INCLUDE_DIR (and some for C)

Note that none of this is necessary on Linux/g++ but @rijobro put it in for MacOS. Possibly FindOpenMP.cmake has moved on.

KrisThielemans commented 4 months ago

@francescaleek could you check if STIR is actually built with OpenMP (i.e. open the builds/STIR/build build dir in the CMake GUI and see if STIR_OPENMP is ON), to understand if this is a generic problem, or specific to parallelproj,

francescaleek commented 4 months ago

Yes, STIR_ENABLE_OPENMP is ON

KrisThielemans commented 4 months ago

@francescaleek confirms NiftyReg fails with the same problem. STIR not, but likely it is going ahead without OpenMP

KrisThielemans commented 4 months ago

Related: #502

KrisThielemans commented 4 months ago

There's an additional problem with NiftyReg: https://github.com/KCL-BMEIS/niftyreg/issues/109. NiftyReg PR at https://github.com/KCL-BMEIS/niftyreg/pull/110