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
FX: ensure node stack trace survives copying #69368
Conversation
Summary: Before this PR, copying a node would lose the stack trace. This PR ensures that the stack trace is preserved across copies. This is useful because quantization passes would like to start allowing the user to preserve stack traces, and we use the copy behavior. Test Plan: ``` python test/test_fx.py TestFX.test_stack_traces ``` Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]
CI Flow Status⚛️ CI FlowRuleset - Version:
You can add a comment to the PR and tag @pytorchbot with the following commands: # ciflow rerun, "ciflow/default" will always be added automatically
@pytorchbot ciflow rerun
# ciflow rerun with additional labels "-l <ciflow/label_name>", which is equivalent to adding these labels manually and trigger the rerun
@pytorchbot ciflow rerun -l ciflow/scheduled -l ciflow/slow For more information, please take a look at the CI Flow Wiki. |
🔗 Helpful links
💊 CI failures summary and remediationsAs of commit 594409c (more details on the Dr. CI page):
ci.pytorch.org: 1 failedThis comment was automatically generated by Dr. CI (expand for details).Please report bugs/suggestions to the (internal) Dr. CI Users group. |
Summary: Before this PR, copying a node would lose the stack trace. This PR ensures that the stack trace is preserved across copies. This is useful because quantization passes would like to start allowing the user to preserve stack traces, and we use the copy behavior. Test Plan: ``` python test/test_fx.py TestFX.test_stack_traces ``` Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 0a077d9068f4d0917e50495a2256a1bea372b590 Pull Request resolved: #69368
@vkuzo has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
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.
Approving, but I would encourage you to consider my comment
torch/fx/graph.py
Outdated
@@ -747,6 +747,7 @@ def node_copy(self, node: Node, arg_transform: Callable[[Node], 'Argument'] = la | |||
assert isinstance(kwargs, dict) | |||
result_node = self.create_node(node.op, node.target, args, kwargs, node.name, node.type) | |||
result_node.meta = copy.copy(node.meta) | |||
result_node.stack_trace = copy.copy(node.stack_trace) |
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.
Can we move the stack trace into meta
instead?
Summary: Before this PR, copying a node would lose the stack trace. This PR ensures that the stack trace is preserved across copies. This is useful because quantization passes would like to start allowing the user to preserve stack traces, and we use the copy behavior. Test Plan: ``` python test/test_fx.py TestFX.test_stack_traces ``` Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D32835248](https://our.internmc.facebook.com/intern/diff/D32835248) [ghstack-poisoned]
@vkuzo has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Summary: Before this PR, copying a node would lose the stack trace. This PR ensures that the stack trace is preserved across copies. This is useful because quantization passes would like to start allowing the user to preserve stack traces, and we use the copy behavior. Test Plan: ``` python test/test_fx.py TestFX.test_stack_traces ``` Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D32835248](https://our.internmc.facebook.com/intern/diff/D32835248) [ghstack-poisoned]
Summary: Before this PR, copying a node would lose the stack trace. This PR ensures that the stack trace is preserved across copies. This is useful because quantization passes would like to start allowing the user to preserve stack traces, and we use the copy behavior. Test Plan: ``` python test/test_fx.py TestFX.test_stack_traces ``` Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 63be485a56141ebccd278d89214553555bbe2e26 Pull Request resolved: #69368
@vkuzo has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Summary: Pull Request resolved: #69368 Before this PR, copying a node would lose the stack trace. This PR ensures that the stack trace is preserved across copies. This is useful because quantization passes would like to start allowing the user to preserve stack traces, and we use the copy behavior. Test Plan: ``` python test/test_fx.py TestFX.test_stack_traces ``` Imported from OSS Reviewed By: jamesr66a Differential Revision: D32835248 fbshipit-source-id: 91610fd8d05f5683cfa5e11fb6f9f3feacb8e241
Summary: Pull Request resolved: #69368 Before this PR, copying a node would lose the stack trace. This PR ensures that the stack trace is preserved across copies. This is useful because quantization passes would like to start allowing the user to preserve stack traces, and we use the copy behavior. Test Plan: ``` python test/test_fx.py TestFX.test_stack_traces ``` Imported from OSS Reviewed By: jamesr66a Differential Revision: D32835248 fbshipit-source-id: 91610fd8d05f5683cfa5e11fb6f9f3feacb8e241
Stack from ghstack:
Summary:
Before this PR, copying a node would lose the stack trace. This PR
ensures that the stack trace is preserved across copies.
This is useful because quantization passes would like to start
allowing the user to preserve stack traces, and we use the copy
behavior.
Test Plan:
Reviewers:
Subscribers:
Tasks:
Tags:
Differential Revision: D32835248