Refactor ShapeGuardPrinter for future C++ addiiton #140968
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/140968
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (2 Unrelated Failures)As of commit dd4c1da with merge base d0fd42e ( BROKEN TRUNK - The following job failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
UNSTABLE - The following job failed but was likely due to flakiness present on trunk and has been marked as unstable:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
anijain2305
left a comment
anijain2305
left a comment
There was a problem hiding this comment.
Refactor LGTM. cc @ezyang for extra eyes.
|
I only skimmed but it doesn't look obviously wrong |
|
The downside to doing it this way is you will have to add call sites for C++ printing everywhere again. But maybe it's not so bad if there are only two. |
|
I had the same thought as first, but the diff is not that bad. See https://github.com/pytorch/pytorch/pull/139899/files#diff-03557db7303b8540f095b4f0d9cd2280e1f42f534f67d8695f756ec6c02d3ec7L4406-R5159 |
|
@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 |
Pull Request resolved: pytorch#140968 Approved by: https://github.com/anijain2305 ghstack dependencies: pytorch#140597
ghstack-source-id: d5ccc51 Pull Request resolved: pytorch/pytorch#140968
6 participants
Stack from ghstack (oldest at bottom):
cc @ezyang @SherlockNoMad @EikanWang @jgong5 @wenzhe-nrv @voznesenskym @penguinwu @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @jiayisunx @chenyang78 @kadeng @chauhang @amjames