8000 [inductor][dynamo] Include operator name in size/stride/alignment assertion by karthickai · Pull Request #152353 · pytorch/pytorch · GitHub
[go: up one dir, main page]
Skip to content

[inductor][dynamo] Include operator name in size/stride/alignment assertion #152353

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

[inductor][dynamo] Include operator name in size/stride/alignment assertion #152353

wants to merge 14 commits into from

Conversation

karthickai
Copy link
Collaborator
@karthickai karthickai commented Apr 28, 2025 •
edited by pytorch-bot bot
Loading

Fixes #151930

This PR updates the assert_size_stride and assert_alignment functions in guards.cpp to accept an optional op_name argument and includes it in the error messages.

The corresponding type stubs in guards.pyi are updated to match the new function arg.

In inductor/ir.py extracts the operator name from the FX graph and passes it into the codegen_size_asserts and codegen_alignment_asserts functions, so that generated assertions in Triton code include the op name for better debugging.

Added unit tests inside test_torchinductor.py.

  • Verified both successful and failing assertion cases include the operator name.
  • Verified that generated Triton code contains the op name inside the asserts.

cc @H-Huang @awgu @wanchaol @fegin @fduwjj @wz337 @wconstab @d4l3k @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @ipiszy @chenyang78 @kadeng @muchulee8 @amjames @chauhang @aakhundov @shunting314 @eellison

@pytorch-bot PyTorch Bot
Copy link
pytorch-bot bot commented Apr 28, 2025 •
edited
Loading

🔗 Helpful Links

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

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

❌ 1 New Failure

As of commit 14bfc7d with merge base c5ebc12 (image):

NEW FAILURE - The following job has failed:

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

@linux-foundation-easycla Linux Foundation: EasyCLA
Copy link
linux-foundation-easycla bot commented Apr 28, 2025 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@karthickai
Copy link
Collaborator Author

@pytorchbot label "topic: not user facing"

@pytorch-bot pytorch-bot bot added the topic: not user facing topic category label Apr 28, 2025
@jerryzh168 jerryzh168 added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Apr 29, 2025
jansel
jansel requested changes Apr 29, 2025
View reviewed changes
@jansel
Copy link
Contributor
jansel commented Apr 29, 2025

Also looks like lints are failing.

@karthickai
Copy link
Collaborator Author

Thank you @jansel, I’ve pushed updates with your suggestions.

shunting314
shunting314 previously approved these changes Apr 29, 2025
View reviewed changes
@shunting314
Copy link
Contributor

The lint job still fails. Not sure if this is some transient error though

@jansel
Copy link
Contributor
jansel commented Apr 29, 2025

You should be able to fix the lints with:

make setup_lint
lintrunner -a

@karthickai
Copy link
Collaborator Author

@jansel @shunting314 I believe the lint error is now resolved with the latest commit.

jansel
jansel previously approved these changes Apr 30, 2025
View reviewed changes
@jansel
Copy link
Contributor
jansel commented Apr 30, 2025

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Apr 30, 2025
@pytorchmergebot
Copy link
Collaborator

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
Copy link
Collaborator

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

Dig deeper by viewing the failures on hud

Details for Dev Infra team Raised by workflow job

Failing merge rule: Core Maintainers

@karthickai
Copy link
Collaborator Author

@pytorchbot label "skip-url-lint"

@pytorch-bot pytorch-bot bot added skip-url-lint and removed ciflow/trunk Trigger trunk jobs on your pull request labels May 1, 2025
@karthickai
Copy link
Collaborator Author

cc @jansel @shunting314. I've added the skip-url-lint label to bypass transient outages in the Lint link check. In the latest commit, I updated existing tests to reflect recent changes in assert_size_stride and fixed the get_op_name to avoid duplicate namespace segments when combining op_namespace and target.

jansel
jansel requested changes May 1, 2025
View reviewed changes
Copy link
Contributor
@jansel jansel left a comment

Choose a reason for hiding this comment

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

Test failures?

@karthickai
Copy link
Collaborator Author

@pytorchbot label ciflow/inductor ciflow/trunk

@pytorch-bot pytorch-bot bot added ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request labels May 13, 2025
@pytorch-bot pytorch-bot bot added oncall: distributed Add this issue/PR to distributed oncall triage queue ciflow/inductor and removed ciflow/trunk Trigger trunk jobs on your pull request ciflow/inductor labels May 14, 2025
@karthickai
Copy link
Collaborator Author

@pytorchbot label ciflow/inductor ciflow/trunk

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label May 14, 2025
@karthickai
Copy link
Collaborator Author

@pytorchbot label ciflow/trunk

@jansel
Copy link
Contributor
jansel commented May 14, 2025

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

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
Copy link
Collaborator

Merge failed

Reason: 1 jobs have failed, first few of them are: trunk / linux-jammy-rocm-py3.10 / test (default, 1, 2, linux.rocm.gpu.2)

Details for Dev Infra team Raised by workflow job

@jansel
Copy link
Contributor
jansel commented May 15, 2025

@pytorchbot merge -i

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged while ignoring the following 1 checks: trunk / linux-jammy-rocm-py3.10 / test (default, 1, 2, linux.rocm.gpu.2)

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

@jeanschmidt
Copy link
Contributor

@pytorchbot revert -m "seems to have broken a few internal tests, @jansel may you help the author get his PR merged?" -c ghfirst

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

pytorchmergebot added a commit that referenced this pull request May 16, 2025
@pytorchmergebot
Revert "[inductor][dynamo] Include operator name in size/stride/align…
4d073af 
…ment assertion (#152353)"

This reverts commit 725bbb6.

Reverted #152353 on behalf of https://github.com/jeanschmidt due to seems to have broken a few internal tests, @jansel may you help the author get his PR merged? ([comment](#152353 (comment)))
@pytorchmergebot
Copy link
Collaborator

@karthickai your PR has been successfully reverted.

@pytorch-bot pytorch-bot bot dismissed jansel’s stale review May 16, 2025 08:20

This PR was reopened (likely due to being reverted), so your approval was removed. Please request another review.

@jeanschmidt
Copy link
Contributor

Forgot to share internal ref: D74804066

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jansel jansel jansel left review comments

@shunting314 shunting314 shunting314 left review comments

@anijain2305 anijain2305 Awaiting requested review from anijain2305

Assignees
No one assigned
Labels
ci-no-td Do not run TD on this PR ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request keep-going Don't stop on first failure, keep running tests until the end Merged module: dynamo module: inductor oncall: distributed Add this issue/PR to distributed oncall triage queue open source Reverted skip-url-lint topic: not user facing topic category triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add operator name to the size/strides/alignment assertion
8 participants
@karthickai @jansel @shunting314 @pytorchmergebot @huydhn @jeanschmidt @jerryzh168 @pytorchbot
0