-
Notifications
You must be signed in to change notification settings - Fork 683
Arm backend: Fix torch.matmul() failures for 2D tensor inputs #14624
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
Arm backend: Fix torch.matmul() failures for 2D tensor inputs #14624
Conversation
- ConvertMmToBmmPass converts an MM node to BMM nodes, turns input and output tensors from rank-2 to rank-3 via unsqueeze/squeeze, and inserts q-dq before and after BMM node when necessary. - After ConvertMmToBmmPass: x -> q -> dq -> unsqueeze -> q_2 -> dq_2 -> \ bmm -> q_4 -> dq_4 / y -> q_1 -> dq_1 -> unsqueeze -> q_3 -> dq_3 -> - Therefore, if the original matmul was 2D, the bmm already has DQ nodes on its inputs and Q node on its output. If AnnotateDecomposedMatmulPass (pytorch#10654) is still applied in this case, it produces illegal sequences such as: x -> q -> unsqueeze -> q_2 (invalid) - Fix by checking whether the BMM is already surrounded by DQ nodes on its inputs and Q nodes on its output. Change-Id: I9949d59b0b4a96fa34a88b0734014567ea6f24cc Signed-off-by: Yufeng Shi <yufeng.shi@arm.com> Co-authored-by: Oscar Andersson <oscar.andersson@arm.com>
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/14624
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 New Failure, 2 Unrelated FailuresAs of commit bb2fbb9 with merge base dcc3978 ( NEW FAILURE - The following job has failed:
BROKEN TRUNK - The following jobs failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
If possible, I suggest we get this fix into the 1.0 release branch |
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.
Thanks @YufengShi-dudu for the bug fix. Please mark the PR to be cherry-picked in 1.0. Thanks again.
@pytorchbot cherry-pick --onto release/1.0 -c regression |
- ConvertMmToBmmPass converts an MM node to BMM nodes, turns input and output tensors from rank-2 to rank-3 via unsqueeze/squeeze, and inserts q-dq before and after BMM node when necessary. - After ConvertMmToBmmPass: ``` x -> q -> dq -> unsqueeze -> q_2 -> dq_2 -> \ bmm -> q_4 -> dq_4 / y -> q_1 -> dq_1 -> unsqueeze -> q_3 -> dq_3 -> ``` - Therefore, if the original matmul was 2D, the bmm already has DQ nodes on its inputs and Q node on its output. If AnnotateDecomposedMatmulPass (#10654) is still applied in this case, it produces illegal sequences such as: x -> q -> unsqueeze -> q_2 (invalid) - Fix by checking whether the BMM is already surrounded by DQ nodes on its inputs and Q nodes on its output. Change-Id: I9949d59b0b4a96fa34a88b0734014567ea6f24cc cc @digantdesai @freddan80 @per @zingo @oscarandersson8218 Signed-off-by: Yufeng Shi <yufeng.shi@arm.com> Co-authored-by: Oscar Andersson <oscar.andersson@arm.com> (cherry picked from commit 9a7fb42)
Cherry picking #14624The cherry pick PR is at #14845 and it is recommended to link a regression cherry pick PR with an issue. The following tracker issues are updated: Details for Dev Infra teamRaised by workflow job |
ConvertMmToBmmPass converts an MM node to BMM nodes, turns input and output tensors from rank-2 to rank-3 via unsqueeze/squeeze, and inserts q-dq before and after BMM node when necessary.
After ConvertMmToBmmPass:
Therefore, if the original matmul was 2D, the bmm already has DQ nodes on its inputs and Q node on its output. If AnnotateDecomposedMatmulPass (Arm backend: Add support for single input matmul #10654) is still applied in this case, it produces illegal sequences such as: x -> q -> unsqueeze -> q_2 (invalid)
Fix by checking whether the BMM is already surrounded by DQ nodes on its inputs and Q nodes on its output.
Change-Id: I9949d59b0b4a96fa34a88b0734014567ea6f24cc
cc @digantdesai @freddan80 @per @zingo @oscarandersson8218