Skip to content

Commit

Permalink
Medley of (hopefully) uncontroversial fixups towards building with no…
Browse files Browse the repository at this point in the history
… exceptions (kokkos#7337)

* Abort on TeamPolicy::set_scratch_team() precondition violation

Call abort for invalid level argument instead of throwing

* Prefer GTetst native support to report failure

* Avoid ASSERT_NO_THROW in range policy tests

* Drop EXPECT_NO_THROW in reduce test

* Abort on the host-side as well on subview precondition violation

* Abort instead of throwing if parsing tools settings failed

* Fix warning comparison of integers of different signs

* Get rid of all *_NO_THROW in algorithms tests

* Get rid of a few more *_NO_THROW in core tests

* Abort on error in shared allocation instead of throwing

* Per review adding comments that code w/o assertions

is meant to check that it does not throw

Co-Authored-By: Bruno Turcksin <[email protected]>

---------

Co-authored-by: Bruno Turcksin <[email protected]>
  • Loading branch information
dalg24 and Rombur authored Sep 13, 2024
1 parent 827e7da commit b5509ab
Show file tree
Hide file tree
Showing 14 changed files with 62 additions and 65 deletions.
14 changes: 7 additions & 7 deletions algorithms/unit_tests/TestBinSortA.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -246,11 +246,11 @@ TEST(TEST_CATEGORY, BinSortEmptyView) {
// does not matter if we use int or something else
Kokkos::View<int*, ExecutionSpace> v("v", 0);

// test all exposed public sort methods
ASSERT_NO_THROW(Sorter.sort(ExecutionSpace(), v, 0, 0));
ASSERT_NO_THROW(Sorter.sort(v, 0, 0));
ASSERT_NO_THROW(Sorter.sort(ExecutionSpace(), v));
ASSERT_NO_THROW(Sorter.sort(v));
// test all exposed public sort methods are callable and do not throw
Sorter.sort(ExecutionSpace(), v, 0, 0);
Sorter.sort(v, 0, 0);
Sorter.sort(ExecutionSpace(), v);
Sorter.sort(v);
}

TEST(TEST_CATEGORY, BinSortEmptyKeysView) {
Expand All @@ -263,7 +263,7 @@ TEST(TEST_CATEGORY, BinSortEmptyKeysView) {
BinOp_t binOp(5, 0, 10);
Kokkos::BinSort<KeyViewType, BinOp_t> Sorter(ExecutionSpace{}, kv, binOp);

ASSERT_NO_THROW(Sorter.create_permute_vector(ExecutionSpace{}));
Sorter.create_permute_vector(ExecutionSpace{}); // does not throw
}

// BinSort may delegate sorting within bins to std::sort when running on host
Expand All @@ -282,7 +282,7 @@ TEST(TEST_CATEGORY, BinSort_issue_7221) {
Kokkos::BinSort<KeyViewType, BinOp_t> Sorter(ExecutionSpace{}, kv, binOp,
/*sort_within_bins*/ true);

ASSERT_NO_THROW(Sorter.create_permute_vector(ExecutionSpace{}));
Sorter.create_permute_vector(ExecutionSpace{}); // does not throw
}

} // namespace Test
Expand Down
5 changes: 3 additions & 2 deletions algorithms/unit_tests/TestSort.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -229,9 +229,10 @@ TEST(TEST_CATEGORY, SortEmptyView) {
// does not matter if we use int or something else
Kokkos::View<int*, ExecutionSpace> v("v", 0);

// checking that it does not throw
// TODO check the synchronous behavior of the calls below
ASSERT_NO_THROW(Kokkos::sort(ExecutionSpace(), v));
ASSERT_NO_THROW(Kokkos::sort(v));
Kokkos::sort(ExecutionSpace(), v);
Kokkos::sort(v);
}

} // namespace Test
Expand Down
16 changes: 8 additions & 8 deletions algorithms/unit_tests/TestSortByKey.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ TEST(TEST_CATEGORY, SortByKeyEmptyView) {
Kokkos::View<int *, ExecutionSpace> keys("keys", 0);
Kokkos::View<float *, ExecutionSpace> values("values", 0);

ASSERT_NO_THROW(
Kokkos::Experimental::sort_by_key(ExecutionSpace(), keys, values));
// checking that it does not throw
Kokkos::Experimental::sort_by_key(ExecutionSpace(), keys, values);
}

// Test #7036
Expand All @@ -95,8 +95,8 @@ TEST(TEST_CATEGORY, SortByKeyEmptyViewHost) {
Kokkos::View<int *, ExecutionSpace> keys("keys", 0);
Kokkos::View<float *, ExecutionSpace> values("values", 0);

ASSERT_NO_THROW(
Kokkos::Experimental::sort_by_key(ExecutionSpace(), keys, values));
// checking that it does not throw
Kokkos::Experimental::sort_by_key(ExecutionSpace(), keys, values);
}

TEST(TEST_CATEGORY, SortByKey) {
Expand Down Expand Up @@ -183,12 +183,12 @@ TEST(TEST_CATEGORY, SortByKeyStaticExtents) {
Kokkos::View<int[10], ExecutionSpace> keys("keys");

Kokkos::View<int[10], ExecutionSpace> values_static("values_static");
ASSERT_NO_THROW(
Kokkos::Experimental::sort_by_key(space, keys, values_static));
// checking that it does not throw
Kokkos::Experimental::sort_by_key(space, keys, values_static);

Kokkos::View<int *, ExecutionSpace> values_dynamic("values_dynamic", 10);
ASSERT_NO_THROW(
Kokkos::Experimental::sort_by_key(space, keys, values_dynamic));
// checking that it does not throw
Kokkos::Experimental::sort_by_key(space, keys, values_dynamic);
}

template <typename ExecutionSpace, typename Keys, typename Values>
Expand Down
15 changes: 4 additions & 11 deletions algorithms/unit_tests/TestStdAlgorithmsConstraints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,7 @@ TEST(std_algorithms, expect_no_overlap) {
auto last_st0 = first_st0 + strided_view_1d_0.extent(0);
auto first_st1 = KE::begin(strided_view_1d_1); // [3, 15)
// Does not overlap since offset (=3) is not divisible by stride (=2)
EXPECT_NO_THROW(
{ KE::Impl::expect_no_overlap(first_st0, last_st0, first_st1); });
KE::Impl::expect_no_overlap(first_st0, last_st0, first_st1);

// Iterating over the same range without overlapping
Kokkos::View<value_type[2][extent0], Kokkos::LayoutLeft> static_view_2d{
Expand All @@ -160,9 +159,7 @@ TEST(std_algorithms, expect_no_overlap) {
auto sub_last_s0 = sub_first_s0 + sub_static_view_1d_0.extent(0);
auto sub_first_s1 = KE::begin(sub_static_view_1d_1); // 1, 3, 5, ...

EXPECT_NO_THROW({
KE::Impl::expect_no_overlap(sub_first_s0, sub_last_s0, sub_first_s1);
});
KE::Impl::expect_no_overlap(sub_first_s0, sub_last_s0, sub_first_s1);

Kokkos::View<value_type**, Kokkos::LayoutLeft> dynamic_view_2d{
"std-algo-test-2d-contiguous-view-dynamic", 2, extent0};
Expand All @@ -172,9 +169,7 @@ TEST(std_algorithms, expect_no_overlap) {
auto sub_last_d0 = sub_first_d0 + sub_dynamic_view_1d_0.extent(0);
auto sub_first_d1 = KE::begin(sub_dynamic_view_1d_1); // 1, 3, 5, ...

EXPECT_NO_THROW({
KE::Impl::expect_no_overlap(sub_first_d0, sub_last_d0, sub_first_d1);
});
KE::Impl::expect_no_overlap(sub_first_d0, sub_last_d0, sub_first_d1);

Kokkos::LayoutStride layout2d{2, 3, extent0, 2 * 3};
Kokkos::View<value_type**, Kokkos::LayoutStride> strided_view_2d{
Expand All @@ -185,9 +180,7 @@ TEST(std_algorithms, expect_no_overlap) {
auto sub_last_st0 = sub_first_st0 + sub_strided_view_1d_0.extent(0);
auto sub_first_st1 = KE::begin(sub_strided_view_1d_1); // 1, 7, 13, ...

EXPECT_NO_THROW({
KE::Impl::expect_no_overlap(sub_first_st0, sub_last_st0, sub_first_st1);
});
KE::Impl::expect_no_overlap(sub_first_st0, sub_last_st0, sub_first_st1);
}

} // namespace stdalgos
Expand Down
2 changes: 1 addition & 1 deletion core/src/Kokkos_ExecPolicy.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@ struct ScratchRequest {
}
};

// Throws a runtime exception if level is not `0` or `1`
// Causes abnormal program termination if level is not `0` or `1`
void team_policy_check_valid_storage_level_argument(int level);

/** \brief Execution policy for parallel work over a league of teams of
Expand Down
2 changes: 1 addition & 1 deletion core/src/View/Kokkos_ViewMapping.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ struct SubviewExtents {
const int n = snprintf(buffer, LEN, "Kokkos::subview bounds error (");
error(buffer + n, LEN - n, 0, 0, dim, args...);

Kokkos::Impl::throw_runtime_exception(std::string(buffer));))
Kokkos::abort(buffer);))

KOKKOS_IF_ON_DEVICE(((void)dim;
Kokkos::abort("Kokkos::subview bounds error");
Expand Down
2 changes: 1 addition & 1 deletion core/src/impl/Kokkos_Core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -979,7 +979,7 @@ void Kokkos::Impl::parse_environment_variables(
Tools::Impl::parse_environment_variables(tools_init_arguments);
if (init_result.result ==
Tools::Impl::InitializationStatus::environment_argument_mismatch) {
Impl::throw_runtime_exception(init_result.error_message);
Kokkos::abort(init_result.error_message.c_str());
}
combine(settings, tools_init_arguments);

Expand Down
2 changes: 1 addition & 1 deletion core/src/impl/Kokkos_ExecPolicy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ void team_policy_check_valid_storage_level_argument(int level) {
std::stringstream ss;
ss << "TeamPolicy::set_scratch_size(/*level*/ " << level
<< ", ...) storage level argument must be 0 or 1 to be valid\n";
Impl::throw_runtime_exception(ss.str());
abort(ss.str().c_str());
}
}

Expand Down
27 changes: 12 additions & 15 deletions core/src/impl/Kokkos_SharedAlloc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ bool SharedAllocationRecord<void, void>::is_sane(
}

if (nullptr != Kokkos::atomic_exchange(&root->m_next, root_next)) {
Kokkos::Impl::throw_runtime_exception(
Kokkos::abort(
"Kokkos::Impl::SharedAllocationRecord failed is_sane unlocking");
}
}
Expand All @@ -97,7 +97,7 @@ bool SharedAllocationRecord<void, void>::is_sane(

bool SharedAllocationRecord<void, void>::is_sane(
SharedAllocationRecord<void, void>*) {
Kokkos::Impl::throw_runtime_exception(
Kokkos::abort(
"Kokkos::Impl::SharedAllocationRecord::is_sane only works with "
"KOKKOS_ENABLE_DEBUG enabled");
return false;
Expand Down Expand Up @@ -129,18 +129,17 @@ SharedAllocationRecord<void, void>* SharedAllocationRecord<void, void>::find(
}

if (nullptr != Kokkos::atomic_exchange(&arg_root->m_next, root_next)) {
Kokkos::Impl::throw_runtime_exception(
Kokkos::abort(
"Kokkos::Impl::SharedAllocationRecord failed locking/unlocking");
}
return r;
}
#else
SharedAllocationRecord<void, void>* SharedAllocationRecord<void, void>::find(
SharedAllocationRecord<void, void>* const, void* const) {
Kokkos::Impl::throw_runtime_exception(
Kokkos::abort(
"Kokkos::Impl::SharedAllocationRecord::find only works with "
"KOKKOS_ENABLE_DEBUG "
"enabled");
"KOKKOS_ENABLE_DEBUG enabled");
return nullptr;
}
#endif
Expand Down Expand Up @@ -188,13 +187,13 @@ SharedAllocationRecord<void, void>::SharedAllocationRecord(
Kokkos::memory_fence();

if (nullptr != Kokkos::atomic_exchange(&m_root->m_next, this)) {
Kokkos::Impl::throw_runtime_exception(
Kokkos::abort(
"Kokkos::Impl::SharedAllocationRecord failed locking/unlocking");
}
#endif

} else {
Kokkos::Impl::throw_runtime_exception(
Kokkos::abort(
"Kokkos::Impl::SharedAllocationRecord given nullptr allocation");
}
}
Expand All @@ -204,8 +203,7 @@ void SharedAllocationRecord<void, void>::increment(
const int old_count = Kokkos::atomic_fetch_add(&arg_record->m_count, 1);

if (old_count < 0) { // Error
Kokkos::Impl::throw_runtime_exception(
"Kokkos::Impl::SharedAllocationRecord failed increment");
Kokkos::abort("Kokkos::Impl::SharedAllocationRecord failed increment");
}
}

Expand All @@ -219,8 +217,7 @@ SharedAllocationRecord<void, void>* SharedAllocationRecord<
ss << "Kokkos allocation \"";
ss << arg_record->get_label();
ss << "\" is being deallocated after Kokkos::finalize was called\n";
auto s = ss.str();
Kokkos::Impl::throw_runtime_exception(s);
Kokkos::abort(ss.str().c_str());
}

#ifdef KOKKOS_ENABLE_DEBUG
Expand Down Expand Up @@ -256,7 +253,7 @@ SharedAllocationRecord<void, void>* SharedAllocationRecord<
// Unlock the list:
if (nullptr !=
Kokkos::atomic_exchange(&arg_record->m_root->m_next, root_next)) {
Kokkos::Impl::throw_runtime_exception(
Kokkos::abort(
"Kokkos::Impl::SharedAllocationRecord failed decrement unlocking");
}

Expand All @@ -273,7 +270,7 @@ SharedAllocationRecord<void, void>* SharedAllocationRecord<
"= %d\n",
arg_record->m_alloc_ptr->m_label, old_count);
fflush(stderr);
Kokkos::Impl::throw_runtime_exception(
Kokkos::abort(
"Kokkos::Impl::SharedAllocationRecord failed decrement count");
}

Expand Down Expand Up @@ -317,7 +314,7 @@ void SharedAllocationRecord<void, void>::print_host_accessible_records(
void SharedAllocationRecord<void, void>::print_host_accessible_records(
std::ostream&, const char* const, const SharedAllocationRecord* const,
const bool) {
Kokkos::Impl::throw_runtime_exception(
Kokkos::abort(
"Kokkos::Impl::SharedAllocationRecord::print_host_accessible_records"
" only works with KOKKOS_ENABLE_DEBUG enabled");
}
Expand Down
6 changes: 2 additions & 4 deletions core/unit_test/TestGraph.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -393,10 +393,8 @@ TEST_F(TEST_CATEGORY_FIXTURE(graph), zero_work_reduce) {
// Ensure that an empty graph can be submitted.
TEST_F(TEST_CATEGORY_FIXTURE(graph), empty_graph) {
auto graph = Kokkos::Experimental::create_graph(ex, [](auto) {});
ASSERT_NO_THROW({
graph.instantiate();
graph.submit(ex);
});
graph.instantiate();
graph.submit(ex);
ex.fence();
}

Expand Down
17 changes: 10 additions & 7 deletions core/unit_test/TestRangePolicyConstructors.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ TEST(TEST_CATEGORY, range_policy_round_trip_conversion_fires) {
ASSERT_DEATH((void)Policy(0, W(&n)), msg);
#else
::testing::internal::CaptureStderr();
ASSERT_NO_THROW((void)Policy(0, W(&n)));
(void)Policy(0, W(&n));
auto s = std::string(::testing::internal::GetCapturedStderr());
#ifdef KOKKOS_ENABLE_DEPRECATION_WARNINGS
if (Kokkos::show_warnings()) {
Expand All @@ -164,13 +164,16 @@ struct B { // round-trip conversion would not compile
};

TEST(TEST_CATEGORY, range_policy_one_way_convertible_bounds) {
using Policy = Kokkos::RangePolicy<>;
using Policy = Kokkos::RangePolicy<>;
using IndexType = Policy::index_type;

static_assert(std::is_convertible_v<B, Policy::index_type>);
static_assert(!std::is_convertible_v<Policy::index_type, B>);
static_assert(std::is_convertible_v<B, IndexType>);
static_assert(!std::is_convertible_v<IndexType, B>);

int const n = 1;
ASSERT_NO_THROW((void)Policy(0, B(&n)));
Policy policy(0, B(&n));
EXPECT_EQ(policy.begin(), static_cast<IndexType>(0));
EXPECT_EQ(policy.end(), static_cast<IndexType>(1));
}

TEST(TEST_CATEGORY, range_policy_check_sign_changes) {
Expand All @@ -193,7 +196,7 @@ TEST(TEST_CATEGORY, range_policy_check_sign_changes) {
{
::testing::internal::CaptureStderr();
std::int64_t n = std::numeric_limits<std::int64_t>::max();
ASSERT_NO_THROW((void)UInt32Policy(0, n));
(void)UInt32Policy(0, n);
auto s = std::string(::testing::internal::GetCapturedStderr());
#ifdef KOKKOS_ENABLE_DEPRECATION_WARNINGS
if (Kokkos::show_warnings()) {
Expand All @@ -204,7 +207,7 @@ TEST(TEST_CATEGORY, range_policy_check_sign_changes) {
{
::testing::internal::CaptureStderr();
std::int64_t n = std::numeric_limits<std::int64_t>::min();
ASSERT_NO_THROW((void)UInt32Policy(n, 0));
(void)UInt32Policy(n, 0);
auto s = std::string(::testing::internal::GetCapturedStderr());
#ifdef KOKKOS_ENABLE_DEPRECATION_WARNINGS
if (Kokkos::show_warnings()) {
Expand Down
4 changes: 2 additions & 2 deletions core/unit_test/TestReduce.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -645,8 +645,8 @@ TEST(TEST_CATEGORY, reduction_with_large_iteration_count) {
const int64_t N = pow(2LL, 39LL) - pow(2LL, 8LL) + 1;
Kokkos::RangePolicy<TEST_EXECSPACE, Kokkos::IndexType<int64_t>> p(0, N);
double nu = 0;
EXPECT_NO_THROW(Kokkos::parallel_reduce(
"sample reduction", p, FunctorReductionWithLargeIterationCount(), nu));
Kokkos::parallel_reduce("sample reduction", p,
FunctorReductionWithLargeIterationCount(), nu);
ASSERT_DOUBLE_EQ(nu, double(N));
}
#endif
Expand Down
8 changes: 6 additions & 2 deletions core/unit_test/TestViewCtorDimMatch.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,12 @@ void test_matching_arguments_rank_helper(std::index_sequence<Is...>) {
constexpr int nargs = sizeof...(Is);
using view_type = Kokkos::View<RankType>;
if (nargs == rank || nargs == dynrank) {
EXPECT_NO_THROW({ view_type v("v", ((Is * 0) + 1)...); });
EXPECT_NO_THROW({ view_type v(nullptr, ((Is * 0) + 1)...); });
{ // does not throw
view_type v("v", ((Is * 0) + 1)...);
}
{ // does not throw
view_type v(nullptr, ((Is * 0) + 1)...);
}
} else {
ASSERT_DEATH(
{ view_type v("v", ((Is * 0) + 1)...); },
Expand Down
7 changes: 4 additions & 3 deletions core/unit_test/TestViewIsAssignable.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

#include <Kokkos_Core.hpp>

#include <gtest/gtest.h>

namespace Test {
namespace Impl {
template <class ViewTypeDst, class ViewTypeSrc>
Expand All @@ -36,8 +38,7 @@ struct TestAssignability {
static void try_assign(
ViewTypeDst&, ViewTypeSrc&,
std::enable_if_t<!MappingType::is_assignable>* = nullptr) {
Kokkos::Impl::throw_runtime_exception(
"TestAssignability::try_assign: Unexpected call path");
FAIL() << "TestAssignability::try_assign: Unexpected call path";
}

template <class... Dimensions>
Expand All @@ -50,7 +51,7 @@ struct TestAssignability {
bool is_assignable = Kokkos::is_assignable(dst, src);

if (sometimes) {
ASSERT_NO_THROW(try_assign<mapping_type>(dst, src));
try_assign<mapping_type>(dst, src);
}
ASSERT_EQ(always, is_always_assignable)
<< Kokkos::Impl::TypeInfo<ViewTypeSrc>::name() << " to "
Expand Down

0 comments on commit b5509ab

Please sign in to comment.