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

[Inductor] Properly package target info for triton.compile #125241

Closed
wants to merge 1 commit into from

Conversation

alexbaden
Copy link
Contributor

@alexbaden alexbaden commented Apr 30, 2024

Triton updated the interface for triton.compile triton-lang/triton@5162346

The target argument to compile needs to be wrapped in a GPUTarget object. Without proper wrapping, we hit an assert in compile. If that assert is removed, Triton attempts to read device info from Torch while inside a torch thread, which hits an in bad fork assert. This change is required for compatibility with latest commits in Triton. The implementation is backwards compatible, so existing versions of Triton that work now continue to work.

cc @ezyang @msaroufim @bdhirsh @anijain2305 @chauhang @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @peterbell10 @ipiszy @yf225 @chenyang78 @kadeng @muchulee8 @ColinPeppler @amjames @desertfire

Copy link

pytorch-bot bot commented Apr 30, 2024

🔗 Helpful Links

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

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

✅ No Failures

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

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

Copy link

linux-foundation-easycla bot commented Apr 30, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: alexbaden / name: Alex Baden (aeabcb2)

@cpuhrsch cpuhrsch requested a review from jansel April 30, 2024 19:53
@cpuhrsch cpuhrsch added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Apr 30, 2024
@jansel
Copy link
Contributor

jansel commented May 3, 2024

@pytorchbot merge

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

Merge failed

Reason: This PR needs a release notes: label
If your changes are user facing and intended to be a part of release notes, please use a label starting with release notes:.

If not, please add the topic: not user facing label.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "topic: not user facing"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Details for Dev Infra team Raised by workflow job

@alexbaden
Copy link
Contributor Author

@pytorchbot label "topic: not user facing"

@pytorch-bot pytorch-bot bot added the topic: not user facing topic category label May 4, 2024
@alexbaden
Copy link
Contributor Author

@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

@alexbaden alexbaden deleted the alex/GPUTarget_inductor branch May 4, 2024 14:25
@huydhn
Copy link
Contributor

huydhn commented May 4, 2024

@pytorchbot revert -m 'Sorry for reverting your change but it is failing inductor tests on ROCm https://hud.pytorch.org/pytorch/pytorch/commit/8a1af95b0979d85c4fe32a75e797323ad81f298d' -c nosignal

I will add ciflow/inductor to run the test on your PR

@huydhn
Copy link
Contributor

huydhn commented May 4, 2024

Oh, the branch has been deleted, so if you try to reland the change in a different PR, please add ciflow/inductor.

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

@alexbaden your PR has been successfully reverted.

pytorchmergebot added a commit that referenced this pull request May 4, 2024
@alexbaden alexbaden restored the alex/GPUTarget_inductor branch May 4, 2024 22:38
alexbaden added a commit to alexbaden/pytorch that referenced this pull request May 4, 2024
…25241)

Triton updated the interface for `triton.compile` triton-lang/triton@5162346

The `target` argument to compile needs to be wrapped in a `GPUTarget` object. Without proper wrapping, we hit an assert in `compile`. If that assert is removed, Triton attempts to read device info from Torch while inside a torch thread, which hits an in bad fork assert. This change is required for compatibility with latest commits in Triton. The implementation is backwards compatible, so existing versions of Triton that work now continue to work.

Pull Request resolved: pytorch#125241
Approved by: https://github.com/jansel
@huydhn
Copy link
Contributor

huydhn commented May 5, 2024

I think I might revert this wrongly as the failure still occurs after the revert https://hud.pytorch.org/pytorch/pytorch/commit/6d30803d64953955df63da56833bf4eb52249aae. Let me know when you want to reland the change, I can stamp and land it.

@alexbaden
Copy link
Contributor Author

Do you want me to open a new PR? GitHub is not letting me reopen this one, possibly because the original branch was deleted.

@huydhn
Copy link
Contributor

huydhn commented May 5, 2024

Do you want me to open a new PR? GitHub is not letting me reopen this one, possibly because the original branch was deleted.

Creating a new PR is ok.

alexbaden added a commit to alexbaden/pytorch that referenced this pull request May 5, 2024
…25241)

Triton updated the interface for `triton.compile` triton-lang/triton@5162346

The `target` argument to compile needs to be wrapped in a `GPUTarget` object. Without proper wrapping, we hit an assert in `compile`. If that assert is removed, Triton attempts to read device info from Torch while inside a torch thread, which hits an in bad fork assert. This change is required for compatibility with latest commits in Triton. The implementation is backwards compatible, so existing versions of Triton that work now continue to work.

Pull Request resolved: pytorch#125241
Approved by: https://github.com/jansel
@alexbaden
Copy link
Contributor Author

Re-submitted as #125553

pytorchmergebot pushed a commit that referenced this pull request May 6, 2024
Triton updated the interface for `triton.compile` triton-lang/triton@5162346

The `target` argument to compile needs to be wrapped in a `GPUTarget` object. Without proper wrapping, we hit an assert in `compile`. If that assert is removed, Triton attempts to read device info from Torch while inside a torch thread, which hits an in bad fork assert. This change is required for compatibility with latest commits in Triton. The implementation is backwards compatible, so existing versions of Triton that work now continue to work.

Re-submitting this after #125241 was reverted due to an unrelated CI issue.

Pull Request resolved: #125553
Approved by: https://github.com/huydhn
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request Merged module: inductor oncall: pt2 open source Reverted 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
Development

Successfully merging this pull request may close these issues.

None yet

6 participants