Skip to content

abort on cmake config if openmp requested but not found - #2893

Merged
paulromano merged 1 commit into
openmc-dev:developfrom
gridley:is-openmp-capable
Mar 6, 2024
Merged

paulromano merged 1 commit into
openmc-dev:developfrom
gridley:is-openmp-capable

Conversation

@gridley

@gridley gridley commented Mar 6, 2024

Copy link
Copy Markdown
Contributor

Description

We should not let CMake silently fail if CMake can't find OpenMP. Currently, the CMake flags for OpenMP can be set, but if it's not available on the system (a rarity but for Macs by default it seems!) the configuration proceeds anyway. I found it strange that this fancy Apple silicon was lagging, and of course observing the output header, no thread count info was provided!

This is an unexpected failure with a one-line fix. If a user wishes to not use OpenMP, it should be reflected in the CMake configuration flags.

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 15) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

@paulromano paulromano left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Agreed — if OPENMC_USE_OPENMP is enabled, it shouldn't let it slide if OpenMP is not found. Thanks!

@paulromano
paulromano enabled auto-merge (squash) March 6, 2024 03:43
@paulromano
paulromano merged commit aa0516f into openmc-dev:develop Mar 6, 2024
@gridley
gridley deleted the is-openmp-capable branch July 10, 2024 18:23
church89 pushed a commit to openmsr/openmc that referenced this pull request Jul 18, 2024
apingegno pushed a commit to apingegno/openmc that referenced this pull request May 7, 2026
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