-
Notifications
You must be signed in to change notification settings - Fork 572
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
Remove usage of subpackages from TriBITS Kokkos build and downstream Trilinos packages (#11545) #11808
Conversation
Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection Is Not Necessary for this Pull Request. |
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: bartlettroscoe |
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' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
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.
The changes made to Kokkos Kernels and MueLu look good to me!
CC: @ccober6, @crtrott, @srajama1, @jwillenbring @trilinos/compadre, @trilinos/intrepid2, @trilinos/kokkos-kernels, @trilinos/kokkos, @trilinos/muelu, @trilinos/panzer, @trilinos/phalanx, @trilinos/sacado, @trilinos/shylu, @trilinos/stk, @trilinos/stokhos, @trilinos/teuchos, @trilinos/tpetra, @trilinos/trilinoscouplings, @trilinos/xpetra, There are changes in the PR to your packages related to the refactoring to remove subpackages from Kokkos. This technically breaks the backward compatibility of your packages but it unlikely there are any customers that would experience any breakages (see comments above). NOTE: @lucbv has already approved this PR so I could just merge it but I wanted to give other developers a chance to take a look before I merge this. But I am happy to back out this merge if we discover if downstream customers experience problems with this. |
@bartlettroscoe thanks for the heads-up. It looks good to me, I'll bring the stk changes to the sierra repository. |
Status Flag 'Pre-Merge Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED AND APPROVED by [ lucbv ]! |
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' - AutoMerge IS ENABLED, but the Label AT: AUTOMERGE is not set. Either set Label AT: AUTOMERGE or manually merge the PR... |
The issues with Kokkos were resolved with the merge of kokkos/kokkos#6104 which are duplicated in this PR
Thanks @ndellingwood!
I will go ahead and merge and then we can address any lingering issues in a follow-on PR. Mostly we need to see if close SNL customers will have anything break from these changes. |
NOTE: I just noticed that:
are just changing the names of tests of the GenConfig tool and don't need to match what is currently in Trilinos. Therefore, it is not needed to merge these MRs to the GenConfig system. |
…rilinos#11545, trilinos#11808) These are renamings that should have happened as part of PR trilinos#11808 but somehow got missed. This commit also removes enables for the pakages KokkosContainers and KokkosCore which don't exist anymore.
…s:develop' (ab899a0). * trilinos-develop: (22 commits) Remove non-existant subdir kokkos-kernels/common/common (trilinos#11921, trilinos#11863) Teuchos: Fixing cmake logic Teuchos: Fixing catch() issues with C++ language drift TrilinosSS: include <omp.h> (Fix trilinos#11867) MueLu hierarchical: Fix build error Tpetra: Changes to StaticView for Kokkos PTHREAD to THREADS change Teuchos: Automatically enabling Tecuhos_ENABLE_THREAD_SAFE if you have Kokkos THREADS or OPENMP for the host Stokhos: Add missing KOKKOS_INLINE_FUNCTION to fix build errors on HIP Phalanx: Remove usage of undefined var Kokkos_INCLUDE_DIRS (trilinos#11545) Kokkos: Mark HWLOC as a TriBITS TPL as well (trilinos#11938) Update for removal of Kokkos subpackages and Kokkos test renamings (trilinos#11545, trilinos#11808) KokkosKernels: Remove non-existent common/src/[impl,tpls] include dirs (trilinos#11545) Add test simpleBuildAgainstTrilinos_by_package_build_tree_name (trilinos#11545) Pass in and define compilers before calling find_package(Trilinos) (trilinos#11545) Add `Kokkos::all_libs` alias target for compatibility with TriBITS/Trilinos (trilinos#6157) Export Kokkos_ENABLE_<OPTION> that are relevant Do not append to Kokkos_OPTIONS variables those in the do not export list Expand list of kokkos options not to export with cmake Tpetra: Don't use std::binary_function Tpetra: Fixing missing HIP tesT ...
…s:develop' (ab899a0). * trilinos-develop: (22 commits) Remove non-existant subdir kokkos-kernels/common/common (trilinos#11921, trilinos#11863) Teuchos: Fixing cmake logic Teuchos: Fixing catch() issues with C++ language drift TrilinosSS: include <omp.h> (Fix trilinos#11867) MueLu hierarchical: Fix build error Tpetra: Changes to StaticView for Kokkos PTHREAD to THREADS change Teuchos: Automatically enabling Tecuhos_ENABLE_THREAD_SAFE if you have Kokkos THREADS or OPENMP for the host Stokhos: Add missing KOKKOS_INLINE_FUNCTION to fix build errors on HIP Phalanx: Remove usage of undefined var Kokkos_INCLUDE_DIRS (trilinos#11545) Kokkos: Mark HWLOC as a TriBITS TPL as well (trilinos#11938) Update for removal of Kokkos subpackages and Kokkos test renamings (trilinos#11545, trilinos#11808) KokkosKernels: Remove non-existent common/src/[impl,tpls] include dirs (trilinos#11545) Add test simpleBuildAgainstTrilinos_by_package_build_tree_name (trilinos#11545) Pass in and define compilers before calling find_package(Trilinos) (trilinos#11545) Add `Kokkos::all_libs` alias target for compatibility with TriBITS/Trilinos (trilinos#6157) Export Kokkos_ENABLE_<OPTION> that are relevant Do not append to Kokkos_OPTIONS variables those in the do not export list Expand list of kokkos options not to export with cmake Tpetra: Don't use std::binary_function Tpetra: Fixing missing HIP tesT ...
…s:develop' (ab899a0). * trilinos-develop: (22 commits) Remove non-existant subdir kokkos-kernels/common/common (trilinos#11921, trilinos#11863) Teuchos: Fixing cmake logic Teuchos: Fixing catch() issues with C++ language drift TrilinosSS: include <omp.h> (Fix trilinos#11867) MueLu hierarchical: Fix build error Tpetra: Changes to StaticView for Kokkos PTHREAD to THREADS change Teuchos: Automatically enabling Tecuhos_ENABLE_THREAD_SAFE if you have Kokkos THREADS or OPENMP for the host Stokhos: Add missing KOKKOS_INLINE_FUNCTION to fix build errors on HIP Phalanx: Remove usage of undefined var Kokkos_INCLUDE_DIRS (trilinos#11545) Kokkos: Mark HWLOC as a TriBITS TPL as well (trilinos#11938) Update for removal of Kokkos subpackages and Kokkos test renamings (trilinos#11545, trilinos#11808) KokkosKernels: Remove non-existent common/src/[impl,tpls] include dirs (trilinos#11545) Add test simpleBuildAgainstTrilinos_by_package_build_tree_name (trilinos#11545) Pass in and define compilers before calling find_package(Trilinos) (trilinos#11545) Add `Kokkos::all_libs` alias target for compatibility with TriBITS/Trilinos (trilinos#6157) Export Kokkos_ENABLE_<OPTION> that are relevant Do not append to Kokkos_OPTIONS variables those in the do not export list Expand list of kokkos options not to export with cmake Tpetra: Don't use std::binary_function Tpetra: Fixing missing HIP tesT ...
…s:develop' (ab899a0). * trilinos-develop: (23 commits) Remove non-existant subdir kokkos-kernels/common/common (trilinos#11921, trilinos#11863) Teuchos: Fixing cmake logic Teuchos: Fixing catch() issues with C++ language drift fastilu: Fix memory leak. TrilinosSS: include <omp.h> (Fix trilinos#11867) MueLu hierarchical: Fix build error Tpetra: Changes to StaticView for Kokkos PTHREAD to THREADS change Teuchos: Automatically enabling Tecuhos_ENABLE_THREAD_SAFE if you have Kokkos THREADS or OPENMP for the host Stokhos: Add missing KOKKOS_INLINE_FUNCTION to fix build errors on HIP Phalanx: Remove usage of undefined var Kokkos_INCLUDE_DIRS (trilinos#11545) Kokkos: Mark HWLOC as a TriBITS TPL as well (trilinos#11938) Update for removal of Kokkos subpackages and Kokkos test renamings (trilinos#11545, trilinos#11808) KokkosKernels: Remove non-existent common/src/[impl,tpls] include dirs (trilinos#11545) Add test simpleBuildAgainstTrilinos_by_package_build_tree_name (trilinos#11545) Pass in and define compilers before calling find_package(Trilinos) (trilinos#11545) Add `Kokkos::all_libs` alias target for compatibility with TriBITS/Trilinos (trilinos#6157) Export Kokkos_ENABLE_<OPTION> that are relevant Do not append to Kokkos_OPTIONS variables those in the do not export list Expand list of kokkos options not to export with cmake Tpetra: Don't use std::binary_function ...
…s:develop' (ab899a0). * trilinos-develop: (23 commits) Remove non-existant subdir kokkos-kernels/common/common (trilinos#11921, trilinos#11863) Teuchos: Fixing cmake logic Teuchos: Fixing catch() issues with C++ language drift fastilu: Fix memory leak. TrilinosSS: include <omp.h> (Fix trilinos#11867) MueLu hierarchical: Fix build error Tpetra: Changes to StaticView for Kokkos PTHREAD to THREADS change Teuchos: Automatically enabling Tecuhos_ENABLE_THREAD_SAFE if you have Kokkos THREADS or OPENMP for the host Stokhos: Add missing KOKKOS_INLINE_FUNCTION to fix build errors on HIP Phalanx: Remove usage of undefined var Kokkos_INCLUDE_DIRS (trilinos#11545) Kokkos: Mark HWLOC as a TriBITS TPL as well (trilinos#11938) Update for removal of Kokkos subpackages and Kokkos test renamings (trilinos#11545, trilinos#11808) KokkosKernels: Remove non-existent common/src/[impl,tpls] include dirs (trilinos#11545) Add test simpleBuildAgainstTrilinos_by_package_build_tree_name (trilinos#11545) Pass in and define compilers before calling find_package(Trilinos) (trilinos#11545) Add `Kokkos::all_libs` alias target for compatibility with TriBITS/Trilinos (trilinos#6157) Export Kokkos_ENABLE_<OPTION> that are relevant Do not append to Kokkos_OPTIONS variables those in the do not export list Expand list of kokkos options not to export with cmake Tpetra: Don't use std::binary_function ...
…os#11545, trilinos#11808) This duplication resulted from running a simple automated script that created a commit in PR trilinos#11808.
…os#11545, trilinos#11808) This duplication resulted from running a simple automated script that created a commit in PR trilinos#11808.
…os#11545, trilinos#11808) This duplication resulted from running a simple automated script that created a commit in PR trilinos#11808.
…os#11545, trilinos#11808) This duplication resulted from running a simple automated script that created a commit in PR trilinos#11808.
…os#11545, trilinos#11808) This duplication resulted from running a simple automated script that created a commit in PR trilinos#11808.
…os#11545, trilinos#11808) This duplication resulted from running a simple automated script that created a commit in PR trilinos#11808.
CC: @trilinos/compadre, @trilinos/intrepid2, @trilinos/kokkos-kernels, @trilinos/kokkos, @trilinos/muelu, @trilinos/panzer, @trilinos/phalanx, @trilinos/sacado, @trilinos/shylu, @trilinos/stk, @trilinos/stokhos, @trilinos/teuchos, @trilinos/tpetra, @trilinos/trilinoscouplings, @trilinos/xpetra
Description
This PR takes a straightforward approach to remove the usage of subpackages from the TriBITS build of Kokkos.
Some of the more significant changes made in this PR include:
Core
,Containers
,Algorithms
, orSimd
(and all of the associated changes that go with that).core
,containers
,algorithms
, andsimd
are always enabled all the time as well as all of their targets and tests.Kokkos<Subpkg>_XXX
toKokkos_<Subpkg>XXX
.Dependencies.cmake
files now listKokkos
and notKokkosCore
,KokkosContainers
, etc.<Package>_ENABLE_Kokkos<Spkg>
are now just one var<Package>_ENABLE_Kokkos
(and all of the changes associated with that including user cache vars<Package>_ENABLE_Kokkos<Spkg>
now becoming<Package>_ENABLE_Kokkos
).<Package>_Confg.h[.in]
files with the suffixes_KOKKOSCORE
,_KOKKOSCONTAINERS
, etc. are now just_KOKKOS
and this change is propagated to all C++ source files as well. (This touched a lot of source files.)Any of the above could break backward compatibility for downstream software depending on what they depend on. Below are some cases where this PR breaks backward compatibility for Trilinos customers:
The user configuring Trilinos has Kokkos enabled but has disabled support for
KokkosContainers
inMuelu
by passing inMueLu_ENABLE_KokkosContainers=OFF
. That can be fixed by changing it toMueLu_ENABLE_Kokkos=OFF
Downstream (even non-CMake-based) software depends on the
#define
macros coming from Trilinos configured header files. For example, if downstream C++ software had ifndefs like#ifdef HAVE_SACADO_KOKKOSCORE
, then that code would be broken. That would be fixed by changing it to#ifdef HAVE_SACADO_KOKKOS
See the commit log messages for more details.
NOTES:
The refactorings in the Kokkos package itself was all done manually but the bulk of the other refactorings were done using scripts provided in this email. (Will provide more detailed notes later.)
There is some more cleanup that could be performed in the various packages to remove duplication and redundant code (as will be obvious if you look over the changes carefully) but the code as is should work correctly.
This technically breaks backward comparability for both downstream TriBITS packages and downstream non-TriBITS cmake projects. But it is likely that most downstream non-TriBITS CMake projects likely will not notice any changes since they likely just call
find_package(Kokkos)
orfind_package(Trilinos)
and, for example, to contents of the configured fileKokkosCore_Config.h
are unchanged after this refactoring.It is possible to provide for a little more backward compatibility like maintaining the macro suffixes
_KOKKOSCORE
,_KOKKOSCONTAINERS
, etc. that would not require changes to source files in Trilinos or source files in downstream projects (through included<Package>_Config.h
files) but this would require a lot of manual CMake code in the impacted<packageDir>/CMakeLists.txt
files.Tasks
[ ] Merge https://gitlab-ex.sandia.gov/trilinos-devops-consolidation/code/GenConfig/-/merge_requests/92 (test name changes)... Skipped, see below[ ] Merge https://gitlab-ex.sandia.gov/trilinos-project/son-ini-files/-/merge_requests/3 (test name changes)... Skipped, see below