[dynamo] clarify graph break handling/logging in symbolic_convert #166587
Conversation
[ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/166587
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit cbe5f15 with merge base 7a0cd8e ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
…convert" cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames Lucaskabela [ghstack-poisoned]
…convert" cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames Lucaskabela [ghstack-poisoned]
test/dynamo/test_error_messages.py
Outdated
| """\ | ||
| Graph break in user code at test_error_messages.py:N | ||
| Graph Break Reason: STORE_ATTR-caused graph break | ||
| Graph Break Reason: Encountered graph break when attempting to store an object's attribute: |
There was a problem hiding this comment.
can we add reference to bytecode instr at the end of this reason? (STORE_ATTR)
This just makes it very attention grabbing/clear imo
| frame_loc[1], | ||
| reason, | ||
| ) | ||
| # [NOTE] graph break handling in symbolic_convert |
There was a problem hiding this comment.
Love this block comment here :) Any reason it is a serious of individual line comments as opposed to
block?
There was a problem hiding this comment.
Are you referring to why I'm not using triple quotes? I believe it's not recommended to use triple quotes for docs unless we're writing doc strings for functions.
| extra_msg: str = "", | ||
| ) -> None: | ||
| log_graph_break( | ||
| assert self.should_compile_partial_graph() |
There was a problem hiding this comment.
Good catch on the ordering here
…convert" cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames Lucaskabela [ghstack-poisoned]
|
Rebased |
|
@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 |
Stack from ghstack (oldest at bottom):
cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @chenyang78 @kadeng @chauhang @amjames @Lucaskabela