-
Notifications
You must be signed in to change notification settings - Fork 10
Update EKAT to require C++20 #400
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Asking the obv question: are we sure that E3SM will build everywhere with C++20 required? If not, this my hold back any ekat update in e3sm. |
|
It will not build with all of our current default compilers. In particular, the "intel" compiler on Chrysalis. Possibly others. |
bartgol
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Only possible concern is whether e3sm is ready for c++20 across all machines.
|
@bartgol @rljacob Good to know. I created this PR while I'm testing E3SM on the target machines, but since we already know there will be some that are not compatible, we will need to find a different way to handle this. Kokkos 5 is going to require C++20, what would the process be to eventually update compilers on Chrysalis (and maybe others) so that we can upgrade? Or is this going to be a bigger roadblock? I guess the first step is for me to test exactly which compilers do not work. |
|
One solution would be to mis with a cmake2.0 way of specifying the std, via something like if (NOT CMAKE_CXX_STANDARD)
target_compile_features(ekat_core PUBLIC cxx_std_20)
endif()Then, in the mach files for old compilers we keep lines like which will force ekat to honor the required standard. Of course, for machines where we do set 17 in the mach files, we won't be able to build anything that needs kokkos 5.0 (e.g. any F case, since EAM still use kokkos for the SL transport). This means that we cannot switch to kokkos 5 until we can abandon compilers that only support c++17. |
|
On Chrysalis:
|
Ugh, that's underwhelming. I wonder if this is on the chrys IT folks' radar... |
Yeah, I want to try and make a small reproducer outside of E3SM. Make sure it's not something we are doing. |
5695852 to
9b9729c
Compare
9b9729c to
8244207
Compare
74e70c9 to
11b44e4
Compare
This option was not actually forcing the cxx version when I changed to 20...
Resolves some C++20 errors
11b44e4 to
8b79f4f
Compare
|
Update:
@rljacob @bartgol How should we proceed? Can we require users of Chrysalis start switching to oneapi? Do we need to have a frozen version of Kokkos as a tpl for the other compilers? Could we require anyone using ekat (running with EAMxx) use oneapi? |
Did you run both gnu and intel on pm-cpu? I think @ndkeen is also using
I think we can keep both intel and oneapi. But we won't be able to run anything that uses kokkos with the intel compiler. I think this includes also any EAM case though, since EAM uses hommexx's SL transport by default, so this may impact a lot of ppl. @jgfouca wild option: can CIME update a submodule conditionally on the compiler (or any other XML var)? E.g., can we do something like if compiler is "intel":
update_submodule(f'{SRCROOT}/externals/ekat', 'v1.1', recursive=True) to switch the submodule to an older tag? |
|
We will require Chrysalis users to use oneapi-ifx. It will be made the default. ifort will still be an option for anyone who needs it and they have to use an earlier hash. We should make a plan on when this goes to master since its a breaking change for chrysalis. |
We should prob make this change first, and then update ekat/kokkos (and the required cxx standard). Maybe mid-late March? I think 2 months may be a good enough window for ppl to switch to the new compiler. |
|
@bartgol , CIME does not support that currently. We could, in theory, add support for configuration that allows for customizing submodules, but that seems kinda hacky. |
|
Testing update
I will next go to frontier, then I think we will have all the info we need to move forward. |
Update EKAT to require C++20
Motivation
Needed to update to Kokkos 5.0.
Testing
CI already uses C++20.