Skip to content

Update Omega to C++20 - #405

Merged
sbrus89 merged 5 commits into
E3SM-Project:developfrom
mwarusz:omega/cxx20
Oct 2, 2026
Merged

sbrus89 merged 5 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
12 of 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!

@sbrus89

sbrus89 commented Sep 24, 2026

Copy link
Copy Markdown
Collaborator

@mwarusz, I'm testing this in conjunction with #566, and am seeing compile errors related to spdlog on Frontier with craygnu-mphipcc:

/autofs/nccs-svm1_proj/cli115/sbrus/polaris_worktrees/testing/e3sm_submodules/Omega/components/omega/external/spdlog/include/spdlog/fmt/bundled/base.h:440:3: error: reference to __host__ function 'assert_fail' in __host__ __device__ function
  440 |   FMT_ASSERT(std::is_unsigned<Int>::value || value >= 0, "negative value");
      |   ^
/autofs/nccs-svm1_proj/cli115/sbrus/polaris_worktrees/testing/e3sm_submodules/Omega/components/omega/external/spdlog/include/spdlog/fmt/bundled/base.h:397:19: note: expanded from macro 'FMT_ASSERT'
  397 |          : ::fmt::assert_fail(__FILE__, __LINE__, (message)))
      |                   ^
/autofs/nccs-svm1_proj/cli115/sbrus/polaris_worktrees/testing/e3sm_submodules/Omega/components/omega/external/spdlog/include/spdlog/fmt/bundled/base.h:1859:20: note: called by 'append<char>'
 1859 |       auto count = to_unsigned(end - begin);
      |                    ^
/autofs/nccs-svm1_proj/cli115/sbrus/polaris_worktrees/testing/e3sm_submodules/Omega/components/omega/external/spdlog/include/spdlog/fmt/bundled/base.h:354:27: note: 'assert_fail' declared here
  354 | FMT_NORETURN FMT_API void assert_fail(const char* file, int line,
      |                           ^
/autofs/nccs-svm1_proj/cli115/sbrus/polaris_worktrees/testing/e3sm_submodules/Omega/components/omega/external/spdlog/include/spdlog/fmt/bundled/base.h:440:3: error: reference to __host__ function 'assert_fail' in __host__ __device__ function
  440 |   FMT_ASSERT(std::is_unsigned<Int>::value || value >= 0, "negative value");
      |   ^
/autofs/nccs-svm1_proj/cli115/sbrus/polaris_worktrees/testing/e3sm_submodules/Omega/components/omega/external/spdlog/include/spdlog/fmt/bundled/base.h:397:19: note: expanded from macro 'FMT_ASSERT'
  397 |          : ::fmt::assert_fail(__FILE__, __LINE__, (message)))
      |                   ^
/autofs/nccs-svm1_proj/cli115/sbrus/polaris_worktrees/testing/e3sm_submodules/Omega/components/omega/external/spdlog/include/spdlog/fmt/bundled/base.h:1859:20: note: called by 'append<wchar_t>'
 1859 |       auto count = to_unsigned(end - begin);
      |                    ^
/autofs/nccs-svm1_proj/cli115/sbrus/polaris_worktrees/testing/e3sm_submodules/Omega/components/omega/external/spdlog/include/spdlog/fmt/bundled/base.h:354:27: note: 'assert_fail' declared here
  354 | FMT_NORETURN FMT_API void assert_fail(const char* file, int line,
      |                           ^
2 errors generated when compiling for gfx90a.

@mwarusz

mwarusz commented Sep 28, 2026 •

Copy link
Copy Markdown
Member Author

@sbrus89

I rebased and pushed a workaround. I also made a small change to the definition of INNER_LAMBDA in hierarchical parallelism to avoid one million warnings that we otherwise get in C++20 mode on frontier.

@sbrus89

sbrus89 commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator

Local merge passes CTests and omega_pr suite on Frontier with craygnu and craygnu-mphipcc. Also passes omega_pr on Aurora with oneapi-ifxgpu and e3sm_omega_developer in pm-cpu with gnu:

Polaris omega_pr suite

  • Baseline workdir: /ccs/home/brus/run/polaris_baseline_omega_pr_omega_craygnu_mpich/
  • Baseline build: /ccs/home/brus/run/polaris_baseline_omega_pr_omega_craygnu_mpich/ocean_build
  • PR build: /ccs/home/brus/run/polaris_cxx20_omega_pr_omega_craygnu_mpich/ocean_build
  • PR workdir: /ccs/home/brus/run/polaris_cxx20_omega_pr_omega_craygnu_mpich
  • Machine: frontier
  • Partition: batch
  • Compiler: craygnu
  • Build type: Release
  • Log: /ccs/home/brus/run/polaris_cxx20_omega_pr_omega_craygnu_mpich/polaris_omega_pr.o5574597
  • Result: All tests passed

Polaris omega_pr suite

  • Baseline workdir: /ccs/home/brus/run/polaris_baseline_omega_pr_omega_craygnu-mphipcc_mpich/
  • Baseline build: /ccs/home/brus/run/polaris_baseline_omega_pr_omega_craygnu-mphipcc_mpich/ocean_build
  • PR build: /ccs/home/brus/run/polaris_cxx20_omega_pr_omega_craygnu-mphipcc_mpich/ocean_build
  • PR workdir: /ccs/home/brus/run/polaris_cxx20_omega_pr_omega_craygnu-mphipcc_mpich
  • Machine: frontier
  • Partition: batch
  • Compiler: craygnu-mphipcc
  • Build type: Release
  • Log: /ccs/home/brus/run/polaris_cxx20_omega_pr_omega_craygnu-mphipcc_mpich/polaris_omega_pr.o5575057
  • Result: All tests passed

Polaris omega_pr suite

  • Baseline workdir: /home/brus/run/polaris_omega_pr_baseline_oneapi-ifxgpu_mpich/
  • Baseline build: /home/brus/run/polaris_omega_pr_baseline_oneapi-ifxgpu_mpich/ocean_build
  • PR build: /home/brus/run/polaris_cxx20_omega_pr_omega_oneapi-ifxgpu_mpich/ocean_build
  • PR workdir: /home/brus/run/polaris_cxx20_omega_pr_omega_oneapi-ifxgpu_mpich
  • Machine: aurora
  • Compiler: oneapi-ifxgpu
  • Build type: Release
  • Log: /home/brus/run/polaris_cxx20_omega_pr_omega_oneapi-ifxgpu_mpich/polaris_omega_pr.o8898746
  • Result: All tests passed

@sbrus89
sbrus89 merged commit fc9eaef into E3SM-Project:develop Oct 2, 2026
1 check passed
sbrus89 added a commit that referenced this pull request Oct 13, 2026
The changes to Omega's build system to support C++20, brought in by #405, only
affected standalone builds. After #566, we are using Kokkos5 and need C++20 in
coupled mode (in addition to standalone). This PR moves setting the 
CMAKE_CXX_STANDARD and CMAKE_CXX_STANDARD_REQUIRED into the common
macro, so that they apply to both couple and standalone builds.
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