Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add decompositions for copy variants of view ops #128416

Open
wants to merge 45 commits into
base: gh/rec/14/base
Choose a base branch
from

Conversation

Copy link

pytorch-bot bot commented Jun 11, 2024

🔗 Helpful Links

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

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

⏳ No Failures, 118 Pending

As of commit b5d251b with merge base 1e61cb8 (image):
💚 Looks good so far! There are no failures yet. 💚

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

@pytorch-bot pytorch-bot bot added ciflow/inductor ciflow/mps Run MPS tests (subset of trunk) module: inductor oncall: distributed Add this issue/PR to distributed oncall triage queue release notes: onnx torch.onnx related changes that should show up in the release notes labels Jun 11, 2024
rec added a commit that referenced this pull request Jun 11, 2024
ghstack-source-id: a290ff22d81cdf09038063a6bd2b5c47e103b460
Pull Request resolved: #128416
@albanD albanD removed their request for review June 11, 2024 13:30
@rec rec closed this Jun 12, 2024
rec added a commit to rec/pytorch that referenced this pull request Jun 13, 2024
ghstack-source-id: a290ff22d81cdf09038063a6bd2b5c47e103b460
Pull Request resolved: pytorch#128416
rec added a commit to rec/pytorch that referenced this pull request Jun 13, 2024
ghstack-source-id: a290ff22d81cdf09038063a6bd2b5c47e103b460
Pull Request resolved: pytorch#128416
rec added a commit to rec/pytorch that referenced this pull request Jun 13, 2024
ghstack-source-id: a290ff22d81cdf09038063a6bd2b5c47e103b460
Pull Request resolved: pytorch#128416
rec added a commit to rec/pytorch that referenced this pull request Jun 13, 2024
ghstack-source-id: a290ff22d81cdf09038063a6bd2b5c47e103b460
Pull Request resolved: pytorch#128416
rec added a commit that referenced this pull request Jun 13, 2024
ghstack-source-id: 32dce226e1e801cc8278a3dbe985e22d0292f039
Pull Request resolved: #128416
rec added a commit that referenced this pull request Jun 14, 2024
ghstack-source-id: 32dce226e1e801cc8278a3dbe985e22d0292f039
Pull Request resolved: #128416
rec added a commit that referenced this pull request Jul 3, 2024
ghstack-source-id: 8e4e90c06a7b861eec2ee10f0b0eee11496df8b2
Pull Request resolved: #128416
@rec rec mentioned this pull request Jul 3, 2024
rec added a commit to rec/pytorch that referenced this pull request Jul 4, 2024
ghstack-source-id: 8e4e90c06a7b861eec2ee10f0b0eee11496df8b2
Pull Request resolved: pytorch#128416
@rec
Copy link
Collaborator Author

rec commented Jul 4, 2024

The single ❌ seems to be caused by the SEV, so this is ready to go.

rec added a commit to rec/pytorch that referenced this pull request Jul 4, 2024
ghstack-source-id: 8e4e90c06a7b861eec2ee10f0b0eee11496df8b2
Pull Request resolved: pytorch#128416
rec added a commit to rec/pytorch that referenced this pull request Jul 4, 2024
ghstack-source-id: 90438a927c50ab4ab31c12e18f11846af0423723
Pull Request resolved: pytorch#128416
rec added a commit to rec/pytorch that referenced this pull request Jul 4, 2024
ghstack-source-id: 90438a927c50ab4ab31c12e18f11846af0423723
Pull Request resolved: pytorch#128416
@rec
Copy link
Collaborator Author

rec commented Jul 8, 2024

@pytorchbot: merge

Copy link

pytorch-bot bot commented Jul 8, 2024

❌ 🤖 pytorchbot command failed:

@pytorchbot: error: argument command: invalid choice: ':' (choose from 'merge', 'revert', 'rebase', 'label', 'drci', 'cherry-pick', 'close')

usage: @pytorchbot [-h] {merge,revert,rebase,label,drci,cherry-pick,close} ...

Try @pytorchbot --help for more info.

@rec
Copy link
Collaborator Author

rec commented Jul 8, 2024

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Jul 8, 2024
@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: Approvers from one of the following sets are needed:

  • superuser (pytorch/metamates)
  • Core Reviewers (mruberry, lezcano, Skylion007, ngimel, peterbell10)
  • Core Maintainers (soumith, gchanan, ezyang, dzhulgakov, malfet)
Details for Dev Infra team Raised by workflow job

Failing merge rule: Core Maintainers

cc mrshenli pritamdamania87 zhaojuanmao satgera gqchen aazzolini osalpekar jiayisuse H-Huang kwen2501 awgu fegin XilunWu wanchaol fduwjj wz337 tianyu-l wconstab chauhang d4l3k voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire

[ghstack-poisoned]
rec added a commit that referenced this pull request Jul 8, 2024
ghstack-source-id: 4eba26fa68813477bd7eb7620e23dd2c8bb1f413
Pull Request resolved: #128416
Copy link
Collaborator

@lezcano lezcano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one minor point, otherwise looks good.
Thank you for the comprehensive PR!

torch/_refs/__init__.py Outdated Show resolved Hide resolved
rec added a commit to rec/pytorch that referenced this pull request Jul 9, 2024
ghstack-source-id: 4eba26fa68813477bd7eb7620e23dd2c8bb1f413
Pull Request resolved: pytorch#128416
cc mrshenli pritamdamania87 zhaojuanmao satgera gqchen aazzolini osalpekar jiayisuse H-Huang kwen2501 awgu fegin XilunWu wanchaol fduwjj wz337 tianyu-l wconstab chauhang d4l3k voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire

[ghstack-poisoned]
rec added a commit that referenced this pull request Jul 9, 2024
ghstack-source-id: 3053377639c5a5bcf864bf8e13aa298b10b053e0
Pull Request resolved: #128416
Copy link
Collaborator

@lezcano lezcano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple nits

Comment on lines 322 to 323
params = *sig.parameters.values(), out_param
params = sorted(params, key=lambda p: p.kind)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this change necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without that line, if there's a **kwds parameter, it's placed before the out= parameter, and you get an exception ValueError: wrong parameter order: variadic keyword parameter before keyword-only parameter.

I added a comment to this effect.


from typing import Callable, NamedTuple, Optional, overload, Sequence, Tuple
from typing import Callable, Dict, NamedTuple, Optional, overload, Sequence, Tuple
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from typing import Callable, Dict, NamedTuple, Optional, overload, Sequence, Tuple
from typing import Callable, NamedTuple, Optional, overload, Sequence, Tuple

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

aten_fn = getattr(aten, fn.__name__)
annotations = fn.__annotations__
fn = out_wrapper()(aten_fn)
fn.__annotations__.update(annotations)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit. Move it to after _fn.__name__ to keep all the metaprogramming together.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

rec added a commit to rec/pytorch that referenced this pull request Jul 9, 2024
ghstack-source-id: 3053377639c5a5bcf864bf8e13aa298b10b053e0
Pull Request resolved: pytorch#128416
cc mrshenli pritamdamania87 zhaojuanmao satgera gqchen aazzolini osalpekar jiayisuse H-Huang kwen2501 awgu fegin XilunWu wanchaol fduwjj wz337 tianyu-l wconstab chauhang d4l3k voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire

[ghstack-poisoned]
rec added a commit that referenced this pull request Jul 9, 2024
ghstack-source-id: ac370ec2fd6d0dffe0e79a6a24b836af869f05d1
Pull Request resolved: #128416
@rec
Copy link
Collaborator Author

rec commented Jul 9, 2024

A couple nits

Nits are good to find - they decrease the difficulty of the code for the next programmer!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/inductor ciflow/mps Run MPS tests (subset of trunk) ciflow/trunk Trigger trunk jobs on your pull request module: inductor oncall: distributed Add this issue/PR to distributed oncall triage queue open source release notes: onnx torch.onnx related changes that should show up in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants