Kokkos::Serial::is_initialized always true

[ibaned]

I have a library that does

bool do_finalize = false;
void my_initialize() {
  if (!Kokkos::DefaultExecutionSpace::is_initialized()) {
    Kokkos::initialize();
    do_finalize = true;
  }
}
void my_finalize() {
  if (do_finalize) Kokkos::finalize();
}

The problem is that Kokkos::Serial::is_initialized() is hardcoded to always return true (actually, the int 1, which is also inconsistent with the bool return types of other exec spaces).
Until Kokkos::is_initialized exists (#1060), this is the only way I know to properly protect calls to Kokkos::initialize within downstream library initialize calls.
Furthermore, Kokkos::Serial actually does manage scratch memory, so if its finalize method is not called then memory is leaked and my tests fail under Valgrind.

Edit: downstream*

[mhoemmen]

Huh, this is good to know. I've been simplifying how Tpetra initializes Kokkos, and due to #1060, my changes would have used the initialization status of the default execution space as a proxy for Kokkos' initialization state. Good to know that Kokkos::Serial isn't quite right.

[ibaned]

@mhoemmen yea this really bugs me. I opened a pull request (#1185) to fix this, but we're in the middle of a promotion, so I don't know when it'll get merged.

[mhoemmen]

@ibaned I'm tempted to apply it to Trilinos first, and reply if necessary until it gets merged in to Kokkos master. Would you object to this, post review?

[ibaned]

@mhoemmen hopefully the Kokkos team can discuss this a bit at our noon meeting today, and come up with a plan. I'm all for applying it ASAP. If we decide we don't want to disrupt the promotion too much, we can patch it onto Trilinos using the checkin script either before or after the new Kokkos snapshot.

[dsunder]

The global Kokkos::is_initialized() works correctly (it is on the current develop branch waiting to be push to master with the current promotion). The individual execution space initialize/finalize are being rewritten to call the global initialize/finalize and in version 3 of Kokkos they will be move to the Kokkos::Impl namespace.

The main difficulty in replacing the individual execution space initialize/finalize with the global stems from trying to reinitialize the Cuda execution space. The OpenMP, Threads, and Serial execution spaces can be safely reinitialized. I haven't tested the OpenMPTarget, Qthreads, or ROCm backends.

For example consider the following code

Kokkos::OpenMP::initialize(nt); // calls Kokkos::initialize(), Cuda initialized to GPU 0
Kokkos::Cuda::initialize(1); // Problematic: tries to reinitialize Cuda to GPU 1

A better way of doing this is to not call the individuals initializes at all.

Kokkos::InitArguments args;
args.num_threads = nt;
args.device_id = 1;
Kokkos::initialize(args);

[ibaned]

@dsunder does Kokkos::is_initialized work correctly even when only the Serial space is enabled, i.e. does it not rely on the broken Serial::is_initialized ?

[dsunder]

The global Kokkos::is_initialized currently only checks if the global Kokkos::initialize(...) has been called.

[ibaned]

That works for my purposes! Thanks

[crtrott]

Looks like this broke our nightlies

[crtrott]

I believe what needs to happen is that kokkos initialize initializes serial always if it is configure time enabled, ie even if openmp is on