Skip to content

fix[cartesian]: Disable OpenMP multithreading for DaCe backends#2491

Merged
FlorianDeconinck merged 11 commits intoGridTools:mainfrom
FlorianDeconinck:fix/macos_apple_clang_openmp_deactivate
Feb 25, 2026
Merged

fix[cartesian]: Disable OpenMP multithreading for DaCe backends#2491
FlorianDeconinck merged 11 commits intoGridTools:mainfrom
FlorianDeconinck:fix/macos_apple_clang_openmp_deactivate

Conversation

@FlorianDeconinck
Copy link
Copy Markdown
Contributor

@FlorianDeconinck FlorianDeconinck commented Feb 20, 2026

Description

This PR deactivate OpenMP when detecting MacOS (Darwin) as the OS. based on a new configuration.

This was motivated by apple-clang not shipping with openmp by default and the way to acquire it is requires brew or other workaround we won't expect out of our users.

In detail the PR:

  • add an GT4PY_CARTESIAN_ENABLE_OPENMP env var and configuration flag
  • strips includes and omp_get_max_threads from codegen on the glue code of stencil DaCe backends when all the computes are sequential (non MacOS related),
  • put an option in the StencilBuilder to centralize deactivation of OpenMP and turn it off when detecting Darwin (+ warning),
  • makes all maps sequential as part of the OIR -> TreeIR process based on the above option,
  • strips openmp flags from the command line based on the above option.

Requirements

  • All fixes and/or new features come with corresponding tests.

@havogt
Copy link
Copy Markdown
Contributor

havogt commented Feb 23, 2026

Adding @tehrengruber as reviewer to check with PMAP people if they are running with openmp on mac for development.

Copy link
Copy Markdown
Contributor

@romanc romanc left a comment

Choose a reason for hiding this comment

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

I'd also suggest to add minimal tests (e.g. we run daily CI on MacOS hardware) to catch obvious errors like deactivating OpenMP on the wrong platform.

Also, we might wanna allow OpenMP on MacOS systems if used in combination with GCC (like we do - without issues - on the daily CI).

Comment thread src/gt4py/cartesian/stencil_builder.py Outdated
Comment thread src/gt4py/cartesian/stencil_builder.py Outdated
Comment thread src/gt4py/cartesian/stencil_builder.py Outdated
@FlorianDeconinck
Copy link
Copy Markdown
Contributor Author

My bad - this was supposed to be a draft. Thanks for the review, I'll be back on it today

@FlorianDeconinck FlorianDeconinck marked this pull request as draft February 23, 2026 14:14
@FlorianDeconinck FlorianDeconinck changed the title fix[cartesian]: MacOS & OpenMP: skip the multithreading fix[cartesian]: Disable OpenMP multithreading for DaCe backends Feb 23, 2026
@FlorianDeconinck FlorianDeconinck marked this pull request as ready for review February 23, 2026 20:46
@FlorianDeconinck
Copy link
Copy Markdown
Contributor Author

PR has been modified to introduce a wider GT4Py flag to deactivate OpenMP

@FlorianDeconinck
Copy link
Copy Markdown
Contributor Author

Adding @tehrengruber as reviewer to check with PMAP people if they are running with openmp on mac for development.

This is now not disabled on MacOS by default. Should be transparent to PMAP people

Comment thread src/gt4py/cartesian/config.py
Copy link
Copy Markdown
Contributor

@twicki twicki left a comment

Choose a reason for hiding this comment

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

to me this looks like it is ready to go

Copy link
Copy Markdown
Contributor

@romanc romanc left a comment

Choose a reason for hiding this comment

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

I find it a little weird that the oir to treeir translator is deciding based on gt_config.GT4PY_CARTESIAN_ENABLE_OPENMP while the pyext builder is looking at gt_config.build_settings["openmp"]["use_openmp"]. I guess it's a matter of taste.

@FlorianDeconinck
Copy link
Copy Markdown
Contributor Author

@romanc / @twicki : as discussed this morning - ready for review/merge

Copy link
Copy Markdown
Contributor

@romanc romanc left a comment

Choose a reason for hiding this comment

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

LGTM

@FlorianDeconinck FlorianDeconinck merged commit 89d1c1e into GridTools:main Feb 25, 2026
21 checks passed
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