Skip to content

Conversation

@blattms
Copy link
Member

@blattms blattms commented Apr 16, 2025

We add target OpenMP::OpenMP_CXX to the linker line. Hence we always need to search for it, at least if the module is use by non-OPM modules.

Otherwise you might get the same errror as for the (autopkgtests in Debian](https://salsa.debian.org/science-team/opm-common/-/jobs/7439843):

-- No module specific tests for module 'opm-common' ('OpmCommonMacros.cmake' not found)
-- Setting opm-common_INCLUDE_DIRS=/usr/include;/usr/include/cjson
-- Setting opm-common_LIBRARIES=$<TARGET_FILE:opmcommon>;fmt::fmt;OpenMP::OpenMP_CXX;Boost::system;Boost::unit_test_framework;/usr/lib/x86_64-linux-gnu/libcjson.so
...
 * Vc, C++ Vectorization library, <https://github.com/VcDevel/Vc>
   For use of SIMD instructions
-- Configuring done (5.0s)
CMake Error at src/CMakeLists.txt:1 (add_executable):
  Target "dune-autopkgtest" links to:
    OpenMP::OpenMP_CXX
  but the target was not found.  Possible reasons include:
    * There is a typo in the target name.
    * A find_package call is missing for an IMPORTED target.
    * An ALIAS target is missing.

While this only ocurs if you use opm-common as a dune module, it still would be great to have this in the release. We do have people that are using opm-grid at least.

We add target OpenMP::OpenMP_CXX to the linker line. Hence we always need
to search for it.
@blattms blattms added this to the Release 2025.04 milestone Apr 16, 2025
@blattms blattms requested a review from akva2 April 16, 2025 08:44
@blattms
Copy link
Member Author

blattms commented Apr 16, 2025

jenkins build this please

@akva2
Copy link
Member

akva2 commented Apr 24, 2025

this isn't quite correct as it will override opm's ability to disable opemp. you'll have to carry a patch until i can look into a proper solution.

@blattms
Copy link
Member Author

blattms commented Apr 25, 2025

And this possibility to disable is used by jenkins for testing compilation?

@akva2
Copy link
Member

akva2 commented Apr 25, 2025

nope, jenkins always builds with openmp, the stuff i am referring to is https://github.com/OPM/opm-common/blob/master/cmake/Modules/OpmLibMain.cmake#L61

@blattms
Copy link
Member Author

blattms commented Apr 28, 2025

Yeah, that simply means that nobody ever disables it. Even without the switch you can simply disable it by setting the number of threads to 1 at runtime, right?

Hence I don't even see a need to keep the switch.

@akva2
Copy link
Member

akva2 commented Apr 28, 2025

uhm, that's a somewhat bold claim. i use it all the time while debugging. for instance, running in valgrind gomp causes a lot of analytics.

we can fix this while keeping the option intact. we just need to add the code to the config file instead of the prereqs, but right now there is no pretty mechanism to handle such things, we only have https://github.com/OPM/opm-common/blob/master/CMakeLists.txt#L32 which would mean we had to handle it in every module that uses openmp, a rather sub-optimal approach.

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

Successfully merging this pull request may close these issues.

2 participants