Skip to content

Add support for .to() for NestedTensor backends #87146

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
Closed
ani300 wants to merge 1 commit into from
Closed

Add support for .to() for NestedTensor backends #87146

ani300 wants to merge 1 commit into from

Conversation

@ani300
Copy link
Collaborator

@ani300 ani300 commented Oct 17, 2022

Summary: This commit adds support for moving NestedTensors from CPU to GPU and back. The implementation includes requires implementing empty_like(), which is based on PR#83140.

Test Plan: Added a new unit test based on the unit test for the main .to() implementation. All unit tests must pass, as well as every sandcastle job.

Differential Revision: D40437585

@pytorch-bot
Copy link

pytorch-bot bot commented Oct 17, 2022 •
edited
Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/87146

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures, 1 Pending

As of commit 54edfa1:
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Oct 17, 2022

This pull request was exported from Phabricator. Differential Revision: D40437585

@ani300 ani300 mentioned this pull request Oct 17, 2022
Closed
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Oct 17, 2022

This pull request was exported from Phabricator. Differential Revision: D40437585

drisspg
drisspg reviewed Oct 17, 2022
View reviewed changes
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Oct 17, 2022

This pull request was exported from Phabricator. Differential Revision: D40437585

1 similar comment
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Oct 18, 2022

This pull request was exported from Phabricator. Differential Revision: D40437585

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Oct 18, 2022

This pull request was exported from Phabricator. Differential Revision: D40437585

@ani300 ani300 requested a review from drisspg October 18, 2022 20:24
drisspg
drisspg reviewed Oct 18, 2022
View reviewed changes
drisspg
drisspg reviewed Oct 18, 2022
View reviewed changes
drisspg
drisspg reviewed Oct 18, 2022
View reviewed changes
drisspg
drisspg reviewed Oct 18, 2022
View reviewed changes
drisspg
drisspg reviewed Oct 18, 2022
View reviewed changes
drisspg
drisspg reviewed Oct 18, 2022
View reviewed changes
drisspg
drisspg reviewed Oct 18, 2022
View reviewed changes
Copy link
Contributor

@drisspg drisspg left a comment

Choose a reason for hiding this comment

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

Few small comments but otherwise I think this is very close, great job

ani300 pushed a commit to ani300/pytorch that referenced this pull request Oct 19, 2022
@facebook-github-bot
Add support for .to() for NestedTensor backends (pytorch#87146)
8c37860 
Summary:
Pull Request resolved: pytorch#87146

This commit adds support for moving NestedTensors from CPU to GPU and back. The implementation includes requires implementing empty_like(), which is based on PR#83140.

Test Plan: Added a new unit test based on the unit test for the main .to() implementation. All unit tests must pass, as well as every sandcastle job.

Differential Revision: D40437585

fbshipit-source-id: b54894943c605a619d900bc9cabebf5c0925e37b
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Oct 19, 2022

This pull request was exported from Phabricator. Differential Revision: D40437585

drisspg
drisspg reviewed Oct 19, 2022
View reviewed changes
aten/src/ATen/native/nested/NestedTensorMath.cpp Outdated
@@ -1,5 +1,7 @@
#include <ATen/native/nested/NestedTensorMath.h>

#include <c10/util/Optional.h>
Copy link
Contributor

@drisspg drisspg Oct 19, 2022

Choose a reason for hiding this comment

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

Nit: do we need to add these headers? maybe we should have had the optional.h , but do we need the Copy.h header

drisspg
drisspg approved these changes Oct 19, 2022
View reviewed changes
Copy link
Contributor

@drisspg drisspg left a comment

Choose a reason for hiding this comment

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

👍

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Oct 19, 2022
@facebook-github-bot
Add support for .to() for NestedTensor backends (pytorch#87146)
54edfa1 
Summary:
Pull Request resolved: pytorch#87146

This commit adds support for moving NestedTensors from CPU to GPU and back. The implementation includes requires implementing empty_like(), which is based on PR#83140.

Test Plan: Added a new unit test based on the unit test for the main .to() implementation. All unit tests must pass, as well as every sandcastle job.

Reviewed By: drisspg

Differential Revision: D40437585

fbshipit-source-id: b2b70ba4852c3b4ba4ae5d9ce796a37a902fb80a
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Oct 19, 2022

This pull request was exported from Phabricator. Differential Revision: D40437585

@ani300
Copy link
Collaborator Author

ani300 commented Oct 19, 2022

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

pytorchmergebot commented Oct 20, 2022

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@github-actions
Copy link
Contributor

github-actions bot commented Oct 20, 2022

Hey @ani300.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

@drisspg drisspg mentioned this pull request Nov 1, 2022
Closed
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@drisspg drisspg drisspg approved these changes

@cpuhrsch cpuhrsch Awaiting requested review from cpuhrsch

@mikekgfb mikekgfb Awaiting requested review from mikekgfb

Assignees

No one assigned

Labels

ciflow/trunk Trigger trunk jobs on your pull request fb-exported Merged

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ani300 @facebook-github-bot @pytorchmergebot @drisspg