Refactor device selection at initialization #5211

dalg24 · 2022-07-13T15:43:54Z

Deprecate num_devices and skip_device settings, --kokkos-num-devices command line argument, KOKKOS_NUM_DEVICES, KOKKOS_SKIP_DEVICES, and KOKKOS_RAND_DEVICES environment variable.

Introduce as a replacement

map_device_id_by setting along with --kokkos-map-device-id-by and KOKKOS_MAP_DEVICE_ID_BY which can be set to "random" or "mpi_rank".
KOKKOS_VISIBLE_DEVICES environment variable to specify device ids as a comma-separated sequence of integers

crtrott · 2022-07-13T18:19:49Z

Summary:

KOKKOS_VISIBLE_DEVICES -> externally enforced mask

// These also exist as members in the initialization struct
--kokkos-device-id=2
--kokkos-map-device-id-by=random
--kokkos-map-device-id-by=mpi-rank
(--kokkos-map-device-id-by=[specific-device(2)|2])

core/src/impl/Kokkos_Core.cpp

fnrizzi · 2022-07-14T07:10:56Z

@dalg24 You said KOKKOS_VISIBLE_DEVICES stores a comma separated list of integers, so this is like NVIDIA_VISIBLE_DEVICES env variable right? Since it is a list of integers, what about renaming it KOKKOS_VISIBLE_DEVICE_IDS? I realize this departs from the cuda naming, but IMO it is more expressive of what this env var actually stores.

When we say "device id", it is implied id with respect to enumeration given the by, e.g., nvidia-smi right?

dalg24 · 2022-07-14T11:11:49Z

@dalg24 You said KOKKOS_VISIBLE_DEVICES stores a comma separated list of integers, so this is like NVIDIA_VISIBLE_DEVICES env variable right?

I was going for {CUDA,HIP}_VISIBLE_DEVICES. I had actually forgotten about NVIDIA_VISIBLE_DEVICES. That one actually also supports all, none, void or empty or unset but these make little sense for us.

Since it is a list of integers, what about renaming it KOKKOS_VISIBLE_DEVICE_IDS? I realize this departs from the cuda naming, but IMO it is more expressive of what this env var actually stores.

I like KOKKOS_VISIBLE_DEVICES better because it is shorter, aligned with current vendor practice, and somehow making "device ids visible" rubs me wrong. But we will sees if others agree with you.

When we say "device id", it is implied id with respect to enumeration given the by, e.g., nvidia-smi right?

Yes, in whatever order the vendor runtimes detects them.

…_device_count()

core/src/impl/Kokkos_Core.cpp

… fallback to 0

Co-Authored-By: Christian Trott <crtrott@sandia.gov>

…nd KOKKOS_DEVICE_ID

JBludau

looks good

dalg24 · 2022-07-25T11:54:53Z

core/src/impl/Kokkos_Core.cpp

@@ -780,6 +840,7 @@ void Kokkos::Impl::parse_command_line_arguments(
          !kokkos_num_devices_found) {
        num_devices = std::stoi(num1_only);
        settings.set_num_devices(num_devices);
+        settings.set_map_device_id_by("mpi_rank");


@XzzX @crtrott
I suspect the issue in #5252 comes from this line. Probably needs to move it to L815

dalg24 · 2022-07-25T12:05:44Z

core/src/impl/Kokkos_Core.cpp

-  int use_gpu        = settings.has_device_id() ? settings.get_device_id() : -1;
-  const int ndevices = [](int num_devices) -> int {
-    if (num_devices > 0) return num_devices;
-#if defined(KOKKOS_ENABLE_CUDA)
-    return Cuda::detect_device_count();
-#elif defined(KOKKOS_ENABLE_HIP)
-    return Experimental::HIP::detect_device_count();
-#elif defined(KOKKOS_ENABLE_SYCL)
-    return sycl::device::get_devices(sycl::info::device_type::gpu).size();
-#else
-    return num_devices;
-#endif
-  }(settings.has_num_devices() ? settings.get_num_devices() : -1);
-  const int skip_device =
-      settings.has_skip_device() ? settings.get_skip_device() : 9999;
-
-  // if the exact device is not set, but ndevices was given, assign round-robin
-  // using on-node MPI rank
-  if (use_gpu < 0) {


@XzzX @crtrott I think this is what changed. Will fix.

crtrott added the Blocks Promotion Overview issue for release-blocking bugs label Jul 13, 2022

dalg24 added 2 commits July 13, 2022 21:25

Rename rdevices -> rand_devices to improve readability

e15c208

Add map_device_id_by string option that can be "random" or "mpi_rank"

5519d18

dalg24 force-pushed the refactor_device_selection branch from db45bd3 to ae03cc4 Compare July 14, 2022 02:18

dalg24 marked this pull request as ready for review July 14, 2022 02:18

dalg24 added 4 commits July 13, 2022 22:25

Deprecate num_devices, skip_device, and rand_devices settings

da19e4c

Add KOKKOS_VISIBLE_DEVICES environment variable

d3a6d48

Add unit test for get_visible_devices()

f6ce9ee

Fix typo concur[r]ently

963df48

dalg24 force-pushed the refactor_device_selection branch from ae03cc4 to 963df48 Compare July 14, 2022 02:26

fnrizzi reviewed Jul 14, 2022

View reviewed changes

core/src/impl/Kokkos_Core.cpp Outdated Show resolved Hide resolved

Improve comment above line that would cause segfault if called in get…

c90df15

…_device_count()

masterleinad reviewed Jul 14, 2022

View reviewed changes

core/src/impl/Kokkos_Core.cpp Outdated Show resolved Hide resolved

Prefer abort to segfault

d9509a6

JBludau reviewed Jul 14, 2022

View reviewed changes

crtrott requested changes Jul 14, 2022

View reviewed changes

dalg24 and others added 4 commits July 14, 2022 11:54

Call abort() when map_device_id_by string is not recognized

3da07a9

Improve warning when MPI local rank is not known and say what that we…

182baa4

… fallback to 0

Document --kokkos-map-device-if-by in help message

d2bcf4a

Co-Authored-By: Christian Trott <crtrott@sandia.gov>

Fix warning placeholder for specifying both KOKKOS_MAP_DEVICE_ID_BY a…

66dcbf5

…nd KOKKOS_DEVICE_ID

crtrott approved these changes Jul 14, 2022

View reviewed changes

fnrizzi approved these changes Jul 14, 2022

View reviewed changes

JBludau approved these changes Jul 14, 2022

View reviewed changes

dalg24 merged commit d924206 into kokkos:develop Jul 14, 2022

dalg24 deleted the refactor_device_selection branch July 14, 2022 22:07

dalg24 mentioned this pull request Jul 15, 2022

Harmonize device selection and allow SYCL to circumvent it when no GPU is available #5222

Merged

ndellingwood mentioned this pull request Jul 15, 2022

Trilinos nightly test failures, Kokkos defaultdevicetype.visible_devices unit test with cuda/9.2 #5225

Closed

XzzX mentioned this pull request Jul 25, 2022

strange behavior of CudaUVMSpace in develop branch #5252

Closed

dalg24 commented Jul 25, 2022

View reviewed changes

dalg24 mentioned this pull request Jul 25, 2022

Restore old behavior in device selection when user specifies nothing #5259

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor device selection at initialization #5211

Refactor device selection at initialization #5211

dalg24 commented Jul 13, 2022

crtrott commented Jul 13, 2022

fnrizzi commented Jul 14, 2022 •

edited

dalg24 commented Jul 14, 2022

JBludau left a comment

dalg24 Jul 25, 2022

dalg24 Jul 25, 2022

Refactor device selection at initialization #5211

Refactor device selection at initialization #5211

Conversation

dalg24 commented Jul 13, 2022

crtrott commented Jul 13, 2022

fnrizzi commented Jul 14, 2022 • edited

dalg24 commented Jul 14, 2022

JBludau left a comment

Choose a reason for hiding this comment

dalg24 Jul 25, 2022

Choose a reason for hiding this comment

dalg24 Jul 25, 2022

Choose a reason for hiding this comment

fnrizzi commented Jul 14, 2022 •

edited