Forward AD formulas batch 2 by albanD · Pull Request #57863 · pytorch/pytorch

albanD · 2021-05-07T21:17:19Z

Slow gradcheck also passes for this PR and can be found here: #57976

Stack from ghstack:

Forward AD formulas batch 3 #58094 Forward AD formulas batch 3
Forward AD formulas batch 2 #57863 Forward AD formulas batch 2

Differential Revision: D28387763

[ghstack-poisoned]

facebook-github-bot · 2021-05-07T21:17:25Z

💊 CI failures summary and remediations

As of commit 697868e (more details on the Dr. CI page):

1/1 failures possibly* introduced in this PR
- 1/1 non-scanned failure(s)

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

Follow this link to opt-out of these comments for your Pull Requests.

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

Click here to manually regenerate this comment.

[ghstack-poisoned]

ghstack-source-id: 099f688 Pull Request resolved: #57863

[ghstack-poisoned]

ghstack-source-id: 4cd3eae Pull Request resolved: #57863

[ghstack-poisoned]

ghstack-source-id: 5b32b08 Pull Request resolved: pytorch#57863

[ghstack-poisoned]

albanD · 2021-05-12T18:18:03Z

@albanD has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Slow gradcheck also passes for this PR and can be found here: #57976 Differential Revision: [D28387763](https://our.internmc.facebook.com/intern/diff/D28387763) [ghstack-poisoned]

soulitzer

Haven't looked at the actual formulas, but here are some comments on the rest

torch/testing/_internal/common_methods_invocations.py

soulitzer · 2021-05-14T17:48:11Z

test/test_autograd.py


 def run_grad_and_gradgrad_checks(test_case, name, test_name, apply_method, output_variable,
-                                 input_variables, run_gradgradcheck=True, check_batched_grad=True):
+                                 input_variables, run_gradgradcheck=True, check_batched_grad=True,


These changes to test_autograd are for method tests? Maybe you could add a similar check that checks_forward_ad=True would raise an error unless its explicitly set to True?

This will induce larger changes because some other tests will need to be updated as well. I'll do that in a follow up PR.

Slow gradcheck also passes for this PR and can be found here: #57976 Differential Revision: [D28387763](https://our.internmc.facebook.com/intern/diff/D28387763) [ghstack-poisoned]

albanD · 2021-05-24T12:56:01Z

@albanD has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

zou3519 · 2021-05-25T13:51:03Z

tools/autograd/load_derivatives.py

        required_inputs = set()
        for arg in args_with_derivatives:
-            if arg.type == 'TensorList':
+            if arg.type == 'at::TensorList':


Is this a bugfix? (Should this be arg.type == 'at::TensorList' or arg.type == 'TensorList'?)

This is a bugfix indeed. This code wasn't actually used before and it is now that we have the "cat" formula.

Should we assert 'TensorList' not in arg.type after this if statement? Would make it easier to catch bugs like this in the future

I don't think so. It was just me not knowing that the namespace is included in this name.
I think there should be methods on the type to check that directly but I couldn't find it.

tools/autograd/derivatives.yaml

torch/csrc/autograd/FunctionsManual.cpp

zou3519

Formula for cross looks fishy, other than that the formulas lgtm

Slow gradcheck also passes for this PR and can be found here: #57976 Differential Revision: [D28387763](https://our.internmc.facebook.com/intern/diff/D28387763) [ghstack-poisoned]

zou3519

Don't forget to revert the changes to the submodules, otherwise this lgtm

soulitzer

Still seems weird to me that the Tensor variant of clamp does not raise an error since we added formulas for both scalar/tensor variants but only added support_forward_ad=True for the scalar variant in OpInfos. Any idea whats up with that?

LGTM otherwise though.

albanD · 2021-06-02T14:39:11Z

Still seems weird to me that the Tensor variant of clamp does not raise an error since we added formulas for both scalar/tensor variants but only added support_forward_ad=True for the scalar variant in OpInfos. Any idea whats up with that?

We did not add the formula for the Tensor variant actually, only the multiple scalar versions. So this is expected.

Slow gradcheck also passes for this PR and can be found here: #57976 Differential Revision: [D28387763](https://our.internmc.facebook.com/intern/diff/D28387763) [ghstack-poisoned]

albanD · 2021-06-02T14:46:25Z

@albanD has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

ghstack-source-id: 46ae249 Pull Request resolved: pytorch#57863

albanD · 2021-06-02T20:00:20Z

@albanD has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

soulitzer · 2021-06-02T23:15:10Z

We did not add the formula for the Tensor variant actually, only the multiple scalar versions. So this is expected.

Aren't clamp_max.Tensor and clamp_min.Tensor tensor variants? Or maybe I'm missing something?

albanD · 2021-06-03T12:52:19Z

They are, but for the clamp_min/max functions. The Tensor variant for clamp is clamp.Tensor which is not getting a forward formula.

facebook-github-bot · 2021-06-03T14:34:53Z

@albanD merged this pull request in d095ec7.

Summary: Pull Request resolved: pytorch#57863 Test Plan: Imported from OSS Reviewed By: zou3519 Differential Revision: D28387763 Pulled By: albanD fbshipit-source-id: e1b60ab728bb05b9e3323ee0dc7e401aaf5b8817

Forward AD formulas batch 2

222d2f7

[ghstack-poisoned]

albanD requested a review from soulitzer as a code owner May 7, 2021 21:17

This was referenced May 7, 2021

Add forward AD gradcheck #57633

Closed

Add forward AD test for op info #57701

Closed

Codegen inplace forward AD formula from out of place one if needed #57767

Closed

facebook-github-bot added the cla signed label May 7, 2021

albanD mentioned this pull request May 7, 2021

Forward AD formulas batch 1 #57768

Closed

Update on "Forward AD formulas batch 2"

d6d00d0

[ghstack-poisoned]

albanD added a commit that referenced this pull request May 7, 2021

Forward AD formulas batch 2

40d3daa

ghstack-source-id: 099f688 Pull Request resolved: #57863

Update on "Forward AD formulas batch 2"

0a828aa

[ghstack-poisoned]

Update on "Forward AD formulas batch 2"

8bae846

[ghstack-poisoned]

albanD added a commit that referenced this pull request May 7, 2021

Forward AD formulas batch 2

641e1c4

ghstack-source-id: 4cd3eae Pull Request resolved: #57863

This was referenced May 10, 2021

[DO NOT REVIEW] Slow gradcheck test for forward AD gradcheck #57952

Closed

[DO NOT REVIEW] Slow gradcheck test for forward AD gradcheck #57976

Closed

albanD added a commit that referenced this pull request May 10, 2021

Forward AD formulas batch 2

718424a

ghstack-source-id: 4cd3eae Pull Request resolved: #57863

Update on "Forward AD formulas batch 2"

8fda63b

[ghstack-poisoned]

albanD mentioned this pull request May 11, 2021

Forward AD formulas batch 3 #58094

Closed

albanD added a commit to albanD/pytorch that referenced this pull request May 11, 2021

Forward AD formulas batch 2

504c09d

ghstack-source-id: 5b32b08 Pull Request resolved: pytorch#57863

albanD added 2 commits May 12, 2021 10:09

Update on "Forward AD formulas batch 2"

72d408e

[ghstack-poisoned]

Update on "Forward AD formulas batch 2"

724bb40

[ghstack-poisoned]

Update on "Forward AD formulas batch 2"

4b28e4b

Slow gradcheck also passes for this PR and can be found here: #57976 Differential Revision: [D28387763](https://our.internmc.facebook.com/intern/diff/D28387763) [ghstack-poisoned]

This was referenced May 13, 2021

Revert "Revert D28387765: Add forward AD gradcheck" #58229

Closed

Revert "Revert D28387767: Add forward AD test for op info" #58230

Closed

Revert "Revert D28387764: Codegen inplace forward AD formula from out of place one if needed" #58231

Closed

Update on "Forward AD formulas batch 2"

59f50bb

Slow gradcheck also passes for this PR and can be found here: #57976 Differential Revision: [D28387763](https://our.internmc.facebook.com/intern/diff/D28387763) [ghstack-poisoned]

albanD mentioned this pull request May 13, 2021

update abs forward ad formula #58235

Closed

Update on "Forward AD formulas batch 2"

8aef07f

Slow gradcheck also passes for this PR and can be found here: #57976 Differential Revision: [D28387763](https://our.internmc.facebook.com/intern/diff/D28387763) [ghstack-poisoned]

soulitzer reviewed May 14, 2021

View reviewed changes

Update on "Forward AD formulas batch 2"

8d6bd8b

Slow gradcheck also passes for this PR and can be found here: #57976 Differential Revision: [D28387763](https://our.internmc.facebook.com/intern/diff/D28387763) [ghstack-poisoned]

zou3519 reviewed May 25, 2021

View reviewed changes

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

zou3519 reviewed May 25, 2021

View reviewed changes

torch/csrc/autograd/FunctionsManual.cpp Outdated Show resolved Hide resolved

zou3519 reviewed May 25, 2021

View reviewed changes

torch/csrc/autograd/FunctionsManual.cpp Outdated Show resolved Hide resolved

zou3519 reviewed May 25, 2021

View reviewed changes

Update on "Forward AD formulas batch 2"

c998508

Slow gradcheck also passes for this PR and can be found here: #57976 Differential Revision: [D28387763](https://our.internmc.facebook.com/intern/diff/D28387763) [ghstack-poisoned]

zou3519 approved these changes Jun 1, 2021

View reviewed changes

soulitzer reviewed Jun 1, 2021

View reviewed changes

Update on "Forward AD formulas batch 2"

697868e

Slow gradcheck also passes for this PR and can be found here: #57976 Differential Revision: [D28387763](https://our.internmc.facebook.com/intern/diff/D28387763) [ghstack-poisoned]

albanD added a commit to albanD/pytorch that referenced this pull request Jun 2, 2021

Forward AD formulas batch 2

ce2ea2f

ghstack-source-id: 46ae249 Pull Request resolved: pytorch#57863

facebook-github-bot closed this in d095ec7 Jun 3, 2021

facebook-github-bot added the Merged label Jun 3, 2021

facebook-github-bot deleted the gh/albanD/91/head branch June 7, 2021 14:17

Conversation

albanD commented May 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented May 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CI failures summary and remediations

Uh oh!

albanD commented May 12, 2021

Uh oh!

soulitzer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

soulitzer May 14, 2021

Choose a reason for hiding this comment

Uh oh!

albanD May 26, 2021

Choose a reason for hiding this comment

Uh oh!

albanD commented May 24, 2021

Uh oh!

zou3519 May 25, 2021

Choose a reason for hiding this comment

Uh oh!

albanD May 25, 2021

Choose a reason for hiding this comment

Uh oh!

zou3519 May 25, 2021

Choose a reason for hiding this comment

Uh oh!

albanD May 26, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zou3519 left a comment

Choose a reason for hiding this comment

Uh oh!

zou3519 left a comment

Choose a reason for hiding this comment

Uh oh!

soulitzer left a comment

Choose a reason for hiding this comment

Uh oh!

albanD commented Jun 2, 2021

Uh oh!

albanD commented Jun 2, 2021

Uh oh!

albanD commented Jun 2, 2021

Uh oh!

soulitzer commented Jun 2, 2021

Uh oh!

albanD commented Jun 3, 2021

Uh oh!

facebook-github-bot commented Jun 3, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

albanD commented May 7, 2021 •

edited

Loading

facebook-github-bot commented May 7, 2021 •

edited

Loading