[inductor] More fixes on the keys of constants and signature dictionaries#135406
[inductor] More fixes on the keys of constants and signature dictionaries#135406Jokeren wants to merge 8 commits intopytorch:mainfrom
constants and signature dictionaries#135406Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/135406
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit a0527af with merge base 21a64d5 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
constants and signature dictionaries
|
@pytorchbot label "module: inductor" |
|
@pytorchbot label "topic: not user facing" |
shunting314
left a comment
There was a problem hiding this comment.
Would there be any backward compatibility issue?
i.e., if PyTorch is running with an older version of triton, with triton still handle the new metadata format properly?
Are you asking about whether old triton could handle the new format? I think it happened about 6 months ago. |
|
Seems fine, though maybe we need some sort of triton version check. |
|
I think there also need to be some tests on the triton side to check the key type of the dictionaries. I'll go add them soon. |
|
Triton PR: triton-lang/triton#4693 |
|
Can someone help me kick off the CI again? Thanks |
|
Is it good to merge? I probably don't have the permission to ask the pytorchbot to merge |
|
@pytorchbot merge |
Merge startedYour 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 |
|
@pytorchbot revert -m "Reverting as it is breaking triton_mtia internal signals @jansel could you have a look and help get those changes merged?" -c ghfirst |
|
@pytorchbot successfully started a revert job. Check the current status here. |
…re` dictionaries (#135406)" This reverts commit e54b559. Reverted #135406 on behalf of https://github.com/jeanschmidt due to Reverting as it is breaking triton_mtia internal signals @jansel could you have a look and help get those changes merged? ([comment](#135406 (comment)))
|
@Jokeren your PR has been successfully reverted. |
This PR was reopened (likely due to being reverted), so your approval was removed. Please request another review.
|
@jfix71 how hard would it be to update MTIA Triton to use this new API? |
|
@jansel I think it should be a pretty reasonable change, but we'd want to import the PR and update the diff internally to tag on our changes before landing, if that's a possibility. What's the standard workflow expected for these sorts of changes, so we don't have to fix forward? |
We fixed MTIA yesterday check D62887579and D62896747. Changes should be landed in fresh master. Could you please try recreate fbsource Phabricator diff out of this pr? It should be all green for you, I tested it on your changes from this Github PR D62888678. |
|
@Myrthan has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
…ionaries (pytorch#135406) Previous PR forgets to change two other places that also create `constants` and `signature`. pytorch#135170 Pull Request resolved: pytorch#135406 Approved by: https://github.com/jansel
…re` dictionaries (pytorch#135406)" This reverts commit e54b559. Reverted pytorch#135406 on behalf of https://github.com/jeanschmidt due to Reverting as it is breaking triton_mtia internal signals @jansel could you have a look and help get those changes merged? ([comment](pytorch#135406 (comment)))
|
@jfix71 cause there was no update for some time, do you think it is wise to use "@ pytorchbot merge"? will the bot merge it on top of fresh changes? Or this PR has to be rebased first? Otherwise I have to recreate the new github PR, but I don't want to take the ownership from @Jokeren Thanks. It's fine to create a new PR. In terms of tracing issues in the future if needed, you can just add me as a co-author in the PR so that I will be notified. |
Great, let me do this right now. |
|
Thanks. I think it should work |
|
#136514 has been merged, we can abandon this PR I believe. Thank you! |
|
Great. Thanks! |
Previous PR forgets to change two other places that also create
constantsandsignature. #135170cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @ipiszy @yf225 @chenyang78 @kadeng @muchulee8 @ColinPeppler @amjames @desertfire @chauhang