Skip to content

Conversation

@dssgabriel
Copy link
Collaborator

This PR is based on #188 (so we'll need to merge that first) and leverages the newly introduced member type aliases on the comm space concept to implement a CommunicationSpace-generic datatype conversion API. The present PR supersedes #186.

The new implementation is generic over both the communication space and the base type to convert, and remains fully compile-time known.

Some notable changes:

  • declarative-style: no cascading if constexpr statements (similar implementation strategy to e.g., std::is_integral)
  • exposed as top-level function in the KokkosComm:: namespace
    • not an implementation detail anymore (the OSU perf tests were relying on it)
  • added a datatype_for(T&&) utility so we can retrieve the datatype from a variable, e.g.:
    using CS = KokkosComm::DefaultCommunicationSpace;
    
    const int& ref = 42;
    CS::datatype_type type = KokkosComm::datatype_for<CS>(ref);

I have kept the mpi/impl/types.hpp file for now, as it defines the logic for creating MPI datatypes (in mpi_view_type), although it appears this functionality is currently unused in Kokkos Comm.

The remaining changes propagate this new API throughout the rest of the code base.

@dssgabriel dssgabriel self-assigned this Nov 28, 2025
@dssgabriel dssgabriel added C-enhancement Category: an enhancement or bug fix A-core Area: KokkosComm core library implementation labels Nov 28, 2025
@dssgabriel dssgabriel changed the title refactor: CommunicationSpace-generic datatype conversion refactor: comm space generic datatype conversion Nov 28, 2025
@dssgabriel dssgabriel force-pushed the refactor/datatype-conversion branch 2 times, most recently from 667eca0 to aadfdca Compare November 28, 2025 14:43
@dssgabriel
Copy link
Collaborator Author

I am unsure why the Open MPI builds fail in the CI. Everything builds and passes on my machine with Open MPI 5.0.8

Here is my workflow for anyone willing to try and reproduce:

spack load [email protected] [email protected] [email protected] [email protected]

mkdir -p build/_deps

# Kokkos 5.0.0 w/ OpenMP backend
git clone https://github.com/kokkos/kokkos.git --depth 1 --branch 5.0.0 build/_deps/kokkos-src
cmake -S build/_deps/kokkos-src -B build/_deps/kokkos-build-omp -G Ninja -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_EXTENSIONS=OFF -DKokkos_ENABLE_OPENMP=ON
cmake --build build/_deps/kokkos-build-omp -j
cmake --install build/_deps/kokkos-build-omp --prefix build/_deps/kokkos-install-omp

cmake -B build/kokkoscomm-mpi-omp -G Ninja -DCMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_COLOR_DIAGNOSTICS=ON -DCMAKE_CXX_STANDARD=20 -DCMAKE_CXX_EXTENSIONS=OFF -DCMAKE_CXX_FLAGS='-Wall -Wextra -Wpedantic -Wmissing-include-dirs' -DKokkos_ROOT=build/_deps/kokkos-install-omp -DKokkosComm_ENABLE_MPI=ON -DKokkosComm_ENABLE_TESTS=ON -DKokkosComm_ENABLE_PERFTESTS=ON
cmake --build build/kokkoscomm-mpi-omp -j
ctest --test-dir build/kokkoscomm-mpi-omp --output-on-failure

@dssgabriel dssgabriel force-pushed the refactor/datatype-conversion branch from bc21ada to fff3960 Compare November 28, 2025 18:33
@cedricchevalier19
Copy link
Member

@dssgabriel, a related? openmpi issue: open-mpi/ompi#10017.

@dssgabriel dssgabriel force-pushed the refactor/datatype-conversion branch from fff3960 to ca0b82e Compare December 2, 2025 10:38
@cwpearson cwpearson added the SNL-CI-APPROVAL Required to run SNL CI on non-SNL contributions label Dec 4, 2025
Overhaul datatype conversion:
- fully generic: over communication space & type
  - fully compile-time known
  - declarative-style (similar implementation strategy to e.g.,
`std::is_integral`)
- exposed as top-level function in the KokkosComm:: namespace
  - not an implementation detail anymore (some tests were relying on it)

Signed-off-by: Gabriel Dos Santos <[email protected]>
@cedricchevalier19 cedricchevalier19 force-pushed the refactor/datatype-conversion branch from ad037cd to 4911943 Compare December 8, 2025 18:07
@cedricchevalier19 cedricchevalier19 merged commit 7c3d596 into kokkos:develop Dec 8, 2025
7 of 10 checks passed
@dssgabriel dssgabriel deleted the refactor/datatype-conversion branch December 9, 2025 01:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-core Area: KokkosComm core library implementation C-enhancement Category: an enhancement or bug fix SNL-CI-APPROVAL Required to run SNL CI on non-SNL contributions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants