Skip to content

Insert aten.add into fallback_ops, and fix Tensor -> Scalar conversion in ir.FallbackKernel#140624

Closed
benjaminglass1 wants to merge 9 commits into
gh/benjaminglass1/21/basefrom
gh/benjaminglass1/21/head
Closed

Insert aten.add into fallback_ops, and fix Tensor -> Scalar conversion in ir.FallbackKernel#140624
benjaminglass1 wants to merge 9 commits into
gh/benjaminglass1/21/basefrom
gh/benjaminglass1/21/head

Conversation

[ghstack-poisoned]
@pytorch-bot

pytorch-bot Bot commented Nov 13, 2024

Copy link
Copy Markdown

🔗 Helpful Links

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

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

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

✅ No Failures

As of commit 209ee40 with merge base 93aef68 (image):
💚 Looks good so far! There are no failures yet. 💚

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

Comment thread torch/_inductor/ir.py
aten.add.Tensor: aten.add.Scalar,
aten.div.Tensor: aten.div.Scalar,
aten.divide.Tensor: aten.divide.Scalar,
aten.floor_divide: aten.floor_divide.Scalar,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Note for reviewers: this is intentional; floor_divide's Tensor operator has no suffix.

[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
@benjaminglass1 benjaminglass1 self-assigned this Nov 15, 2024
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
@benjaminglass1 benjaminglass1 marked this pull request as ready for review November 18, 2024 16:51
[ghstack-poisoned]
"aten.adaptive_max_pool3d.default",
"aten.adaptive_max_pool3d_backward.default",
"aten.add.Scalar",
"aten.add.Tensor",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we have to add this as a fallback? aten.add.Tensor should be a simple pointwise op and properly codegen-ed by Inductor.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@desertfire I agree, and I'm unsure why we ran into this, but the complex fallback test specifically required this op. I'd also note that we already codegen for mul.Scalar and mul.Tensor below...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@desertfire It appears that the test in question (test_complex_fallback) is deliberately triggering a situation where inductor will choose to fallback to aten.add (adding complex numbers). We can continue to skip this test indefinitely if you'd prefer not to add new fallback ops, but passing the test will require this fallback.

I know that there's some work ongoing with better supporting complex numbers in torch, but I don't really know the status of that, so this may be needed for a while.

[ghstack-poisoned]
pytorchmergebot pushed a commit that referenced this pull request Nov 19, 2024
pytorchmergebot pushed a commit that referenced this pull request Nov 19, 2024
pytorchmergebot pushed a commit to jakeharmon8/pytorch that referenced this pull request Nov 20, 2024
…n in ir.FallbackKernel (pytorch#140624)

The code in ir.FallbackKernel will long-term be obviated by the solution for pytorch#90923.

Closes pytorch#131334.

Pull Request resolved: pytorch#140624
Approved by: https://github.com/desertfire
pytorchmergebot pushed a commit to jakeharmon8/pytorch that referenced this pull request Nov 20, 2024
pytorchmergebot pushed a commit to jakeharmon8/pytorch that referenced this pull request Nov 20, 2024
pobin6 pushed a commit to pobin6/pytorch that referenced this pull request Dec 5, 2024
…n in ir.FallbackKernel (pytorch#140624)

The code in ir.FallbackKernel will long-term be obviated by the solution for pytorch#90923.

Closes pytorch#131334.

Pull Request resolved: pytorch#140624
Approved by: https://github.com/desertfire
pobin6 pushed a commit to pobin6/pytorch that referenced this pull request Dec 5, 2024
pobin6 pushed a commit to pobin6/pytorch that referenced this pull request Dec 5, 2024
@github-actions github-actions Bot deleted the gh/benjaminglass1/21/head branch December 20, 2024 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants