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

Converts cudf over to supporting thrust existing in a nested namespace #11440

Closed

Conversation

robertmaynard
Copy link
Contributor

@robertmaynard robertmaynard commented Aug 2, 2022

Description

This is one way that cudf can be updated to support a nested version of thrust.
This version requires rapidsai/rmm#1077

Tasks

  • namespaced thrust calls
  • namespace cub calls
  • remove thrust types from public API
  • remove cub types from public API

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@github-actions github-actions bot added CMake CMake build issue conda Java Affects Java cuDF API. Python Affects Python cuDF API. libcudf Affects libcudf (C++/CUDA) code. labels Aug 2, 2022
@robertmaynard robertmaynard changed the base branch from branch-22.08 to branch-22.10 August 2, 2022 19:29
@github-actions github-actions bot removed Python Affects Python cuDF API. Java Affects Java cuDF API. labels Aug 2, 2022
@robertmaynard robertmaynard added feature request New feature or request breaking Breaking change and removed gpuCI labels Aug 2, 2022
@@ -585,6 +585,7 @@ target_include_directories(
target_compile_definitions(
cudf PUBLIC "$<$<COMPILE_LANGUAGE:CXX>:${CUDF_CXX_DEFINITIONS}>"
"$<BUILD_INTERFACE:$<$<COMPILE_LANGUAGE:CUDA>:${CUDF_CUDA_DEFINITIONS}>>"
PRIVATE "THRUST_WRAPPED_NAMESPACE=rapids"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This currently is PRIVATE on purpose since making it public would break consumers that use Thrust and don't call cudf API that have thrust types.

@github-actions
Copy link

github-actions bot commented Sep 2, 2022

This PR has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this PR if it is no longer required. Otherwise, please respond with a comment indicating any updates. This PR will be labeled inactive-90d if there is no activity in the next 60 days.

@robertmaynard
Copy link
Contributor Author

Closing this as we are going to use a different approach

@robertmaynard robertmaynard deleted the thrust_namespace branch September 8, 2022 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking change CMake CMake build issue feature request New feature or request libcudf Affects libcudf (C++/CUDA) code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant