Skip to content

Remove OpenMP search when not using likwid#303

Merged
EmilyBourne merged 2 commits into
SciCompMod:mainfrom
tpadioleau:remove-unnecessary-dependency-search
Jul 9, 2026
Merged

Remove OpenMP search when not using likwid#303
EmilyBourne merged 2 commits into
SciCompMod:mainfrom
tpadioleau:remove-unnecessary-dependency-search

Conversation

@tpadioleau

Copy link
Copy Markdown
Contributor

This PR removes the search of OpenMP when not using likwid. This mimics the search happening in the build stage,

GMGPolar/CMakeLists.txt

Lines 57 to 60 in 9356b29

if(GMGPOLAR_USE_LIKWID)
find_package(OpenMP REQUIRED COMPONENTS CXX)
find_package(LIKWID REQUIRED)
endif()

Merge Request - GuideLine Checklist

Guideline to check code before resolve WIP and approval, respectively.
As many checkboxes as possible should be ticked.

Checks by code author:

Always to be checked:

  • There is at least one issue associated with the pull request.
  • New code adheres with the coding guidelines
  • No large data files have been added to the repository. Maximum size for files should be of the order of KB not MB. In particular avoid adding of pdf, word, or other files that cannot be change-tracked correctly by git.

If functions were changed or functionality was added:

  • Tests for new functionality has been added
  • A local test was succesful

If new functionality was added:

  • There is appropriate documentation of your work. (use doxygen style comments)

If new third party software is used:

  • Did you pay attention to its license? Please remember to add it to the wiki after successful merging.

If new mathematical methods or epidemiological terms are used:

  • Are new methods referenced? Did you provide further documentation?

Checks by code reviewer(s):

  • Is the code clean of development artifacts e.g., unnecessary comments, prints, ...
  • The ticket goals for each associated issue are reached or problems are clearly addressed (i.e., a new issue was introduced).
  • There are appropriate unit tests and they pass.
  • The git history is clean and linearized for the merge request. All reviewers should squash commits and write a simple and meaningful commit message.
  • Coverage report for new code is acceptable.
  • No large data files have been added to the repository. Maximum size for files should be of the order of KB not MB. In particular avoid adding of pdf, word, or other files that cannot be change-tracked correctly by git.

@tpadioleau

Copy link
Copy Markdown
Contributor Author

@mknaranja do you mind reviewing ? :)

@mknaranja

Copy link
Copy Markdown
Member

are you not using OpenMP anymore ?

And have you mixed up LIKWID/MUMPS above ?

Just to be sure that's everything is as intended ;)

@tpadioleau

Copy link
Copy Markdown
Contributor Author

are you not using OpenMP anymore ?

I just aligned the build and install cmake files. Note that grepping omp in source files is now empty. But in the end I prefer to let @EmilyBourne or @AbdelhadiKara confirm that OpenMP is gone.

And have you mixed up LIKWID/MUMPS above ?

Where exactly ?

Just to be sure that's everything is as intended ;)

Sure you are very right

@mknaranja

Copy link
Copy Markdown
Member

"This PR removes the search of OpenMP when not using likwid" --> you meant MUMPS, right?

@julianlitz

julianlitz commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

If Likwid=Off and Mumps=Off, OpenMP is not used anymore.

Likwid=On: The Likwid markers are initialized inside a parallel region -> OpenMP is used here.

AI: "When MUMPS=ON, we don't need find_package(OpenMP) in our CMake, since our code never calls OpenMP directly. MUMPS can be built with or without OpenMP (see GMGPolar README.md file) if built with it, the dependency is already baked into MUMPS's own library/link config, so it comes along transitively when we link against MUMPS."

Thus I would move the +1 code addition to the if(Likwid) section.

(For the Mumps section I would guess that
find_dependency(MUMPS) is enough and "components OpemMP Metis" isnt needed.)

@julianlitz julianlitz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image

@tpadioleau

tpadioleau commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Yes of course you are correct let me fix this.

@tpadioleau

Copy link
Copy Markdown
Contributor Author

Fixed in 8fcc596.

That said, do we still have any OpenMP calls in the repo ? I don't see omp.h nor any pragma omp.

@julianlitz

julianlitz commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

The only ones are: At the bottom of include/Definitions/global_definitions.h.

We are probably missing an omp.h include in that file. I never tested if Likwid still works.

@julianlitz julianlitz self-requested a review July 9, 2026 06:03
@EmilyBourne EmilyBourne merged commit 7ea8655 into SciCompMod:main Jul 9, 2026
7 checks passed
@tpadioleau tpadioleau deleted the remove-unnecessary-dependency-search branch July 9, 2026 07:34
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.

4 participants