Skip to content

Commit

Permalink
review updates:
Browse files Browse the repository at this point in the history 
- formatting
- enable uncommented test
- add static_assert to warn on mismatching alignment
- check ginkgo build config matches kokkos
- increase types test
- remove unnecessary if-def guard
- use memory space type for mapping instead of variable
- set cxx standard to 17 for kokkos extension
- documentation
- move implementation out of class
- add install doc

Co-authored-by: Pratik Nayak <[email protected]>
Co-authored-by: Yu-Hsiang M. Tsai <[email protected]>
  • Loading branch information
@MarcelKoch @pratikvn @yhmtsai
3 people committed May 6, 2024
1 parent 036abdd commit 7e3d888
Show file tree
Hide file tree
Showing 12 changed files with 295 additions and 180 deletions.
5 changes: 5 additions & 0 deletions INSTALL.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,11 @@ Ginkgo adds the following additional switches to control what is being built:
this option see the
[`ARCHITECTURES` specification list](https://github.com/ginkgo-project/CudaArchitectureSelector/blob/master/CudaArchitectureSelector.cmake#L58)
section in the documentation of the CudaArchitectureSelector CMake module.
* `-DGINKGO_EXTENSION_KOKKOS={ON, OFF}` enables an extension that can map simple Ginkgo
types (`gko::array`, `gko::matrix::Dense`) to equivalent native Kokkos types. `OFF` by
default. Requires Kokkos to be installed if set to `ON`. To use the mapping it is
required to additionally link against `Ginkgo::ext::kokkos` (or `Ginkgo::ext` for short).
More details on the usage of this extension can be found in the example `kokkos-assembly`.

Additionally, the following CMake options have effect on the build process:

Expand Down
2 changes: 1 addition & 1 deletion dev_tools/scripts/regroup
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ IncludeBlocks: Regroup
IncludeCategories:
- Regex: '^<(nlohmann|gflags|gtest|papi).*'
Priority: 3
- Regex: '^<(omp|cu|hip|thrust|CL/|cooperative|oneapi|mpi|nvToolsExt).*'
- Regex: '^<(omp|cu|hip|thrust|CL/|cooperative|oneapi|mpi|nvToolsExt|Kokkos_Core).*'
Priority: 2
- Regex: '^<ginkgo.*'
Priority: 5
Expand Down
11 changes: 7 additions & 4 deletions examples/kokkos-assembly/kokkos-assembly.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
// SPDX-License-Identifier: BSD-3-Clause

#include <iostream>
#include <Kokkos_Core.hpp>
#include <string>


#include <Kokkos_Core.hpp>


#include <ginkgo/extensions/kokkos.hpp>
#include <ginkgo/ginkgo.hpp>

Expand Down Expand Up @@ -47,8 +49,9 @@ struct mapper<device_matrix_data<ValueType, IndexType>, MemorySpace> {
* @param row_idxs Pointer to the row indices
* @param col_idxs Pointer to the column indices
* @param values Pointer to the values
*
* @return An object which has each gko::array of the
* devive_matrix_data mapped to a Kokkos view
* device_matrix_data mapped to a Kokkos view
*/
static type map(size_type size, IndexType_c* row_idxs,
IndexType_c* col_idxs, ValueType_c* values)
Expand All @@ -66,7 +69,7 @@ struct mapper<device_matrix_data<ValueType, IndexType>, MemorySpace> {
static type<ValueType, IndexType> map(
device_matrix_data<ValueType, IndexType>& md)
{
assert_compatibility(md, MemorySpace{});
assert_compatibility<MemorySpace>(md);
return type<ValueType, IndexType>::map(
md.get_num_stored_elements(), md.get_row_idxs(), md.get_col_idxs(),
md.get_values());
Expand All @@ -75,7 +78,7 @@ struct mapper<device_matrix_data<ValueType, IndexType>, MemorySpace> {
static type<const ValueType, const IndexType> map(
const device_matrix_data<ValueType, IndexType>& md)
{
assert_compatibility(md, MemorySpace{});
assert_compatibility<MemorySpace>(md);
return type<const ValueType, const IndexType>::map(
md.get_num_stored_elements(), md.get_const_row_idxs(),
md.get_const_col_idxs(), md.get_const_values());
Expand Down
20 changes: 11 additions & 9 deletions extensions/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
add_library(ginkgo_ext INTERFACE)
add_library(Ginkgo::ext ALIAS ginkgo_ext)
set_property(TARGET ginkgo_ext PROPERTY EXPORT_NAME ext)

if(GINKGO_EXTENSION_KOKKOS)
if (GINKGO_EXTENSION_KOKKOS)
find_package(Kokkos 4.1 REQUIRED)

set(GINKGO_Kokkos_VERSION ${Kokkos_VERSION} PARENT_SCOPE)
Expand All @@ -13,19 +9,25 @@ if(GINKGO_EXTENSION_KOKKOS)
set_property(TARGET ginkgo_ext_kokkos PROPERTY EXPORT_NAME ext::kokkos)

target_link_libraries(ginkgo_ext_kokkos INTERFACE Kokkos::kokkos)
target_compile_features(ginkgo_ext_kokkos INTERFACE cxx_std_17)

target_link_libraries(ginkgo_ext INTERFACE ginkgo_ext_kokkos)
configure_file("${Ginkgo_SOURCE_DIR}/include/ginkgo/extensions/kokkos/config.hpp.in"
"${Ginkgo_BINARY_DIR}/include/ginkgo/extensions/kokkos/config.hpp"
@ONLY
)

ginkgo_install_library(ginkgo_ext_kokkos)
install(FILES "${Ginkgo_BINARY_DIR}/include/ginkgo/extensions/kokkos/config.hpp"
DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/ginkgo/extensions/kokkos"
)
install(DIRECTORY "${Ginkgo_SOURCE_DIR}/include/ginkgo/extensions/kokkos"
DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/ginkgo/extensions"
FILES_MATCHING PATTERN "*.hpp"
)
install(FILES "${Ginkgo_SOURCE_DIR}/include/ginkgo/extensions/kokkos.hpp"
DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/ginkgo/extensions"
)
endif()

ginkgo_install_library(ginkgo_ext)
endif ()

if (GINKGO_BUILD_TESTS)
add_subdirectory(test)
Expand Down
34 changes: 19 additions & 15 deletions extensions/test/kokkos/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,31 +11,35 @@ function(_ginkgo_check_build_config _build)
endfunction()

if(Kokkos_ENABLE_CUDA)
set(resource_type "cudagpu")
set(definitions "GKO_COMPILING_CUDA")
_ginkgo_check_build_config(cuda)
set(resource_type "cudagpu")
set(definitions "GKO_COMPILING_CUDA")
elseif(Kokkos_ENABLE_HIP)
set(resource_type "hipgpu")
set(definitions "GKO_COMPILING_HIP")
_ginkgo_check_build_config(hip)
set(resource_type "hipgpu")
set(definitions "GKO_COMPILING_HIP")
elseif(Kokkos_ENABLE_SYCL)
set(resource_type "sycl")
set(definitions "GKO_COMPILING_DPCPP")
_ginkgo_check_build_config(sycl)
set(resource_type "sycl")
set(definitions "GKO_COMPILING_DPCPP")
else()
set(resource_type "cpu")
if(Kokkos_ENABLE_OMP)
set(definitions "GKO_COMPILING_OMP")
endif()
set(resource_type "cpu")
if(Kokkos_ENABLE_OPENMP)
_ginkgo_check_build_config(omp)
set(definitions "GKO_COMPILING_OMP")
endif()
endif()

function(create_gtest_main_kokkos)
add_library(ginkgo_gtest_main_kokkos STATIC kokkos_main.cpp ${PROJECT_SOURCE_DIR}/core/test/gtest/resources.cpp)
target_link_libraries(ginkgo_gtest_main_kokkos PUBLIC Ginkgo::ginkgo GTest::GTest Ginkgo::ext::kokkos)
target_compile_definitions(ginkgo_gtest_main_kokkos PRIVATE ${definitions})
ginkgo_compile_features(ginkgo_gtest_main_kokkos)
add_library(ginkgo_gtest_main_kokkos STATIC kokkos_main.cpp ${PROJECT_SOURCE_DIR}/core/test/gtest/resources.cpp)
target_link_libraries(ginkgo_gtest_main_kokkos PUBLIC Ginkgo::ginkgo GTest::GTest Ginkgo::ext::kokkos)
target_compile_definitions(ginkgo_gtest_main_kokkos PRIVATE ${definitions})
ginkgo_compile_features(ginkgo_gtest_main_kokkos)
endfunction()
create_gtest_main_kokkos()

function(ginkgo_create_test_kokkos test_name)
ginkgo_create_test(${test_name} NO_GTEST_MAIN RESOURCE_TYPE ${resource_type} ADDITIONAL_LIBRARIES Ginkgo::ext::kokkos ginkgo_gtest_main_kokkos ${ARGN})
ginkgo_create_test(${test_name} NO_GTEST_MAIN RESOURCE_TYPE ${resource_type} ADDITIONAL_LIBRARIES Ginkgo::ext::kokkos ginkgo_gtest_main_kokkos ${ARGN})
endfunction()

ginkgo_create_test_kokkos(types)
Expand Down
3 changes: 3 additions & 0 deletions extensions/test/kokkos/spaces.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
//
// SPDX-License-Identifier: BSD-3-Clause

#include <Kokkos_Core.hpp>


#include <gtest/gtest.h>


Expand Down
80 changes: 72 additions & 8 deletions extensions/test/kokkos/types.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
#include <sstream>


#include <Kokkos_Core.hpp>


#include <gtest/gtest.h>


Expand All @@ -19,6 +22,7 @@

using DefaultMemorySpace = Kokkos::DefaultExecutionSpace::memory_space;


template <typename ValueType>
class ArrayMapper : public ::testing::Test {
protected:
Expand All @@ -40,14 +44,20 @@ TYPED_TEST(ArrayMapper, CanMapDefault)

auto mapped_array = gko::ext::kokkos::map_data(this->array);

using array_type =
using mapped_type =
std::remove_cv_t<std::remove_reference_t<decltype(mapped_array)>>;
static_assert(
std::is_same_v<array_type,
std::is_same_v<mapped_type,
Kokkos::View<kokkos_value_type*, DefaultMemorySpace,
Kokkos::MemoryTraits<Kokkos::Unmanaged>>>);
ASSERT_EQ(reinterpret_cast<std::uintptr_t>(mapped_array.data()),
reinterpret_cast<std::uintptr_t>(this->array.get_data()));
ASSERT_EQ(mapped_array.extent(0), this->array.get_size());
ASSERT_EQ(mapped_array.size(), this->array.get_size());
ASSERT_EQ(mapped_array.stride(0), 1);
}


TYPED_TEST(ArrayMapper, CanMapConst)
{
using value_type = typename TestFixture::value_type;
Expand All @@ -57,12 +67,17 @@ TYPED_TEST(ArrayMapper, CanMapConst)
auto mapped_array = gko::ext::kokkos::map_data(
const_cast<const gko::array<value_type>&>(this->array));

using array_type =
using mapped_type =
std::remove_cv_t<std::remove_reference_t<decltype(mapped_array)>>;
static_assert(std::is_same_v<
array_type,
mapped_type,
Kokkos::View<const kokkos_value_type*, DefaultMemorySpace,
Kokkos::MemoryTraits<Kokkos::Unmanaged>>>);
ASSERT_EQ(reinterpret_cast<std::uintptr_t>(mapped_array.data()),
reinterpret_cast<std::uintptr_t>(this->array.get_data()));
ASSERT_EQ(mapped_array.extent(0), this->array.get_size());
ASSERT_EQ(mapped_array.size(), this->array.get_size());
ASSERT_EQ(mapped_array.stride(0), 1);
}


Expand All @@ -89,15 +104,25 @@ TYPED_TEST(DenseMapper, CanMapDefault)

auto mapped_mtx = gko::ext::kokkos::map_data(this->mtx);

using mtx_type =
using mapped_type =
std::remove_cv_t<std::remove_reference_t<decltype(mapped_mtx)>>;
static_assert(
std::is_same_v<mtx_type,
std::is_same_v<mapped_type,
Kokkos::View<kokkos_value_type**, Kokkos::LayoutStride,
DefaultMemorySpace,
Kokkos::MemoryTraits<Kokkos::Unmanaged>>>);
ASSERT_EQ(reinterpret_cast<std::uintptr_t>(mapped_mtx.data()),
reinterpret_cast<std::uintptr_t>(this->mtx->get_values()));
ASSERT_EQ(mapped_mtx.extent(0), this->mtx->get_size()[0]);
ASSERT_EQ(mapped_mtx.extent(1), this->mtx->get_size()[1]);
ASSERT_EQ(mapped_mtx.size(),
this->mtx->get_size()[0] * this->mtx->get_size()[1]);
ASSERT_EQ(mapped_mtx.span(), this->mtx->get_num_stored_elements());
ASSERT_EQ(mapped_mtx.stride(0), this->mtx->get_stride());
ASSERT_EQ(mapped_mtx.stride(1), 1);
}


TYPED_TEST(DenseMapper, CanMapConst)
{
using value_type = typename TestFixture::value_type;
Expand All @@ -107,11 +132,50 @@ TYPED_TEST(DenseMapper, CanMapConst)
auto mapped_mtx = gko::ext::kokkos::map_data(
const_cast<const gko::matrix::Dense<value_type>*>(this->mtx.get()));

using mtx_type =
using mapped_type =
std::remove_cv_t<std::remove_reference_t<decltype(mapped_mtx)>>;
static_assert(
std::is_same_v<mtx_type,
std::is_same_v<mapped_type,
Kokkos::View<const kokkos_value_type**,
Kokkos::LayoutStride, DefaultMemorySpace,
Kokkos::MemoryTraits<Kokkos::Unmanaged>>>);
ASSERT_EQ(reinterpret_cast<std::uintptr_t>(mapped_mtx.data()),
reinterpret_cast<std::uintptr_t>(this->mtx->get_values()));
ASSERT_EQ(mapped_mtx.extent(0), this->mtx->get_size()[0]);
ASSERT_EQ(mapped_mtx.extent(1), this->mtx->get_size()[1]);
ASSERT_EQ(mapped_mtx.size(),
this->mtx->get_size()[0] * this->mtx->get_size()[1]);
ASSERT_EQ(mapped_mtx.span(), this->mtx->get_num_stored_elements());
ASSERT_EQ(mapped_mtx.stride(0), this->mtx->get_stride());
ASSERT_EQ(mapped_mtx.stride(1), 1);
}


TYPED_TEST(DenseMapper, CanMapStrided)
{
using mtx_type = typename TestFixture::mtx_type;
using value_type = typename TestFixture::value_type;
using kokkos_value_type =
typename gko::ext::kokkos::detail::value_type<value_type>::type;
std::unique_ptr<mtx_type> mtx = mtx_type ::create(
this->exec, gko::dim<2>{2, 2},
gko::array<value_type>{this->exec, {1, 2, -1, 3, 4, -10}}, 3);

auto mapped_mtx = gko::ext::kokkos::map_data(mtx);

using mapped_type =
std::remove_cv_t<std::remove_reference_t<decltype(mapped_mtx)>>;
static_assert(
std::is_same_v<mapped_type,
Kokkos::View<kokkos_value_type**, Kokkos::LayoutStride,
DefaultMemorySpace,
Kokkos::MemoryTraits<Kokkos::Unmanaged>>>);
ASSERT_EQ(reinterpret_cast<std::uintptr_t>(mapped_mtx.data()),
reinterpret_cast<std::uintptr_t>(mtx->get_values()));
ASSERT_EQ(mapped_mtx.extent(0), mtx->get_size()[0]);
ASSERT_EQ(mapped_mtx.extent(1), mtx->get_size()[1]);
ASSERT_EQ(mapped_mtx.size(), mtx->get_size()[0] * mtx->get_size()[1]);
ASSERT_EQ(mapped_mtx.span(), mtx->get_num_stored_elements());
ASSERT_EQ(mapped_mtx.stride(0), mtx->get_stride());
ASSERT_EQ(mapped_mtx.stride(1), 1);
}
12 changes: 0 additions & 12 deletions include/ginkgo/config.hpp.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,16 +112,4 @@
// clang-format on


/* Is the Kokkos extension enabled? */
// clang-format off
#cmakedefine01 GINKGO_EXTENSION_KOKKOS
// clang-format on


/* Do we need to check the type alignment in the Kokkos type maps? */
// clang-format off
#cmakedefine01 GINKGO_EXTENSION_KOKKOS_CHECK_TYPE_ALIGNMENT
// clang-format on


#endif // GKO_INCLUDE_CONFIG_H
3 changes: 2 additions & 1 deletion include/ginkgo/extensions/kokkos.hpp
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
// SPDX-FileCopyrightText: 2017-2023 The Ginkgo authors
// SPDX-FileCopyrightText: 2017 - 2024 The Ginkgo authors
//
// SPDX-License-Identifier: BSD-3-Clause

#ifndef GKO_PUBLIC_EXTENSIONS_KOKKOS_HPP_
#define GKO_PUBLIC_EXTENSIONS_KOKKOS_HPP_


#include <ginkgo/extensions/kokkos/config.hpp>
#include <ginkgo/extensions/kokkos/spaces.hpp>
#include <ginkgo/extensions/kokkos/types.hpp>

Expand Down
15 changes: 15 additions & 0 deletions include/ginkgo/extensions/kokkos/config.hpp.in
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// SPDX-FileCopyrightText: 2024 The Ginkgo authors
//
// SPDX-License-Identifier: BSD-3-Clause

#ifndef GINKGO_INCLUDE_GINKGO_EXTENSIONS_KOKKOS_CONFIG_HPP_IN
#define GINKGO_INCLUDE_GINKGO_EXTENSIONS_KOKKOS_CONFIG_HPP_IN


/* Do we need to check the type alignment in the Kokkos type maps? */
// clang-format off
#cmakedefine01 GINKGO_EXTENSION_KOKKOS_CHECK_TYPE_ALIGNMENT
// clang-format on


#endif // GINKGO_INCLUDE_GINKGO_EXTENSIONS_KOKKOS_CONFIG_HPP_IN
Loading

0 comments on commit 7e3d888

Please sign in to comment.