-
Notifications
You must be signed in to change notification settings - Fork 685
Non-fatal error when ET_SWITCH encounters unsupported dtype #13359
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
Non-fatal error when ET_SWITCH encounters unsupported dtype #13359
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/13359
Note: Links to docs will display an error until the docs builds have been completed. ❌ 16 New Failures, 2 Unrelated FailuresAs of commit 362cd53 with merge base 9359481 ( NEW FAILURES - The following jobs have 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. |
This pull request was exported from Phabricator. Differential Revision: D80141272 |
…13359) Summary: Modify `ET_INTERNAL_SWITCH` macro, so that we fail non-fatally in the default clause when we encounter an unsupported dtype. Differential Revision: D80141272
ca35dd9
to
c66f982
Compare
This pull request was exported from Phabricator. Differential Revision: D80141272 |
…13359) Summary: Modify `ET_INTERNAL_SWITCH` macro, so that we fail non-fatally in the default clause when we encounter an unsupported dtype. Differential Revision: D80141272
c66f982
to
d710186
Compare
This pull request was exported from Phabricator. Differential Revision: D80141272 |
…13359) Summary: Modify `ET_INTERNAL_SWITCH` macro, so that we fail non-fatally in the default clause when we encounter an unsupported dtype. Differential Revision: D80141272
d710186
to
e0b2494
Compare
This pull request was exported from Phabricator. Differential Revision: D80141272 |
…13359) Summary: Modify `ET_INTERNAL_SWITCH` macro, so that we fail non-fatally in the default clause when we encounter an unsupported dtype. Differential Revision: D80141272
e0b2494
to
dfce728
Compare
This pull request was exported from Phabricator. Differential Revision: D80141272 |
…13359) Summary: Modify `ET_INTERNAL_SWITCH` macro, so that we fail non-fatally in the default clause when we encounter an unsupported dtype. Differential Revision: D80141272
dfce728
to
1292927
Compare
This pull request was exported from Phabricator. Differential Revision: D80141272 |
…13359) Summary: Modify `ET_INTERNAL_SWITCH` macro, so that we fail non-fatally in the default clause when we encounter an unsupported dtype. Differential Revision: D80141272
0831ea6
to
62618fc
Compare
This pull request was exported from Phabricator. Differential Revision: D80141272 |
…13359) Summary: Modify `ET_INTERNAL_SWITCH` macro, so that we fail non-fatally in the default clause when we encounter an unsupported dtype. Differential Revision: D80141272
62618fc
to
855fecb
Compare
This pull request was exported from Phabricator. Differential Revision: D80141272 |
switch (_st) { \ | ||
__VA_ARGS__ \ | ||
default: \ | ||
CONTEXT.fail(torch::executor::Error::InvalidArgument); \ |
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.
Nit: Leave a comment along the lines "this sets the return code which should be checked by the caller"
|
||
ET_SWITCH_REALHBBF16_TYPES(a_type, ctx, op_name, CTYPE_A, [&] { | ||
CTYPE_A b_casted; | ||
CTYPE_A b_casted{}; |
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.
this took me a minute -- we now return all-zeros output and signal a failure when there's an unsupported dtype instead of terminating the process. this seems fine.
"Unhandled dtype %s for %s", | ||
::executorch::runtime::toString(t.scalar_type()), | ||
op_name); | ||
} |
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.
Sadly, load_and_convert
has undefined behavior in this case. We should probably return CTYPE_COMPUTE();
to be safe.
"Unhandled dtype %s for %s", | ||
::executorch::runtime::toString(t.scalar_type()), | ||
op_name); | ||
} |
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.
ditto undefined behavior
"Unhandled dtype %s for %s", | ||
::executorch::runtime::toString(t.scalar_type()), | ||
op_name); | ||
} |
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.
ditto undefined behavior; for the convert_and_store
case it seems easiest to just skip the write to be safe.
"Unhandled dtype %s for %s", | ||
::executorch::runtime::toString(t.scalar_type()), | ||
op_name); | ||
} |
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.
ditto undefined behavior
(void)context; | ||
resize_out_tensor(weight, indices, out, weight_nbit); | ||
return quantized_embedding_xbit_out( | ||
auto context = KernelRuntimeContext(); |
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.
nit: KernelRuntimeContext context;
855fecb
to
b511926
Compare
Summary: Modify `ET_INTERNAL_SWITCH` macro, so that we fail non-fatally in the default clause when we encounter an unsupported dtype. Reviewed By: digantdesai Differential Revision: D80141272
…13359) Summary: Modify `ET_INTERNAL_SWITCH` macro, so that we fail non-fatally in the default clause when we encounter an unsupported dtype. Reviewed By: digantdesai Differential Revision: D80141272
b511926
to
cc91a52
Compare
This pull request was exported from Phabricator. Differential Revision: D80141272 |
1 similar comment
This pull request was exported from Phabricator. Differential Revision: D80141272 |
…13359) Summary: Pull Request resolved: pytorch#13359 Modify `ET_INTERNAL_SWITCH` macro, so that we fail non-fatally in the default clause when we encounter an unsupported dtype. Reviewed By: digantdesai Differential Revision: D80141272
cc91a52
to
ff75457
Compare
…13359) Summary: Modify `ET_INTERNAL_SWITCH` macro, so that we fail non-fatally in the default clause when we encounter an unsupported dtype. Reviewed By: digantdesai Differential Revision: D80141272
ff75457
to
362cd53
Compare
This pull request was exported from Phabricator. Differential Revision: D80141272 |
Differential Revision: D80141272 Pull Request resolved: pytorch#13359
Summary: Modify
ET_INTERNAL_SWITCH
macro, so that we fail non-fatally in the default clause when we encounter an unsupported dtype.Differential Revision: D80141272