Add refs/decomps for dot/vdot #108194

lezcano · 2023-08-29T23:03:17Z

Stack from ghstack (oldest at bottom):

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx @peterbell10 @ipiszy @ngimel @yf225 @chenyang78 @kadeng @muchulee8 @aakhundov

Follow-up on #108127 (comment) [ghstack-poisoned]

pytorch-bot · 2023-08-29T23:03:19Z

🔗 Helpful Links

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

📄 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 the bot commands wiki or our office hours

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

✅ No Failures

As of commit 3632437 with merge base b7624fc ():
💚 Looks good so far! There are no failures yet. 💚

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

Follow-up on #108127 (comment) ghstack-source-id: 829d50f Pull Request resolved: #108194

peterbell10 · 2023-08-29T23:44:24Z

torch/testing/_internal/common_methods_invocations.py

    yield ErrorInput(SampleInput(make_input(1, 1), args=(make_input(3),)),
                     error_regex='1D tensors expected')
    yield ErrorInput(SampleInput(make_input(9), args=(make_input(3),)),
                     error_regex='inconsistent tensor size')
-    if device != "cpu":
-        ErrorInput(SampleInput(make_input(3), args=(make_input(3, device="cpu"),)),
-                   error_regex='Expected all tensors to be on the same device')


Why are we changing the tests here? Shouldn't you fix the refs to match the test.

We don't check the device in almost any op, so I figured that it's a bit unnecessary. I can roll back.

As to the other, checking for that error disallows using the elementwise wrapper, which is, if anything, a good thing. One of the initial designs of primtorch was that we should add more support for ops if it's easy to do so.

Adding more functionality to the ref seems fine, but removing testing from eager definitely seems bad,

What do you suggest then? Should I just xfail these tests?

Could you override the error_inputs_func of the ref info to pass in an is_ref=True option?

Fair enough. Done.

peterbell10 · 2023-08-29T23:46:39Z

torch/testing/_internal/common_methods_invocations.py

+        "_refs.dot",
+        torch_opinfo_name="dot",
+        # .conj() returns a view on complex tensors??
+        validate_view_consistency=False,


Why aren't we getting the view behavior right here?

At a guess, return torch.dot(self, other.conj()).conj() probably needs to come outside of elementwise_type_promotion_wrapper.

Not quite. I checked under this branch:

elif other.is_conj(): return torch.dot(self, other.conj()).conj()

which is the one that triggers the issue, and torch.dot(self, other.conj()) is not a view, but torch.dot(self, other.conj()).conj() is a view.
On the other hand, if I put the same code in ATen, it says that torch.dot(self, other.conj()).conj() is conj but not a view... I'm submitting a different fix.

Nvm, the other fix doesn't work. I'm not quite sure what's going on there.
I'm voting for leaving as is right now as we don't even support complex numbers at the moment in inductor.

Okay the eager result having is_conj() but not _is_view() is weird but not really our fault here, so I guess thats fine. This comment should be updated to reflect that though; .conj() is expected to return a view so the comment doesn't make much sense.

Follow-up on #108127 (comment) cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

Follow-up on #108127 (comment) ghstack-source-id: 399c51e Pull Request resolved: #108194

Follow-up on #108127 (comment) cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

Follow-up on #108127 (comment) ghstack-source-id: 81642a5 Pull Request resolved: #108194

Follow-up on #108127 (comment) cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

Follow-up on #108127 (comment) ghstack-source-id: 849d47b Pull Request resolved: #108194

lezcano · 2023-08-30T16:22:25Z

@pytorchbot merge

pytorchmergebot · 2023-08-30T16:30:15Z

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

pytorchmergebot · 2023-08-30T16:55:58Z

Merge failed

Reason: 1 jobs have failed, first few of them are: trunk / macos-12-py3-arm64-mps / test (default, 1, 1)

Details for Dev Infra team

Raised by workflow job

Follow-up on #108127 (comment) cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

Follow-up on #108127 (comment) ghstack-source-id: 992cf08 Pull Request resolved: #108194

lezcano · 2023-08-31T12:13:31Z

aten/src/ATen/native/cuda/Blas.cpp

-  TORCH_CHECK(
-      self.device() == other.device(),
-      "expected all tensors to be on the same device. Found: ",
-      self.device(),
-      ", ",
-      other.device());


@peterbell10 removing this as it's unreachable. We currently check this in the dispatcher and throw

Expected all tensors to be on the same device, but found at least two devices, cpu and cuda:0! (when checking argument for argument tensor in method wrapper_CUDA__dot

lezcano · 2023-08-31T12:14:15Z

@pytorchbot merge

pytorchmergebot · 2023-08-31T12:17:02Z

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

Add refs/decomps for dot/vdot

105d735

Follow-up on #108127 (comment) [ghstack-poisoned]

lezcano requested review from mruberry and ngimel as code owners August 29, 2023 23:03

lezcano mentioned this pull request Aug 29, 2023

Add reference for linalg.vecdot #108188

Closed

github-actions bot added the ciflow/inductor label Aug 29, 2023

lezcano added a commit that referenced this pull request Aug 29, 2023

Add refs/decomps for dot/vdot

b049d1d

Follow-up on #108127 (comment) ghstack-source-id: 829d50f Pull Request resolved: #108194

lezcano requested a review from peterbell10 August 29, 2023 23:05

lezcano added module: inductor release notes: inductor labels Aug 29, 2023

lezcano mentioned this pull request Aug 29, 2023

Support CUDA autocast for torch.linalg.vecdot() #108127

Closed

pytorchbot added the open source label Aug 29, 2023

peterbell10 requested changes Aug 29, 2023

View reviewed changes

Update on "Add refs/decomps for dot/vdot"

0f37c37

Follow-up on #108127 (comment) cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

lezcano added a commit that referenced this pull request Aug 30, 2023

Add refs/decomps for dot/vdot

8ba7db3

Follow-up on #108127 (comment) ghstack-source-id: 399c51e Pull Request resolved: #108194

Update on "Add refs/decomps for dot/vdot"

67b6c8f

Follow-up on #108127 (comment) cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

lezcano added a commit that referenced this pull request Aug 30, 2023

Add refs/decomps for dot/vdot

1ed7f40

Follow-up on #108127 (comment) ghstack-source-id: 81642a5 Pull Request resolved: #108194

Update on "Add refs/decomps for dot/vdot"

d53cd75

Follow-up on #108127 (comment) cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

Update on "Add refs/decomps for dot/vdot"

162ade2

Follow-up on #108127 (comment) cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

lezcano added a commit that referenced this pull request Aug 30, 2023

Add refs/decomps for dot/vdot

6d77a28

Follow-up on #108127 (comment) ghstack-source-id: 849d47b Pull Request resolved: #108194

peterbell10 approved these changes Aug 30, 2023

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Aug 30, 2023

pytorchmergebot added the merging label Aug 30, 2023

pytorchmergebot removed the merging label Aug 30, 2023

Update on "Add refs/decomps for dot/vdot"

3632437

Follow-up on #108127 (comment) cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

lezcano added a commit that referenced this pull request Aug 31, 2023

Add refs/decomps for dot/vdot

cdf2814

Follow-up on #108127 (comment) ghstack-source-id: 992cf08 Pull Request resolved: #108194

lezcano requested a review from kulinseth as a code owner August 31, 2023 12:12

lezcano commented Aug 31, 2023

View reviewed changes

pytorchmergebot added the merging label Aug 31, 2023

pytorchmergebot added Merged and removed merging labels Aug 31, 2023

pytorchmergebot closed this in 239ee76 Aug 31, 2023

facebook-github-bot deleted the gh/Lezcano/233/head branch October 1, 2023 14:23

Add refs/decomps for dot/vdot #108194

Add refs/decomps for dot/vdot #108194

Uh oh!

Conversation

lezcano commented Aug 29, 2023 • edited by pytorch-bot bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Aug 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

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

✅ No Failures

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lezcano commented Aug 30, 2023

Uh oh!

pytorchmergebot commented Aug 30, 2023

Merge started

Uh oh!

pytorchmergebot commented Aug 30, 2023

Merge failed

Uh oh!

lezcano Aug 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lezcano commented Aug 31, 2023

Uh oh!

pytorchmergebot commented Aug 31, 2023

Merge started

Uh oh!

Uh oh!

lezcano commented Aug 29, 2023 •

edited by pytorch-bot bot

Loading

pytorch-bot bot commented Aug 29, 2023 •

edited

Loading

lezcano Aug 31, 2023 •

edited

Loading