Generalize toAccumulateType() #108248

qqaatw · 2023-08-30T16:58:10Z

Stack from ghstack (oldest at bottom):

-> Generalize toAccumulateType() #108248

Trying to address this comment: #106666 (comment)

cc @albanD @kulinseth

[ghstack-poisoned]

pytorch-bot · 2023-08-30T16:58:20Z

🔗 Helpful Links

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

📄 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 b9fe25c with merge base 92f4a7b ():
💚 Looks good so far! There are no failures yet. 💚

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

ghstack-source-id: fa89f03 Pull Request resolved: #108248

aten/src/ATen/AccumulateType.h

albanD

Super cool update!

aten/src/ATen/AccumulateType.h

Trying to address this comment: #106666 (comment) cc albanD kulinseth [ghstack-poisoned]

ghstack-source-id: 1f6a2ea Pull Request resolved: #108248

Trying to address this comment: #106666 (comment) cc albanD kulinseth [ghstack-poisoned]

ghstack-source-id: 97a2b15 Pull Request resolved: #108248

torch/csrc/autograd/FunctionsManual.cpp

albanD · 2023-09-06T18:43:19Z

aten/src/ATen/AccumulateType.h

+#define CUDA_ACC_TYPE(t, acc_t) ACC_TYPE(t, acc_t, c10::DeviceType::CUDA)
+#define CPU_ACC_TYPE(t, acc_t) ACC_TYPE(t, acc_t, c10::DeviceType::CPU)
+
+MPS_ACC_TYPE(BFloat16, float);


This looks great! Thanks for the refactor!

Trying to address this comment: #106666 (comment) cc albanD kulinseth [ghstack-poisoned]

ghstack-source-id: 905afd1 Pull Request resolved: #108248

Trying to address this comment: #106666 (comment) cc albanD kulinseth [ghstack-poisoned]

ghstack-source-id: c8f5e64 Pull Request resolved: #108248

albanD

Change sounds good.
I just want a quick stamp from Kulin to make sure the double complex handling is fine.

albanD · 2023-09-13T22:55:24Z

aten/src/ATen/AccumulateType.h

+MPS_ACC_TYPE(bool, bool);
+MPS_ACC_TYPE(c10::complex<Half>, c10::complex<float>);
+MPS_ACC_TYPE(c10::complex<float>, c10::complex<float>);
+MPS_ACC_TYPE(c10::complex<double>, c10::complex<float>);


cc @kulinseth just to confirm that this is ok to have this for now.

Can we issue a warning that we are falling back to using float for double types ?

I'm a bit concerned about that would be somewhat verbose from a user's point of view since accumulate type is an internal thing of which a regular user probably doesn't have idea.

What do you think @albanD?

I'm not sure what is the best place to put this warning for sure. Also note that the user would need to have created a compleex to begin with to reach this.
So I would say that not having a warning here is fine?

If the current state is fine, this PR needs your approval @albanD. Thanks!

kulinseth

Other than the warning . This looks good to me

albanD

SGTM

qqaatw · 2023-09-29T17:56:54Z

@pytorchbot merge -r

pytorchmergebot · 2023-09-29T17:58:34Z

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

Trying to address this comment: #106666 (comment) cc albanD kulinseth [ghstack-poisoned]

pytorchmergebot · 2023-09-29T17:58:47Z

Successfully rebased gh/qqaatw/19/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/108248)

ghstack-source-id: 63e414e Pull Request resolved: #108248

pytorchmergebot · 2023-09-29T17:59:59Z

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-09-29T18:15:55Z

Merge failed

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

Details for Dev Infra team

Raised by workflow job

Trying to address this comment: #106666 (comment) cc albanD kulinseth [ghstack-poisoned]

ghstack-source-id: ea08786 Pull Request resolved: #108248

qqaatw · 2023-10-02T06:14:23Z

@pytorchbot merge

pytorchmergebot · 2023-10-02T06:18:04Z

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-10-02T06:50:45Z

Merge failed

Reason: 1 jobs have failed, first few of them are: trunk / macos-12-py3-arm64 / build

Details for Dev Infra team

Raised by workflow job

qqaatw · 2023-10-02T16:30:24Z

@pytorchbot merge

pytorchmergebot · 2023-10-02T16:34:14Z

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

Generalize toAccumulateType()

87fcbd8

[ghstack-poisoned]

pytorch-bot bot added ciflow/mps Run MPS tests (subset of trunk) release notes: mps Release notes category labels Aug 30, 2023

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

Generalize toAccumulateType()

7b87f6e

ghstack-source-id: fa89f03 Pull Request resolved: #108248

pytorchbot added the open source label Aug 30, 2023

qqaatw commented Aug 30, 2023

View reviewed changes

aten/src/ATen/AccumulateType.h Outdated Show resolved Hide resolved

albanD reviewed Aug 31, 2023

View reviewed changes

aten/src/ATen/AccumulateType.h Outdated Show resolved Hide resolved

aten/src/ATen/AccumulateType.h Outdated Show resolved Hide resolved

Update on "Generalize toAccumulateType()"

1765ffd

Trying to address this comment: #106666 (comment) cc albanD kulinseth [ghstack-poisoned]

Update on "Generalize toAccumulateType()"

73d7844

Trying to address this comment: #106666 (comment) cc albanD kulinseth [ghstack-poisoned]

qqaatw added a commit that referenced this pull request Sep 4, 2023

Generalize toAccumulateType()

5649585

ghstack-source-id: 1f6a2ea Pull Request resolved: #108248

Update on "Generalize toAccumulateType()"

deb1cb7

Trying to address this comment: #106666 (comment) cc albanD kulinseth [ghstack-poisoned]

qqaatw added a commit that referenced this pull request Sep 4, 2023

Generalize toAccumulateType()

e12803f

ghstack-source-id: 97a2b15 Pull Request resolved: #108248

qqaatw marked this pull request as ready for review September 6, 2023 14:30

qqaatw requested review from kulinseth and soulitzer as code owners September 6, 2023 14:30

albanD reviewed Sep 6, 2023

View reviewed changes

Update on "Generalize toAccumulateType()"

f2c8348

Trying to address this comment: #106666 (comment) cc albanD kulinseth [ghstack-poisoned]

Update on "Generalize toAccumulateType()"

24ce043

Trying to address this comment: #106666 (comment) cc albanD kulinseth [ghstack-poisoned]

qqaatw added a commit that referenced this pull request Sep 7, 2023

Generalize toAccumulateType()

73f0020

ghstack-source-id: 905afd1 Pull Request resolved: #108248

Update on "Generalize toAccumulateType()"

ce83816

Trying to address this comment: #106666 (comment) cc albanD kulinseth [ghstack-poisoned]

qqaatw added a commit that referenced this pull request Sep 12, 2023

Generalize toAccumulateType()

6168d12

ghstack-source-id: c8f5e64 Pull Request resolved: #108248

qqaatw requested a review from albanD September 12, 2023 16:36

albanD reviewed Sep 13, 2023

View reviewed changes

kulinseth approved these changes Sep 19, 2023

View reviewed changes

albanD approved these changes Sep 29, 2023

View reviewed changes

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

Update on "Generalize toAccumulateType()"

d461e4b

Trying to address this comment: #106666 (comment) cc albanD kulinseth [ghstack-poisoned]

pytorchmergebot pushed a commit that referenced this pull request Sep 29, 2023

Generalize toAccumulateType()

83e06c6

ghstack-source-id: 63e414e Pull Request resolved: #108248

pytorchmergebot added the merging label Sep 29, 2023

pytorchmergebot removed the merging label Sep 29, 2023

Update on "Generalize toAccumulateType()"

b9fe25c

Trying to address this comment: #106666 (comment) cc albanD kulinseth [ghstack-poisoned]

qqaatw added a commit that referenced this pull request Oct 2, 2023

Generalize toAccumulateType()

5f0acf7

ghstack-source-id: ea08786 Pull Request resolved: #108248

pytorchmergebot added the merging label Oct 2, 2023

pytorchmergebot removed the merging label Oct 2, 2023

pytorchmergebot added the merging label Oct 2, 2023

pytorchmergebot added Merged and removed merging labels Oct 2, 2023

pytorchmergebot closed this in a3c1e3c Oct 2, 2023

facebook-github-bot deleted the gh/qqaatw/19/head branch October 6, 2023 14:23

Generalize toAccumulateType() #108248

Generalize toAccumulateType() #108248

Uh oh!

Conversation

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

Uh oh!

Uh oh!

pytorch-bot bot commented Aug 30, 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/108248

✅ No Failures

Uh oh!

Uh oh!

albanD left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

albanD Sep 6, 2023

Choose a reason for hiding this comment

Uh oh!

albanD left a comment

Choose a reason for hiding this comment

Uh oh!

albanD Sep 13, 2023

Choose a reason for hiding this comment

Uh oh!

kulinseth Sep 19, 2023

Choose a reason for hiding this comment

Uh oh!

qqaatw Sep 19, 2023

Choose a reason for hiding this comment

Uh oh!

albanD Sep 19, 2023

Choose a reason for hiding this comment

Uh oh!

qqaatw Sep 20, 2023

Choose a reason for hiding this comment

Uh oh!

kulinseth left a comment

Choose a reason for hiding this comment

Uh oh!

albanD left a comment

Choose a reason for hiding this comment

Uh oh!

qqaatw commented Sep 29, 2023

Uh oh!

pytorchmergebot commented Sep 29, 2023

Uh oh!

pytorchmergebot commented Sep 29, 2023

Uh oh!

pytorchmergebot commented Sep 29, 2023

Merge started

Uh oh!

pytorchmergebot commented Sep 29, 2023

Merge failed

Uh oh!

qqaatw commented Oct 2, 2023

Uh oh!

pytorchmergebot commented Oct 2, 2023

Merge started

Uh oh!

pytorchmergebot commented Oct 2, 2023

Merge failed

Uh oh!

qqaatw commented Oct 2, 2023

Uh oh!

pytorchmergebot commented Oct 2, 2023

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

qqaatw commented Aug 30, 2023 •

edited

Loading

pytorch-bot bot commented Aug 30, 2023 •

edited

Loading