Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Discussion] Provide public, templated APIs for libcudf features #6495

Open
jrhemstad opened this issue Oct 12, 2020 · 0 comments
Open

[Discussion] Provide public, templated APIs for libcudf features #6495

jrhemstad opened this issue Oct 12, 2020 · 0 comments
Labels
feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. proposal Change current process or code

Comments

@jrhemstad
Copy link
Contributor

jrhemstad commented Oct 12, 2020

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 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 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:

* A negative value `i` in the `gather_map` is interpreted as `i+n`, where
* `n` is the number of rows in the `source_table`.

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 #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_views 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 #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/tables.
  4. Nullable inputs

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.

@jrhemstad jrhemstad added feature request New feature or request proposal Change current process or code libcudf Affects libcudf (C++/CUDA) code. labels Oct 12, 2020
@jrhemstad jrhemstad changed the title [FEA] Provide public, templated APIs for libcudf features [Discussion] Provide public, templated APIs for libcudf features Oct 12, 2020
@rapidsai rapidsai deleted a comment from github-actions bot Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. proposal Change current process or code
Projects
None yet
Development

No branches or pull requests

2 participants