-
Notifications
You must be signed in to change notification settings - Fork 754
Arm backend: Match arg ranks for min/max ops #16181
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
base: main
Are you sure you want to change the base?
Arm backend: Match arg ranks for min/max ops #16181
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/16181
Note: Links to docs will display an error until the docs builds have been completed. ❌ 3 New Failures, 4 Unrelated FailuresAs of commit 821b308 with merge base 3a262ef ( NEW FAILURES - The following jobs have failed:
FLAKY - The following job failed but was likely due to flakiness present on trunk:
BROKEN TRUNK - The following jobs failed but was present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
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. |
|
int32 clamp already does a full op to handle the rank difference. If this is not working properly you should add a test that triggers this error or if it is a replacement, you should remove the full op |
- Int32 clamp is decomposed into min/max, but input_tensor and min_arg/max_arg can end up with different ranks (e.g. (1,) vs ()) after subsequent passes. This violates the TOSA requirements for TOSA.MINIMUM and TOSA.MAXIMUM in the node visitors. - Fix this by matching the input ranks in MatchArgRanksPass Change-Id: I47a3bb73832ddc35553cd3e5b65085155eb9ca48 Signed-off-by: Yufeng Shi <[email protected]> Co-authored-by: Ryan O'Shea <[email protected]> Co-authored-by: Rob Elliott <[email protected]>
e50aca5 to
b28d0a6
Compare
Hey Ryan, thanks for your comment. I realize This issue was found by some internal model tests and cannot be easily reproduced with an op test, but we have verified the fix against those internal models. Thank you! |
|
Arm tests might fail on test_index_tensor_tosa @oscarandersson8218 is working on a fix |
Change-Id: I47a3bb73832ddc35553cd3e5b65085155eb9ca48
cc @freddan80 @per @zingo @oscarandersson8218 @digantdesai