-
Notifications
You must be signed in to change notification settings - Fork 25.7k
Grab bag of (mostly) typing improvements #158075
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
Grab bag of (mostly) typing improvements #158075
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/158075
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit c00bba8 with merge base 70b4a88 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
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.
Typing nits
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.
Once these nits are resolved. Looks good.
|
BC lint is failing on updating some existing types from |
|
With this amount of test failures, I'm going to back out the changes to |
|
@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 |
Summary: Collects some scattershot improvements made while attempting to enable training for AOTInductor. Non-typing changes are: 1. Swapping a few custom searches for the output node in an FX graph for calling `graph.output_node()`. 2. Removing two unused parameters from `torch.export._unlift._unlift`. 3. Switching handles to constants in `cpp_wrapper_cpu` to use C++ references for memory efficiency. 4. Cleaning out unused, unexported imports from `torch/export/__init__.py`, and adding one missing export to `__all__`. X-link: pytorch/pytorch#158075 Approved by: https://github.com/Skylion007 Reviewed By: ZainRizvi Differential Revision: D78691998 fbshipit-source-id: cdd51fe27cef89786ac7728775c103f5c11fcb1b
Stack from ghstack (oldest at bottom):
Collects some scattershot improvements made while attempting to enable training for AOTInductor. Non-typing changes are:
graph.output_node().torch.export._unlift._unlift.cpp_wrapper_cputo use C++ references for memory efficiency.torch/export/__init__.py, and adding one missing export to__all__.cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @ipiszy @chenyang78 @kadeng @muchulee8 @amjames @chauhang @aakhundov @coconutruben @Lucaskabela