Include tensor shapes in get_broadcast_target_size error message #7944
Conversation
|
Stack from ghstack (oldest at bottom): |
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/7944
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 New Failure, 4 PendingAs of commit f2538df with merge base 77f18b2 ( NEW FAILURE - The following job has failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
facebook-github-bot
added
the
CLA Signed
|
would like feedback on this one before sending more PRs to log tensor shapes broadly |
| InvalidArgument, | ||
| "Two input tensors should be broadcastable.\n"); | ||
| if (!tensors_are_broadcastable_between(a_size, b_size)) { | ||
| const auto a_shape_str = tensor_shape_to_c_string( |
There was a problem hiding this comment.
This new API is great, keeping the caller totally ignorant of how big a buffer to allocate.
| tensors_are_broadcastable_between(a_size, b_size), | ||
| InvalidArgument, | ||
| "Two input tensors should be broadcastable.\n"); | ||
| if (!tensors_are_broadcastable_between(a_size, b_size)) { |
There was a problem hiding this comment.
Maybe thrown an UNLIKELY on this, but compared to the work done during broadcasting this is probably inconsequential
| if (!tensors_are_broadcastable_between(a_size, b_size)) { | |
| if (ET_UNLIKELY(!tensors_are_broadcastable_between(a_size, b_size))) { |
swolchok
added
the
release notes: devtools
| #endif | ||
| ET_LOG( | ||
| Error, | ||
| "Two input tensors should be broadcastable but got shapes %s and %s.", | ||
| a_shape_str.data(), | ||
| b_shape_str.data()); |
There was a problem hiding this comment.
I think it'd be less surprising for readers if the if/endif completely covered the scopes of the locals that are defined in it, so they don't need to do the extra mental jump of "I guess ET_LOG evaluates to nothing when ET_LOG_ENABLED is false"
| #endif | |
| ET_LOG( | |
| Error, | |
| "Two input tensors should be broadcastable but got shapes %s and %s.", | |
| a_shape_str.data(), | |
| b_shape_str.data()); | |
| ET_LOG( | |
| Error, | |
| "Two input tensors should be broadcastable but got shapes %s and %s.", | |
| a_shape_str.data(), | |
| b_shape_str.data()); | |
| #endif |
This is the motivating example for #7902.
Test Plan: Injected failure to new broadcast_test and saw shapes in error message.