-
Couldn't load subscription status.
- Fork 701
[et] generate debug handle before opeartor decomposition #11997
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
Changes from 4 commits
3a66602
641e9b5
5fad521
b51110f
01c49fd
535659e
588a868
dba90f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,31 +4,75 @@ | |
| # This source code is licensed under the BSD-style license found in the | ||
| # LICENSE file in the root directory of this source tree. | ||
|
|
||
| from typing import Dict | ||
|
|
||
| from executorch.exir.graph_module import bfs_trace_with_node_process | ||
| from executorch.exir.pass_base import ExportPass | ||
| from torch.export import ExportedProgram | ||
| from torch.fx import GraphModule | ||
| from torch.fx import GraphModule, Node | ||
| from torch.fx.passes.infra.pass_base import PassResult | ||
|
|
||
|
|
||
| class DebugHandleGeneratorPass(ExportPass): | ||
| def call(self, graph_module: GraphModule) -> PassResult: | ||
| """Lower a quantized reference model (with reference quantized operator patterns) | ||
| to executorch backend, that has a canonical set of quantized operators | ||
| """Generate debug handles for each node in the graph module and its submodule except | ||
| placeholder and output nodes. The debug handle is generated starting from 1 and | ||
| incrementally. The debug handle of a node is the same as the node sharing the same | ||
| greatest ancestor node in the export flow. | ||
| """ | ||
|
|
||
| index = 1 | ||
| FROM_NODE_KEY = "from_node" | ||
| DEBUG_HANDLE_KEY = "debug_handle" | ||
|
|
||
| source_node_id_to_debug_handle: Dict[str, int] = {} | ||
|
|
||
| def _get_greatest_ancestor_node_identifier(node: Node) -> str: | ||
| """Get the identifier of the greatest ancestor node of the given node. | ||
| The identifier is the concatenation of the node name and graph id of the | ||
| greatest ancestor node, where the graph id is the unique id for every graph | ||
| module in the export flow and node name is unique within the same graph module. | ||
| """ | ||
|
|
||
| node_source = node.meta[FROM_NODE_KEY] | ||
| node_source = node_source[-1] | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is only populated for unlifted graph when There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no it is not only populate that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There in my question around to_edge_transform_and_lower. although that specifically is unrelated to this pr |
||
|
|
||
| while len(node_source.from_node) > 0: | ||
| node_source = node_source.from_node[-1] | ||
|
|
||
| return node_source.name + str(node_source.graph_id) | ||
|
|
||
| def _extract_debug_handles_from_node(node: Node) -> None: | ||
| """ | ||
| Generate a debug handle based on node's oldest ancestor node's name | ||
| and graph id, or return None if the node does not need to be traced. | ||
| """ | ||
|
|
||
| if node.op == "placeholder" or node.op == "output": | ||
| # placeholder and output nodes don't have debug handle | ||
| return | ||
|
|
||
| assert ( | ||
| FROM_NODE_KEY in node.meta | ||
| ), f"Node {node} does not have meta key {FROM_NODE_KEY}" | ||
|
|
||
| greatest_ancestor_node_id = _get_greatest_ancestor_node_identifier(node) | ||
|
|
||
| debug_handle = ( | ||
| len(source_node_id_to_debug_handle) + 1 | ||
| if greatest_ancestor_node_id not in source_node_id_to_debug_handle | ||
| else source_node_id_to_debug_handle[greatest_ancestor_node_id] | ||
| ) | ||
|
Comment on lines
+61
to
+65
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok one of my concern is this: is this gonna impact any existing debug utils that rely on debug handle? earlier each node had its own debug handle, but not we are giving a bunch of nodes that might belong to the same original node (e.g. full aten graph nodes?). Other question is how does this work for delegates that use to_edge_tranform_and_lower, if this pass is run only after to_edge? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For first question, yes like devtool and some other infra or utils are depending on the debgu handle and in the following diffs i will do the migration. There're some testcases rely on debug handle for check and I've talked with the owners and they are ok with the update. Now ci is all clean. For second question, not sure if I fully understand your concern, but our current debug handle generation pass also locates at the end of to_edge. Our update does not change the location. But we do have concern about whether or not the change of debug handle generation may impact the correctness of the debug handels in the downstream e.g. in delegate, though ci is all clean. And we will pay attention to it when working on the numeric descrepancy dtector on delegated model. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually we should discuss this second paragraph further. THe issue is that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the first paragraph, do you forsee any disruption in existing user flows for debugging? That would be one thing that would make me block this PR, so having an answer on that would be nice There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
No operator decomposition should be happened inside
no. One heart break context is no debug tool is actually using debug handle right now. The numerical discrepancy detector will be the first one. |
||
|
|
||
| def _extract_debug_handles_from_node(node): | ||
| nonlocal index | ||
| node.meta["debug_handle"] = index | ||
| index += 1 | ||
| source_node_id_to_debug_handle[greatest_ancestor_node_id] = debug_handle | ||
| node.meta[DEBUG_HANDLE_KEY] = debug_handle | ||
|
|
||
| bfs_trace_with_node_process(graph_module, _extract_debug_handles_from_node) | ||
|
|
||
| return PassResult(graph_module, True) | ||
|
|
||
|
|
||
| # TODO(gasoonjia): generate missing debug handles using `from_node` info | ||
| def generate_missing_debug_handles(ep: ExportedProgram): | ||
| """ | ||
| This pass is used to generate missing debug handles for the graph module and its submodules. | ||
|
|
||
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.
Put before and after state
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.
i've updated the comment to reflect the function behavior before, during and after the migration.