Fix bug in optimized mul's broadcast handling #11590
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/11590
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 2341203 with merge base c6c3616 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
facebook-github-bot
added
the
CLA Signed
This PR needs a
|
kernels/optimized/cpu/op_mul.cpp
Outdated
| // broadcasting rule dictate that the tensor whose dimensionality matches | ||
| // the output is the one that we should pick. | ||
| // e.g. a[6] and b[1, 1, 6] would imply out = [1, 1, 6], but if we resize | ||
| // via a we will crash |
There was a problem hiding this comment.
this is a good catch, but I think the original error was to use resize_tensor() in the first place when this op was updated to support broadcasting. The right function to use is resize_to_broadcast_target_size(a, b, out), which is already being used inside some of the if/else. This should be moved out and to the top, before the case work begins.
by the way, this same error is now present in your other PR for le
Summary: When we have two tensors that match exactly in size but not in dims, the output resize does not work correctly in cases like this a[6] b[1, 1, 6] -> out [1, 1, 6] but current code tried to resize using a.sizes() Test Plan: tests added Reviewers: Subscribers: Tasks: Tags:
kimishpatel
force-pushed
the
fix_op_mul_broadcast
branch
from
79aea79 to
2341203
Compare
Summary:
When we have two tensors that match exactly in size but not in dims, the output resize does not work correctly in cases like this a[6] b[1, 1, 6] -> out [1, 1, 6] but current code tried to resize using a.sizes()
Test Plan:
tests added
Reviewers:
Subscribers:
Tasks:
Tags:
Summary
[PLEASE REMOVE] See CONTRIBUTING.md's Pull Requests for ExecuTorch PR guidelines.
[PLEASE REMOVE] If this PR closes an issue, please add a
Fixes #<issue-id>line.[PLEASE REMOVE] If this PR introduces a fix or feature that should be the upcoming release notes, please add a "Release notes: " label. For a list of available release notes labels, check out CONTRIBUTING.md's Pull Requests.
Test plan
[PLEASE REMOVE] How did you test this PR? Please write down any manual commands you used and note down tests that you have written if applicable.