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.

Sign up for GitHub

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

Jump to bottom

[PluggableDevice] add DEVICE_DEFAULT for session/transpose ops #50623

Merged
merged 1 commit into from
Aug 30, 2021
Merged

[PluggableDevice] add DEVICE_DEFAULT for session/transpose ops #50623

merged 1 commit into from
Aug 30, 2021

Conversation

quintinwang5
Copy link
Contributor

Add DEVICE_DEFAULT for session/transpose ops. This PR is for PluggableDevice.

@google-ml-butler google-ml-butler bot added the size:S CL Change Size: Small label Jul 6, 2021
@google-cla google-cla bot added the cla: yes label Jul 6, 2021
@gbaned gbaned self-assigned this Jul 6, 2021
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Jul 6, 2021
@gbaned gbaned added the comp:core issues related to core part of tensorflow label Jul 6, 2021
@gbaned gbaned requested a review from saxenasaurabh July 6, 2021 10:49
@gbaned gbaned added the awaiting review Pull request awaiting review label Jul 9, 2021
@jzhoulon
Copy link
Contributor

@saxenasaurabh Can you help to review it? there are also several other PRs related with DEVICE_DEFAULT, thanks.

https://github.com/tensorflow/tensorflow/pull/50623

https://github.com/tensorflow/tensorflow/pull/50610

https://github.com/tensorflow/tensorflow/pull/50609

https://github.com/tensorflow/tensorflow/pull/50608

https://github.com/tensorflow/tensorflow/pull/50605

https://github.com/tensorflow/tensorflow/pull/50604

https://github.com/tensorflow/tensorflow/pull/50603

https://github.com/tensorflow/tensorflow/pull/50602

https://github.com/tensorflow/tensorflow/pull/50601

https://github.com/tensorflow/tensorflow/pull/50600

https://github.com/tensorflow/tensorflow/pull/50581

https://github.com/tensorflow/tensorflow/pull/50580

https://github.com/tensorflow/tensorflow/pull/50578

@saxenasaurabh
Copy link
Member

Hey @jzhoulon,

@saxenasaurabh Can you help to review it? there are also several other PRs related with DEVICE_DEFAULT, thanks.

https://github.com/tensorflow/tensorflow/pull/50623

https://github.com/tensorflow/tensorflow/pull/50610

https://github.com/tensorflow/tensorflow/pull/50609

https://github.com/tensorflow/tensorflow/pull/50608

https://github.com/tensorflow/tensorflow/pull/50605

https://github.com/tensorflow/tensorflow/pull/50604

https://github.com/tensorflow/tensorflow/pull/50603

https://github.com/tensorflow/tensorflow/pull/50602

https://github.com/tensorflow/tensorflow/pull/50601

https://github.com/tensorflow/tensorflow/pull/50600

https://github.com/tensorflow/tensorflow/pull/50581

https://github.com/tensorflow/tensorflow/pull/50580

https://github.com/tensorflow/tensorflow/pull/50578

Hey @jzhoulon, these are on my radar. I will try to review this week or next. Thanks for the patience!

penpornk
penpornk requested changes Aug 20, 2021
View reviewed changes
Copy link
Member

@penpornk penpornk left a comment

Choose a reason for hiding this comment

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

Thank you for the PR!

tensorflow/core/kernels/transpose_op.cc
@@ -91,6 +91,18 @@ REGISTER_KERNEL_BUILDER(Name("InvertPermutation")
.HostMemory("y"),
InvertPermutationOp<int64>);

REGISTER_KERNEL_BUILDER(Name("InvertPermutation")
.Device(DEVICE_DEFAULT)
Copy link
Member

Choose a reason for hiding this comment

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

Can we just replace the registration for DEVICE_GPU with DEVICE_DEFAULT? The DEVICE_DEFAULT rule would work for DEVICE_GPU too. Same for other PRs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@penpornk Hi, I've changed all of my DEVICE_DEFAULT PRs. Please review.

PR Queue automation moved this from Assigned Reviewer to Reviewer Requested Changes Aug 20, 2021
@penpornk penpornk removed the awaiting review Pull request awaiting review label Aug 20, 2021
@quintinwang5 quintinwang5 force-pushed the quintinwang/session_transpose_default_device branch 3 times, most recently from c881f88 to 2582f23 Compare August 23, 2021 06:57
penpornk
penpornk approved these changes Aug 27, 2021
View reviewed changes
Copy link
Member

@penpornk penpornk left a comment

Choose a reason for hiding this comment

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

Thank you for the changes!

@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Aug 27, 2021
PR Queue automation moved this from Reviewer Requested Changes to Approved by Reviewer Aug 27, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Aug 27, 2021
@copybara-service copybara-service bot merged commit 9a679b3 into tensorflow:master Aug 30, 2021
PR Queue automation moved this from Approved by Reviewer to Merged Aug 30, 2021
@google-ml-butler google-ml-butler bot removed the ready to pull PR ready for merge process label Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@penpornk penpornk penpornk approved these changes

@saxenasaurabh saxenasaurabh Awaiting requested review from saxenasaurabh

Assignees

@gbaned gbaned

Labels
cla: yes comp:core issues related to core part of tensorflow size:S CL Change Size: Small
Projects
PR Queue
  
Merged
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@quintinwang5 @jzhoulon @saxenasaurabh @penpornk @kokoro-team @gbaned