Qualcomm AI Engine Direct - Support Debug Handle and Integrate IntermediateOutputCapturer#16316
Qualcomm AI Engine Direct - Support Debug Handle and Integrate IntermediateOutputCapturer#16316winskuo-quic wants to merge 3 commits intopytorch:mainfrom
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/16316
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (1 Unrelated Failure)As of commit cc92ab4 with merge base 8e8d97e ( UNSTABLE - The following job is marked as unstable, possibly due to flakiness on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
Bot
added
the
CLA Signed
This PR needs a
|
|
Is the PR ready to be reviewed now? |
winskuo-quic
force-pushed
the
dev1/winskuo/debug_handle
branch
2 times, most recently
from
9a7ca59 to
dc72614
Compare
Hi @Gasoonjia, |
|
Hi @cccclai, @Gasoonjia, @kimishpatel, Also, I would also like to get some suggestions on the official API to retrieve an edge IR. The current way of retrieving an edge IR is through: executorch/examples/qualcomm/utils.py Line 499 in 0fb422f However, I encountered following issues when retrieving edge IR using the above method.
Thanks |
|
I think instead of using the edge graph IR as the ground truth for comparsion, it will be great if we can use the export program ET stack get at the first place (e.g. the export graph of executorch/examples/qualcomm/utils.py Line 480 in 0fb422f You can see how we calculate intermediate output numercal descrepancy: executorch/devtools/inspector/_inspector.py Line 1407 in 0fb422f https://github.com/pytorch/executorch/blob/0fb422f9c59e0e5526c0082352a583baf0510fb7/exir/passes/debug_handle_generator_pass.py here's pass for debug handle generation, where the debug handle of a node is the same as the node sharing the same greatest ancestor node in the export flow. |
|
Here's an example of how our current API works on VIT model on xnnpack backend: https://gist.github.com/Gasoonjia/db6285ac39ad5759b95c7a92d37cd4f8 and below is the expected output. For some ops like layernorm there're still some issue i need to fix.
I would love to chat with you regarding how we can make the pipeline works on qualcomm backend! |
winskuo-quic
force-pushed
the
dev1/winskuo/debug_handle
branch
from
dc72614 to
00c4f7e
Compare
|
Hi @Gasoonjia, |
| def call(self, graph_module: torch.fx.GraphModule): | ||
| handle_counter = 1 | ||
| visited = set() | ||
| for node in graph_module.graph.nodes: |
There was a problem hiding this comment.
not sure if Qualcomm can handle conditional graph. If so i think the way you are adding debug handle might not able to equip debug handle to all branches. You can follow what i'm doing here:
https://github.com/pytorch/executorch/blob/main/exir/passes/debug_handle_generator_pass.py#L14
There was a problem hiding this comment.
I would like to confirm on definition of conditional graph. Initially, I thought we can't have conditional graph (e.g., if else) in Aten. The only way to do it is through operations such as where op, which still makes it a single graph. We actually use a lot of for loop for graph traversing in our passes. If you find this being a concern, please let me know and I'll take a look at all our other passes.
There was a problem hiding this comment.
conditiion graph refers to some branch like torch.cond. https://github.com/pytorch/executorch/blob/main/exir/tests/test_passes.py#L1275 is an example.
For conditional graph like torch.cond we will capture the graph for both branch.
There was a problem hiding this comment.
Thanks for sharing the operation. I think we currently does not have this op. I think I can add a TODO or some checks for now and actually made the change if QNN supports this operation in future. This will probably require a bigger refactor since all our passes are now using for loops.
| assert ( | ||
| source_node.name in visited | ||
| ), "Graph is not traversed in topological order, unexpected behavior." | ||
| node.meta[QCOM_DEBUG_HANDLE] = source_node.meta[QCOM_DEBUG_HANDLE] |
There was a problem hiding this comment.
Curious why do we need to set the get_item node the same debug handle as the soruce node? Since it will introduce duplicate debug handle in the graph and im a little bit worried if it could cause any issue in the down stream,
There was a problem hiding this comment.
I am actually just following the same behavior prior to this pass. For node's that has multi-output, it will have get_item node as users. I noticed that the debug_handle for all get_item node and node itself all have the same debug_handle, as shown below.
There was a problem hiding this comment.
I feel like the reason of get_item and its user sharing the same debug handle is because of exporting in strict mode and both aten_topk and its get_item function are from same source.
For here i don't have strong preference of whether or not we should make get_item's debug handle the same as source node, as long as it can support Qualcomm's requirement.
| tensor_name = f"{node.name}_{wrapper_idx}" | ||
|
|
||
| # Only append special namings when enable tensor dump, since longer name results bigger .pte | ||
| if (handle_id := node.meta.get(QCOM_DEBUG_HANDLE)) and self.enable_tensor_dump: |
There was a problem hiding this comment.
wondering if we still need this file since we will migrate to devtool infra?
There was a problem hiding this comment.
I believe this can be eventually deprecated as we discussed offline. However, since currently we don't have an official API to bring debug_handle to runtime, we will need to stick with parsing tensor name during runtime to extract debug_handle ID.
There was a problem hiding this comment.
oh will the tensor name here be part of runtime?
There was a problem hiding this comment.
During runtime, we will get tensor name like relu__debugID_1, where 1 here is the debug handle. We use regex in runtime to parse the ID 1 and save it to ETDUMP as the key.
I agree we will migrate to devtool infra, but due to some of the limitations where we don't really have an official flow to pass debug_handle ID related info to runtime, we need to keep it this way so the debugger can still function after this PR is merged.
There was a problem hiding this comment.
Thx for detailed explanation! Mind share me the ptr of how you retreive the op name during runtime?
There was a problem hiding this comment.
Of course! Please refer to the message below.
winskuo-quic
force-pushed
the
dev1/winskuo/debug_handle
branch
from
00c4f7e to
7ec377f
Compare
| std::string qnn_tensor_name = | ||
| std::string(QNN_TENSOR_VER_PTR(output_tensor)->name); | ||
| if (std::regex_search(qnn_tensor_name, match, re)) { | ||
| debug_handle_id = static_cast<uint32_t>(std::stoul(match[1].str())); |
There was a problem hiding this comment.
Hi @Gasoonjia,
This is where we parse the qnn_tensor_name and get the debug_handle id.
winskuo-quic
force-pushed
the
dev1/winskuo/debug_handle
branch
from
403300d to
cc92ab4
Compare
|
Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as |
|
hi @winskuo-quic any related PR recently need my support or review? |
Hi @Gasoonjia, |
2 participants
Summary
Additional Topics:
Test plan
python backends/qualcomm/tests/test_qnn_delegate.py -k TestExampleUtilsScript.test_intermediate_debugger -s $DEVICE --model SM8650 --build_folder build-android/ --executorch_root . --image_dataset ../imagenet-mini/val/ --artifact ./e2e_test_debugpython backends/qualcomm/tests/test_qnn_delegate.py -k TestQNNQuantizedUtils.test_qnn_backend_dump_intermediate_outputs_simple_model --model SM8550 --device $DEVICE --build_folder build-androidpython backends/qualcomm/tests/test_qnn_delegate.py -k TestQNNQuantizedUtils.test_qnn_backend_dump_intermediate_outputs_topk --model SM8550 --device $DEVICE --build_folder build-android