Add PALS_LOCAL_RANKID to list of on-node local RANKID env-vars#2
Conversation
There was a problem hiding this comment.
Pull request overview
Extends Kokkos’ CPU-side MPI rank discovery to recognize PALS-provided local-rank environment variables, improving interoperability on systems using PALS.
Changes:
- Add
PALS_LOCAL_RANKIDto the set of environment variables checked to determine MPI local rank on a node.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Is this mod already in kokkos main repo? If no, would it make sense to put in a PR in the kokkos repo itself? It seems like kokkos should support what's needed on Aurora... |
|
okay, in the main repo, submitted PR kokkos/pull/8890 |
|
This was merged in E3SM-Project/EKAT#372. Closing. |
|
Shouldn't we merge this PR and make ekat point to E3SM-Project/kokkos/master? If this branch gets deleted, we have a broken submodule in ekat. And we should probably not point to an unmerged feature branch. Another issue is what will happen when someone adds another fix to this kokkos fork. They will likely fork from master, so if this branch is not merged in master, we will lose this fix. I think we should merge, and update e3sm-master. @tcclevenger I don't know if you were planning something else, maybe in view of kokkos 5 upgrade, so I'm letting you decide. But leaving things as they are may be dangerous down the road... |
|
Yes, I wasn't thinking about the state of the submodule branch, we should definitely merge this and update in EKAT. I'll create the PR. |
|
Wait, @bartgol I think I'm now confused. Currently, we have the branch |
|
I went ahead and reorged the I may further consolidate into a single commit from the E3SM side, then when updating to kokkos 5 we can decide how much of those extra changes are necessary (some are quite old, I believe), and the PALS commit will drop off. EKAT currently points to the tip of this branch. |
|
@tcclevenger yes, the e3sm-master branch had a couple of commits that were added through the years to fix a couple of issues in kokkos. Honestly, I don't know for sure how much each of those still was. I see that the current version of e3sm-master still has them (at least the ones I remember). And I am now noticing this PR was into e3sm-master, so I think all was good. I got confused cause I thought we were merging into E3SM-Project/kokkos/master or something, which would have been wrong. I think all is good. |
Add PALS_LOCAL_RANKID to list of on-node local RANKID env-vars.
[BFB]