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

Make thrust nosync execution policy the default thrust policy #2302

Merged
merged 1 commit into from
May 13, 2024

Conversation

abc99lr
Copy link
Contributor

@abc99lr abc99lr commented May 9, 2024

Testing. Do not merge.

Based on the discussions from #2293, it's a good idea to test if we could use nosync thrust calls by default. This PR changes the current rmm::exec_policy to its async version rmm::exec_policy_nosync.

@abc99lr abc99lr requested a review from a team as a code owner May 9, 2024 17:38
Copy link

copy-pr-bot bot commented May 9, 2024

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions bot added the cpp label May 9, 2024
@abc99lr
Copy link
Contributor Author

abc99lr commented May 9, 2024

Tagging @cjnolet to enable testing

@cjnolet
Copy link
Member

cjnolet commented May 9, 2024

@abc99lr for future reference, you should be in the rapids org and the raft-triage group. The reason CI isn't automatically starting on this PR is because you haven't signed your commits. This is something you should be able to set up once and it'll "just work" on your dev system going forward. Here's the instructions to do this.

@cjnolet cjnolet added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels May 9, 2024
@cjnolet
Copy link
Member

cjnolet commented May 9, 2024

/ok to test

@abc99lr
Copy link
Contributor Author

abc99lr commented May 10, 2024

It seems this PR fails pr / conda-cpp-tests / tests (amd64, 3.11, 12.2.2, ubuntu22.04, v100, latest) (push). But I checked the log and all the status show success. Should we rerun the CI? Or maybe I missed something?

Here is the link to the log.
https://github.com/rapidsai/raft/actions/runs/9024355165/job/24800696954?pr=2302

@cjnolet
Copy link
Member

cjnolet commented May 10, 2024

@abc99lr yeah it's weird indeed. i"m rerunnig that test checker. It's possible it could have been a chance occurrence.

@cjnolet cjnolet changed the title [Do not merge] Test thrust nosync execution policy Make thrust nosync execution policy the default thrust policy May 13, 2024
@cjnolet
Copy link
Member

cjnolet commented May 13, 2024

/merge

@rapids-bot rapids-bot bot merged commit 0b55c33 into rapidsai:branch-24.06 May 13, 2024
62 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cpp improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
Development

Successfully merging this pull request may close these issues.

2 participants