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

Implement HOST_UDF aggregation for reduction and groupby #17249

Draft
wants to merge 68 commits into
base: branch-25.02
Choose a base branch
from

Conversation

ttnghia
Copy link
Contributor

@ttnghia ttnghia commented Nov 5, 2024

This implements HOST_UDF aggregation, allowing to execute a host-side user-defined function (UDF) through libcudf aggregation framework.

  • A host-side function can be an arbitrarily independent function running on the host machine. It may or may not call other device kernels depending on its implementation.
  • Such user-defined function needs to follow the libcudf provided interface (cudf::host_udf_base). The interface provides the ability to fully interact with libcudf aggregation framework.
  • Since it is implemented on the user application side, it has a very high degree of freedom to perform arbitrary operations to satisfy the user's need.

Closes #16633.


Usage

  1. Define a functor deriving from cudf::host_udf_base and implement the required virtual functions declared in that base struct. For example:
struct my_aggregation : cudf::host_udf_base {
   ...
};
  1. Create an instance of libcudf HOST_UDF aggregation which is constructed from an instance of the functor defined above. For example:
auto agg = cudf::make_host_udf_aggregation<cudf::groupby_aggregation>(
    std::make_unique<my_aggregation>());
  1. Perform aggregation operation on the created instance.

Signed-off-by: Nghia Truong <[email protected]>
@ttnghia ttnghia self-assigned this Nov 5, 2024
@github-actions github-actions bot added libcudf Affects libcudf (C++/CUDA) code. CMake CMake build issue labels Nov 5, 2024
@ttnghia ttnghia added feature request New feature or request 2 - In Progress Currently a work in progress Spark Functionality that helps Spark RAPIDS and removed CMake CMake build issue labels Nov 5, 2024
@github-actions github-actions bot added the CMake CMake build issue label Nov 5, 2024
@github-actions github-actions bot added Python Affects Python cuDF API. Java Affects Java cuDF API. cudf.pandas Issues specific to cudf.pandas cudf.polars Issues specific to cudf.polars pylibcudf Issues specific to the pylibcudf package labels Nov 19, 2024
@ttnghia ttnghia changed the base branch from branch-24.12 to branch-25.02 November 19, 2024 23:56
@ttnghia ttnghia removed Python Affects Python cuDF API. CMake CMake build issue Java Affects Java cuDF API. cudf.pandas Issues specific to cudf.pandas cudf.polars Issues specific to cudf.polars pylibcudf Issues specific to the pylibcudf package labels Nov 19, 2024
Signed-off-by: Nghia Truong <[email protected]>
Signed-off-by: Nghia Truong <[email protected]>
@harrism harrism removed their request for review November 27, 2024 19:51
MERGE_TDIGEST(32), // This can take a delta argument for accuracy level
HISTOGRAM(33),
MERGE_HISTOGRAM(34);
HOST_UDF(26),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Append HOST_UDF to the tail? then do not touch other aggregation values.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately HOST_UDF is classified as "UDF" which should be grouped together with other UDF kinds. That's why I have to insert it in the middle.

Copy link
Member

@PointKernel PointKernel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The majority of the complexity in host_udf_base arises from its handling of both reduction and groupby operations. I mainly went through examples and tests and found this feature quite powerful overall. However, the design supporting both reduction and groupby could use some refinement. For now, I recommend focusing exclusively on groupby in the current work, which will help define the simplest possible interface for this new aggregation. Once that's established, we can extend the feature to include reduction. This approach will also help refine the scope of the PR, making it easier to review. @ttnghia What do you think?

cpp/include/cudf/aggregation.hpp Show resolved Hide resolved
Comment on lines +102 to +103
rmm::cuda_stream_view stream;
rmm::device_async_resource_ref mr;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A larger scope of changes is required since the given stream and mr need to be used for all the factories like make_lists_column, etc.

@ttnghia ttnghia marked this pull request as draft December 13, 2024 19:49
Copy link

copy-pr-bot bot commented Dec 13, 2024

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@ttnghia
Copy link
Contributor Author

ttnghia commented Dec 13, 2024

This will be closed and replaced by multiple smaller PRs for easier review:

@@ -122,44 +122,7 @@ ConfigureTest(TIMESTAMPS_TEST wrappers/timestamps_test.cu)
# * groupby tests ---------------------------------------------------------------------------------
ConfigureTest(
GROUPBY_TEST
groupby/argmin_tests.cpp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't intend for this change to be checked in, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh this was temporarily modified for local tests, and should be reverted now in the new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team CMake CMake build issue feature request New feature or request Java Affects Java cuDF API. libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change Spark Functionality that helps Spark RAPIDS
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

[FEA] Allow to run groupby/reduction with externally derived aggregations
6 participants