-
Notifications
You must be signed in to change notification settings - Fork 26.3k
[dynamo] added github_cli to detect unimplemented_v2 calls #155610
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
[dynamo] added github_cli to detect unimplemented_v2 calls #155610
Conversation
[ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/155610
Note: Links to docs will display an error until the docs builds have been completed. ❌ 8 New Failures, 4 Cancelled Jobs, 1 Unrelated FailureAs of commit 8fd7623 with merge base 8a396c5 ( NEW FAILURES - The following jobs have failed:
CANCELLED JOBS - The following jobs were cancelled. Please retry:
BROKEN TRUNK - The following job failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
Some questions:
Note: I also have to test this after this PR gets merged to see if the checks work as intended. |
[ghstack-poisoned]
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.
Addressing your comments: we can follow inspiration from how the pull workflow runs (pull.yml), or the lint runs, or how my verification script verify_dynamo.py runs. I was imagining that we would do something more similar to the pull/lint workflows. We can get opinions from devinfra.
You can test by uploading a dummy test PR on top of this PR.
My Reply to comment: Ok yeah, the way it is set up right now is similar to pull.yml.
[ghstack-poisoned]
cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames [ghstack-poisoned]
cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames [ghstack-poisoned]
cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames [ghstack-poisoned]
cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames [ghstack-poisoned]
cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames [ghstack-poisoned]
cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames [ghstack-poisoned]
cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames [ghstack-poisoned]
|
@pytorchbot merge -f "Already went through testing" |
Merge startedYour change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
|
@williamwen42 @Sidharth123-cpu I'm seeing this test failing on many developer's PR since today: https://hud.pytorch.org/failure?name=Check%20unimplemented_v2%20calls%20%2F%20check-unimplemented-calls&jobName=check-unimplemented-calls&failureCaptures=ModuleNotFoundError%3A%20No%20module%20named%20%27torch%27 Could we have a fix on this? |
|
@zhxchen17 Thanks for bringing this up. Looking at the reason why and a fix for it right now. Also feel free to ignore the warning and continue. |
|
@pytorchbot revert -m "Looks like it fails on every pull request, based on https://github.com/pytorch/pytorch/actions/workflows/check-unimplemented-calls.yml, but it does not run on trunk" -c weird |
|
@pytorchbot successfully started a revert job. Check the current status here. |
…155610)" This reverts commit 5dd07c7. Reverted #155610 on behalf of https://github.com/malfet due to Looks like it fails on every pull request, based on https://github.com/pytorch/pytorch/actions/workflows/check-unimplemented-calls.yml, but it does not run on trunk ([comment](#155610 (comment)))
|
@Sidharth123-cpu your PR has been successfully reverted. |
malfet
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.
Please include someone from the dev infra team when adding a new workflow to the PyTorch repo. Namely majority of the workflows(except for service ones, like autolabeller) should be either opt in(i.e. run only on ciflow/ trigger), periodic or run on push to trunk.
This PR adds the workflow of whenever a dev makes a PR that contains files under torch/_dynamo, we check for any unimplemented_v2() callsites and if any of them have been modified in some sort of way, the workflow fails and lists them exactly which callsites and let's them know what the command lines are to update the registry.
Stack from ghstack (oldest at bottom):
cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @chenyang78 @kadeng @chauhang @amjames