[QST] Changing `cudf::rolling_window` API to have exclusive `preceding` parameter
- Dominant language
- C++
- Stars
- 9.8k
- Forks
- 1.1k
- Avg merge
- 3d 6m
- Merged PRs (30d)
- 278
Description
**Should we change `cudf::rolling_window` API so that the `preceding` parameter is exclusive?**
This is the current API for `cudf::rolling_window`:
```cpp
std::unique_ptr rolling_window(
column_view const& input,
size_type preceding_window,
size_type following_window,
size_type min_periods,
std::unique_ptr const& agg,
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource());
```
The question is whether `preceding` should be exclusive or inclusive. Current examples of behaviour:
```cpp
// auto col = {1, 2, 3, 4, 5};
cudf::rolling_window(col, 2, 1, 1, ???); // gets you [1, 2], [1, 2, 3], [2, 3, 4], [3, 4, 5], [4, 5]
cudf::rolling_window(col, 2, 1, 1, min); // 1, 1, 2, 3, 4
cudf::rolling_window(col, 2, 1, 1, max); // 2, 3, 4, 5, 5
cudf::rolling_window(col, 2, 1, 1, sum); // 3, 6, 9, 12, 9
// auto col = {1, 2, 3, 4, 5};
cudf::rolling_window(col, 1, 1, 1, ???); // gets you [1, 2], [2, 3], [3, 4], [4, 5], [5]
cudf::rolling_window(col, 1, 1, 1, min); // 1, 2, 3, 4, 5
cudf::rolling_window(col, 1, 1, 1, max); // 2, 3, 4, 5, 5
cudf::rolling_window(col, 1, 1, 1, sum); // 3, 5, 7, 9, 5
// auto col = {1, 2, 3, 4, 5};
cudf::rolling_window(col, 1, 0, 1, ???); // gets you [1], [2], [3], [4], [5]
cudf::rolling_window(col, 1, 0, 1, min); // 1, 2, 3, 4, 5
cudf::rolling_window(col, 1, 0, 1, max); // 1, 2, 3, 4, 5
cudf::rolling_window(col, 1, 0, 1, sum); // 1, 2, 3, 4, 5
```
Recommendation would be that all of the `preceding` parameters we be "reduce by 1" and the current index would be included by default. Therefore window length would always be `preceding` + `following` + 1. However, need to consider the API that takes a column of window sizes and also the future changes @mythrocks will make.
**Previous discussion:**
* https://github.com/rapidsai/cudf/pull/3305#discussion_r352659894
* https://github.com/rapidsai/cudf/pull/3305#discussion_r353878021
**Relevant SQL Links:**
* https://docs.microsoft.com/en-us/sql/t-sql/queries/select-over-clause-transact-sql?view=sql-server-ver15
* https://www.red-gate.com/simple-talk/sql/learn-sql-server/window-functions-in-sql-server-part-2-the-frame/
Contributor guide
Assessment
This issue has not been assessed yet.