[BUG] cleanup template code of cpp/src/rolling/rolling.cu

[karthikeyann]

clean up template code of https://github.com/rapidsai/cudf/blob/branch-0.15/cpp/src/rolling/rolling.cu

and respective unit test codes
https://github.com/rapidsai/cudf/blob/branch-0.15/cpp/tests/rolling/rolling_test.cpp
https://github.com/rapidsai/cudf/blob/branch-0.15/cpp/tests/grouped_rolling/grouped_rolling_test.cpp

• (cudf::is_numeric<ColumnType>() or is_comparable_countable_op) this will always be true inside the if (cudf::is_numeric<ColumnType>()) condition.
• Also is_comparable_countable_op is already covered by (op == aggregation::MIN) or (op == aggregation::MAX) or (op == aggregation::COUNT_VALID) or (op == aggregation::COUNT_ALL)
• retain the template type aggregation::Kind till last level template function
• is_mean can be eliminated in rolling_store_output_functor, The logic for this can be inferred from the other parameters. (check [REVIEW] disallow SUM and MEAN of timestamp types #5319)
• launch<T>, kernel_launcher<T> could share InputType tparam.
• CUDF_FAIL should come before output column creation in string specialization
• any more improvements, code review the whole file cpp/src/rolling/rolling.cu

[github-actions]

This issue has been labeled inactive-90d due to no recent activity in the past 90 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed.

[github-actions]

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

[GregoryKimball]

This looks almost complete to me, with the exception of some test cleanup left over from #8158.

• (cudf::is_numeric() or is_comparable_countable_op) this will always be true inside the if (cudf::is_numeric()) condition.

#8158 moved is_comparable_countable_op into rolling_test.hpp the test code is still not refactored.

• Also is_comparable_countable_op is already covered by (op == aggregation::MIN) or (op == aggregation::MAX) or (op == aggregation::COUNT_VALID) or (op == aggregation::COUNT_ALL)

#8158 moved is_comparable_countable_op into rolling_test.hpp the test code is still not refactored.

• retain the template type aggregation::Kind till last level template function

The aggregation::Kind template type is retained in DeviceRolling, DeviceRollingArgMinMaxBase, DeviceRollingArgMinMaxString and DeviceRollingArgMinMaxStruct. (https://github.com/rapidsai/cudf/blob/branch-22.08/cpp/src/rolling/rolling_detail.cuh)

• is_mean can be eliminated in rolling_store_output_functor, The logic for this can be inferred from the other parameters. (check [REVIEW] disallow SUM and MEAN of timestamp types [REVIEW] disallow SUM and MEAN of timestamp types #5319)

is_mean has been eliminated. Closed by #5319

• launch, kernel_launcher could share InputType tparam.

rolling_detail.cuh now uses a templated operator() and the kernels are no longer templated

• CUDF_FAIL should come before output column creation in string specialization

CUDF_EXPECTS now appears at the top of the rolling kernels.

• any more improvements, code review the whole file cpp/src/rolling/rolling.cu

rolling.cu has been re-reviewed in #6512, #6277, #8052, #8158, #8809