Skip to content

Implement copy_, fill_, and ones_like for Nested Tensors backends#87728

Closed
ani300 wants to merge 1 commit intopytorch:masterfrom
ani300:export-D40689594
Closed

Implement copy_, fill_, and ones_like for Nested Tensors backends#87728
ani300 wants to merge 1 commit intopytorch:masterfrom
ani300:export-D40689594

Conversation

@ani300
Copy link
Collaborator

@ani300 ani300 commented Oct 25, 2022

Summary: This diff implements copy_ in order to allow pinned memory transfers for nested tensors, as well as fill_ and ones_like, to test whether nested tensors can be created with other factory functions.

Test Plan: Pass all CI and sandcastle jobs.

Reviewed By: mikekgfb

Differential Revision: D40689594

@pytorch-bot
Copy link

pytorch-bot bot commented Oct 25, 2022

🔗 Helpful Links

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

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

❌ 1 Failures

As of commit ecb2460:

The following jobs have failed:

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

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Why this change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it was done automatically from a linter suggestion, I can revert it

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add unit tests for these ops?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll add them

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

@ani300 ani300 requested a review from cpuhrsch October 25, 2022 21:18
Copy link
Contributor

@cpuhrsch cpuhrsch Oct 25, 2022

Choose a reason for hiding this comment

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

This won't work if value is anything but a scalar. In the test you only test it with scalar. Do you need to support this overload for your use case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No. I can add the test that fails with a non-0-dimensional tensor too. I think the reason this exists is so that you can do something like t.fill_(other_t[0]), which returns a 0-dimensional tensor instead of a scalar.

…torch#87728)

Summary:
Pull Request resolved: pytorch#87728

This diff implements copy_ in order to allow pinned memory transfers for nested tensors, as well as fill_ and ones_like, to test whether nested tensors can be created with other factory functions.

Test Plan: Pass all CI and sandcastle jobs.

Reviewed By: mikekgfb

Differential Revision: D40689594

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

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

@ani300 ani300 requested a review from cpuhrsch October 25, 2022 22:48
@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Oct 26, 2022
@ani300
Copy link
Collaborator Author

ani300 commented Oct 26, 2022

@pytorchbot merge -f "unrelated failure"

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes).

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

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.

@cpuhrsch cpuhrsch added topic: new features topic category release notes: nested tensor Changes that have a direct impact on nested tensors labels Oct 26, 2022
kulinseth pushed a commit to kulinseth/pytorch that referenced this pull request Nov 5, 2022
…torch#87728)

Summary: This diff implements copy_ in order to allow pinned memory transfers for nested tensors, as well as fill_ and ones_like, to test whether nested tensors can be created with other factory functions.

Test Plan: Pass all CI and sandcastle jobs.

Reviewed By: mikekgfb

Differential Revision: D40689594

Pull Request resolved: pytorch#87728
Approved by: https://github.com/cpuhrsch
kulinseth pushed a commit to kulinseth/pytorch that referenced this pull request Dec 10, 2022
…torch#87728)

Summary: This diff implements copy_ in order to allow pinned memory transfers for nested tensors, as well as fill_ and ones_like, to test whether nested tensors can be created with other factory functions.

Test Plan: Pass all CI and sandcastle jobs.

Reviewed By: mikekgfb

Differential Revision: D40689594

Pull Request resolved: pytorch#87728
Approved by: https://github.com/cpuhrsch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk Trigger trunk jobs on your pull request fb-exported Merged release notes: nested tensor Changes that have a direct impact on nested tensors topic: new features topic category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants