-
Notifications
You must be signed in to change notification settings - Fork 621
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
Enhance error messages in case of not supported data types in operators #2211
Conversation
), DALI_FAIL(make_string("Unsupported output type: ", input.type().id()))) // NOLINT | ||
), DALI_FAIL(make_string("Unsupported input type: ", output_type_))) // NOLINT |
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.
The error messages are reversed wrt to the value shown.
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.
good catch
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.
What is your opinion on unifying the "Invalid ... type " and "Unsupported .... type " in error messages?
dali/operators/generic/reshape.cc
Outdated
@@ -223,7 +223,7 @@ void Reshape<Backend>::ShapeFromInput(const TensorListLike &tl, bool relative) { | |||
(int8_t, uint8_t, int16_t, uint16_t, int32_t, uint32_t, int64_t, uint64_t), | |||
(this->ShapeFromInput(view<const type>(tl));), | |||
(DALI_FAIL(make_string(OpName(), ": shape input must have integral type; got: ", | |||
tl.type().name(), " (id = ", static_cast<int>(tl.type().id()), ")"));) | |||
tl.type().name(), " type: ", tl.type().id()));) |
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.
I think that typeid is readily converted to a string, so this would print the name twice.
tl.type().name(), " type: ", tl.type().id()));) | |
tl.type().id()));) |
I think these are distinct concepts. E.g. using a floating point number for an index is invalid, whereas resizing uint32_t image is unsupported. |
!build |
CI MESSAGE: [1573075]: BUILD STARTED |
CI MESSAGE: [1573075]: BUILD FAILED |
Some more changes needed: remove internal id from known types, differentiate better between no type set, invalid type and valid run-time registered type.
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Joaquin Anton <janton@nvidia.com>
cb58abc
to
4a58ea9
Compare
!build |
CI MESSAGE: [1574232]: BUILD STARTED |
CI MESSAGE: [1574232]: BUILD FAILED |
CI MESSAGE: [1574232]: BUILD PASSED |
Why we need this PR?
Pick one, remove the rest
What happened in this PR?
Fill relevant points, put NA otherwise. Replace anything inside []
Added DALIDataType string conversion
Adjusted every TYPE_SWITCH instance that was not printing the invalid type
Several ops
Changes in the way we handle type names
N/A
N/A
JIRA TASK: [Use DALI-XXXX or NA]