EAMxx: Kokkos 4.5 update, fixes and workarounds for Kokkos update, Aurora support #6916

oksanaguba · 2025-01-19T03:54:29Z

No description provided.

oksanaguba · 2025-01-19T20:48:57Z

this commit is running on sunspot

~/ess/v1-jan19/run/atm.log.11365537.amn-0001.250119-204314

oksanaguba · 2025-01-19T20:50:49Z

i should have not committed to here turning off compose and mam, will remove once this PR is finalized.

ambrad · 2025-01-19T21:32:29Z

Looks good. A couple of points:

In addition to any usual pre-merge testing, I strongly recommend running e3sm_scream_v1_medres with this PR on Frontier, pm-gpu, and Chrysalis, as well as homme_integration on Chrysalis.
~~I'm thinking the EKAT update isn't right.~~ No, it's fine. I looked more closely. I'm seeing some commits from Luca that diff, but I suspect that's intended. Edit 2: Ok, maybe my initial analysis was right: there seems to be some unintended diffs in ekat.

ambrad · 2025-01-19T21:33:29Z

components/eamxx/CMakeLists.txt

+if (Kokkos_ENABLE_SYCL)
+  #enable_language(SYCL)
+  set (EAMXX_ENABLE_GPU TRUE CACHE BOOL "" FORCE)
+  set (SYCL_BUILD TRUE CACHE BOOL "" FORCE) #needed for yakl if kokkos vars are not visible there?


You could turn off COMPOSE here, I believe. (I'll fix the COMPOSE issues once I'm on the machine.)

ambrad · 2025-01-19T21:34:08Z

components/eamxx/src/dynamics/homme/CMakeLists.txt

@@ -23,7 +23,8 @@ set(BUILD_HOMME_PREQX_KOKKOS OFF CACHE BOOL "")
 set(BUILD_HOMME_PESE         OFF CACHE BOOL "")
 set(BUILD_HOMME_SWIM         OFF CACHE BOOL "")
 set(BUILD_HOMME_PRIM         OFF CACHE BOOL "")
-set(HOMME_ENABLE_COMPOSE     ON  CACHE BOOL "")
+#set(HOMME_ENABLE_COMPOSE     ON  CACHE BOOL "")
+set(HOMME_ENABLE_COMPOSE     OFF  CACHE BOOL "")


Just noting here this is what needs to be removed from this PR.

ambrad · 2025-01-19T21:35:41Z

components/eamxx/src/physics/rrtmgp/CMakeLists.txt

+
+#<<<<<<< HEAD
+#find_library(NETCDF_C netcdf HINTS $ENV{NETCDF_C_PATH}/lib)
+#target_link_libraries(scream_rrtmgp_yakl ${NETCDF_C} rrtmgp scream_share)


Can you explain this change? The second line I'm guessing is due to Kokkos, but what about the NETCDF_C line?

i need to ask Jim about this -- iirc i had to set io libs in general, but also some cmake variables for io for rrtmgp separately. so i left this file messy, because i still need to sort it out.

oksanaguba · 2025-01-19T21:54:50Z

i id not understant EKAT comment -- i have an ekat branch, oksanaguba/spot, which i maintained only to point to a different kokkos. so i periodically merge master into it, but not too frequently, so it may be behind the ekat used in master, but its kokkos points not to e3sm kokkos, but to kokkos develop branch.

this is another issue to sort out (but i would not know how).

ambrad · 2025-01-19T22:07:07Z

i id not understant EKAT comment

Re: EKAT, it appears there are missing commits on your branch w.r.t. the submodule point in E3SM: E3SM-Project/EKAT@oksanaguba/spot...4231383

@bartgol am I seeing that correctly? Also, is current EKAT master ready to be used in E3SM?

If so, Oksana, I recommend rebasing your ekat branch on master, then pointing the ekat submodule to that cleaned-up branch.

More generally, testing across machines will help to flush out issues, if there are any, including any with ekat.

oksanaguba · 2025-01-19T22:13:14Z

there are some December commits in the diff you posted, yes, at least those are expected. my ekat branch was not updated too frequently. will do what you suggested, thanks.

ambrad · 2025-01-19T22:25:16Z

As an example, here is one commit I'm either confused about or is truly missing from your branch: E3SM-Project/EKAT@5804bfa. This commit was merged in late Nov 2024, which is why I'm thinking there's a fair bit missing.

Edit: I think EKAT PR 347 was the last merged into your branch. PRs 349-356 are missing. (357-359 are in EKAT master but not used yet in E3SM, and 348 was closed without merging.)

Thus, what I wrote above is probably the correct solution: Rebase your branch on EKAT master.

bartgol

Looks good to me. I have a couple of questions/remarks, but nothing too important. I'm going to approve, modulo the fix of EKAT submodule that you are already working on.

bartgol · 2025-01-20T16:28:58Z

cime_config/machines/cmake_macros/oneapi-ifxgpu.cmake

-string(APPEND CMAKE_CXX_FLAGS_RELEASE " -O2")
-string(APPEND CMAKE_Fortran_FLAGS_DEBUG   " -O0 -g -fpe0")
+
+#adding -g here leads to linker internal errors


And yet I see -g below. Is this comment outdated, and link is fine now? If so, we should remove it to avoid confusion.

"just -g" is in debug flags only, and i haven't tried to build debug. so i only changed release flags.

i was able to compile and to run with "just -g" on sunspot, however, the folder size for e3sm build gets from say 4 gb to 140 gb with full -g. last i tried "just -g" on aurora, there were linking issues. so this is all WIP. i'd say keep this as is now and sort it later.

What I meant is that the comment is confusing: it makes it sound like we should not use -g b/c of link errors, but we do use it across the board. Maybe the comment needs to be clarified. Something like "WARNING: the -g flag seems to have a wild behavior sometimes, with side effects ranging from VERY large binaries to link errors. If you experience errors, remove it".

thanks, done

bartgol · 2025-01-20T16:38:10Z

components/eamxx/src/dynamics/homme/physics_dynamics_remapper.cpp

@@ -426,6 +426,10 @@ do_remap_fwd()
  const int team_size = std::min(256, std::min(128*m_num_phys_cols,32*(concurrency/this->m_num_fields+31)/32));
 #endif

+#ifdef KOKKOS_ENABLE_SYCL
+  const int team_size = 4;


I am puzzled by this. Do we really want just 4 on sycl? It's staggeringly different from other GPU platforms' default...

the change is from Nov 2023. i looked at my notes and did not see why it was needed. i will try to remove it.

This is for the P-D remapper (not the GllFvPhys one), so its relevance is somewhat limited (doesn't run at runtime, usually). So not a huge deal. But if things work without, no point in limiting team size to 4.

used hip TS calculation and it worked at least for ne4 run.

bartgol · 2025-01-20T16:47:33Z

components/eamxx/src/physics/rrtmgp/CMakeLists.txt

+#<<<<<<< HEAD
+#find_library(NETCDF_C netcdf HINTS $ENV{NETCDF_C_PATH}/lib)
+#target_link_libraries(scream_rrtmgp_yakl ${NETCDF_C} rrtmgp scream_share)
+#=======
 find_library(NETCDF_C netcdf HINTS ${NetCDF_C_PATH}/lib)


While at it, do you mind adding ${NetCDF_C_PATH}/lib64 to the hints? Since lib64 is quite common, we should be proactive and add it to the list of hints (e..g, my laptop and workstation use lib64)

thanks, done

ambrad · 2025-01-20T18:54:02Z

Looks like you need to merge ekat master into your branch one more time to get the macro name fix.

oksanaguba · 2025-01-21T17:09:22Z

ran this commit with kokkos 4.5.01 tag tail ~/ess/v1-jan21/run/atm.log.11682723.amn-0001.250121-164543 on sunspot.

ambrad

Looks great.

Note that SCREAM nightlies run on master, not next, so I always run e3sm_scream_v1_medres on Frontier, Chrysalis, and pm-gpu before merging a complex PR to avoid having to fix master later. I recommend it to others, as well.

ambrad · 2025-01-21T18:52:29Z

I added Conrad to the reviewers since he's working on upgrading Kokkos for multiple reasons, including Aurora readiness.

ambrad · 2025-01-21T18:56:08Z

CI is failing with a checkout on the ekat submodule.

  Error: fatal: remote error: upload-pack: not our ref 1eae1ad5a01cdb510726624ac647e7f96dd883d0
  Error: fatal: Fetched in submodule path 'externals/ekat', but it did not contain 1eae1ad5a01cdb510726624ac647e7f96dd883d0. Direct fetching of that commit failed.

Looking at the branch, https://github.com/E3SM-Project/EKAT/commits/oksanaguba/spot/, it's unclear where 1eae1a is coming from. @oksanaguba I suggest checking the submodule state in this PR w.r.t. to your ekat branch.

oksanaguba · 2025-01-21T19:10:29Z

@ambrad my ekat branch had to use kokkos main repo. if CI somehow does not know to add a remote (the default remote in ekat is poinitng to e3sm kokkos (whatever it is)), then it would not be able to fetch my commit.

ambrad · 2025-01-21T19:13:54Z

my ekat branch had to use kokkos main repo.

I'm sure I'm confused, but on your ekat branch I see this as the Kokkos submodule: https://github.com/E3SM-Project/kokkos/tree/e6e5c4598d16d756db62225ab7f937ee833bd660. This appears to be in E3SM-Project/kokkos.

ambrad · 2025-01-21T19:16:00Z

Re: commit 1eae1a, I don't see where it's coming from. I don't see it in your EKAT spot branch, but I could easily be mistaken.

ambrad · 2025-01-21T20:14:18Z

Re: Kokkos: Ok, I'm guessing what's happening is you have your local submodule pointed to the Kokkos repo with this commit, but, as you wrote, the submodule metadata points to E3SM-Project/Kokkos. Generally, we have to put some extra commits on top of a Kokkos version, which is why E3SM-Project/Kokkos exists. Thus, I suggest you start by creating a branch in E3SM-Project/Kokkos with the desired Kokkos state and then use this in the submodule. That will let testing proceed.

oksanaguba · 2025-01-21T20:36:05Z

i'll ask Conrad as he prob already has a branch

oksanaguba · 2025-01-23T02:49:17Z

i pointed the branch to an e3sm kokkos branch, but haven't had a chance to see if CI is happier. also, will do more testing tomorrow.

Previously, there were two. Now, with two additional template parameters, there are eight. This is to try to work around an Nvidia internal compiler error on one platform.

…sue.

bartgol · 2025-02-15T00:56:08Z

@oksanaguba In order to get the CI running and verify things work, I rebased on master (the conflict prevented CI from running). The only real conflict was the rrtmgp one (the workflow file I knew how to handle): in there, there were two ways to specify the netcdf C path. I stuck with the version from master. If that's not what you think is needed, I also pushed a branch oksanaguba/eamxx/spot-clean-bartgol-bkp, so you can revert to the state before the rebase.

bartgol · 2025-02-15T01:27:36Z

Note: CI tests are failing since we did not regen baselines once Jim's PR got merged. I triggered the generate job now, but it will be a while until it gets through the queue. We can simply re-run the failed job later to check if this PR is bfb or not.

bartgol · 2025-02-15T01:34:29Z

Ok, some good news and bad news. The CUDA dbg CI job is finally getting past the bld phase, and running tests. The bad news is that, despite the rebase on master, we get some errors. E.g., for homme_shoc_cld_p3_rrtmgp_pg2:

  - Iteration   4 completed       [100%]
[EAMxx] Finalize ...
Constructor for Kokkos::View 'UNMANAGED' has mismatched number of arguments. The number of arguments = 0 neither matches the dynamic rank = 1 nor the total rank = 1

I will have to investigate hopping on blake on monday. The interesting thing is that the error shows up during finalization (as you can see from the fact that the unit test completed 100% of the iterations).

The release build only has the baseline_cmp fails, which should go away if we retest after the baseline-gen job runs.

ambrad · 2025-02-15T01:57:23Z

The interesting thing is that the error shows up during finalization (as you can see from the fact that the unit test completed 100% of the iterations).

I think I know the cause. It's some new nullify routines I created to handle Kokkos's new (>= 4.3, I think) way of dealing with view-of-view initi/finalization. I'll push a fix.

oksanaguba mentioned this pull request Jan 19, 2025

[wip] aurora config #6915

Closed

oksanaguba requested review from amametjanov, jgfouca and ambrad January 19, 2025 20:49

ambrad requested a review from bartgol January 19, 2025 21:29

ambrad reviewed Jan 19, 2025

View reviewed changes

ambrad assigned oksanaguba Jan 19, 2025

bartgol mentioned this pull request Jan 20, 2025

Change name of an internal macro to avoid name clash E3SM-Project/EKAT#360

Merged

bartgol approved these changes Jan 20, 2025

View reviewed changes

ambrad approved these changes Jan 21, 2025

View reviewed changes

ambrad requested a review from tcclevenger January 21, 2025 18:51

ambrad and others added 14 commits February 14, 2025 17:49

Hommexx/SL: Fix atomic_compare_exchang usage.

15a97b4

Homme(xx)/SL: Handle view-of-view cleanup without SequentialHostInit.

5ffa3bb

replace old io paths in eamxx

2f10c5e

Hommexx/SL: Need to retain mirror views for ref counting on GPU.

d3ba47b

Homme/eamxx: fix cmake syntax to link libraries

57bdee2

EAMxx: Fix logical-vs-integer issue.

4e1fb5c

Update ekat to get kokkos setting fix

2f53ba5

fix MAM variable

066289b

Hommexx: Remove use of a quantity Kokkos no longer provides.

103f671

Hommexx: remove deprecated (and unused by Kokkkos) set_num_device usage

935bb6f

aurora entry for gpu runs, perf restored

ddecb1c

Homme(xx)/SL: Break up kernel into eight instantiations.

8c2d63b

Previously, there were two. Now, with two additional template parameters, there are eight. This is to try to work around an Nvidia internal compiler error on one platform.

EAMxx: Update EKAT submodule.

b68a02a

EAMxx: Turn MAM off in old Frontier s/w stack due to compiler-side is…

8bc8505

…sue.

bartgol force-pushed the oksanaguba/eamxx/spot-clean branch from d4e4894 to 4af0429 Compare February 15, 2025 00:54

bartgol force-pushed the oksanaguba/eamxx/spot-clean branch from 4af0429 to 0c944e5 Compare February 15, 2025 01:08

Workflows: hard-code SCREAM_SMALL_KERNELS=ON for CUDA dbg eamxx builds

1e0d4a3

bartgol force-pushed the oksanaguba/eamxx/spot-clean branch from 0c944e5 to 1e0d4a3 Compare February 15, 2025 01:09

Homme/SL: Add rank args to array nullify routines.

99a7cda

ambrad changed the title ~~[wip] aurora config, clean up~~ [wip] Aurora changes with required Kokkos update Feb 15, 2025

ambrad changed the title ~~[wip] Aurora changes with required Kokkos update~~ [wip] Aurora changes, required Kokkos update, fixes and workarounds for Kokkos update Feb 15, 2025

ambrad changed the title ~~[wip] Aurora changes, required Kokkos update, fixes and workarounds for Kokkos update~~ [wip] EAMxx: Aurora support, required Kokkos update, fixes and workarounds for Kokkos update Feb 15, 2025

ambrad changed the title ~~[wip] EAMxx: Aurora support, required Kokkos update, fixes and workarounds for Kokkos update~~ EAMxx: Aurora support, required Kokkos update, fixes and workarounds for Kokkos update Feb 15, 2025

ambrad added EAMxx PRs focused on capabilities for EAMxx and removed wip work in progress labels Feb 20, 2025

mahf708 changed the title ~~EAMxx: Aurora support, required Kokkos update, fixes and workarounds for Kokkos update~~ EAMxx: Kokkos 4.5 update, fixes and workarounds for Kokkos update, Aurora support Feb 20, 2025

EAMxx: Kokkos 4.5 update, fixes and workarounds for Kokkos update, Aurora support #6916

Are you sure you want to change the base?

EAMxx: Kokkos 4.5 update, fixes and workarounds for Kokkos update, Aurora support #6916

Conversation

oksanaguba commented Jan 19, 2025

oksanaguba commented Jan 19, 2025

oksanaguba commented Jan 19, 2025 • edited Loading

ambrad commented Jan 19, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oksanaguba commented Jan 19, 2025

ambrad commented Jan 19, 2025

oksanaguba commented Jan 19, 2025

ambrad commented Jan 19, 2025 • edited Loading

bartgol left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ambrad commented Jan 20, 2025

oksanaguba commented Jan 21, 2025

ambrad left a comment

Choose a reason for hiding this comment

ambrad commented Jan 21, 2025

ambrad commented Jan 21, 2025

oksanaguba commented Jan 21, 2025

ambrad commented Jan 21, 2025

ambrad commented Jan 21, 2025

ambrad commented Jan 21, 2025

oksanaguba commented Jan 21, 2025

oksanaguba commented Jan 23, 2025

bartgol commented Feb 15, 2025

bartgol commented Feb 15, 2025

bartgol commented Feb 15, 2025 • edited Loading

ambrad commented Feb 15, 2025 • edited Loading

oksanaguba commented Jan 19, 2025 •

edited

Loading

ambrad commented Jan 19, 2025 •

edited

Loading

ambrad commented Jan 19, 2025 •

edited

Loading

bartgol commented Feb 15, 2025 •

edited

Loading

ambrad commented Feb 15, 2025 •

edited

Loading