NVIDIA / NVIDIA/cudf

Investigate if empty cluster handling could be simplified in tdigest aggregating

Open
#16,901 0 comments 0 reactions 0 assignees View on GitHub
improvement libcudf
Dominant language
C++
Stars
9.8k
Forks
1.1k
Avg merge
3d 6m
Merged PRs (30d)
278

Description

There are two APIs in libcudf for tdigest groupby aggregation, `cudf::tdigest::detail::group_tdigest()` and `cudf::tdigest::detail::group_merge_tdigest()`. The former takes the input as numeric values, and the latter takes tdigest columns. The numeric value column can contain nulls as it is a regular column. However, the tdigest column cannot contain nulls. Instead, it can contain an empty cluster for a group if all input values in the group to compute a tdigest were null.

To handle nulls, we are currently using a workaround based on explicit stubs. When all values are null in a group, we put a stub as a placeholder for an empty cluster to be created later. After the core computation is done, these stubs are removed before the result is returned. This workaround not only is certainly adding complexity to the implementation, but also might be adding some unnecessary overhead to handle empty clusters during the computation.

We should investigate if this workaround is absolutely necessary, and remove it if possible.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.