Skip to content

Update Omega to C++20 - #405

Open
mwarusz wants to merge 3 commits into
E3SM-Project:developfrom
mwarusz:omega/cxx20
Open

Update Omega to C++20#405
mwarusz wants to merge 3 commits into
E3SM-Project:developfrom
mwarusz:omega/cxx20

Conversation

@mwarusz

@mwarusz mwarusz commented May 12, 2026

Copy link
Copy Markdown
Member

This draft PR updates Omega C++ standard to C++20. E3SM is in the process to upgrading to Kokkos 5, which requires C++20. This PR has the necessary fixes on the Omega side.

Checklist

  • Linting
  • Building
    • CMake build does not produce any new warnings from changes in this PR
  • Testing
    • Add a comment to the PR titled Testing with the following:
      • Which machines CTest unit tests
        have been run on and indicate that are all passing.
      • The Polaris omega_pr test suite
        has passed, using the Polaris e3sm_submodules/Omega baseline
      • Document machine(s), compiler(s), and the build path(s) used for -p for both the baseline (Polaris e3sm_submodules/Omega) and the PR build
      • Indicate "All tests passed" or document failing tests

@grnydawn
grnydawn self-requested a review May 14, 2026 18:16

@grnydawn grnydawn left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I reviewed the code visually, and it looks good to me. All tests are passed on Frontier(craygnu-mphipcc, craycray-mphipcc, and craygnu), PM-CPU(gnu), PM-GPU(gnugpu), and Chrysalis(gnu). Approved.

@grnydawn

Copy link
Copy Markdown

Due to replacing the spdlog submodule with a new version and location, the changes to the logging module in this PR conflict with those in the develop branch.

@sbrus89

sbrus89 commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

@mwarusz, does E3SM-Project#8559 getting merged mean it is time to do another E3SM/master merge into Omega and then merge this PR?

@mwarusz

mwarusz commented Aug 13, 2026

Copy link
Copy Markdown
Member Author

@sbrus89

Yes, that would be good, as long as we don't run into any problems. I still need to rebase this PR. I can try to do a local merge after that to see if everything works as expected and, if there are no issues, we can go ahead with the switch. How does that sound ?

@grnydawn grnydawn left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The updates in this PR are a subset of those in the previous PR. Previously, all tests passed on Frontier, PM-CPU, PM-GPU, and Chrysalis. The current PR also passed all CTests on PM-GPU and Chrysalis. Approved.

@mwarusz
mwarusz marked this pull request as ready for review August 14, 2026 19:46
@sbrus89 sbrus89 mentioned this pull request Aug 18, 2026
20 tasks
@sbrus89

sbrus89 commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

@mwarusz, will this PR fix #368, or are more changes needed in another PR?

@mwarusz

mwarusz commented Aug 18, 2026

Copy link
Copy Markdown
Member Author

@sbrus89

No. At first, I misread E3SM-Project#8559. That PR only changes the standard and doesn't update Kokkos. The PR we need to fix #368 is E3SM-Project#8632, which hasn't been merged into E3SM yet.

I am not sure if we want to merge this PR and do a master merge now, or do we want to wait for E3SM-Project#8632. I will leave this decision up to you.

@sbrus89

sbrus89 commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Thanks, that's what I had suspected. Let's wait for E3SM-Project#8632 before we merge #521. Given the level of testing associated with merging master into develop, I think it makes sense to do it once rather than twice. It's good to know all the pieces are almost in place to fix #368. Thanks for helping with this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants