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

Simplify rmm::exec_policy and refactor Thrust support #647

Merged
merged 10 commits into from
Dec 9, 2020

Conversation

harrism
Copy link
Member

@harrism harrism commented Dec 4, 2020

Closes #620.

This PR improves rmm::exec_policy so you can now just pass rmm::exec_policy(stream) rather than rmm::exec_policy(stream)->on(stream). Also refactors the rmm::device_vector alias into its own header, and the new rmm::exec_policy into its own header. The old thrust_rmm_allocator.h still exists for backwards compatibility. We should probably deprecate it and the old exec_policy.

#620 describes the motivation for this PR very well.

@GPUtester
Copy link
Contributor

Please update the changelog in order to start CI tests.

View the gpuCI docs here.

@harrism harrism added 2 - In Progress Currently a work in progress breaking Breaking change improvement Improvement / enhancement to an existing function labels Dec 4, 2020
@harrism harrism changed the title Refactoring thrust support. Simplify rmm::exec_policy and refactor Thrust support Dec 4, 2020
@harrism harrism added this to PR-WIP in v0.18 Release via automation Dec 4, 2020
@harrism harrism removed the 2 - In Progress Currently a work in progress label Dec 4, 2020
@harrism harrism marked this pull request as ready for review December 4, 2020 04:15
@harrism harrism requested a review from a team as a code owner December 4, 2020 04:15
@harrism harrism requested review from rongou, codereport and jrhemstad and removed request for rongou December 4, 2020 04:15
@harrism harrism moved this from PR-WIP to PR-Needs review in v0.18 Release Dec 4, 2020
@harrism harrism added the 3 - Ready for review Ready for review by team label Dec 4, 2020
README.md Outdated Show resolved Hide resolved
@harrism
Copy link
Member Author

harrism commented Dec 8, 2020

We should not merge this until corresponding PRs for cuDF and cuSpatial are merged, and this is tested with cuML and cuGraph.

Update: determined that this PR is non-breaking for cuML and cuGraph, which have not been updated to use rmm::cuda_stream_view. So I'm relabeling this PR as non-breaking. It only breaks libcudf and cuSpatial because I previously updated those libraries to use rmm::cuda_stream_view.

@harrism harrism added non-breaking Non-breaking change and removed breaking Breaking change labels Dec 8, 2020
@harrism harrism self-assigned this Dec 8, 2020
v0.18 Release automation moved this from PR-Needs review to PR-Reviewer approved Dec 8, 2020
@harrism harrism added 6 - Okay to Auto-Merge and removed 3 - Ready for review Ready for review by team labels Dec 9, 2020
@rapids-bot rapids-bot bot merged commit ce0d6ec into rapidsai:branch-0.18 Dec 9, 2020
v0.18 Release automation moved this from PR-Reviewer approved to Done Dec 9, 2020
kkraus14 pushed a commit to rapidsai/cudf that referenced this pull request Dec 10, 2020
Updates libcudf to use the new, simplified `rmm::exec_policy` and include the new refactored headers `rmm/exec_policy.hpp` and `rmm/device_vector.hpp`

The new `exec_policy` can be passed directly to Thrust, no longer any need to call `rmm::exec_policy(stream)->on(stream)`.

Depends on rapidsai/rmm#647
rapids-bot bot pushed a commit to rapidsai/cuspatial that referenced this pull request Dec 10, 2020
Updates libcudf to use the new, simplified rmm::exec_policy and include the new refactored headers rmm/exec_policy.hpp and rmm/device_vector.hpp

The new exec_policy can be passed directly to Thrust, no longer any need to call rmm::exec_policy(stream)->on(stream).

Depends on rapidsai/rmm#647

Authors:
  - Mark Harris <mharris@nvidia.com>

Approvers:
  - Paul Taylor
  - Christopher Harris

URL: #331
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
No open projects
v0.18 Release
  
Done
Development

Successfully merging this pull request may close these issues.

[FEA] Simplify rmm::exec_policy
4 participants