-
Notifications
You must be signed in to change notification settings - Fork 26.3k
Port amax to structured kernel
#72124
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
Conversation
CI Flow Status⚛️ CI FlowRuleset - Version:
|
🔗 Helpful links
💊 CI failures summary and remediationsAs of commit efb3ecd (more details on the Dr. CI page):
🕵️ 1 new failure recognized by patternsThe following CI failures do not appear to be due to upstream breakages:
|
a75c29e to
51c68ee
Compare
amax to structured kernelamax to structured kernel
4a2ebde to
aa0d82b
Compare
|
I see this error from CI (link): I'm not sure what's causing that just from staring at it though- I'd have to repro locally. It looks like the kernel eventually calls You can repro that OpInfo test with `python test/test_ops.py TestCommonCPU.test_dtypes_amax_cpu |
|
ahh I think I found it. In your meta function, you use this to get the output dtype: And it looks like My guess is that you used that helper function because it's used by some similar ops in that file, which seems totally reasonable. The easiest fix would probably be to just directly put the logic in (@ansley lmk if you have any questions about the diagnosis!) cc @anjali411 - it looks like the other reduction ops in this file have the same problem - if the input tensor is complex, the output tensor is forced not to be complex: Do you know if that's intended behavior for reduction ops? Otherwise I can file an issue for it. |
|
@bdhirsh that's the intended behavior for norm and std. we should disable |
aa0d82b to
5e719fe
Compare
dfc8777 to
1bd8b7f
Compare
bdhirsh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for changing toValueType() in the PR too!
I'll approve, but I think you accidentally included changes to third_party/* folders - just back out those changes before landing!
1bd8b7f to
efb3ecd
Compare
|
@ansley has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
Hey @ansley. |
Summary: Pull Request resolved: pytorch/pytorch#72124 Reviewed By: bdhirsh Differential Revision: D34215708 Pulled By: ansley fbshipit-source-id: fee887e331cb8bd9fab3d9d958ff13ac8d07be27 (cherry picked from commit 94dbb5b7e7e14a663dc02ecf5013fad10b8701b3)
Summary: Pull Request resolved: pytorch/pytorch#72124 Reviewed By: bdhirsh Differential Revision: D34215708 Pulled By: ansley fbshipit-source-id: fee887e331cb8bd9fab3d9d958ff13ac8d07be27 (cherry picked from commit 94dbb5b7e7e14a663dc02ecf5013fad10b8701b3)
Summary: Pull Request resolved: pytorch/pytorch#72124 Reviewed By: bdhirsh Differential Revision: D34215708 Pulled By: ansley fbshipit-source-id: fee887e331cb8bd9fab3d9d958ff13ac8d07be27 (cherry picked from commit 94dbb5b7e7e14a663dc02ecf5013fad10b8701b3)
Summary: Pull Request resolved: pytorch/pytorch#72124 Reviewed By: bdhirsh Differential Revision: D34215708 Pulled By: ansley fbshipit-source-id: fee887e331cb8bd9fab3d9d958ff13ac8d07be27 (cherry picked from commit 94dbb5b7e7e14a663dc02ecf5013fad10b8701b3)
Summary: Pull Request resolved: pytorch/pytorch#72124 Reviewed By: bdhirsh Differential Revision: D34215708 Pulled By: ansley fbshipit-source-id: fee887e331cb8bd9fab3d9d958ff13ac8d07be27 (cherry picked from commit 94dbb5b7e7e14a663dc02ecf5013fad10b8701b3)
Summary: Pull Request resolved: pytorch/pytorch#72124 Reviewed By: bdhirsh Differential Revision: D34215708 Pulled By: ansley fbshipit-source-id: fee887e331cb8bd9fab3d9d958ff13ac8d07be27 (cherry picked from commit 94dbb5b7e7e14a663dc02ecf5013fad10b8701b3)
Summary: Pull Request resolved: pytorch/pytorch#72124 Reviewed By: bdhirsh Differential Revision: D34215708 Pulled By: ansley fbshipit-source-id: fee887e331cb8bd9fab3d9d958ff13ac8d07be27 (cherry picked from commit 94dbb5b7e7e14a663dc02ecf5013fad10b8701b3)
Summary: Pull Request resolved: pytorch/pytorch#72124 Reviewed By: bdhirsh Differential Revision: D34215708 Pulled By: ansley fbshipit-source-id: fee887e331cb8bd9fab3d9d958ff13ac8d07be27 (cherry picked from commit 94dbb5b7e7e14a663dc02ecf5013fad10b8701b3)
Summary: Pull Request resolved: pytorch/pytorch#72124 Reviewed By: bdhirsh Differential Revision: D34215708 Pulled By: ansley fbshipit-source-id: fee887e331cb8bd9fab3d9d958ff13ac8d07be27 (cherry picked from commit 94dbb5b7e7e14a663dc02ecf5013fad10b8701b3)
Summary: Pull Request resolved: pytorch/pytorch#72124 Reviewed By: bdhirsh Differential Revision: D34215708 Pulled By: ansley fbshipit-source-id: fee887e331cb8bd9fab3d9d958ff13ac8d07be27 (cherry picked from commit 94dbb5b7e7e14a663dc02ecf5013fad10b8701b3)
Summary: Pull Request resolved: pytorch/pytorch#72124 Reviewed By: bdhirsh Differential Revision: D34215708 Pulled By: ansley fbshipit-source-id: fee887e331cb8bd9fab3d9d958ff13ac8d07be27 (cherry picked from commit 94dbb5b7e7e14a663dc02ecf5013fad10b8701b3)
No description provided.