-
Notifications
You must be signed in to change notification settings - Fork 101
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
thd_incorrelate and OMP #278
Comments
I'm still getting this. For the thd_incorrelate.c error I think the ifdef logic just needs to be tidied a little to work when USE_OMP is not set as a compile definition. Here is a full list of failures in case it is helpful, this is emitted by requesting that the build system continue with up to 100 errors (
All cmake variables (`cmake -LA`) ?BUILD_SHARED_LIBS:BOOL=ON |
Thanks for the error message output; it didn't show up compiling on my Ubuntu or Mac, making it hard to investigate. Re. thd_incorrelate.c: indeed, I think that is the issue, and @afni-rwcox is looking at it. I briefly looked at the enable_mcw_malloc(). I suspect there is something not matching with environment variables, as there are a lot of "ifdef"s and "defined(...)"s around. I guess in general the Cmake as "USE_OMP" set to 0? Maybe that is why I don't see many of these things (USE_OMP is also in an if-condition around enable_mcw_malloc()). Also, is there a list of environment variables that Cmake uses (like, its ~/.afnirc)? I probably missed it, but looked in the afni/cmake tree and didn't see something that had that. |
I don't think environment variables are involved here. There's cmake variables:
There's targets:
There's compile definitions:
So given all that... in cmake what should be happening is the following:
I will open a PR that makes the cmake system more strictly align to the above ideal. |
addresses issues raised in #278 Now if USE_OMP is set by the user to OFF, an attempt at finding openmp is not done. In general if OpenMP::OpenMP_C is not a defined target then afni targets do not link against omp or define USE_OMP as a compile definition. Any deviation is a problem with the targets and should be fixed. An example of a succesful implementation is: https://github.com/afni/afni/blob/7bc375964a0a54323447842d6564a536e045476e/src/CMakeLists_binaries.txt#L250-L263
addresses issues raised in #278 Now if USE_OMP is set by the user to OFF, an attempt at finding openmp is not done. In general if OpenMP::OpenMP_C is not a defined target then afni targets do not link against omp or define USE_OMP as a compile definition. Any deviation is a problem with the targets and should be fixed. An example of a succesful implementation is: https://github.com/afni/afni/blob/7bc375964a0a54323447842d6564a536e045476e/src/CMakeLists_binaries.txt#L250-L263
I am getting an error when compiling thd_incorrelate.c in the cmake build on MacOS.
I can't remember too well but I don't think I ever managed to compile libmri openmp... without USE_OMP defined for it any instances of OpenMP code are appropriately removed by the C preprocessor... it seems this is not the case for thd_correlate.c
I don't know whether the intention is to support openmp for this file. The quick fix is to wrap the omp lines appropriately so that they are not compiled. The more thorough fix of supporting compilation of libmri in the cmake build is likely not feasible. Perhaps an intermediate solution would be to use openmp for thd_incorrelate.c... I don't have a sense of how entangled it is with other files grouped using the thd_objs cmake target.
The text was updated successfully, but these errors were encountered: