NVIDIA / NVIDIA/cudf

[Discussion] Provide public, templated APIs for libcudf features

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

Description

## Description

Today, libcudf does not provide function templates in it's public interface. This is because for most users, the input data type is runtime information. Therefore, libcudf APIs operate on type-erased `column_view` objects where the runtime type information is stored in the `data_type` member and internally we dispatch to the appropriately typed code path, e.g., via the `type_dispatcher`.

However, this design lacks the power and flexibility of iterator based interfaces. E.g., there is no way to fuse operations with something like a `thrust::transform_iterator` and instead requires materializing intermediate results. Internally, we side-step this issue with function templates in the `detail::` API that _do_ operate on iterators. We can do this because inside a type-dispatched code path we _know_ the type of the underlying data and can invoke the proper template instantiation (or use device-side dispatch in some cases like with the `indexalator`). This allows greater flexibility and reuse of functionality within the library.

For example, the public [`cudf::gather`](https://github.com/rapidsai/cudf/blob/421fddb8b52dff8c8152158ca5f379f0f7255b98/cpp/include/cudf/copying.hpp#L64-L68) API expects the `gather_map` to be specified as a `column_view`. The implementation of this public API uses the `indexalator` to convert the type-erased input `column_view` into an iterator and forwards to a [`detail::gather` API](https://github.com/rapidsai/cudf/blob/421fddb8b52dff8c8152158ca5f379f0f7255b98/cpp/include/cudf/detail/gather.cuh#L617-L624) that expects the `gather_map` to be an iterator.

It would be nice if we could offer external users some of this same power and flexibility, e.g., there may be cases where a user knows that a particular input will always be a certain type (or small set of types) and could invoke a function template with an iterator that fuses some operations to avoid intermediate materializations.

`gather` is a good example of this where this would be useful. Today, `gather` implements non-standard C++ behavior where a negative index wraps around from the end of the array: https://github.com/rapidsai/cudf/blob/421fddb8b52dff8c8152158ca5f379f0f7255b98/cpp/include/cudf/copying.hpp#L46-L47

This is accomplished internally via a `transform_iterator` over the elements of the `gather_map`. Instead, if there were a public iterator-based `gather` function, users could provide the `transform_iterator` themselves and the `gather` implementation wouldn't be required to do the negative value wrapping. This is desirable because there are situations where we don't want negative values to wrap around, see https://github.com/rapidsai/cudf/issues/6479.

## Summary

I would like to see libcudf essentially provide two complementary public APIs:

1. A function-template interface operating on iterators as much as possible
2. A non-template interface operating on `column_view`s that just calls the APIs in 1.

To a certain extent, libcudf is already designed this way. Many public APIs call `detail::` APIs that operate on iterators, e.g., `gather`, `apply_boolean_mask`, `copy_if`, etc. This is a convention typically followed in libcudf (though not often enough). We should document and standardize more formally on this pattern (see https://github.com/rapidsai/cudf/issues/6470).

## Additional Points of Consideration

1. The inability of Cython to define `__device__` functors/lambdas limits the ability of Cython/Python to take advantage of any function template API
- There may be a solution here long term if we extended Cython to allow `__device__` keyword and then compile the Cython lib with `nvcc`, but there's some amount of work here.

2. Testing
- If we bless function template APIs as public, do we need to test those directly in addition to the existing public APIs? That would dramatically increase the surface area of the library. (My personal opinion is that testing the non-template APIs would be sufficient)

3. Outputs
- Nullable outputs makes output iterators impossible to do efficiently. So outputs would still need to be `column/table`s.

4. Nullable inputs
- We should standardize nullable input iterators to return a `thrust::optional`, see https://github.com/rapidsai/cudf/issues/6470

## Additional Context

This is an idea I've had bouncing around for quite a while now. I wanted to capture all of my thoughts in a single issue for discussion. It's not something I believe we _must_ do, but we should at least explore it.

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.