Add backward support for rudimentary NestedTensor.sum(dim) #82625

mikaylagawarecki · 2022-08-01T23:17:31Z

Per offline discussion, this will be updated to use expand once expand semantics for nested tensor have been fleshed out.

Next steps will be to add support for other features for forward sum mentioned on #82387 and likewise update the backward

Stack from ghstack (oldest at bottom):

[ghstack-poisoned]

ghstack-source-id: ef1b279 Pull Request resolved: #82625

facebook-github-bot · 2022-08-01T23:17:38Z

🔗 Helpful links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/82625
📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓Need help or want to give feedback on the CI? Visit our office hours

✅ No Failures (0 Pending)

As of commit 7ef27d3 (more details on the Dr. CI page):

Expand to see more

💚 💚 Looks good so far! There are no failures yet. 💚 💚

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

[ghstack-poisoned]

ghstack-source-id: 6929abd Pull Request resolved: #82625

tools/autograd/derivatives.yaml

albanD · 2022-08-02T14:32:02Z

This was already discussed in https://docs.google.com/document/d/1dVMH33IK2C7n3Ee8LBtfWw2i8MzxkAIkvuUo_hZY63k/edit#
Any reason for not going with that is proposed there?

[ghstack-poisoned]

aten/src/ATen/native/native_functions.yaml

tools/autograd/derivatives.yaml

aten/src/ATen/native/native_functions.yaml

tools/autograd/derivatives.yaml

test/test_nestedtensor.py

albanD · 2022-08-12T15:04:19Z

test/test_nestedtensor.py

+            self.assertEqual(nt_sum, torch.nested_tensor(ub2_sum))
+            # test backward
+            if requires_grad:
+                # generate gradient tensor that has the same size as the output


It would be nice to have torch.randn_like(nt_sum) :D

Could I hold off on this until Driss implements empty_like in #83140, I think that the implementation for randn_like might be similar

Of course!

btw driss is using nt = self.random_nt(device, dtype, 4, (4, 4)) in a few of his tests. Maybe that is a good replacement?

In any case, this is a small detail that can be done later.

Ho you're already using that now :D

test/test_nestedtensor.py

Per offline discussion, this will be updated to use expand once expand semantics for nested tensor have been fleshed out. [ghstack-poisoned]

ghstack-source-id: 087ad38 Pull Request resolved: #82625

albanD

SGTM
I didn't look into details into the kernel though (but I guess it's fine as it's going to be removed soon).

mikaylagawarecki · 2022-08-16T18:41:52Z

@pytorchbot merge

pytorchmergebot · 2022-08-16T18:45:23Z

@pytorchbot successfully started a merge job. Check the current status here and land check progress here.
The merge job was triggered with the land checks (-l) flag. If you did not specify this flag yourself, you are likely enrolled in the land checks rollout. This means that your change will be merged once all checks on your PR and the land checks have passed (ETA 4 Hours). If you need to coordinate lands between different changes and cannot risk a land race, please add the ciflow/trunk label to your PR and wait for signal to complete, and then land your changes in proper order. Having trunk, pull, and Lint pre-run on a PR will bypass land checks and the ETA should be immediate. If this is not the intended behavior, feel free to use some of the other merge options in the wiki.
Please reach out to the PyTorch DevX Team with feedback or questions!

pytorchmergebot · 2022-08-16T19:40:49Z

Merge failed due to Failed to merge; some land checks failed: pull, pull / linux-bionic-py3.7-clang9 / test (default, 2, 2, linux.2xlarge), pull / linux-bionic-py3.7-clang9 / test (crossref, 2, 2, linux.2xlarge)
Raised by https://github.com/pytorch/pytorch/actions/runs/2870295706 If you believe this is an error, you can use the old behavior with @pytorchbot merge -g (optionally with the ciflow/trunk to get land checks) or use @pytorchbot merge -f "some reason here". For more information, see the bot wiki.
Please reach out to the PyTorch DevX Team with feedback or questions!

mikaylagawarecki · 2022-08-16T20:45:32Z

@pytorchbot merge

pytorchmergebot · 2022-08-16T20:46:58Z

@pytorchbot successfully started a merge job. Check the current status here and land check progress here.
The merge job was triggered with the land checks (-l) flag. If you did not specify this flag yourself, you are likely enrolled in the land checks rollout. This means that your change will be merged once all checks on your PR and the land checks have passed (ETA 4 Hours). If you need to coordinate lands between different changes and cannot risk a land race, please add the ciflow/trunk label to your PR and wait for signal to complete, and then land your changes in proper order. Having trunk, pull, and Lint pre-run on a PR will bypass land checks and the ETA should be immediate. If this is not the intended behavior, feel free to use some of the other merge options in the wiki.
Please reach out to the PyTorch DevX Team with feedback or questions!

pytorchmergebot · 2022-08-16T21:37:21Z

Merge failed
Reason: Failed to merge; some land checks failed: pull, pull / linux-bionic-py3.7-clang9 / test (default, 2, 2, linux.2xlarge), pull / linux-bionic-py3.7-clang9 / test (crossref, 2, 2, linux.2xlarge)
Raised by https://github.com/pytorch/pytorch/actions/runs/2870978847 If you believe this is an error, you can use the old behavior with @pytorchbot merge -g (optionally with the ciflow/trunk to get land checks) or use @pytorchbot merge -f "some reason here". For more information, see the bot wiki.
Please reach out to the PyTorch DevX Team with feedback or questions!

mikaylagawarecki · 2022-08-16T22:06:31Z

@pytorchbot rebase -s

pytorchmergebot · 2022-08-16T22:08:04Z

@pytorchbot successfully started a rebase job. Check the current status here

Per offline discussion, this will be updated to use expand once expand semantics for nested tensor have been fleshed out. Next steps will be to add support for other features for forward sum mentioned on #82387 and likewise update the backward [ghstack-poisoned]

pytorchmergebot · 2022-08-16T22:08:21Z

Successfully rebased gh/mikaylagawarecki/75/orig onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via ghstack checkout https://github.com/pytorch/pytorch/pull/82625)

ghstack-source-id: 4f6f637 Pull Request resolved: #82625

mikaylagawarecki · 2022-08-16T22:10:38Z

@pytorchbot merge

pytorchmergebot · 2022-08-16T22:11:58Z

@pytorchbot successfully started a merge job. Check the current status here and land check progress here.
The merge job was triggered with the land checks (-l) flag. If you did not specify this flag yourself, you are likely enrolled in the land checks rollout. This means that your change will be merged once all checks on your PR and the land checks have passed (ETA 4 Hours). If you need to coordinate lands between different changes and cannot risk a land race, please add the ciflow/trunk label to your PR and wait for signal to complete, and then land your changes in proper order. Having trunk, pull, and Lint pre-run on a PR will bypass land checks and the ETA should be immediate. If this is not the intended behavior, feel free to use some of the other merge options in the wiki.
Please reach out to the PyTorch DevX Team with feedback or questions!

Per offline discussion, this will be updated to use expand once expand semantics for nested tensor have been fleshed out. Next steps will be to add support for other features for forward sum mentioned on #82387 and likewise update the backward Pull Request resolved: #82625 Approved by: https://github.com/albanD

pytorchmergebot · 2022-08-17T01:17:40Z

Merge failed
Reason: Failed to merge; some land checks failed: pull, pull / linux-focal-py3.7-gcc7 / test (default, 1, 2, linux.2xlarge), pull / linux-bionic-py3.7-clang9 / test (default, 2, 2, linux.2xlarge), pull / linux-bionic-py3.7-clang9 / test (crossref, 2, 2, linux.2xlarge), pull / linux-focal-py3.7-clang7-asan / test (default, 3, 5, linux.2xlarge), pull / win-vs2019-cpu-py3 / test (default, 2, 2, windows.4xlarge), trunk, trunk / parallelnative-linux-focal-py3.7-gcc7 / test (default, 1, 2, linux.2xlarge), trunk / macos-12-py3-arm64 / test (default, 2, 2, macos-m1-12), trunk / macos-12-py3-x86-64 / test (default, 2, 2, macos-12)
Raised by https://github.com/pytorch/pytorch/actions/runs/2871401403 If you believe this is an error, you can use the old behavior with @pytorchbot merge -g (optionally with the ciflow/trunk to get land checks) or use @pytorchbot merge -f "some reason here". For more information, see the bot wiki.
Please reach out to the PyTorch DevX Team with feedback or questions!

mikaylagawarecki · 2022-08-17T14:54:03Z

@pytorchbot merge

pytorchmergebot · 2022-08-17T14:55:38Z

@pytorchbot successfully started a merge job. Check the current status here and land check progress here.
The merge job was triggered with the land checks (-l) flag. If you did not specify this flag yourself, you are likely enrolled in the land checks rollout. This means that your change will be merged once all checks on your PR and the land checks have passed (ETA 4 Hours). If you need to coordinate lands between different changes and cannot risk a land race, please add the ciflow/trunk label to your PR and wait for signal to complete, and then land your changes in proper order. Having trunk, pull, and Lint pre-run on a PR will bypass land checks and the ETA should be immediate. If this is not the intended behavior, feel free to use some of the other merge options in the wiki.
Please reach out to the PyTorch DevX Team with feedback or questions!

Per offline discussion, this will be updated to use expand once expand semantics for nested tensor have been fleshed out. Next steps will be to add support for other features for forward sum mentioned on #82387 and likewise update the backward Pull Request resolved: #82625 Approved by: https://github.com/albanD

…82625) Summary: Per offline discussion, this will be updated to use expand once expand semantics for nested tensor have been fleshed out. Next steps will be to add support for other features for forward sum mentioned on #82387 and likewise update the backward Pull Request resolved: #82625 Approved by: https://github.com/albanD Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/bd0ad7a84f125435f9e0685f86b1ca2efd2bd43b Reviewed By: atalman Differential Revision: D38830753 Pulled By: mikaylagawarecki fbshipit-source-id: 6e4db6ab37714e2fa2637d7213a7cbe6dfadab06

Add backward support for rudimentary NestedTensor.sum(dim)

81e6c9b

[ghstack-poisoned]

facebook-github-bot added the cla signed label Aug 1, 2022

mikaylagawarecki added a commit that referenced this pull request Aug 1, 2022

Add backward support for rudimentary NestedTensor.sum(dim)

c84ea41

ghstack-source-id: ef1b279 Pull Request resolved: #82625

Update on "Add backward support for rudimentary NestedTensor.sum(dim)"

89c2de5

[ghstack-poisoned]

Update on "Add backward support for rudimentary NestedTensor.sum(dim)"

0592c85

[ghstack-poisoned]

mikaylagawarecki added a commit that referenced this pull request Aug 2, 2022

Add backward support for rudimentary NestedTensor.sum(dim)

e31f3fe

ghstack-source-id: 6929abd Pull Request resolved: #82625

mikaylagawarecki commented Aug 2, 2022

View reviewed changes

tools/autograd/derivatives.yaml Show resolved Hide resolved

albanD reviewed Aug 2, 2022

View reviewed changes

tools/autograd/derivatives.yaml Outdated Show resolved Hide resolved

Update on "Add backward support for rudimentary NestedTensor.sum(dim)"

7884c75

[ghstack-poisoned]

bdhirsh reviewed Aug 12, 2022

View reviewed changes

aten/src/ATen/native/native_functions.yaml Outdated Show resolved Hide resolved

albanD reviewed Aug 12, 2022

View reviewed changes

mikaylagawarecki mentioned this pull request Aug 12, 2022

[easy] Fix .sizes() call in saved_variable.cpp for nested tensor #83356

Closed

Update on "Add backward support for rudimentary NestedTensor.sum(dim)"

f88401d

Per offline discussion, this will be updated to use expand once expand semantics for nested tensor have been fleshed out. [ghstack-poisoned]

Update on "Add backward support for rudimentary NestedTensor.sum(dim)"

b70ff7e

Per offline discussion, this will be updated to use expand once expand semantics for nested tensor have been fleshed out. [ghstack-poisoned]

Update on "Add backward support for rudimentary NestedTensor.sum(dim)"

920ce09

Per offline discussion, this will be updated to use expand once expand semantics for nested tensor have been fleshed out. [ghstack-poisoned]

mikaylagawarecki added a commit that referenced this pull request Aug 12, 2022

Add backward support for rudimentary NestedTensor.sum(dim)

521cf22

ghstack-source-id: 087ad38 Pull Request resolved: #82625

mikaylagawarecki requested a review from jbschlosser August 15, 2022 15:28

mikaylagawarecki marked this pull request as ready for review August 15, 2022 15:29

mikaylagawarecki requested a review from soulitzer as a code owner August 15, 2022 15:29

albanD approved these changes Aug 15, 2022

View reviewed changes

pytorchmergebot pushed a commit that referenced this pull request Aug 16, 2022

Add backward support for rudimentary NestedTensor.sum(dim)

ece6abe

ghstack-source-id: 4f6f637 Pull Request resolved: #82625

pytorchmergebot added the Merged label Aug 17, 2022

pytorchmergebot closed this in bd0ad7a Aug 17, 2022

mikaylagawarecki added release notes: nested tensor Changes that have a direct impact on nested tensors topic: improvements topic category labels Aug 17, 2022

facebook-github-bot deleted the gh/mikaylagawarecki/75/head branch August 21, 2022 14:19

mikaylagawarecki mentioned this pull request Aug 23, 2022

Tracking nested tensor functions with backward kernels registered in derivatives.yaml #83910

Open

2 tasks

Add backward support for rudimentary NestedTensor.sum(dim) #82625

Add backward support for rudimentary NestedTensor.sum(dim) #82625

Uh oh!

Conversation

mikaylagawarecki commented Aug 1, 2022 • edited by pytorchmergebot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Aug 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful links

✅ No Failures (0 Pending)

Uh oh!

Uh oh!

Uh oh!

albanD commented Aug 2, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

albanD Aug 12, 2022

Choose a reason for hiding this comment

Uh oh!

mikaylagawarecki Aug 15, 2022

Choose a reason for hiding this comment

Uh oh!

albanD Aug 15, 2022

Choose a reason for hiding this comment

Uh oh!

albanD Aug 15, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

albanD left a comment

Choose a reason for hiding this comment

Uh oh!

mikaylagawarecki commented Aug 16, 2022

Uh oh!

pytorchmergebot commented Aug 16, 2022

Uh oh!

pytorchmergebot commented Aug 16, 2022

Uh oh!

mikaylagawarecki commented Aug 16, 2022

Uh oh!

pytorchmergebot commented Aug 16, 2022

Uh oh!

pytorchmergebot commented Aug 16, 2022

Uh oh!

mikaylagawarecki commented Aug 16, 2022

Uh oh!

pytorchmergebot commented Aug 16, 2022

Uh oh!

pytorchmergebot commented Aug 16, 2022

Uh oh!

mikaylagawarecki commented Aug 16, 2022

Uh oh!

pytorchmergebot commented Aug 16, 2022

Uh oh!

pytorchmergebot commented Aug 17, 2022

Uh oh!

mikaylagawarecki commented Aug 17, 2022

Uh oh!

pytorchmergebot commented Aug 17, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mikaylagawarecki commented Aug 1, 2022 •

edited by pytorchmergebot

Loading

facebook-github-bot commented Aug 1, 2022 •

edited

Loading