From 2dabec8e6d32b0e2a928f8fd082485d4d2cee07d Mon Sep 17 00:00:00 2001 From: Carl Pearson Date: Tue, 4 Jun 2024 14:43:24 -0600 Subject: [PATCH 1/4] Bump gtest to May 28,2024. Auto patch for nvcc. Google Test does not currently plan to support nvcc, so we introduce a small patch here to make it work. --- unit_tests/CMakeLists.txt | 25 ++++++++++++++-- unit_tests/cmake/gtest_a7f443b8.patch | 43 +++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 3 deletions(-) create mode 100644 unit_tests/cmake/gtest_a7f443b8.patch diff --git a/unit_tests/CMakeLists.txt b/unit_tests/CMakeLists.txt index 8cb9d1d2..e91db589 100644 --- a/unit_tests/CMakeLists.txt +++ b/unit_tests/CMakeLists.txt @@ -18,16 +18,35 @@ endif() FetchContent_Declare( googletest - URL https://github.com/google/googletest/archive/530d5c8c84abd2a46f38583ee817743c9b3a42b4.zip # 12-18-2023 + URL https://github.com/google/googletest/archive/a7f443b80b105f940225332ed3c31f2790092f47.zip # 05-28-2024 ) # For Windows: Prevent overriding the parent project's compiler/linker settings set(gtest_force_shared_crt ON CACHE BOOL "" FORCE) -# FetchContent_MakeAvailable(googletest) -# was making install install googletest as well +# FetchContent_MakeAvailable(googletest) was making install install googletest as well # EXCLUDE_FROM_ALL here seems to be the magic +FetchContent_GetProperties(googletest) if (NOT googletest_POPULATED) FetchContent_Populate(googletest) add_subdirectory(${googletest_SOURCE_DIR} ${googletest_BINARY_DIR} EXCLUDE_FROM_ALL) + + set(PATCH_PATH "${PROJECT_SOURCE_DIR}/cmake/gtest_a7f443b8.patch") + message(STATUS "patching gtest @ ${googletest_SOURCE_DIR} with ${PATCH_PATH}") + execute_process( + COMMAND patch -N -p1 -i "${PATCH_PATH}" + WORKING_DIRECTORY "${googletest_SOURCE_DIR}" + RESULT_VARIABLE patch_result + ) + + # patch exists with 1 if some hunks were skipped, or 2 for real problems. + # we want to tolerate hunks being skipped in case of a partial cmake, + # where the patch was already applied. + # unfortunately this might consume some other errors + if(patch_result GREATER 1) + message(FATAL_ERROR "Failed to patch gtest for nvcc: ${patch_result}") + else() + message(WARNING "Possible error while patching gtest for nvcc. This is okay if the only messages above are about ignored hunks.") + endif() + endif() # Standalone MPI smoke tests (do not use KokkosComm) diff --git a/unit_tests/cmake/gtest_a7f443b8.patch b/unit_tests/cmake/gtest_a7f443b8.patch new file mode 100644 index 00000000..33702c49 --- /dev/null +++ b/unit_tests/cmake/gtest_a7f443b8.patch @@ -0,0 +1,43 @@ +diff --color -ruN orig/googletest/include/gtest/gtest-typed-test.h patched/googletest/include/gtest/gtest-typed-test.h +--- orig/googletest/include/gtest/gtest-typed-test.h 2024-05-28 08:42:39 ++++ patched/googletest/include/gtest/gtest-typed-test.h 2024-06-04 14:10:13 +@@ -200,10 +200,11 @@ + template \ + class GTEST_TEST_CLASS_NAME_(CaseName, TestName) \ + : public CaseName { \ ++ public: \ ++ void TestBody() override; \ + private: \ + typedef CaseName TestFixture; \ + typedef gtest_TypeParam_ TypeParam; \ +- void TestBody() override; \ + }; \ + GTEST_INTERNAL_ATTRIBUTE_MAYBE_UNUSED static bool \ + gtest_##CaseName##_##TestName##_registered_ = \ +@@ -272,10 +273,11 @@ + namespace GTEST_SUITE_NAMESPACE_(SuiteName) { \ + template \ + class TestName : public SuiteName { \ ++ public: \ ++ void TestBody() override; \ + private: \ + typedef SuiteName TestFixture; \ + typedef gtest_TypeParam_ TypeParam; \ +- void TestBody() override; \ + }; \ + GTEST_INTERNAL_ATTRIBUTE_MAYBE_UNUSED static bool \ + gtest_##TestName##_defined_ = \ +diff --color -ruN orig/googletest/include/gtest/internal/gtest-internal.h patched/googletest/include/gtest/internal/gtest-internal.h +--- orig/googletest/include/gtest/internal/gtest-internal.h 2024-05-28 08:42:39 ++++ patched/googletest/include/gtest/internal/gtest-internal.h 2024-06-04 14:10:48 +@@ -1499,8 +1499,9 @@ + GTEST_TEST_CLASS_NAME_(test_suite_name, \ + test_name) &&) noexcept = delete; /* NOLINT */ \ + \ +- private: \ ++ public: \ + void TestBody() override; \ ++ private: \ + GTEST_INTERNAL_ATTRIBUTE_MAYBE_UNUSED static ::testing::TestInfo* const \ + test_info_; \ + }; \ From 767760990c5ed98b69cdb46b1d6acb815fc8ef44 Mon Sep 17 00:00:00 2001 From: Carl Pearson Date: Tue, 4 Jun 2024 14:50:52 -0600 Subject: [PATCH 2/4] only warn about gtest patch on non-zero --- unit_tests/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/unit_tests/CMakeLists.txt b/unit_tests/CMakeLists.txt index e91db589..92308d3c 100644 --- a/unit_tests/CMakeLists.txt +++ b/unit_tests/CMakeLists.txt @@ -37,13 +37,13 @@ if (NOT googletest_POPULATED) RESULT_VARIABLE patch_result ) - # patch exists with 1 if some hunks were skipped, or 2 for real problems. + # patch exits with 1 if some hunks were skipped, or 2 for real problems. # we want to tolerate hunks being skipped in case of a partial cmake, # where the patch was already applied. # unfortunately this might consume some other errors if(patch_result GREATER 1) message(FATAL_ERROR "Failed to patch gtest for nvcc: ${patch_result}") - else() + elseif(patch_result GREATER 0) message(WARNING "Possible error while patching gtest for nvcc. This is okay if the only messages above are about ignored hunks.") endif() From 85d7e23fff292590fd3b4490f8f01d2a3dd205e0 Mon Sep 17 00:00:00 2001 From: Carl Pearson Date: Wed, 5 Jun 2024 09:41:00 -0600 Subject: [PATCH 3/4] remove gtest patch --- unit_tests/CMakeLists.txt | 19 ------------ unit_tests/cmake/gtest_a7f443b8.patch | 43 --------------------------- 2 files changed, 62 deletions(-) delete mode 100644 unit_tests/cmake/gtest_a7f443b8.patch diff --git a/unit_tests/CMakeLists.txt b/unit_tests/CMakeLists.txt index 92308d3c..93ea170f 100644 --- a/unit_tests/CMakeLists.txt +++ b/unit_tests/CMakeLists.txt @@ -28,25 +28,6 @@ FetchContent_GetProperties(googletest) if (NOT googletest_POPULATED) FetchContent_Populate(googletest) add_subdirectory(${googletest_SOURCE_DIR} ${googletest_BINARY_DIR} EXCLUDE_FROM_ALL) - - set(PATCH_PATH "${PROJECT_SOURCE_DIR}/cmake/gtest_a7f443b8.patch") - message(STATUS "patching gtest @ ${googletest_SOURCE_DIR} with ${PATCH_PATH}") - execute_process( - COMMAND patch -N -p1 -i "${PATCH_PATH}" - WORKING_DIRECTORY "${googletest_SOURCE_DIR}" - RESULT_VARIABLE patch_result - ) - - # patch exits with 1 if some hunks were skipped, or 2 for real problems. - # we want to tolerate hunks being skipped in case of a partial cmake, - # where the patch was already applied. - # unfortunately this might consume some other errors - if(patch_result GREATER 1) - message(FATAL_ERROR "Failed to patch gtest for nvcc: ${patch_result}") - elseif(patch_result GREATER 0) - message(WARNING "Possible error while patching gtest for nvcc. This is okay if the only messages above are about ignored hunks.") - endif() - endif() # Standalone MPI smoke tests (do not use KokkosComm) diff --git a/unit_tests/cmake/gtest_a7f443b8.patch b/unit_tests/cmake/gtest_a7f443b8.patch deleted file mode 100644 index 33702c49..00000000 --- a/unit_tests/cmake/gtest_a7f443b8.patch +++ /dev/null @@ -1,43 +0,0 @@ -diff --color -ruN orig/googletest/include/gtest/gtest-typed-test.h patched/googletest/include/gtest/gtest-typed-test.h ---- orig/googletest/include/gtest/gtest-typed-test.h 2024-05-28 08:42:39 -+++ patched/googletest/include/gtest/gtest-typed-test.h 2024-06-04 14:10:13 -@@ -200,10 +200,11 @@ - template \ - class GTEST_TEST_CLASS_NAME_(CaseName, TestName) \ - : public CaseName { \ -+ public: \ -+ void TestBody() override; \ - private: \ - typedef CaseName TestFixture; \ - typedef gtest_TypeParam_ TypeParam; \ -- void TestBody() override; \ - }; \ - GTEST_INTERNAL_ATTRIBUTE_MAYBE_UNUSED static bool \ - gtest_##CaseName##_##TestName##_registered_ = \ -@@ -272,10 +273,11 @@ - namespace GTEST_SUITE_NAMESPACE_(SuiteName) { \ - template \ - class TestName : public SuiteName { \ -+ public: \ -+ void TestBody() override; \ - private: \ - typedef SuiteName TestFixture; \ - typedef gtest_TypeParam_ TypeParam; \ -- void TestBody() override; \ - }; \ - GTEST_INTERNAL_ATTRIBUTE_MAYBE_UNUSED static bool \ - gtest_##TestName##_defined_ = \ -diff --color -ruN orig/googletest/include/gtest/internal/gtest-internal.h patched/googletest/include/gtest/internal/gtest-internal.h ---- orig/googletest/include/gtest/internal/gtest-internal.h 2024-05-28 08:42:39 -+++ patched/googletest/include/gtest/internal/gtest-internal.h 2024-06-04 14:10:48 -@@ -1499,8 +1499,9 @@ - GTEST_TEST_CLASS_NAME_(test_suite_name, \ - test_name) &&) noexcept = delete; /* NOLINT */ \ - \ -- private: \ -+ public: \ - void TestBody() override; \ -+ private: \ - GTEST_INTERNAL_ATTRIBUTE_MAYBE_UNUSED static ::testing::TestInfo* const \ - test_info_; \ - }; \ From 0e3a3c643876e85886f67b5fc5d065329fefcfe3 Mon Sep 17 00:00:00 2001 From: Carl Pearson Date: Wed, 5 Jun 2024 09:41:22 -0600 Subject: [PATCH 4/4] tests: break out lambdas into helper function --- unit_tests/test_allgather.cpp | 26 +++++++++++++++----------- unit_tests/test_alltoall.cpp | 18 ++++++++++-------- unit_tests/test_reduce.cpp | 22 +++++++++++----------- 3 files changed, 36 insertions(+), 30 deletions(-) diff --git a/unit_tests/test_allgather.cpp b/unit_tests/test_allgather.cpp index 926134af..0627bfb5 100644 --- a/unit_tests/test_allgather.cpp +++ b/unit_tests/test_allgather.cpp @@ -18,6 +18,8 @@ #include "KokkosComm.hpp" +namespace { + template class Allgather : public testing::Test { public: @@ -27,17 +29,14 @@ class Allgather : public testing::Test { using ScalarTypes = ::testing::Types, Kokkos::complex>; TYPED_TEST_SUITE(Allgather, ScalarTypes); -TYPED_TEST(Allgather, 0D) { - using TestScalar = typename TestFixture::Scalar; - +template +void test_allgather_0d() { int rank, size; MPI_Comm_rank(MPI_COMM_WORLD, &rank); MPI_Comm_size(MPI_COMM_WORLD, &size); - const int nContrib = 10; - - Kokkos::View sv("sv"); - Kokkos::View rv("rv", size); + Kokkos::View sv("sv"); + Kokkos::View rv("rv", size); // fill send buffer Kokkos::parallel_for( @@ -51,17 +50,18 @@ TYPED_TEST(Allgather, 0D) { EXPECT_EQ(errs, 0); } -TYPED_TEST(Allgather, 1D_contig) { - using TestScalar = typename TestFixture::Scalar; +TYPED_TEST(Allgather, 0D) { test_allgather_0d(); } +template +void test_allgather_1d_contig() { int rank, size; MPI_Comm_rank(MPI_COMM_WORLD, &rank); MPI_Comm_size(MPI_COMM_WORLD, &size); const int nContrib = 10; - Kokkos::View sv("sv", nContrib); - Kokkos::View rv("rv", size * nContrib); + Kokkos::View sv("sv", nContrib); + Kokkos::View rv("rv", size * nContrib); // fill send buffer Kokkos::parallel_for( @@ -80,3 +80,7 @@ TYPED_TEST(Allgather, 1D_contig) { errs); EXPECT_EQ(errs, 0); } + +TYPED_TEST(Allgather, 1D_contig) { test_allgather_1d_contig(); } + +} // namespace diff --git a/unit_tests/test_alltoall.cpp b/unit_tests/test_alltoall.cpp index 61397734..db0103c2 100644 --- a/unit_tests/test_alltoall.cpp +++ b/unit_tests/test_alltoall.cpp @@ -29,17 +29,16 @@ class Alltoall : public testing::Test { using ScalarTypes = ::testing::Types, Kokkos::complex>; TYPED_TEST_SUITE(Alltoall, ScalarTypes); -TYPED_TEST(Alltoall, 1D_contig) { - using TestScalar = typename TestFixture::Scalar; - +template +void test_alltoall_1d_contig() { int rank, size; MPI_Comm_rank(MPI_COMM_WORLD, &rank); MPI_Comm_size(MPI_COMM_WORLD, &size); const int nContrib = 10; - Kokkos::View sv("sv", size * nContrib); - Kokkos::View rv("rv", size * nContrib); + Kokkos::View sv("sv", size * nContrib); + Kokkos::View rv("rv", size * nContrib); // fill send buffer Kokkos::parallel_for( @@ -59,16 +58,17 @@ TYPED_TEST(Alltoall, 1D_contig) { EXPECT_EQ(errs, 0); } -TYPED_TEST(Alltoall, 1D_inplace_contig) { - using TestScalar = typename TestFixture::Scalar; +TYPED_TEST(Alltoall, 1D_contig) { test_alltoall_1d_contig(); } +template +void test_alltoall_1d_inplace_contig() { int rank, size; MPI_Comm_rank(MPI_COMM_WORLD, &rank); MPI_Comm_size(MPI_COMM_WORLD, &size); const int nContrib = 10; - Kokkos::View rv("rv", size * nContrib); + Kokkos::View rv("rv", size * nContrib); // fill send buffer Kokkos::parallel_for( @@ -88,4 +88,6 @@ TYPED_TEST(Alltoall, 1D_inplace_contig) { EXPECT_EQ(errs, 0); } +TYPED_TEST(Alltoall, 1D_inplace_contig) { test_alltoall_1d_inplace_contig(); } + } // namespace diff --git a/unit_tests/test_reduce.cpp b/unit_tests/test_reduce.cpp index f538f64a..a8d33efe 100644 --- a/unit_tests/test_reduce.cpp +++ b/unit_tests/test_reduce.cpp @@ -18,6 +18,8 @@ #include "KokkosComm.hpp" +namespace { + template class Reduce : public testing::Test { public: @@ -31,17 +33,15 @@ TYPED_TEST_SUITE(Reduce, ScalarTypes); Each rank fills its sendbuf[i] with `rank + i` operation is sum, so recvbuf[i] should be sum(0..size) + i * size - */ -TYPED_TEST(Reduce, 1D_contig) { - using TestScalar = typename TestFixture::Scalar; - +template +void test_reduce_1d_contig() { int rank, size; MPI_Comm_rank(MPI_COMM_WORLD, &rank); MPI_Comm_size(MPI_COMM_WORLD, &size); - Kokkos::View sendv("sendv", 10); - Kokkos::View recvv; + Kokkos::View sendv("sendv", 10); + Kokkos::View recvv; if (0 == rank) { Kokkos::resize(recvv, sendv.extent(0)); } @@ -57,17 +57,17 @@ TYPED_TEST(Reduce, 1D_contig) { Kokkos::parallel_reduce( recvv.extent(0), KOKKOS_LAMBDA(const int &i, int &lsum) { - TestScalar acc = 0; + Scalar acc = 0; for (int r = 0; r < size; ++r) { acc += r + i; } lsum += recvv(i) != acc; - // if (recvv(i) != acc) { - // Kokkos::printf("%f != %f @ %lu\n", double(Kokkos::abs(recvv(i))), - // double(Kokkos::abs(acc)), size_t(i)); - // } }, errs); ASSERT_EQ(errs, 0); } } + +TYPED_TEST(Reduce, 1D_contig) { test_reduce_1d_contig(); } + +} // namespace