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

skip triton template precompilation in 311.0-3.11.7 to workaround 311 cpython bug #125446

Closed
wants to merge 3 commits into from

Conversation

eellison
Copy link
Contributor

@eellison eellison commented May 2, 2024

Stack from ghstack (oldest at bottom):

Fix for #125374. We dont have CI for this specific versions, but I verified locally. THere is a cpython bug from 3.11.0->3.11.7 where the ast parsing state is global, and errors with multiple threads. when dust settles a little around the new process based compilation we can look into migrating.

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

Copy link

pytorch-bot bot commented May 2, 2024

🔗 Helpful Links

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

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

✅ No Failures

As of commit 92efd2e with merge base 1bcbc91 (image):
💚 Looks good so far! There are no failures yet. 💚

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

eellison added a commit that referenced this pull request May 2, 2024
ghstack-source-id: 46d53133c782a2fbfa74dfc9b8ffc256db2a91ae
Pull Request resolved: #125446
@eellison eellison changed the title skip precompilation in 311.0-3.11.7 to workaround 311 cpython bug skip triton template precompilation in 311.0-3.11.7 to workaround 311 cpython bug May 2, 2024
…karound 311 cpython bug"


Fix for #125374. We dont have CI for this specific versions, but I verified locally. THere is a cpython bug from 3.11->3.17 where the ast parsing state is global, and errors with multiple threads. when dust settles a little around the new process based compilation we can look into migrating. 


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

[ghstack-poisoned]
eellison added a commit that referenced this pull request May 2, 2024
ghstack-source-id: 1a256ddbc98ec5714140865516fab6aee0e70d8e
Pull Request resolved: #125446
@eellison eellison requested review from Chillee and cpuhrsch May 2, 2024 23:47
@@ -64,6 +64,14 @@ class KernelNamespace:
extern_kernels = KernelNamespace()


@functools.lru_cache(None)
def warn_ast_parse_3_11():
log.warning(
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we actually need a warning here? It doesn't matter that much right - only some compilation time improvement? In that case, I think most users wouldn't care.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now that we have a ton more mm configs, it actually matters a good amount.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd lean towards not having it (i think warning spew is really bad), but if you really want a warning, at least mention what's going to happen if they don't upgrade.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, fair. I'll remove and then after this is landed try to add a workaround in triton.

…karound 311 cpython bug"


Fix for #125374. We dont have CI for this specific versions, but I verified locally. THere is a cpython bug from 3.11.0->3.11.7 where the ast parsing state is global, and errors with multiple threads. when dust settles a little around the new process based compilation we can look into migrating.  


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

[ghstack-poisoned]
eellison added a commit that referenced this pull request May 3, 2024
ghstack-source-id: 15b720c5c7e6fb9c0e9a0901313065c32074565f
Pull Request resolved: #125446
@eellison
Copy link
Contributor Author

eellison 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
@eellison eellison added topic: not user facing topic category and removed ciflow/trunk Trigger trunk jobs on your pull request labels May 3, 2024
@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

@github-actions github-actions bot deleted the gh/eellison/640/head branch June 5, 2024 01:53
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.

None yet

3 participants