From 375b3f73ab64810a620422c963acaf92f2e87062 Mon Sep 17 00:00:00 2001
From: MithunR <mythrocks@gmail.com>
Date: Thu, 1 Jun 2023 15:27:50 -0700
Subject: [PATCH 01/14] WIP: Code in place. Refafctoring tests.

---
 .../rolling/detail/range_window_bounds.hpp    |  8 +++-
 cpp/src/rolling/grouped_rolling.cu            |  2 +-
 cpp/src/rolling/range_window_bounds.cpp       | 25 +++++++++++
 .../rolling/grouped_rolling_range_test.cpp    | 45 +++++++++++++------
 4 files changed, 64 insertions(+), 16 deletions(-)

diff --git a/cpp/src/rolling/detail/range_window_bounds.hpp b/cpp/src/rolling/detail/range_window_bounds.hpp
index 76af3f5a774..176a2bd0ddd 100644
--- a/cpp/src/rolling/detail/range_window_bounds.hpp
+++ b/cpp/src/rolling/detail/range_window_bounds.hpp
@@ -29,6 +29,7 @@ template <typename RangeType>
 constexpr bool is_supported_range_type()
 {
   return cudf::is_duration<RangeType>() || cudf::is_fixed_point<RangeType>() ||
+         cudf::is_floating_point<RangeType>() ||
          (std::is_integral_v<RangeType> && !cudf::is_boolean<RangeType>());
 }
 
@@ -39,6 +40,7 @@ constexpr bool is_supported_order_by_column_type()
 {
   return cudf::is_timestamp<ColumnType>() || cudf::is_fixed_point<ColumnType>() ||
          (std::is_integral_v<ColumnType> && !cudf::is_boolean<ColumnType>()) ||
+         cudf::is_floating_point<ColumnType>() ||
          std::is_same_v<ColumnType, cudf::string_view>;
 }
 
@@ -64,7 +66,8 @@ struct range_type_impl {
 template <typename ColumnType>
 struct range_type_impl<
   ColumnType,
-  std::enable_if_t<std::is_integral_v<ColumnType> && !cudf::is_boolean<ColumnType>(), void>> {
+  std::enable_if_t<std::is_floating_point_v<ColumnType> ||
+    (std::is_integral_v<ColumnType> && !cudf::is_boolean<ColumnType>()), void>> {
   using type     = ColumnType;
   using rep_type = ColumnType;
 };
@@ -98,7 +101,8 @@ void assert_non_negative([[maybe_unused]] T const& value)
 
 template <typename RangeT,
           typename RepT,
-          CUDF_ENABLE_IF(std::is_integral_v<RangeT> && !cudf::is_boolean<RangeT>())>
+          CUDF_ENABLE_IF(std::is_floating_point_v<RangeT>
+              || (std::is_integral_v<RangeT> && !cudf::is_boolean<RangeT>()))>
 RepT range_comparable_value_impl(scalar const& range_scalar,
                                  bool,
                                  data_type const&,
diff --git a/cpp/src/rolling/grouped_rolling.cu b/cpp/src/rolling/grouped_rolling.cu
index 861effe8dfb..7a084d7ced4 100644
--- a/cpp/src/rolling/grouped_rolling.cu
+++ b/cpp/src/rolling/grouped_rolling.cu
@@ -314,7 +314,7 @@ struct device_value_accessor {
    *
    * @param[in] col_ column device view of cudf column
    */
-  __device__ device_value_accessor(column_device_view const& col_) : col{col_}
+  explicit __device__ device_value_accessor(column_device_view const& col_) : col{col_}
   {
     cudf_assert(type_id_matches_device_storage_type<T>(col.type().id()) &&
                 "the data type mismatch");
diff --git a/cpp/src/rolling/range_window_bounds.cpp b/cpp/src/rolling/range_window_bounds.cpp
index 35da9e3f8ec..c9a3c9080f5 100644
--- a/cpp/src/rolling/range_window_bounds.cpp
+++ b/cpp/src/rolling/range_window_bounds.cpp
@@ -30,6 +30,7 @@ namespace {
  * This makes it possible to copy construct and copy assign `range_window_bounds` objects.
  */
 struct range_scalar_constructor {
+  /*
   template <typename T, CUDF_ENABLE_IF(not detail::is_supported_range_type<T>())>
   std::unique_ptr<scalar> operator()(scalar const& range_scalar_) const
   {
@@ -58,6 +59,30 @@ struct range_scalar_constructor {
     return std::make_unique<fixed_point_scalar<T>>(
       static_cast<fixed_point_scalar<T> const&>(range_scalar_));
   }
+  */
+
+  template <typename T>
+  std::unique_ptr<scalar> operator()(scalar const& range_scalar_) const
+  {
+      if constexpr (std::is_floating_point_v<T> || (std::is_integral_v<T> && not cudf::is_boolean<T>())) {
+        return std::make_unique<numeric_scalar<T>>(
+            static_cast<numeric_scalar<T> const&>(range_scalar_));
+      }
+      else if constexpr (cudf::is_fixed_point<T>()) {
+        return std::make_unique<fixed_point_scalar<T>>(
+            static_cast<fixed_point_scalar<T> const&>(range_scalar_));
+      }
+      else if constexpr (cudf::is_duration<T>()) {
+        return std::make_unique<duration_scalar<T>>(
+            static_cast<duration_scalar<T> const&>(range_scalar_));
+      }
+      else {
+        assert(not detail::is_supported_range_type<T>());
+        CUDF_FAIL(
+            "Unsupported range type. "
+            "Only Durations, fixed-point, floating point, and non-boolean integral range types are allowed.");
+      }
+  }
 };
 
 }  // namespace
diff --git a/cpp/tests/rolling/grouped_rolling_range_test.cpp b/cpp/tests/rolling/grouped_rolling_range_test.cpp
index 6321b193e26..379b29c05be 100644
--- a/cpp/tests/rolling/grouped_rolling_range_test.cpp
+++ b/cpp/tests/rolling/grouped_rolling_range_test.cpp
@@ -46,20 +46,46 @@ using bigints_column  = fwcw<int64_t>;
 using strings_column  = cudf::test::strings_column_wrapper;
 using column_ptr      = std::unique_ptr<cudf::column>;
 
-struct BaseGroupedRollingRangeOrderByDecimalTest : public cudf::test::BaseFixture {
+template <typename T>
+struct BaseGroupedRollingRangeOrderByTest : public cudf::test::BaseFixture {
   // Stand-in for std::pow(10, n), but for integral return.
   static constexpr std::array<int32_t, 6> pow10{1, 10, 100, 1000, 10000, 100000};
+
   // Test data.
   column_ptr const grouping_keys = ints_column{0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 2, 2, 2, 2}.release();
   column_ptr const agg_values    = ints_column{1, 1, 1, 1, 1, 1, 2, 2, 2, 2, 3, 3, 3, 3}.release();
   cudf::size_type const num_rows = grouping_keys->size();
-};
 
-using base = BaseGroupedRollingRangeOrderByDecimalTest;  // Shortcut to base test class.
+  /**
+   * @brief Get grouped rolling results for specified order-by column and range scale
+   *
+   */
+  [[nodiscard]]
+  column_ptr get_grouped_range_rolling_result(cudf::range_window_bounds const& preceding,
+                                              cudf::range_window_bounds const& following,
+                                              cudf::column_view const& order_by_column,
+                                              numeric::scale_type const& range_scale) const
+  {
+    return cudf::grouped_range_rolling_window(
+        cudf::table_view{{grouping_keys->view()}},
+        order_by_column,
+        cudf::order::ASCENDING,
+        agg_values->view(),
+        preceding,
+        following,
+        1,  // min_periods
+        *cudf::make_sum_aggregation<cudf::rolling_aggregation>());
+  }
+};
 
 template <typename DecimalT>
-struct GroupedRollingRangeOrderByDecimalTypedTest : BaseGroupedRollingRangeOrderByDecimalTest {
+struct GroupedRollingRangeOrderByDecimalTypedTest : BaseGroupedRollingRangeOrderByTest<typename DecimalT::rep> {
   using Rep = typename DecimalT::rep;
+  using base = BaseGroupedRollingRangeOrderByTest<Rep>;
+
+  using base::agg_values;
+  using base::grouping_keys;
+  using base::num_rows;
 
   [[nodiscard]] auto make_fixed_point_range_bounds(typename DecimalT::rep value,
                                                    numeric::scale_type scale) const
@@ -108,6 +134,7 @@ struct GroupedRollingRangeOrderByDecimalTypedTest : BaseGroupedRollingRangeOrder
    * @brief Get grouped rolling results for specified order-by column and range scale
    *
    */
+  [[nodiscard]]
   column_ptr get_grouped_range_rolling_result(cudf::column_view const& order_by_column,
                                               numeric::scale_type const& range_scale) const
   {
@@ -116,15 +143,7 @@ struct GroupedRollingRangeOrderByDecimalTypedTest : BaseGroupedRollingRangeOrder
     auto const following =
       this->make_fixed_point_range_bounds(rescale_range_value(Rep{100}, range_scale), range_scale);
 
-    return cudf::grouped_range_rolling_window(
-      cudf::table_view{{grouping_keys->view()}},
-      order_by_column,
-      cudf::order::ASCENDING,
-      agg_values->view(),
-      preceding,
-      following,
-      1,  // min_periods
-      *cudf::make_sum_aggregation<cudf::rolling_aggregation>());
+    return base::get_grouped_range_rolling_result(preceding, following, order_by_column, range_scale);
   }
 
   /**

From 4ae82998b9948c0b235094b6789d432950448251 Mon Sep 17 00:00:00 2001
From: MithunR <mythrocks@gmail.com>
Date: Fri, 2 Jun 2023 15:23:07 -0700
Subject: [PATCH 02/14] Working tests for floating point types.

Signed-off-by: MithunR <mythrocks@gmail.com>
---
 .../rolling/grouped_rolling_range_test.cpp    | 182 +++++++++++++++++-
 1 file changed, 174 insertions(+), 8 deletions(-)

diff --git a/cpp/tests/rolling/grouped_rolling_range_test.cpp b/cpp/tests/rolling/grouped_rolling_range_test.cpp
index 379b29c05be..fa3d35881fb 100644
--- a/cpp/tests/rolling/grouped_rolling_range_test.cpp
+++ b/cpp/tests/rolling/grouped_rolling_range_test.cpp
@@ -47,7 +47,8 @@ using strings_column  = cudf::test::strings_column_wrapper;
 using column_ptr      = std::unique_ptr<cudf::column>;
 
 template <typename T>
-struct BaseGroupedRollingRangeOrderByTest : public cudf::test::BaseFixture {
+struct BaseGroupedRollingRangeOrderByTest : cudf::test::BaseFixture {
+
   // Stand-in for std::pow(10, n), but for integral return.
   static constexpr std::array<int32_t, 6> pow10{1, 10, 100, 1000, 10000, 100000};
 
@@ -57,14 +58,12 @@ struct BaseGroupedRollingRangeOrderByTest : public cudf::test::BaseFixture {
   cudf::size_type const num_rows = grouping_keys->size();
 
   /**
-   * @brief Get grouped rolling results for specified order-by column and range scale
-   *
+   * @brief Get grouped rolling results for specified order-by column and range bounds.
    */
   [[nodiscard]]
   column_ptr get_grouped_range_rolling_result(cudf::range_window_bounds const& preceding,
                                               cudf::range_window_bounds const& following,
-                                              cudf::column_view const& order_by_column,
-                                              numeric::scale_type const& range_scale) const
+                                              cudf::column_view const& order_by_column) const
   {
     return cudf::grouped_range_rolling_window(
         cudf::table_view{{grouping_keys->view()}},
@@ -78,6 +77,173 @@ struct BaseGroupedRollingRangeOrderByTest : public cudf::test::BaseFixture {
   }
 };
 
+template <typename T>
+struct GroupedRollingRangeOrderByNumericTest : public BaseGroupedRollingRangeOrderByTest<T> {
+  using base = BaseGroupedRollingRangeOrderByTest<T>;
+
+  using base::agg_values;
+  using base::grouping_keys;
+  using base::num_rows;
+  using base::get_grouped_range_rolling_result;
+
+  [[nodiscard]] auto make_range_bounds(T const& value) const
+  {
+      return cudf::range_window_bounds::get(*cudf::make_fixed_width_scalar(value));
+  }
+
+  [[nodiscard]] auto make_unbounded_range_bounds() const
+  {
+      return cudf::range_window_bounds::unbounded(cudf::data_type{cudf::type_to_id<T>()});
+  }
+
+  /// Generate order-by column with values: [0, 100,   200,   300,   ... 1100,   1200,   1300]
+  [[nodiscard]] column_ptr generate_order_by_column() const
+  {
+    auto const begin = thrust::make_transform_iterator(
+        thrust::make_counting_iterator<cudf::size_type>(0),
+        [&](T const& i) -> T { return i * 100; });
+
+    return fwcw<T>(begin, begin + num_rows).release();
+  }
+
+  /**
+   * @brief Run grouped_rolling test with no nulls in the order-by column
+   */
+  void run_test_no_null_oby() const
+  {
+    auto const preceding = make_range_bounds(T{200});
+    auto const following = make_range_bounds(T{100});
+    auto const order_by = generate_order_by_column();
+    auto const results =
+      get_grouped_range_rolling_result(preceding, following, *order_by);
+    auto const expected_results = bigints_column{{2, 3, 4, 4, 4, 3, 4, 6, 8, 6, 6, 9, 12, 9},
+                                                   cudf::test::iterators::no_nulls()};
+    CUDF_TEST_EXPECT_COLUMNS_EQUAL(*results, expected_results);
+  }
+
+  /**
+   * @brief Run grouped_rolling test with nulls in the order-by column
+   * (i.e. 2 nulls at the beginning of each group)
+   *
+   */
+  void run_test_nulls_in_oby() const
+  {
+    auto const preceding = make_range_bounds(T{200});
+    auto const following = make_range_bounds(T{100});
+
+    // Nullify the first two rows of each group in the order_by column.
+    auto const nulled_order_by = [&] {
+      auto col           = generate_order_by_column();
+      auto new_null_mask = create_null_mask(col->size(), cudf::mask_state::ALL_VALID);
+      cudf::set_null_mask(static_cast<cudf::bitmask_type*>(new_null_mask.data()),
+                          0,
+                          2,
+                          false);  // Nulls in first group.
+      cudf::set_null_mask(static_cast<cudf::bitmask_type*>(new_null_mask.data()),
+                          6,
+                          8,
+                          false);  // Nulls in second group.
+      cudf::set_null_mask(static_cast<cudf::bitmask_type*>(new_null_mask.data()),
+                          10,
+                          12,
+                          false);  // Nulls in third group.
+      col->set_null_mask(std::move(new_null_mask), 6);
+      return col;
+    }();
+
+    auto const results =
+        get_grouped_range_rolling_result(preceding, following, *nulled_order_by);
+    auto const expected_results = bigints_column{{2, 2, 2, 3, 4, 3, 4, 4, 4, 4, 6, 6, 6, 6},
+                                               cudf::test::iterators::no_nulls()};
+    CUDF_TEST_EXPECT_COLUMNS_EQUAL(*results, expected_results);
+  }
+
+  /**
+   * @brief Run grouped_rolling test with unbounded preceding and unbounded following.
+   *
+   */
+  void run_test_unbounded_preceding_to_unbounded_following()
+  {
+    auto const order_by  = generate_order_by_column();
+    auto const preceding = make_unbounded_range_bounds();
+    auto const following = make_unbounded_range_bounds();
+    auto results =
+        cudf::grouped_range_rolling_window(cudf::table_view{{grouping_keys->view()}},
+                                           order_by->view(),
+                                           cudf::order::ASCENDING,
+                                           agg_values->view(),
+                                           preceding,
+                                           following,
+                                           1,  // min_periods
+                                           *cudf::make_sum_aggregation<cudf::rolling_aggregation>());
+
+    auto expected_results = bigints_column{{6, 6, 6, 6, 6, 6, 8, 8, 8, 8, 12, 12, 12, 12},
+                                           cudf::test::iterators::no_nulls()};
+    CUDF_TEST_EXPECT_COLUMNS_EQUAL(*results, expected_results);
+  }
+
+  /**
+   * @brief Run grouped_rolling test with unbounded preceding and current row.
+   *
+   */
+  void run_test_unbounded_preceding_to_current_row() {
+    auto const order_by = generate_order_by_column();
+    auto const unbounded_preceding = make_unbounded_range_bounds();
+    auto const current_row = make_range_bounds(T{0});
+    auto const results = cudf::grouped_range_rolling_window(
+        cudf::table_view{{grouping_keys->view()}},
+        order_by->view(),
+        cudf::order::ASCENDING,
+        agg_values->view(),
+        unbounded_preceding,
+        current_row,
+        1,  // min_periods
+        *cudf::make_sum_aggregation<cudf::rolling_aggregation>());
+
+    auto expected_results = bigints_column{{1, 2, 3, 4, 5, 6, 2, 4, 6, 8, 3, 6, 9, 12},
+                                           cudf::test::iterators::no_nulls()};
+    CUDF_TEST_EXPECT_COLUMNS_EQUAL(*results, expected_results);
+  }
+
+  /**
+   * @brief Run grouped_rolling test with current row and unbounded following.
+   */
+  void run_test_current_row_to_unbounded_following() {
+    auto const order_by = generate_order_by_column();
+    auto const unbounded_following = make_unbounded_range_bounds();
+
+    auto const current_row = make_range_bounds(T{0});
+    auto const results = cudf::grouped_range_rolling_window(
+        cudf::table_view{{grouping_keys->view()}},
+        order_by->view(),
+        cudf::order::ASCENDING,
+        agg_values->view(),
+        current_row,
+        unbounded_following,
+        1,  // min_periods
+        *cudf::make_sum_aggregation<cudf::rolling_aggregation>());
+
+    auto expected_results = bigints_column{{6, 5, 4, 3, 2, 1, 8, 6, 4, 2, 12, 9, 6, 3},
+                                           cudf::test::iterators::no_nulls()};
+    CUDF_TEST_EXPECT_COLUMNS_EQUAL(*results, expected_results);
+  }
+};
+
+TYPED_TEST_SUITE(GroupedRollingRangeOrderByNumericTest, cudf::test::FloatingPointTypes);
+
+TYPED_TEST(GroupedRollingRangeOrderByNumericTest, BoundedRanges)
+{
+  this->run_test_no_null_oby();
+  this->run_test_nulls_in_oby();
+}
+
+TYPED_TEST(GroupedRollingRangeOrderByNumericTest, UnboundedRanges)
+{
+  this->run_test_unbounded_preceding_to_unbounded_following();
+  this->run_test_unbounded_preceding_to_current_row();
+  this->run_test_current_row_to_unbounded_following();
+}
+
 template <typename DecimalT>
 struct GroupedRollingRangeOrderByDecimalTypedTest : BaseGroupedRollingRangeOrderByTest<typename DecimalT::rep> {
   using Rep = typename DecimalT::rep;
@@ -143,7 +309,7 @@ struct GroupedRollingRangeOrderByDecimalTypedTest : BaseGroupedRollingRangeOrder
     auto const following =
       this->make_fixed_point_range_bounds(rescale_range_value(Rep{100}, range_scale), range_scale);
 
-    return base::get_grouped_range_rolling_result(preceding, following, order_by_column, range_scale);
+    return base::get_grouped_range_rolling_result(preceding, following, order_by_column);
   }
 
   /**
@@ -228,7 +394,7 @@ struct GroupedRollingRangeOrderByDecimalTypedTest : BaseGroupedRollingRangeOrder
 
   /**
    * @brief Run grouped_rolling test for specified order-by column scale with
-   * unbounded preceding and unbounded following.
+   * unbounded preceding and current row.
    *
    */
   void run_test_unbounded_preceding_to_current_row(numeric::scale_type oby_column_scale)
@@ -258,7 +424,7 @@ struct GroupedRollingRangeOrderByDecimalTypedTest : BaseGroupedRollingRangeOrder
 
   /**
    * @brief Run grouped_rolling test for specified order-by column scale with
-   * unbounded preceding and unbounded following.
+   * current row and unbounded following.
    *
    */
   void run_test_current_row_to_unbounded_following(numeric::scale_type oby_column_scale)

From 1ba40f80289e302e44575625a4749b3ba50d16e2 Mon Sep 17 00:00:00 2001
From: MithunR <mythrocks@gmail.com>
Date: Fri, 2 Jun 2023 15:55:51 -0700
Subject: [PATCH 03/14] Refactored test: Common call to grouped_rolling.

---
 .../rolling/grouped_rolling_range_test.cpp    | 66 +++++++------------
 1 file changed, 25 insertions(+), 41 deletions(-)

diff --git a/cpp/tests/rolling/grouped_rolling_range_test.cpp b/cpp/tests/rolling/grouped_rolling_range_test.cpp
index fa3d35881fb..2d4c621ccb9 100644
--- a/cpp/tests/rolling/grouped_rolling_range_test.cpp
+++ b/cpp/tests/rolling/grouped_rolling_range_test.cpp
@@ -63,7 +63,8 @@ struct BaseGroupedRollingRangeOrderByTest : cudf::test::BaseFixture {
   [[nodiscard]]
   column_ptr get_grouped_range_rolling_result(cudf::range_window_bounds const& preceding,
                                               cudf::range_window_bounds const& following,
-                                              cudf::column_view const& order_by_column) const
+                                              cudf::column_view const& order_by_column,
+                                              cudf::rolling_aggregation const& agg) const
   {
     return cudf::grouped_range_rolling_window(
         cudf::table_view{{grouping_keys->view()}},
@@ -73,7 +74,16 @@ struct BaseGroupedRollingRangeOrderByTest : cudf::test::BaseFixture {
         preceding,
         following,
         1,  // min_periods
-        *cudf::make_sum_aggregation<cudf::rolling_aggregation>());
+        agg);
+  }
+
+  [[nodiscard]]
+  column_ptr get_grouped_range_rolling_sum_result(cudf::range_window_bounds const& preceding,
+                                                  cudf::range_window_bounds const& following,
+                                                  cudf::column_view const& order_by_column) const
+  {
+    return get_grouped_range_rolling_result(preceding, following, order_by_column,
+                                            *cudf::make_sum_aggregation<cudf::rolling_aggregation>());
   }
 };
 
@@ -84,7 +94,7 @@ struct GroupedRollingRangeOrderByNumericTest : public BaseGroupedRollingRangeOrd
   using base::agg_values;
   using base::grouping_keys;
   using base::num_rows;
-  using base::get_grouped_range_rolling_result;
+  using base::get_grouped_range_rolling_sum_result;
 
   [[nodiscard]] auto make_range_bounds(T const& value) const
   {
@@ -115,7 +125,7 @@ struct GroupedRollingRangeOrderByNumericTest : public BaseGroupedRollingRangeOrd
     auto const following = make_range_bounds(T{100});
     auto const order_by = generate_order_by_column();
     auto const results =
-      get_grouped_range_rolling_result(preceding, following, *order_by);
+      get_grouped_range_rolling_sum_result(preceding, following, *order_by);
     auto const expected_results = bigints_column{{2, 3, 4, 4, 4, 3, 4, 6, 8, 6, 6, 9, 12, 9},
                                                    cudf::test::iterators::no_nulls()};
     CUDF_TEST_EXPECT_COLUMNS_EQUAL(*results, expected_results);
@@ -152,7 +162,7 @@ struct GroupedRollingRangeOrderByNumericTest : public BaseGroupedRollingRangeOrd
     }();
 
     auto const results =
-        get_grouped_range_rolling_result(preceding, following, *nulled_order_by);
+        get_grouped_range_rolling_sum_result(preceding, following, *nulled_order_by);
     auto const expected_results = bigints_column{{2, 2, 2, 3, 4, 3, 4, 4, 4, 4, 6, 6, 6, 6},
                                                cudf::test::iterators::no_nulls()};
     CUDF_TEST_EXPECT_COLUMNS_EQUAL(*results, expected_results);
@@ -160,48 +170,30 @@ struct GroupedRollingRangeOrderByNumericTest : public BaseGroupedRollingRangeOrd
 
   /**
    * @brief Run grouped_rolling test with unbounded preceding and unbounded following.
-   *
    */
   void run_test_unbounded_preceding_to_unbounded_following()
   {
     auto const order_by  = generate_order_by_column();
     auto const preceding = make_unbounded_range_bounds();
     auto const following = make_unbounded_range_bounds();
-    auto results =
-        cudf::grouped_range_rolling_window(cudf::table_view{{grouping_keys->view()}},
-                                           order_by->view(),
-                                           cudf::order::ASCENDING,
-                                           agg_values->view(),
-                                           preceding,
-                                           following,
-                                           1,  // min_periods
-                                           *cudf::make_sum_aggregation<cudf::rolling_aggregation>());
+    auto const results = get_grouped_range_rolling_sum_result(preceding, following, *order_by);
 
-    auto expected_results = bigints_column{{6, 6, 6, 6, 6, 6, 8, 8, 8, 8, 12, 12, 12, 12},
-                                           cudf::test::iterators::no_nulls()};
+    auto const expected_results = bigints_column{{6, 6, 6, 6, 6, 6, 8, 8, 8, 8, 12, 12, 12, 12},
+                                                 cudf::test::iterators::no_nulls()};
     CUDF_TEST_EXPECT_COLUMNS_EQUAL(*results, expected_results);
   }
 
   /**
    * @brief Run grouped_rolling test with unbounded preceding and current row.
-   *
    */
   void run_test_unbounded_preceding_to_current_row() {
     auto const order_by = generate_order_by_column();
     auto const unbounded_preceding = make_unbounded_range_bounds();
     auto const current_row = make_range_bounds(T{0});
-    auto const results = cudf::grouped_range_rolling_window(
-        cudf::table_view{{grouping_keys->view()}},
-        order_by->view(),
-        cudf::order::ASCENDING,
-        agg_values->view(),
-        unbounded_preceding,
-        current_row,
-        1,  // min_periods
-        *cudf::make_sum_aggregation<cudf::rolling_aggregation>());
+    auto const results = get_grouped_range_rolling_sum_result(unbounded_preceding, current_row, *order_by);
 
-    auto expected_results = bigints_column{{1, 2, 3, 4, 5, 6, 2, 4, 6, 8, 3, 6, 9, 12},
-                                           cudf::test::iterators::no_nulls()};
+    auto const expected_results = bigints_column{{1, 2, 3, 4, 5, 6, 2, 4, 6, 8, 3, 6, 9, 12},
+                                                 cudf::test::iterators::no_nulls()};
     CUDF_TEST_EXPECT_COLUMNS_EQUAL(*results, expected_results);
   }
 
@@ -213,18 +205,10 @@ struct GroupedRollingRangeOrderByNumericTest : public BaseGroupedRollingRangeOrd
     auto const unbounded_following = make_unbounded_range_bounds();
 
     auto const current_row = make_range_bounds(T{0});
-    auto const results = cudf::grouped_range_rolling_window(
-        cudf::table_view{{grouping_keys->view()}},
-        order_by->view(),
-        cudf::order::ASCENDING,
-        agg_values->view(),
-        current_row,
-        unbounded_following,
-        1,  // min_periods
-        *cudf::make_sum_aggregation<cudf::rolling_aggregation>());
+    auto const results = get_grouped_range_rolling_sum_result(current_row, unbounded_following, *order_by);
 
-    auto expected_results = bigints_column{{6, 5, 4, 3, 2, 1, 8, 6, 4, 2, 12, 9, 6, 3},
-                                           cudf::test::iterators::no_nulls()};
+    auto const expected_results = bigints_column{{6, 5, 4, 3, 2, 1, 8, 6, 4, 2, 12, 9, 6, 3},
+                                                 cudf::test::iterators::no_nulls()};
     CUDF_TEST_EXPECT_COLUMNS_EQUAL(*results, expected_results);
   }
 };
@@ -309,7 +293,7 @@ struct GroupedRollingRangeOrderByDecimalTypedTest : BaseGroupedRollingRangeOrder
     auto const following =
       this->make_fixed_point_range_bounds(rescale_range_value(Rep{100}, range_scale), range_scale);
 
-    return base::get_grouped_range_rolling_result(preceding, following, order_by_column);
+    return base::get_grouped_range_rolling_sum_result(preceding, following, order_by_column);
   }
 
   /**

From dd43cf4e5805fe19b50cfe75805548a5daa3630c Mon Sep 17 00:00:00 2001
From: MithunR <mythrocks@gmail.com>
Date: Fri, 2 Jun 2023 15:57:44 -0700
Subject: [PATCH 04/14] Formatting.

Signed-off-by: MithunR <mythrocks@gmail.com>
---
 .../rolling/detail/range_window_bounds.hpp    | 10 +-
 cpp/src/rolling/range_window_bounds.cpp       | 35 ++++---
 .../rolling/grouped_rolling_range_test.cpp    | 95 ++++++++++---------
 3 files changed, 71 insertions(+), 69 deletions(-)

diff --git a/cpp/src/rolling/detail/range_window_bounds.hpp b/cpp/src/rolling/detail/range_window_bounds.hpp
index 176a2bd0ddd..40da8f6ab18 100644
--- a/cpp/src/rolling/detail/range_window_bounds.hpp
+++ b/cpp/src/rolling/detail/range_window_bounds.hpp
@@ -40,8 +40,7 @@ constexpr bool is_supported_order_by_column_type()
 {
   return cudf::is_timestamp<ColumnType>() || cudf::is_fixed_point<ColumnType>() ||
          (std::is_integral_v<ColumnType> && !cudf::is_boolean<ColumnType>()) ||
-         cudf::is_floating_point<ColumnType>() ||
-         std::is_same_v<ColumnType, cudf::string_view>;
+         cudf::is_floating_point<ColumnType>() || std::is_same_v<ColumnType, cudf::string_view>;
 }
 
 /// Range-comparable representation type for an orderby column type.
@@ -67,7 +66,8 @@ template <typename ColumnType>
 struct range_type_impl<
   ColumnType,
   std::enable_if_t<std::is_floating_point_v<ColumnType> ||
-    (std::is_integral_v<ColumnType> && !cudf::is_boolean<ColumnType>()), void>> {
+                     (std::is_integral_v<ColumnType> && !cudf::is_boolean<ColumnType>()),
+                   void>> {
   using type     = ColumnType;
   using rep_type = ColumnType;
 };
@@ -101,8 +101,8 @@ void assert_non_negative([[maybe_unused]] T const& value)
 
 template <typename RangeT,
           typename RepT,
-          CUDF_ENABLE_IF(std::is_floating_point_v<RangeT>
-              || (std::is_integral_v<RangeT> && !cudf::is_boolean<RangeT>()))>
+          CUDF_ENABLE_IF(std::is_floating_point_v<RangeT> ||
+                         (std::is_integral_v<RangeT> && !cudf::is_boolean<RangeT>()))>
 RepT range_comparable_value_impl(scalar const& range_scalar,
                                  bool,
                                  data_type const&,
diff --git a/cpp/src/rolling/range_window_bounds.cpp b/cpp/src/rolling/range_window_bounds.cpp
index c9a3c9080f5..4138bd2bd3c 100644
--- a/cpp/src/rolling/range_window_bounds.cpp
+++ b/cpp/src/rolling/range_window_bounds.cpp
@@ -64,24 +64,23 @@ struct range_scalar_constructor {
   template <typename T>
   std::unique_ptr<scalar> operator()(scalar const& range_scalar_) const
   {
-      if constexpr (std::is_floating_point_v<T> || (std::is_integral_v<T> && not cudf::is_boolean<T>())) {
-        return std::make_unique<numeric_scalar<T>>(
-            static_cast<numeric_scalar<T> const&>(range_scalar_));
-      }
-      else if constexpr (cudf::is_fixed_point<T>()) {
-        return std::make_unique<fixed_point_scalar<T>>(
-            static_cast<fixed_point_scalar<T> const&>(range_scalar_));
-      }
-      else if constexpr (cudf::is_duration<T>()) {
-        return std::make_unique<duration_scalar<T>>(
-            static_cast<duration_scalar<T> const&>(range_scalar_));
-      }
-      else {
-        assert(not detail::is_supported_range_type<T>());
-        CUDF_FAIL(
-            "Unsupported range type. "
-            "Only Durations, fixed-point, floating point, and non-boolean integral range types are allowed.");
-      }
+    if constexpr (std::is_floating_point_v<T> ||
+                  (std::is_integral_v<T> && not cudf::is_boolean<T>())) {
+      return std::make_unique<numeric_scalar<T>>(
+        static_cast<numeric_scalar<T> const&>(range_scalar_));
+    } else if constexpr (cudf::is_fixed_point<T>()) {
+      return std::make_unique<fixed_point_scalar<T>>(
+        static_cast<fixed_point_scalar<T> const&>(range_scalar_));
+    } else if constexpr (cudf::is_duration<T>()) {
+      return std::make_unique<duration_scalar<T>>(
+        static_cast<duration_scalar<T> const&>(range_scalar_));
+    } else {
+      assert(not detail::is_supported_range_type<T>());
+      CUDF_FAIL(
+        "Unsupported range type. "
+        "Only Durations, fixed-point, floating point, and non-boolean integral range types are "
+        "allowed.");
+    }
   }
 };
 
diff --git a/cpp/tests/rolling/grouped_rolling_range_test.cpp b/cpp/tests/rolling/grouped_rolling_range_test.cpp
index 2d4c621ccb9..bebb271c08e 100644
--- a/cpp/tests/rolling/grouped_rolling_range_test.cpp
+++ b/cpp/tests/rolling/grouped_rolling_range_test.cpp
@@ -48,7 +48,6 @@ using column_ptr      = std::unique_ptr<cudf::column>;
 
 template <typename T>
 struct BaseGroupedRollingRangeOrderByTest : cudf::test::BaseFixture {
-
   // Stand-in for std::pow(10, n), but for integral return.
   static constexpr std::array<int32_t, 6> pow10{1, 10, 100, 1000, 10000, 100000};
 
@@ -60,30 +59,32 @@ struct BaseGroupedRollingRangeOrderByTest : cudf::test::BaseFixture {
   /**
    * @brief Get grouped rolling results for specified order-by column and range bounds.
    */
-  [[nodiscard]]
-  column_ptr get_grouped_range_rolling_result(cudf::range_window_bounds const& preceding,
-                                              cudf::range_window_bounds const& following,
-                                              cudf::column_view const& order_by_column,
-                                              cudf::rolling_aggregation const& agg) const
+  [[nodiscard]] column_ptr get_grouped_range_rolling_result(
+    cudf::range_window_bounds const& preceding,
+    cudf::range_window_bounds const& following,
+    cudf::column_view const& order_by_column,
+    cudf::rolling_aggregation const& agg) const
   {
-    return cudf::grouped_range_rolling_window(
-        cudf::table_view{{grouping_keys->view()}},
-        order_by_column,
-        cudf::order::ASCENDING,
-        agg_values->view(),
-        preceding,
-        following,
-        1,  // min_periods
-        agg);
+    return cudf::grouped_range_rolling_window(cudf::table_view{{grouping_keys->view()}},
+                                              order_by_column,
+                                              cudf::order::ASCENDING,
+                                              agg_values->view(),
+                                              preceding,
+                                              following,
+                                              1,  // min_periods
+                                              agg);
   }
 
-  [[nodiscard]]
-  column_ptr get_grouped_range_rolling_sum_result(cudf::range_window_bounds const& preceding,
-                                                  cudf::range_window_bounds const& following,
-                                                  cudf::column_view const& order_by_column) const
+  [[nodiscard]] column_ptr get_grouped_range_rolling_sum_result(
+    cudf::range_window_bounds const& preceding,
+    cudf::range_window_bounds const& following,
+    cudf::column_view const& order_by_column) const
   {
-    return get_grouped_range_rolling_result(preceding, following, order_by_column,
-                                            *cudf::make_sum_aggregation<cudf::rolling_aggregation>());
+    return get_grouped_range_rolling_result(
+      preceding,
+      following,
+      order_by_column,
+      *cudf::make_sum_aggregation<cudf::rolling_aggregation>());
   }
 };
 
@@ -92,26 +93,25 @@ struct GroupedRollingRangeOrderByNumericTest : public BaseGroupedRollingRangeOrd
   using base = BaseGroupedRollingRangeOrderByTest<T>;
 
   using base::agg_values;
+  using base::get_grouped_range_rolling_sum_result;
   using base::grouping_keys;
   using base::num_rows;
-  using base::get_grouped_range_rolling_sum_result;
 
   [[nodiscard]] auto make_range_bounds(T const& value) const
   {
-      return cudf::range_window_bounds::get(*cudf::make_fixed_width_scalar(value));
+    return cudf::range_window_bounds::get(*cudf::make_fixed_width_scalar(value));
   }
 
   [[nodiscard]] auto make_unbounded_range_bounds() const
   {
-      return cudf::range_window_bounds::unbounded(cudf::data_type{cudf::type_to_id<T>()});
+    return cudf::range_window_bounds::unbounded(cudf::data_type{cudf::type_to_id<T>()});
   }
 
   /// Generate order-by column with values: [0, 100,   200,   300,   ... 1100,   1200,   1300]
   [[nodiscard]] column_ptr generate_order_by_column() const
   {
     auto const begin = thrust::make_transform_iterator(
-        thrust::make_counting_iterator<cudf::size_type>(0),
-        [&](T const& i) -> T { return i * 100; });
+      thrust::make_counting_iterator<cudf::size_type>(0), [&](T const& i) -> T { return i * 100; });
 
     return fwcw<T>(begin, begin + num_rows).release();
   }
@@ -123,11 +123,10 @@ struct GroupedRollingRangeOrderByNumericTest : public BaseGroupedRollingRangeOrd
   {
     auto const preceding = make_range_bounds(T{200});
     auto const following = make_range_bounds(T{100});
-    auto const order_by = generate_order_by_column();
-    auto const results =
-      get_grouped_range_rolling_sum_result(preceding, following, *order_by);
+    auto const order_by  = generate_order_by_column();
+    auto const results   = get_grouped_range_rolling_sum_result(preceding, following, *order_by);
     auto const expected_results = bigints_column{{2, 3, 4, 4, 4, 3, 4, 6, 8, 6, 6, 9, 12, 9},
-                                                   cudf::test::iterators::no_nulls()};
+                                                 cudf::test::iterators::no_nulls()};
     CUDF_TEST_EXPECT_COLUMNS_EQUAL(*results, expected_results);
   }
 
@@ -162,9 +161,9 @@ struct GroupedRollingRangeOrderByNumericTest : public BaseGroupedRollingRangeOrd
     }();
 
     auto const results =
-        get_grouped_range_rolling_sum_result(preceding, following, *nulled_order_by);
-    auto const expected_results = bigints_column{{2, 2, 2, 3, 4, 3, 4, 4, 4, 4, 6, 6, 6, 6},
-                                               cudf::test::iterators::no_nulls()};
+      get_grouped_range_rolling_sum_result(preceding, following, *nulled_order_by);
+    auto const expected_results =
+      bigints_column{{2, 2, 2, 3, 4, 3, 4, 4, 4, 4, 6, 6, 6, 6}, cudf::test::iterators::no_nulls()};
     CUDF_TEST_EXPECT_COLUMNS_EQUAL(*results, expected_results);
   }
 
@@ -176,7 +175,7 @@ struct GroupedRollingRangeOrderByNumericTest : public BaseGroupedRollingRangeOrd
     auto const order_by  = generate_order_by_column();
     auto const preceding = make_unbounded_range_bounds();
     auto const following = make_unbounded_range_bounds();
-    auto const results = get_grouped_range_rolling_sum_result(preceding, following, *order_by);
+    auto const results   = get_grouped_range_rolling_sum_result(preceding, following, *order_by);
 
     auto const expected_results = bigints_column{{6, 6, 6, 6, 6, 6, 8, 8, 8, 8, 12, 12, 12, 12},
                                                  cudf::test::iterators::no_nulls()};
@@ -186,11 +185,13 @@ struct GroupedRollingRangeOrderByNumericTest : public BaseGroupedRollingRangeOrd
   /**
    * @brief Run grouped_rolling test with unbounded preceding and current row.
    */
-  void run_test_unbounded_preceding_to_current_row() {
-    auto const order_by = generate_order_by_column();
+  void run_test_unbounded_preceding_to_current_row()
+  {
+    auto const order_by            = generate_order_by_column();
     auto const unbounded_preceding = make_unbounded_range_bounds();
-    auto const current_row = make_range_bounds(T{0});
-    auto const results = get_grouped_range_rolling_sum_result(unbounded_preceding, current_row, *order_by);
+    auto const current_row         = make_range_bounds(T{0});
+    auto const results =
+      get_grouped_range_rolling_sum_result(unbounded_preceding, current_row, *order_by);
 
     auto const expected_results = bigints_column{{1, 2, 3, 4, 5, 6, 2, 4, 6, 8, 3, 6, 9, 12},
                                                  cudf::test::iterators::no_nulls()};
@@ -200,12 +201,14 @@ struct GroupedRollingRangeOrderByNumericTest : public BaseGroupedRollingRangeOrd
   /**
    * @brief Run grouped_rolling test with current row and unbounded following.
    */
-  void run_test_current_row_to_unbounded_following() {
-    auto const order_by = generate_order_by_column();
+  void run_test_current_row_to_unbounded_following()
+  {
+    auto const order_by            = generate_order_by_column();
     auto const unbounded_following = make_unbounded_range_bounds();
 
     auto const current_row = make_range_bounds(T{0});
-    auto const results = get_grouped_range_rolling_sum_result(current_row, unbounded_following, *order_by);
+    auto const results =
+      get_grouped_range_rolling_sum_result(current_row, unbounded_following, *order_by);
 
     auto const expected_results = bigints_column{{6, 5, 4, 3, 2, 1, 8, 6, 4, 2, 12, 9, 6, 3},
                                                  cudf::test::iterators::no_nulls()};
@@ -229,8 +232,9 @@ TYPED_TEST(GroupedRollingRangeOrderByNumericTest, UnboundedRanges)
 }
 
 template <typename DecimalT>
-struct GroupedRollingRangeOrderByDecimalTypedTest : BaseGroupedRollingRangeOrderByTest<typename DecimalT::rep> {
-  using Rep = typename DecimalT::rep;
+struct GroupedRollingRangeOrderByDecimalTypedTest
+  : BaseGroupedRollingRangeOrderByTest<typename DecimalT::rep> {
+  using Rep  = typename DecimalT::rep;
   using base = BaseGroupedRollingRangeOrderByTest<Rep>;
 
   using base::agg_values;
@@ -284,9 +288,8 @@ struct GroupedRollingRangeOrderByDecimalTypedTest : BaseGroupedRollingRangeOrder
    * @brief Get grouped rolling results for specified order-by column and range scale
    *
    */
-  [[nodiscard]]
-  column_ptr get_grouped_range_rolling_result(cudf::column_view const& order_by_column,
-                                              numeric::scale_type const& range_scale) const
+  [[nodiscard]] column_ptr get_grouped_range_rolling_result(
+    cudf::column_view const& order_by_column, numeric::scale_type const& range_scale) const
   {
     auto const preceding =
       this->make_fixed_point_range_bounds(rescale_range_value(Rep{200}, range_scale), range_scale);

From b567ad4ebaeaa5c33f6923911b169646287cd846 Mon Sep 17 00:00:00 2001
From: MithunR <mythrocks@gmail.com>
Date: Mon, 5 Jun 2023 10:40:17 -0700
Subject: [PATCH 05/14] Better name for the floating point tests.

---
 cpp/tests/rolling/grouped_rolling_range_test.cpp | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/cpp/tests/rolling/grouped_rolling_range_test.cpp b/cpp/tests/rolling/grouped_rolling_range_test.cpp
index bebb271c08e..6a8eea8755e 100644
--- a/cpp/tests/rolling/grouped_rolling_range_test.cpp
+++ b/cpp/tests/rolling/grouped_rolling_range_test.cpp
@@ -216,15 +216,19 @@ struct GroupedRollingRangeOrderByNumericTest : public BaseGroupedRollingRangeOrd
   }
 };
 
-TYPED_TEST_SUITE(GroupedRollingRangeOrderByNumericTest, cudf::test::FloatingPointTypes);
+template <typename FloatingPointType>
+struct GroupedRollingRangeOrderByFloatingPointTest
+  : GroupedRollingRangeOrderByNumericTest<FloatingPointType> {};
 
-TYPED_TEST(GroupedRollingRangeOrderByNumericTest, BoundedRanges)
+TYPED_TEST_SUITE(GroupedRollingRangeOrderByFloatingPointTest, cudf::test::FloatingPointTypes);
+
+TYPED_TEST(GroupedRollingRangeOrderByFloatingPointTest, BoundedRanges)
 {
   this->run_test_no_null_oby();
   this->run_test_nulls_in_oby();
 }
 
-TYPED_TEST(GroupedRollingRangeOrderByNumericTest, UnboundedRanges)
+TYPED_TEST(GroupedRollingRangeOrderByFloatingPointTest, UnboundedRanges)
 {
   this->run_test_unbounded_preceding_to_unbounded_following();
   this->run_test_unbounded_preceding_to_current_row();

From d6eb43495c048b16116a3106a7ee0bbb6b9ecd4e Mon Sep 17 00:00:00 2001
From: MithunR <mythrocks@gmail.com>
Date: Mon, 5 Jun 2023 11:02:57 -0700
Subject: [PATCH 06/14] Switched from enable_if to constexpr.

---
 cpp/src/rolling/range_window_bounds.cpp | 32 -------------------------
 1 file changed, 32 deletions(-)

diff --git a/cpp/src/rolling/range_window_bounds.cpp b/cpp/src/rolling/range_window_bounds.cpp
index 4138bd2bd3c..8d577ab28e5 100644
--- a/cpp/src/rolling/range_window_bounds.cpp
+++ b/cpp/src/rolling/range_window_bounds.cpp
@@ -30,37 +30,6 @@ namespace {
  * This makes it possible to copy construct and copy assign `range_window_bounds` objects.
  */
 struct range_scalar_constructor {
-  /*
-  template <typename T, CUDF_ENABLE_IF(not detail::is_supported_range_type<T>())>
-  std::unique_ptr<scalar> operator()(scalar const& range_scalar_) const
-  {
-    CUDF_FAIL(
-      "Unsupported range type. "
-      "Only Durations, fixed-point, and non-boolean integral range types are allowed.");
-  }
-
-  template <typename T, CUDF_ENABLE_IF(cudf::is_duration<T>())>
-  std::unique_ptr<scalar> operator()(scalar const& range_scalar_) const
-  {
-    return std::make_unique<duration_scalar<T>>(
-      static_cast<duration_scalar<T> const&>(range_scalar_));
-  }
-
-  template <typename T, CUDF_ENABLE_IF(std::is_integral_v<T> && not cudf::is_boolean<T>())>
-  std::unique_ptr<scalar> operator()(scalar const& range_scalar_) const
-  {
-    return std::make_unique<numeric_scalar<T>>(
-      static_cast<numeric_scalar<T> const&>(range_scalar_));
-  }
-
-  template <typename T, CUDF_ENABLE_IF(cudf::is_fixed_point<T>())>
-  std::unique_ptr<scalar> operator()(scalar const& range_scalar_) const
-  {
-    return std::make_unique<fixed_point_scalar<T>>(
-      static_cast<fixed_point_scalar<T> const&>(range_scalar_));
-  }
-  */
-
   template <typename T>
   std::unique_ptr<scalar> operator()(scalar const& range_scalar_) const
   {
@@ -83,7 +52,6 @@ struct range_scalar_constructor {
     }
   }
 };
-
 }  // namespace
 
 range_window_bounds::range_window_bounds(extent_type extent_, std::unique_ptr<scalar> range_scalar_)

From 50149298965d2f9382e304cc36ffb8afc0ea462c Mon Sep 17 00:00:00 2001
From: MithunR <mythrocks@gmail.com>
Date: Mon, 5 Jun 2023 11:30:55 -0700
Subject: [PATCH 07/14] Minor code simplication.

---
 cpp/src/rolling/range_window_bounds.cpp | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/cpp/src/rolling/range_window_bounds.cpp b/cpp/src/rolling/range_window_bounds.cpp
index 8d577ab28e5..febe3f915f3 100644
--- a/cpp/src/rolling/range_window_bounds.cpp
+++ b/cpp/src/rolling/range_window_bounds.cpp
@@ -65,19 +65,18 @@ range_window_bounds::range_window_bounds(extent_type extent_, std::unique_ptr<sc
 
 range_window_bounds range_window_bounds::unbounded(data_type type)
 {
-  return range_window_bounds(extent_type::UNBOUNDED, make_default_constructed_scalar(type));
+  return {extent_type::UNBOUNDED, make_default_constructed_scalar(type)};
 }
 
 range_window_bounds range_window_bounds::current_row(data_type type)
 {
-  return range_window_bounds(extent_type::CURRENT_ROW, make_default_constructed_scalar(type));
+  return {extent_type::CURRENT_ROW, make_default_constructed_scalar(type)};
 }
 
 range_window_bounds range_window_bounds::get(scalar const& boundary)
 {
-  return range_window_bounds{
-    extent_type::BOUNDED,
-    cudf::type_dispatcher(boundary.type(), range_scalar_constructor{}, boundary)};
+  return {extent_type::BOUNDED,
+          cudf::type_dispatcher(boundary.type(), range_scalar_constructor{}, boundary)};
 }
 
 }  // namespace cudf

From 190edd5b178870df0a9b65de372f4750bbc71cea Mon Sep 17 00:00:00 2001
From: MithunR <mythrocks@gmail.com>
Date: Thu, 8 Jun 2023 15:25:19 -0700
Subject: [PATCH 08/14] Switch back from `if constexpr` to `enable_if`.

---
 cpp/src/rolling/range_window_bounds.cpp | 52 ++++++++++++++++---------
 1 file changed, 34 insertions(+), 18 deletions(-)

diff --git a/cpp/src/rolling/range_window_bounds.cpp b/cpp/src/rolling/range_window_bounds.cpp
index febe3f915f3..1e0d711147c 100644
--- a/cpp/src/rolling/range_window_bounds.cpp
+++ b/cpp/src/rolling/range_window_bounds.cpp
@@ -30,26 +30,42 @@ namespace {
  * This makes it possible to copy construct and copy assign `range_window_bounds` objects.
  */
 struct range_scalar_constructor {
-  template <typename T>
+  template <typename T, CUDF_ENABLE_IF(not detail::is_supported_range_type<T>())>
   std::unique_ptr<scalar> operator()(scalar const& range_scalar_) const
   {
-    if constexpr (std::is_floating_point_v<T> ||
-                  (std::is_integral_v<T> && not cudf::is_boolean<T>())) {
-      return std::make_unique<numeric_scalar<T>>(
-        static_cast<numeric_scalar<T> const&>(range_scalar_));
-    } else if constexpr (cudf::is_fixed_point<T>()) {
-      return std::make_unique<fixed_point_scalar<T>>(
-        static_cast<fixed_point_scalar<T> const&>(range_scalar_));
-    } else if constexpr (cudf::is_duration<T>()) {
-      return std::make_unique<duration_scalar<T>>(
-        static_cast<duration_scalar<T> const&>(range_scalar_));
-    } else {
-      assert(not detail::is_supported_range_type<T>());
-      CUDF_FAIL(
-        "Unsupported range type. "
-        "Only Durations, fixed-point, floating point, and non-boolean integral range types are "
-        "allowed.");
-    }
+    CUDF_FAIL(
+      "Unsupported range type. "
+      "Only Durations, fixed-point, and non-boolean integral range types are allowed.");
+  }
+
+  template <typename T, CUDF_ENABLE_IF(cudf::is_duration<T>())>
+  std::unique_ptr<scalar> operator()(scalar const& range_scalar_) const
+  {
+    return std::make_unique<duration_scalar<T>>(
+      static_cast<duration_scalar<T> const&>(range_scalar_));
+  }
+
+  template <typename T,
+            CUDF_ENABLE_IF(std::is_floating_point_v<T> ||
+                           (std::is_integral_v<T> && not cudf::is_boolean<T>()))>
+  std::unique_ptr<scalar> operator()(scalar const& range_scalar_) const
+  {
+    return std::make_unique<numeric_scalar<T>>(
+      static_cast<numeric_scalar<T> const&>(range_scalar_));
+  }
+
+  template <typename T, CUDF_ENABLE_IF(cudf::is_fixed_point<T>())>
+  std::unique_ptr<scalar> operator()(scalar const& range_scalar_) const
+  {
+    return std::make_unique<fixed_point_scalar<T>>(
+      static_cast<fixed_point_scalar<T> const&>(range_scalar_));
+  }
+
+  template <typename T, CUDF_ENABLE_IF(std::is_floating_point_v<T>())>
+  std::unique_ptr<scalar> operator()(scalar const& range_scalar_) const
+  {
+    return std::make_unique<fixed_point_scalar<T>>(
+      static_cast<fixed_point_scalar<T> const&>(range_scalar_));
   }
 };
 }  // namespace

From c340309a2d5aa144b71bf11f76a60b115c8459ce Mon Sep 17 00:00:00 2001
From: MithunR <mythrocks@gmail.com>
Date: Thu, 8 Jun 2023 15:32:12 -0700
Subject: [PATCH 09/14] Fixed CUDF_FAIL message.

---
 cpp/src/rolling/range_window_bounds.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/cpp/src/rolling/range_window_bounds.cpp b/cpp/src/rolling/range_window_bounds.cpp
index 1e0d711147c..f7049f5fd44 100644
--- a/cpp/src/rolling/range_window_bounds.cpp
+++ b/cpp/src/rolling/range_window_bounds.cpp
@@ -35,7 +35,8 @@ struct range_scalar_constructor {
   {
     CUDF_FAIL(
       "Unsupported range type. "
-      "Only Durations, fixed-point, and non-boolean integral range types are allowed.");
+      "Only Durations, fixed-point, floating point, and non-boolean integral range types are "
+      "allowed.");
   }
 
   template <typename T, CUDF_ENABLE_IF(cudf::is_duration<T>())>

From 48135ecc84113154a6bc9e5fdbb80e024aaafc1b Mon Sep 17 00:00:00 2001
From: MithunR <mythrocks@gmail.com>
Date: Fri, 9 Jun 2023 11:58:15 -0700
Subject: [PATCH 10/14] Remove invalid enable-if for float.

---
 cpp/src/rolling/range_window_bounds.cpp | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/cpp/src/rolling/range_window_bounds.cpp b/cpp/src/rolling/range_window_bounds.cpp
index f7049f5fd44..af905646219 100644
--- a/cpp/src/rolling/range_window_bounds.cpp
+++ b/cpp/src/rolling/range_window_bounds.cpp
@@ -61,13 +61,6 @@ struct range_scalar_constructor {
     return std::make_unique<fixed_point_scalar<T>>(
       static_cast<fixed_point_scalar<T> const&>(range_scalar_));
   }
-
-  template <typename T, CUDF_ENABLE_IF(std::is_floating_point_v<T>())>
-  std::unique_ptr<scalar> operator()(scalar const& range_scalar_) const
-  {
-    return std::make_unique<fixed_point_scalar<T>>(
-      static_cast<fixed_point_scalar<T> const&>(range_scalar_));
-  }
 };
 }  // namespace
 

From 554e3ef2e2b172c518eea4b1fb433def5ed8c105 Mon Sep 17 00:00:00 2001
From: MithunR <mythrocks@gmail.com>
Date: Wed, 14 Jun 2023 11:06:02 -0700
Subject: [PATCH 11/14] Update cpp/src/rolling/range_window_bounds.cpp

Co-authored-by: David Wendt <45795991+davidwendt@users.noreply.github.com>
---
 cpp/src/rolling/range_window_bounds.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/cpp/src/rolling/range_window_bounds.cpp b/cpp/src/rolling/range_window_bounds.cpp
index af905646219..adb429c67eb 100644
--- a/cpp/src/rolling/range_window_bounds.cpp
+++ b/cpp/src/rolling/range_window_bounds.cpp
@@ -47,8 +47,7 @@ struct range_scalar_constructor {
   }
 
   template <typename T,
-            CUDF_ENABLE_IF(std::is_floating_point_v<T> ||
-                           (std::is_integral_v<T> && not cudf::is_boolean<T>()))>
+            CUDF_ENABLE_IF(cudf::is_numeric<T> && not cudf::is_boolean<T>())>
   std::unique_ptr<scalar> operator()(scalar const& range_scalar_) const
   {
     return std::make_unique<numeric_scalar<T>>(

From 7ded2a796a830292b8806e270b4578f018c14332 Mon Sep 17 00:00:00 2001
From: MithunR <mythrocks@gmail.com>
Date: Tue, 20 Jun 2023 10:01:54 -0700
Subject: [PATCH 12/14] Fix range-bounds overloads.

---
 cpp/src/rolling/range_window_bounds.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/cpp/src/rolling/range_window_bounds.cpp b/cpp/src/rolling/range_window_bounds.cpp
index adb429c67eb..dd179d4ee76 100644
--- a/cpp/src/rolling/range_window_bounds.cpp
+++ b/cpp/src/rolling/range_window_bounds.cpp
@@ -46,8 +46,7 @@ struct range_scalar_constructor {
       static_cast<duration_scalar<T> const&>(range_scalar_));
   }
 
-  template <typename T,
-            CUDF_ENABLE_IF(cudf::is_numeric<T> && not cudf::is_boolean<T>())>
+  template <typename T, CUDF_ENABLE_IF(cudf::is_numeric<T>() && not cudf::is_boolean<T>())>
   std::unique_ptr<scalar> operator()(scalar const& range_scalar_) const
   {
     return std::make_unique<numeric_scalar<T>>(

From 9599cb37ec095514c09cf46e1a2971133b866fab Mon Sep 17 00:00:00 2001
From: MithunR <mythrocks@gmail.com>
Date: Tue, 20 Jun 2023 10:57:59 -0700
Subject: [PATCH 13/14] Consolidated to cudf::is_numeric.

---
 cpp/src/rolling/detail/range_window_bounds.hpp | 10 ++++------
 cpp/src/rolling/range_window_bounds.cpp        |  3 +--
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/cpp/src/rolling/detail/range_window_bounds.hpp b/cpp/src/rolling/detail/range_window_bounds.hpp
index 40da8f6ab18..669b6a88208 100644
--- a/cpp/src/rolling/detail/range_window_bounds.hpp
+++ b/cpp/src/rolling/detail/range_window_bounds.hpp
@@ -29,8 +29,7 @@ template <typename RangeType>
 constexpr bool is_supported_range_type()
 {
   return cudf::is_duration<RangeType>() || cudf::is_fixed_point<RangeType>() ||
-         cudf::is_floating_point<RangeType>() ||
-         (std::is_integral_v<RangeType> && !cudf::is_boolean<RangeType>());
+         (cudf::is_numeric<RangeType>() && !cudf::is_boolean<RangeType>());
 }
 
 /// Checks if the specified type is a supported target type,
@@ -39,8 +38,8 @@ template <typename ColumnType>
 constexpr bool is_supported_order_by_column_type()
 {
   return cudf::is_timestamp<ColumnType>() || cudf::is_fixed_point<ColumnType>() ||
-         (std::is_integral_v<ColumnType> && !cudf::is_boolean<ColumnType>()) ||
-         cudf::is_floating_point<ColumnType>() || std::is_same_v<ColumnType, cudf::string_view>;
+         (cudf::is_numeric<ColumnType>() && !cudf::is_boolean<ColumnType>()) ||
+         std::is_same_v<ColumnType, cudf::string_view>;
 }
 
 /// Range-comparable representation type for an orderby column type.
@@ -101,8 +100,7 @@ void assert_non_negative([[maybe_unused]] T const& value)
 
 template <typename RangeT,
           typename RepT,
-          CUDF_ENABLE_IF(std::is_floating_point_v<RangeT> ||
-                         (std::is_integral_v<RangeT> && !cudf::is_boolean<RangeT>()))>
+          CUDF_ENABLE_IF(cudf::is_numeric<RangeT>() && !cudf::is_boolean<RangeT>())>
 RepT range_comparable_value_impl(scalar const& range_scalar,
                                  bool,
                                  data_type const&,
diff --git a/cpp/src/rolling/range_window_bounds.cpp b/cpp/src/rolling/range_window_bounds.cpp
index dd179d4ee76..a136f152d25 100644
--- a/cpp/src/rolling/range_window_bounds.cpp
+++ b/cpp/src/rolling/range_window_bounds.cpp
@@ -35,8 +35,7 @@ struct range_scalar_constructor {
   {
     CUDF_FAIL(
       "Unsupported range type. "
-      "Only Durations, fixed-point, floating point, and non-boolean integral range types are "
-      "allowed.");
+      "Only durations, fixed-point, and non-boolean numeric range types are allowed.");
   }
 
   template <typename T, CUDF_ENABLE_IF(cudf::is_duration<T>())>

From d93735f36923e248d678fa063293c61ea68e924a Mon Sep 17 00:00:00 2001
From: MithunR <mythrocks@gmail.com>
Date: Tue, 20 Jun 2023 11:59:11 -0700
Subject: [PATCH 14/14] Fixed straggler for is_numeric() check.

---
 cpp/src/rolling/detail/range_window_bounds.hpp | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/cpp/src/rolling/detail/range_window_bounds.hpp b/cpp/src/rolling/detail/range_window_bounds.hpp
index 669b6a88208..8a53e937f98 100644
--- a/cpp/src/rolling/detail/range_window_bounds.hpp
+++ b/cpp/src/rolling/detail/range_window_bounds.hpp
@@ -64,9 +64,7 @@ struct range_type_impl {
 template <typename ColumnType>
 struct range_type_impl<
   ColumnType,
-  std::enable_if_t<std::is_floating_point_v<ColumnType> ||
-                     (std::is_integral_v<ColumnType> && !cudf::is_boolean<ColumnType>()),
-                   void>> {
+  std::enable_if_t<cudf::is_numeric<ColumnType>() && !cudf::is_boolean<ColumnType>(), void>> {
   using type     = ColumnType;
   using rep_type = ColumnType;
 };