-
Notifications
You must be signed in to change notification settings - Fork 919
Add const-tensor checks #3141
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?
Add const-tensor checks #3141
Conversation
@tensorflow/micro Add consistent const-tensor checks and error messages across these kernels: TRANSPOSE STRIDED_SLICE FILL BROADCAST_TO EXPAND_DIMS Modify associated kernel unit tests. bug=fixes tensorflow#3140
if (dims_data != nullptr) { | ||
// dims must be a const tensor | ||
tensors[0].allocation_type = kTfLiteMmapRo; | ||
} |
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.
Better to have this condition after all the tensor creations are done?
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 code only applies to the dims
tensor, so it is colocated with the tensor creation.
output_data); | ||
tflite::micro::KernelRunner runner = | ||
CreateFillTestRunner(dims_shape, dims_data, value_shape, value_data, | ||
output_shape, output_data); |
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 just want to understand few things here.
- Is this test was successful previously?
- Do we want to make this test fail in the prepare stage?
- Why can't we use TestFill() API here aswell?
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 old kernel allowed the dims
tensor to have changing values. This is incorrect and was corrected in the kernel such that the dims
tensor must be a const-tensor.
As a result, we now want a negative test, and it should fail during the prepare
phase.
We dont use TestFill
because it doesn't support negative tests. We rarely rewrite old unit test code as it is not worth the effort (it would be better to plan an entirely new unit test code base).
@tensorflow/micro
Add consistent const-tensor checks and error messages across these kernels: TRANSPOSE
STRIDED_SLICE
FILL
BROADCAST_TO
EXPAND_DIMS
Modify associated kernel unit tests.
bug=fixes #3140