-
Notifications
You must be signed in to change notification settings - Fork 578
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
Add some constexpr traits to Tpetra Node types #12284
Add some constexpr traits to Tpetra Node types #12284
Conversation
- is_serial: is it Kokkos::Serial - is_cpu: is it a CPU-like device (default memspace is HostSpace) - is_gpu: is it a GPU-like device (default memspace is something other than HostSpace)
@csiefer2 @jhux2 @cwpearson This is probably the way I would deal with the issue @cgcgcg brought up. We can talk about it at next week's meeting |
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 fine
We might want similar variables for use in CMake. And maybe a macro that does something like this
Maybe something similar to MueLu's test ETI could work for passing a function to call? |
@crtrott Any thoughts on the naming scheme / how this fits with future Kokkos plans? |
There are other concerns too --- like getting std::strings names out of the space (Kokkos might already support this) and default communication spaces (e.g. CudaSpace instead of CudaUVMSpace) as well as Execution space instance w/ Priority (@cwpearson 's stuff) |
@cgcgcg do you know why we use checks like:
instead of the simpler
|
@lucbv Because someone wrote that once and then it got copy-pasta'd all over the place. |
@csiefer2 The first two things you mention exist. There are 3 std::string names you can get from a node: node, exec and mem:
For the comm buffer memspace, we have |
Related to kokkos/kokkos#6006. If there are more customers/users requesting that feature we were considering supporting it in |
Status Flag 'Pull Request AutoTester' - User Requested Retest - Label AT: RETEST will be reset after testing. |
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: brian-kelley |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: 1 or more Jobs FAILED Note: Testing will normally be attempted again in approx. 2 Hrs 30 Mins. If a change to the PR source branch occurs, the testing will be attempted again on next available autotester run. Pull Request Auto Testing has FAILED (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
|
#endif | ||
|
||
//! Whether the ExecutionSpace is CPU-like (its default memory space is HostSpace) | ||
static constexpr bool is_cpu = std::is_same_v<typename ExecutionSpace::memory_space, Kokkos::HostSpace>; |
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.
Careful, Kokkos actually also defines HBWSpace
for things like KNL and SapphireRapids which should lead to is_cpu == true
.
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.
Good catch, I'll fix this in a follow up PR
Status Flag 'Pull Request AutoTester' - User Requested Retest - Label AT: RETEST will be reset after testing. |
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: brian-kelley |
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 [ lucbv csiefer2 ]! |
Status Flag 'Pull Request AutoTester' - Pull Request will be Automerged |
Merge on Pull Request# 12284: IS A SUCCESS - Pull Request successfully merged |
Node::is_serial
: is it Kokkos::Serial?Node::is_cpu
: is it a CPU-like device? (default memspace is HostSpace)Node::is_gpu
: is it a GPU-like device? (default memspace is something other than HostSpace)@trilinos/tpetra
Motivation
Possible solution to issue discussed at Tpetra meeting. Instead of having to compare the node or execution space to every possible case (with ifdefs to check if types like
Kokkos::Serial
even exist), this lets users check the Node for common properties more easily.@csiefer2 comments: Basically, I only want to see the words "HIP" "CUDA" and "SYCL" in Trilinos code where it is absolutely necessary for performance. We have a lot of boilerplate where this stuff is littered through Trilinos right now, which makes adding new backends way harder then it needs to be.
Stakeholder Feedback
Testing