-
Notifications
You must be signed in to change notification settings - Fork 407
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
Try to autodetect the number of devices #3013
Conversation
Retest this please. |
1 similar comment
Retest this please. |
namespace Experimental { | ||
HIP::size_type HIP::detect_device_count() { | ||
return HIPInternalDevices::singleton().m_hipDevCount; | ||
} |
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.
Why did you implement it here rather than Kokkos_HIP_Space.cpp
?
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.
HIPInternalDevices
is declared in this file only in this file in an anonymous namespace. If preferred, we can also declare that class in Kokkos_HIP_Instance.hpp
and move the implementation to Kokkos_HIP_Space.cpp
.
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.
Fair enough. We will need to clean that out at some point. I am not asking you to change.
kokkos/core/src/HIP/Kokkos_HIP_Space.cpp
Lines 590 to 593 in 266724d
int HIP::concurrency() { | |
auto const& prop = hip_device_prop(); | |
return prop.maxThreadsPerMultiProcessor * prop.multiProcessorCount; | |
} |
kokkos/core/src/HIP/Kokkos_HIP_Space.cpp
Lines 631 to 633 in 266724d
hipDeviceProp_t const& HIP::hip_device_prop() { | |
return Impl::HIPInternal::singleton().m_deviceProp; | |
} |
Fixes #2975. After talking to @crtrott this is enough at a first step.
We might want to check that all the devices also satisfy the compute capability given or something similar.
So far, I checked that we arrive at the line changed in this pull request with
-1
by default.While there, I also changed the type of the numerous
*_found
variables tobool
. I can rip that out into a new pull request if preferred.