[Reland] [functorch] Fix linalg batch rules to error on non-matrix inputs by samdow · Pull Request #82176 · pytorch/pytorch

samdow · 2022-07-25T22:30:14Z

Stack from ghstack (oldest at bottom):

-> [Reland] [functorch] Fix linalg batch rules to error on non-matrix inputs #82176

[ghstack-poisoned]

facebook-github-bot · 2022-07-25T22:30:20Z

🔗 Helpful links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/82176
📄 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 (2 Pending)

As of commit b78bba1 (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.

samdow · 2022-07-25T22:31:56Z

I wasn't able to add the testing I wanted where we check that the error we get on a vector is the same as a matrix...somehow that failed locally for things where the batch rule wasn't implemented?

Any other thoughts on how to test this are welcome though!

…inputs" [ghstack-poisoned]

functorch/functorch/csrc/VmapGeneratedPlumbing.h

zou3519

some minor comments/questions

functorch/functorch/csrc/BatchRulesHelper.h

functorch/functorch/csrc/BatchRulesLinearAlgebra.cpp

zou3519 · 2022-07-26T15:56:15Z

functorch/functorch/csrc/BatchRulesLinearAlgebra.cpp

+    return moveBatchDimToFront(tensor, batch_dim);
+  }
+
+  static oneOutput apply_one(


(no action required) In c++17 we can create a single function that takes the number of returns as a template argument, using constexpr_if. e.g. https://stackoverflow.com/questions/59046161/how-to-change-the-return-type-depending-on-the-input-c . Unfortunately pytorch is still on c++14

zou3519 · 2022-07-26T17:58:31Z

I wasn't able to add the testing I wanted where we check that the error we get on a vector is the same as a matrix...somehow that failed locally for things where the batch rule wasn't implemented?
Any other thoughts on how to test this are welcome though!

What failed locally?

…inputs" [ghstack-poisoned]

samdow · 2022-07-26T20:54:52Z

I wasn't able to add the testing I wanted where we check that the error we get on a vector is the same as a matrix...somehow that failed locally for things where the batch rule wasn't implemented?
Any other thoughts on how to test this are welcome though!

What failed locally?

So the basic of the test was this: go through the sample inputs for every linalg.* function, find one that's 2D, take a 1D slice of it, run the (non-vmapped) function with that under a try, catch any error and then run the vmapped-version (with the input expanded to be a square of the 1D slice) under self.assertRaisesRegex(<that error>).

Occassionally we'll get errors like:
AssertionError: "linalg.pinv(Float{[5]}): expected a tensor with 2 or more dimensions of float, double, cfloat or cdouble types" does not match "linalg.pinv(Float{[5]}): expected a tensor with 2 or more dimensions of float, double, cfloat or cdouble types"
This is something that I looked into briefly and just couldn't figure out what it thought was different

We could also just do a check that the the first part of this matches the function being called but I'm not fully convinced that it's always correct? I guess I could check that

EDIT: just checked and it's not always true that the first part matches the exact function being called (ex: linalg.svdvals will error with an error starting with linalg.svd)

…inputs" [ghstack-poisoned]

samdow · 2022-07-28T13:40:59Z

@pytorchbot rebase

pytorchmergebot · 2022-07-28T13:42:23Z

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

…inputs" [ghstack-poisoned]

pytorchmergebot · 2022-07-28T13:42:36Z

Successfully rebased gh/samdow/15/orig onto refs/remotes/origin/master, please pull locally before adding more changes (for example, via ghstack checkout https://github.com/pytorch/pytorch/pull/82176)

ghstack-source-id: 319361c Pull Request resolved: #82176

…inputs" [ghstack-poisoned]

ghstack-source-id: baccd55 Pull Request resolved: #82176

samdow · 2022-07-28T18:56:50Z

@zou3519 when you get a chance to look again, this now has the test we discussed offline that just checks that every function in linalg and tests that it errors if being passed a 1D tensor. The only cases it's skipping at (1) vector_norm which should accept 1D vectors or (2) multi_dot which accepts a list of tensors. This gets its own test case since there's rules around what can and can't be 1D

functorch/functorch/csrc/BatchRulesHelper.cpp

functorch/functorch/csrc/BatchRulesHelper.h

…inputs" [ghstack-poisoned]

ghstack-source-id: c516d66 Pull Request resolved: #82176

pytorchmergebot · 2022-08-02T13:59:07Z

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

github-actions · 2022-08-02T14:00:37Z

Hey @samdow.
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.

zengk95 · 2022-08-02T19:31:08Z

@pytorchbot revert -m "This looks like it's breaking functorch tests on master" -c nosignal

pytorchmergebot · 2022-08-02T19:33:57Z

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

pytorchmergebot · 2022-08-02T19:34:03Z

@samdow your PR has been successfully reverted.

…uts (#82176)" This reverts commit 1dfcad8. Reverted #82176 on behalf of https://github.com/zengk95 due to This looks like it's breaking functorch tests on master

…inputs" [ghstack-poisoned]

…n-matrix inputs" [ghstack-poisoned]

ghstack-source-id: 9dd77a0 Pull Request resolved: #82176

…176) (#82176) Summary: X-link: pytorch/pytorch#82176 Approved by: https://github.com/zou3519 Reviewed By: kit1980 Differential Revision: D38359527 Pulled By: samdow fbshipit-source-id: f41ecc073deaa5d45f5e2733e31cc086a33716f0

) (#82176) Summary: Pull Request resolved: #82176 Approved by: https://github.com/zou3519 Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/1dfcad84aaad9c095b4dbbdafd5c351e9a712d45 Reviewed By: kit1980 Differential Revision: D38359527 Pulled By: samdow fbshipit-source-id: f41ecc073deaa5d45f5e2733e31cc086a33716f0

samdow · 2022-08-03T13:43:34Z

@pytorchbot merge

pytorchmergebot · 2022-08-03T13:45:05Z

@pytorchbot successfully started a merge and created land time checks. See merge status here and land check progress here.

…puts (#82176) Pull Request resolved: #82176 Approved by: https://github.com/zou3519

github-actions · 2022-08-03T16:47:09Z

Hey @samdow.
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.

…uts (#82176)" Summary: This reverts commit 1dfcad84aaad9c095b4dbbdafd5c351e9a712d45. Reverted pytorch/pytorch#82176 on behalf of https://github.com/zengk95 due to This looks like it's breaking functorch tests on master Reviewed By: kit1980 Differential Revision: D38394721 fbshipit-source-id: 0aedbc4d8284ce3055df5d6768858199f06f230e

…uts (#82176)" Summary: This reverts commit 1dfcad8. Reverted #82176 on behalf of https://github.com/zengk95 due to This looks like it's breaking functorch tests on master Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/31292599eb8ab41a8f5ac9ced3a5efca1fc0b627 Reviewed By: kit1980 Differential Revision: D38394721 fbshipit-source-id: 0aedbc4d8284ce3055df5d6768858199f06f230e

…puts (#82176) (#82176) Summary: X-link: pytorch/pytorch#82176 Approved by: https://github.com/zou3519 Reviewed By: kit1980 Differential Revision: D38395303 Pulled By: samdow fbshipit-source-id: ea69a28d2d413f4194ed6e96bdeaeaba6c01a3a1

…puts (#82176) (#82176) Summary: Pull Request resolved: #82176 Approved by: https://github.com/zou3519 Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/fbbd036871406a3895607a79dc58dc08551df486 Reviewed By: kit1980 Differential Revision: D38395303 Pulled By: samdow fbshipit-source-id: ea69a28d2d413f4194ed6e96bdeaeaba6c01a3a1

[functorch] Fix linalg batch rules to error on non-matrix inputs

7e9fd03

[ghstack-poisoned]

facebook-github-bot added the cla signed label Jul 25, 2022

samdow mentioned this pull request Jul 25, 2022

[functorch] Add a bunch of low hanging fruit linalg batch rules #82177

Closed

samdow requested a review from zou3519 July 25, 2022 22:30

Update on "[functorch] Fix linalg batch rules to error on non-matrix …

d6fbd77

…inputs" [ghstack-poisoned]

zou3519 reviewed Jul 26, 2022

View reviewed changes

functorch/functorch/csrc/VmapGeneratedPlumbing.h Outdated Show resolved Hide resolved

zou3519 reviewed Jul 26, 2022

View reviewed changes

Update on "[functorch] Fix linalg batch rules to error on non-matrix …

d85c789

…inputs" [ghstack-poisoned]

samdow added 4 commits July 26, 2022 17:32

Update on "[functorch] Fix linalg batch rules to error on non-matrix …

3fcaeb7

…inputs" [ghstack-poisoned]

Update on "[functorch] Fix linalg batch rules to error on non-matrix …

887731f

…inputs" [ghstack-poisoned]

Update on "[functorch] Fix linalg batch rules to error on non-matrix …

d380790

…inputs" [ghstack-poisoned]

Update on "[functorch] Fix linalg batch rules to error on non-matrix …

0bda5b9

…inputs" [ghstack-poisoned]

samdow mentioned this pull request Jul 27, 2022

[functorch] add lstsq batch rule #82325

Closed

samdow added 2 commits July 27, 2022 12:30

Update on "[functorch] Fix linalg batch rules to error on non-matrix …

85be85d

…inputs" [ghstack-poisoned]

Update on "[functorch] Fix linalg batch rules to error on non-matrix …

b92e86c

…inputs" [ghstack-poisoned]

Update on "[functorch] Fix linalg batch rules to error on non-matrix …

a558729

…inputs" [ghstack-poisoned]

pytorchmergebot pushed a commit that referenced this pull request Jul 28, 2022

[functorch] Fix linalg batch rules to error on non-matrix inputs

16144c4

ghstack-source-id: 319361c Pull Request resolved: #82176

Update on "[functorch] Fix linalg batch rules to error on non-matrix …

a90d19c

…inputs" [ghstack-poisoned]

samdow pushed a commit that referenced this pull request Jul 28, 2022

[functorch] Fix linalg batch rules to error on non-matrix inputs

3dcc31d

ghstack-source-id: baccd55 Pull Request resolved: #82176

zou3519 approved these changes Jul 28, 2022

View reviewed changes

functorch/functorch/csrc/BatchRulesHelper.cpp Outdated Show resolved Hide resolved

functorch/functorch/csrc/BatchRulesHelper.h Outdated Show resolved Hide resolved

Update on "[functorch] Fix linalg batch rules to error on non-matrix …

bbb3b30

…inputs" [ghstack-poisoned]

samdow pushed a commit that referenced this pull request Jul 28, 2022

[functorch] Fix linalg batch rules to error on non-matrix inputs

0fe8bb9

ghstack-source-id: c516d66 Pull Request resolved: #82176

pytorchmergebot added the Merged label Aug 2, 2022

pytorchmergebot closed this in 1dfcad8 Aug 2, 2022

pytorchmergebot added the Reverted label Aug 2, 2022

Update on "[functorch] Fix linalg batch rules to error on non-matrix …

488fb09

…inputs" [ghstack-poisoned]

samdow reopened this Aug 2, 2022

samdow changed the title ~~[functorch] Fix linalg batch rules to error on non-matrix inputs~~ [Reland] [functorch] Fix linalg batch rules to error on non-matrix inputs Aug 2, 2022

Update on "[Reland] [functorch] Fix linalg batch rules to error on no…

b78bba1

…n-matrix inputs" [ghstack-poisoned]

samdow pushed a commit that referenced this pull request Aug 2, 2022

[functorch] Fix linalg batch rules to error on non-matrix inputs

0d71504

ghstack-source-id: 9dd77a0 Pull Request resolved: #82176

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

[Reland] [functorch] Fix linalg batch rules to error on non-matrix in…

a93b181

…puts (#82176) Pull Request resolved: #82176 Approved by: https://github.com/zou3519

pytorchmergebot closed this in fbbd036 Aug 3, 2022

facebook-github-bot deleted the gh/samdow/15/head branch August 7, 2022 14:18

samdow mentioned this pull request Aug 12, 2022

Support for batched matrix exponential pytorch/functorch#1002

Closed

Conversation

samdow commented Jul 25, 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 Jul 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful links

✅ No Failures (2 Pending)

Uh oh!

samdow commented Jul 25, 2022

Uh oh!

Uh oh!

zou3519 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zou3519 Jul 26, 2022

Choose a reason for hiding this comment

Uh oh!

zou3519 commented Jul 26, 2022

Uh oh!

samdow commented Jul 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

samdow commented Jul 28, 2022

Uh oh!

pytorchmergebot commented Jul 28, 2022

Uh oh!

pytorchmergebot commented Jul 28, 2022

Uh oh!

samdow commented Jul 28, 2022

Uh oh!

Uh oh!

Uh oh!

pytorchmergebot commented Aug 2, 2022

Uh oh!

github-actions bot commented Aug 2, 2022

Uh oh!

zengk95 commented Aug 2, 2022

Uh oh!

pytorchmergebot commented Aug 2, 2022

Uh oh!

pytorchmergebot commented Aug 2, 2022

Uh oh!

samdow commented Aug 3, 2022

Uh oh!

pytorchmergebot commented Aug 3, 2022

Uh oh!

github-actions bot commented Aug 3, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

samdow commented Jul 25, 2022 •

edited by pytorchmergebot

Loading

facebook-github-bot commented Jul 25, 2022 •

edited

Loading

samdow commented Jul 26, 2022 •

edited

Loading