-
Notifications
You must be signed in to change notification settings - Fork 580
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
Updates for ROCm 6 support #12681
Updates for ROCm 6 support #12681
Conversation
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
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.
Stokhos changes look good.
Thanks guys, is there anyone from the ShyLu team that can double check the rocsolver/blas changes? |
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
@iyamazaki can you review the changes to Tacho? |
For the kokkos-kernels changes in this PR, I cherry-picked kokkos/kokkos-kernels#2050 to the release-candidate-4.2.01 branch so that changes in this PR are not clobbered by the patch release |
@@ -65,6 +65,7 @@ TRIBITS_REPOSITORY_DEFINE_TPLS( | |||
Cusp "cmake/TPLs/" ST | |||
ROCBLAS "cmake/TPLs/" PT | |||
ROCSPARSE "cmake/TPLs/" PT | |||
ROCSOLVER "cmake/TPLs/" PT |
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.
👍 for adding cmake/TPLs/FindTPLROCSOLVER.cmake , this will allow a follow-up revert of kokkos/kokkos-kernels#2037
@ndellingwood Actually, I was curious how you guys were managing the snap-shooting of kokkos/kokkoskernels into Trilinos without submodules. How does one cherry-pick kokkos/kk changes into Trilinos? |
Hi @seanofthemillers , I think what you did hear is just fine - you ensured the changes made to Trilinos were already submitted, reviewed, and approved by devs for the kokkos-kernels stand-alone repo. That is the necessary step to ensure that changes between the repos stay consistent. |
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
Hi @seanofthemillers , the kokkos-kernels snapshot in PR #12707 is merged which includes the kokkos-kernels updates in this PR, would you be able to rebase this PR on top of develop? I'll approve and can add the AUTOMERGE label once that is done. Thanks for your patience waiting on that PR to complete |
24960b1
to
7fd5b26
Compare
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
Thanks @ndellingwood, I've rebased and squashed it down. |
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.
Thanks @seanofthemillers !
@ndellingwood Actually I have a question - If I come across issues in KokkosKernels (and fix it there), is it better for me to patch them into Trilinos manually, or should I submit an issue to for you guys to pull in a fully updated KokkosKernels? I'm not sure what the standard approach is. |
Status Flag 'Pre-Test Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED by label AT: PRE-TEST INSPECTED! Autotester is Removing Label; this inspection will remain valid until a new commit to source branch is performed. |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_PR_gcc-8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-8.3.0-serial
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-8.3.0-debug
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_clang-11.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_cuda-11.4.2-uvm-off
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_intel-2021.3
Jenkins Parameters
Using Repos:
Pull Request Author: seanofthemillers |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: Trilinos_PR_gcc-8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-8.3.0-serial
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-8.3.0-debug
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_clang-11.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_cuda-11.4.2-uvm-off
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_intel-2021.3
Jenkins Parameters
|
Status Flag 'Pre-Merge Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED AND APPROVED by [ ndellingwood ]! |
Status Flag 'Pull Request AutoTester' - Pull Request will be Automerged |
Merge on Pull Request# 12681: IS A SUCCESS - Pull Request successfully merged |
@seanofthemillers yeah, it's a good question. Generally we prefer kokkos-kernels changes to come into Trilinos via snapshot during releases, but we know the period between releases can be quite long and important changes (like in this PR) need to be patched directly to Trilinos (which still requires the separate PR to the kokkos-kernels repo and approvals from the devs). The process you followed in this PR was correct, where you submitted the change to kokkos-kernels and then peeled them off to submit to Trilinos (or could have been done in the opposite order). If you run into issues with merge conflicts between the separate branches feel free to ping me and I'll help to get them cleaned up. Thanks again for the work on this PR! |
@trilinos/ShyLu
Motivation
These changes are meant to support building Trilinos with ROCm 6.0.
The KokkosKernels changes have already been upstreamed in the KokkosKernels repository.
The ShyLu changes are there to fix linking, and to handle some directory changes in ROCm 6.
FYI @jjellio
Stakeholder Feedback
Testing