NVIDIA / NVIDIA/cudf

[FEA] Apply `sliced_child` when calling to `slice`

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

Description

I observe that there are a lot of bugs related to the situations when an API directly accesses the child columns of a sliced column instead of calling to `get_sliced_child`. As such, the `slice` API is a kind of shallow slice, not a deep slice. Maybe shallow slice is more efficient as it can avoid unnecessary slicing of the children columns when we don't care, it has caused a lot of (potential) bugs that cost a lot of developer time.

An instance of such bugs is here: https://github.com/rapidsai/cudf/pull/9218. In the past, I have also dealt with many similar situations but I could catch them immediately through unit tests. If a developer forgets to write unit tests for sliced input, the bug may be there.

I would like to rewrite `slice` into deep slicing, i.e., recursively calling to `slice` on all children columns of the column being sliced. This way, when we access its children column through the APIs `child_begin()`, `child_end`, or `child(idx)` we will have the expected results all the time. Although we have talked about this before and didn't do anything as deep slicing is expensive, I still decided to raise the issue again as it still causes bugs.

An alternative solution to this issue is to rename the existing `slice` API into `shallow_slice` then add another `slice` version that does recursively calling `shallow_slice` on the columns. So, a developer will only call `shallow_slice` if he/she knows exactly that just the shallow version is needed in the context. Otherwise, a more expensive `slice` version will produce the correct results in most situations.

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.