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

[ROCm] move nccl stream to member of StreamGroup #31483

Merged

Conversation

jeffdaily
Copy link
Contributor

This allows the compute and nccl stream to be force-initialized as immediate
siblings which is necessary for ROCm performance.

This allows the compute and nccl stream to be force-initialized as immediate
siblings which is necessary for ROCm performance.
@jeffdaily jeffdaily requested a review from chsigg as a code owner August 9, 2019 16:11
@tensorflow-bot tensorflow-bot bot added the size:M CL Change Size: Medium label Aug 9, 2019
@jeffdaily jeffdaily changed the title move nccl stream to member of StreamGroup [ROCm] move nccl stream to member of StreamGroup Aug 9, 2019
@rthadur rthadur self-assigned this Aug 9, 2019
@rthadur rthadur added this to Assigned Reviewer in PR Queue via automation Aug 9, 2019
@rthadur rthadur added the comp:gpu GPU related issues label Aug 9, 2019
chsigg
chsigg previously approved these changes Aug 12, 2019
PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer Aug 12, 2019
@tensorflow-bot tensorflow-bot bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Aug 12, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Aug 12, 2019
@rthadur rthadur added ready to pull PR ready for merge process and removed ready to pull PR ready for merge process labels Aug 12, 2019
@rthadur
Copy link
Contributor

rthadur commented Aug 13, 2019

@jeffdaily can you please resolve conflicts ?

PR Queue automation moved this from Approved by Reviewer to Reviewer Requested Changes Aug 13, 2019
@rthadur rthadur requested a review from chsigg August 13, 2019 21:42
@rthadur rthadur removed the ready to pull PR ready for merge process label Aug 15, 2019
@mellanox-github
Copy link

Can one of the admins verify this patch?

chsigg
chsigg previously approved these changes Aug 26, 2019
PR Queue automation moved this from Reviewer Requested Changes to Approved by Reviewer Aug 26, 2019
@tensorflow-bot tensorflow-bot bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Aug 26, 2019
@rthadur
Copy link
Contributor

rthadur commented Aug 29, 2019

@jeffdaily can you please resolve conflicts and we can run the tests once you are done, thank you

@rthadur rthadur removed kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Aug 29, 2019
PR Queue automation moved this from Approved by Reviewer to Reviewer Requested Changes Sep 4, 2019
This also disables the multi-node emulation of the nccl_manager_test
for the ROCm platform.
@rthadur rthadur requested a review from chsigg September 4, 2019 22:03
PR Queue automation moved this from Reviewer Requested Changes to Approved by Reviewer Sep 5, 2019
@tensorflow-bot tensorflow-bot bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Sep 5, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Sep 5, 2019
tensorflow-copybara pushed a commit that referenced this pull request Sep 5, 2019
…ccl_stream_early

PiperOrigin-RevId: 267404335
@tensorflow-copybara tensorflow-copybara merged commit ad8b9ff into tensorflow:master Sep 5, 2019
PR Queue automation moved this from Approved by Reviewer to Merged Sep 5, 2019
@jeffdaily jeffdaily deleted the google_upstream_nccl_stream_early branch December 20, 2019 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes comp:gpu GPU related issues ready to pull PR ready for merge process size:M CL Change Size: Medium
Projects
PR Queue
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

7 participants