Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenMP flags #95

Closed
rijobro opened this issue Feb 28, 2018 · 14 comments
Closed

OpenMP flags #95

rijobro opened this issue Feb 28, 2018 · 14 comments
Assignees
Milestone

Comments

@rijobro
Copy link
Contributor

rijobro commented Feb 28, 2018

When building STIR, the user can enable STIR_OpenMP, reconfigure and then if necessary, set OpenMP_CXX_FLAGS, OpenMP_C_FLAGS, OpenMP_CXX_LIB_NAMES and OpenMP_C_LIB_NAMES. These aren't exposed in the superbuild, and so compilation fails on my machine.

Could we also enable a travis test with openMP enabled for STIR?

@KrisThielemans KrisThielemans added this to the v1.1 milestone Feb 28, 2018
@KrisThielemans
Copy link
Member

aargh. macs....

It really shouldn't be necessary to have to set these flags, but indeed we struggled with it for STIR. Have a look there. Note the end statement "CMake doesn't support OpenMP with Apple's version of clang yet." (can't remember where I found that, but I believe it was in some CMake issue).

Can you add your observations on how you did get STIR-OPENMP to work on your system (os, xcode, compiler, cmake) to the STIR issue.

In STIR, we didn't manage to get openmp on Travis yet for OSX, so no point in enabling it for SIRF.(ok, we could do it for linux).

Having said all this, it would make sense to do a find(openmp) in SuperBuild.cmake and use mark_as_superbuild on the Openmp* variables (should only need CXX). feel free to generate a PR for this.

@DANAJK
Copy link
Contributor

DANAJK commented May 12, 2018

Is there a work around? A super build on my Mac currently fails with the error below. In cmake-gui, the BUILD_STIR_WITH_OPENMP is not checked (so it should build?)

CMake Error at /usr/local/Cellar/cmake/3.11.0/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.11.0/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:378 (_FPHSA_FAILURE_MESSAGE)
/usr/local/Cellar/cmake/3.11.0/share/cmake/Modules/FindOpenMP.cmake:451 (find_package_handle_standard_args)
src/CMakeLists.txt:145 (find_package)

-- Configuring incomplete, errors occurred!
See also "/Users/davidatkinson/devel/build/builds/STIR/build/CMakeFiles/CMakeOutput.log".
make[2]: *** [builds/STIR/stamp/STIR-configure] Error 1
make[1]: *** [CMakeFiles/STIR.dir/all] Error 2
make: *** [all] Error 2

@KrisThielemans
Copy link
Member

sorry. this is my update. Travis OSX was down, so I thought it'd work :-;

I don't understand what's going on though. I was specifically trying to avoid this issue by NOT enabling STIR_OPENMP on OSX. Can you attach the CMakeCache.txt of ~/devel/build/ and ~/devel/build/builds/STIR/build/?

As a work-around for now, you can edit your local SIRF-SuperBuild/SuperBuild.cmake, L116 back to

option(BUILD_STIR_WITH_OPENMP "Build STIR with OpenMP acceleration" OFF)

as that worked for you before. (you would have to delete your build directory, or create a new one such that we can debug the old one)

@DANAJK
Copy link
Contributor

DANAJK commented May 12, 2018 via email

@DANAJK
Copy link
Contributor

DANAJK commented May 12, 2018 via email

@KrisThielemans
Copy link
Member

KrisThielemans commented May 12, 2018

unless you changed CMAKE_INSTALL_PREFIX this is another version of gadgetron. Try executing the one in .../INSTALL/bin.

we currently append our directory to the PATH in env_ccppetmr.sh. maybe that's a bad idea (i.e. we should prepend)

@KrisThielemans
Copy link
Member

The SuperBuild CMake should now default to false for STIR-OPENMP for OSX. My mistake was that UNIX is also true on OSX.

By the way @DANAJK, the only way I can see that setting BUILD_STIR_WITH_OPENMP to OFF on your cmake-gui didn't end up in the CMakeCache.txt is if you forgot to press configure again.

Let us know about gadgetron (and if you think we should put our INSTALL/bin first in the path, which would prevent people using other stuff by mistake).

@KrisThielemans
Copy link
Member

@rijobro, regarding your original observation, could you maybe check at some point what Gadgetron does for OPENMP on OSX, as we don't have that problem there.

@DANAJK
Copy link
Contributor

DANAJK commented May 12, 2018 via email

@rijobro
Copy link
Contributor Author

rijobro commented Mar 29, 2019

Sorry, just revisiting this and I can't remember what I need to test. On OSX, BUILD_STIR_WITH_OPENMP defaults to OFF and that works just fine.

Does that mean this issue is resolved?

@KrisThielemans
Copy link
Member

At some point, we have to get OpenMP support on working on Macs. We might then have to pass the flags through that you mentioned in your first comment.
Let's leave this for later though, so I'll reassign to 2.1.

@KrisThielemans KrisThielemans modified the milestones: v2.0, v2.1 Apr 8, 2019
@rijobro rijobro self-assigned this Oct 17, 2019
@rijobro rijobro modified the milestones: v2.1, v2.2 Oct 17, 2019
@KrisThielemans
Copy link
Member

@rijobro says this should now all work with Python and C++, but not with MATLAB.

@KrisThielemans
Copy link
Member

see also #502

@KrisThielemans
Copy link
Member

we won't support MATLAB anymore in the future, so we close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants