Merge pull request kokkos#5384 from kokkos/5382-3.7-deprecate-sort-bool

[3.7] kokkos#5382: Deprecate overloads of Kokkos::sort() taking parameter 'bool always_use_kokkos_sort'
ndellingwood · Aug 25, 2022 · 86cbc84 · 86cbc84
2 parents 31eb58b + bfd1e0d
commit 86cbc84
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -105,6 +105,7 @@
 - Deprecate `Kokkos::common_view_alloc_prop` [\#5059](https://github.com/kokkos/kokkos/pull/5059)
 - Deprecate `Kokkos::is_reducer_type` [\#4957](https://github.com/kokkos/kokkos/pull/4957)
 - Deprecate `OffsetView` constructors taking `index_list_type` [\#4810](https://github.com/kokkos/kokkos/pull/4810)
+- Deprecate overloads of `Kokkos::sort` taking a parameter `bool always_use_kokkos_sort` [\#5382](https://github.com/kokkos/kokkos/issues/5382)
 - Warn about `parallel_reduce` cases that call `join()` with volatile-qualified arguments [\#5215](https://github.com/kokkos/kokkos/pull/5215)
 
 ### Bug Fixes:

diff --git a/algorithms/src/Kokkos_Sort.hpp b/algorithms/src/Kokkos_Sort.hpp
@@ -586,11 +586,7 @@ struct min_max_functor {
 
 template <class ExecutionSpace, class ViewType>
 std::enable_if_t<Kokkos::is_execution_space<ExecutionSpace>::value> sort(
-    const ExecutionSpace& exec, ViewType const& view,
-    bool const always_use_kokkos_sort = false) {
-  if (!always_use_kokkos_sort) {
-    if (Impl::try_std_sort(view, exec)) return;
-  }
+    const ExecutionSpace& exec, ViewType const& view) {
   using CompType = BinOp1D<ViewType>;
 
   Kokkos::MinMaxScalar<typename ViewType::non_const_value_type> result;
@@ -628,12 +624,38 @@ std::enable_if_t<Kokkos::is_execution_space<ExecutionSpace>::value> sort(
   bin_sort.sort(exec, view);
 }
 
+#ifdef KOKKOS_ENABLE_DEPRECATED_CODE_3
+template <class ExecutionSpace, class ViewType>
+KOKKOS_DEPRECATED_WITH_COMMENT(
+    "Use the overload not taking bool always_use_kokkos_sort")
+std::enable_if_t<Kokkos::is_execution_space<ExecutionSpace>::value> sort(
+    const ExecutionSpace& exec, ViewType const& view,
+    bool const always_use_kokkos_sort) {
+  if (!always_use_kokkos_sort && Impl::try_std_sort(view, exec)) {
+    return;
+  } else {
+    sort(exec, view);
+  }
+}
+#endif
+
 template <class ViewType>
-void sort(ViewType const& view, bool const always_use_kokkos_sort = false) {
+void sort(ViewType const& view) {
+  typename ViewType::execution_space exec;
+  sort(exec, view);
+  exec.fence("Kokkos::Sort: fence after sorting");
+}
+
+#ifdef KOKKOS_ENABLE_DEPRECATED_CODE_3
+template <class ViewType>
+KOKKOS_DEPRECATED_WITH_COMMENT(
+    "Use the overload not taking bool always_use_kokkos_sort")
+void sort(ViewType const& view, bool const always_use_kokkos_sort) {
   typename ViewType::execution_space exec;
   sort(exec, view, always_use_kokkos_sort);
   exec.fence("Kokkos::Sort: fence after sorting");
 }
+#endif
 
 template <class ExecutionSpace, class ViewType>
 std::enable_if_t<Kokkos::is_execution_space<ExecutionSpace>::value> sort(

diff --git a/algorithms/unit_tests/TestSort.hpp b/algorithms/unit_tests/TestSort.hpp
@@ -137,7 +137,12 @@ void test_1D_sort_impl(unsigned int n, bool force_kokkos) {
   // Test sorting array with all numbers equal
   ExecutionSpace exec;
   Kokkos::deep_copy(exec, keys, KeyType(1));
+#ifdef KOKKOS_ENABLE_DEPRECATED_CODE_3
   Kokkos::sort(exec, keys, force_kokkos);
+#else
+  (void)force_kokkos;  // suppress warnings about unused variable
+  Kokkos::sort(exec, keys);
+#endif
 
   Kokkos::Random_XorShift64_Pool<ExecutionSpace> g(1931);
   Kokkos::fill_random(keys, g,
@@ -151,7 +156,11 @@ void test_1D_sort_impl(unsigned int n, bool force_kokkos) {
   Kokkos::parallel_reduce(Kokkos::RangePolicy<ExecutionSpace>(exec, 0, n),
                           sum<ExecutionSpace, KeyType>(keys), sum_before);
 
+#ifdef KOKKOS_ENABLE_DEPRECATED_CODE_3
   Kokkos::sort(exec, keys, force_kokkos);
+#else
+  Kokkos::sort(exec, keys);
+#endif
 
   Kokkos::parallel_reduce(Kokkos::RangePolicy<ExecutionSpace>(exec, 0, n),
                           sum<ExecutionSpace, KeyType>(keys), sum_after);
@@ -396,7 +405,7 @@ void test_sort_integer_overflow() {
             Kokkos::Experimental::finite_min<T>::value};
   auto vd = Kokkos::create_mirror_view_and_copy(
       ExecutionSpace(), Kokkos::View<T[2], Kokkos::HostSpace>(a));
-  Kokkos::sort(vd, /*force using Kokkos bin sort*/ true);
+  Kokkos::sort(vd);
   auto vh = Kokkos::create_mirror_view_and_copy(Kokkos::HostSpace(), vd);
   EXPECT_TRUE(std::is_sorted(vh.data(), vh.data() + 2))
       << "view (" << vh[0] << ", " << vh[1] << ") is not sorted";
@@ -407,7 +416,9 @@ void test_sort_integer_overflow() {
 template <class ExecutionSpace, typename KeyType>
 void test_1D_sort(unsigned int N) {
   test_1D_sort_impl<ExecutionSpace, KeyType>(N * N * N, true);
+#ifdef KOKKOS_ENABLE_DEPRECATED_CODE_3
   test_1D_sort_impl<ExecutionSpace, KeyType>(N * N * N, false);
+#endif
 }
 
 template <class ExecutionSpace, typename KeyType>