Implement Shrink operator - #485
Conversation
|
Is this PR ready for review? If yes, you may remove the WIP from the title. Thx. |
It is not ready yet. That's why I did not remove the WIP from the title. It needs some test case additions. Thanks. |
|
Talked with Hari offline. It's very likely the test data is wrong. He is verifying it. We have two ONNX test programs, one is in C++, another is in Python. Another question is: did we check the input shape? I suspect we didn't. |
| @@ -312,8 +312,6 @@ int real_main(int argc, char* argv[]) { | |||
| {"scatter_without_axis", "opset 9 not supported yet"}, | |||
| {"scan_sum", "opset 9 not supported yet"}, | |||
| {"shrink", "opset 9 not supported yet"}, | |||
There was a problem hiding this comment.
Keeping "shrink" test excluded because of bug in test input - onnx/onnx#1823
|
| auto input = p_op_kernel_context->Input<Tensor>(0); | ||
| auto output = p_op_kernel_context->Output(0, input->Shape()); | ||
|
|
||
| auto dtype = input->DataType(); |
There was a problem hiding this comment.
Prefer explicitly using 'const' and * or & with 'auto' when applicable so it's clearer what it represents.
| template <> | ||
| Status ShrinkImpl<bool>(const Tensor* /*input*/, Tensor* /*output*/, float /*bias*/, float /*lambd*/) { | ||
| return ORT_MAKE_STATUS(ONNXRUNTIME, INVALID_ARGUMENT, "Input types for the Shrink operator are constrained to all numeric types only. Got bool type here."); | ||
| } |
There was a problem hiding this comment.
nit: limit line length to 120
https://marketplace.visualstudio.com/items?itemName=PaulHarrington.EditorGuidelines is very helpful to show a vertical guideline at 120
## Describe your changes Fix document generation issues. ## Checklist before requesting a review - [ ] Add unit tests for this change. - [ ] Make sure all tests can pass. - [ ] Update documents if necessary. - [ ] Format your code by running `pre-commit run --all-files` - [ ] Is this a user-facing change? If yes, give a description of this change to be included in the release notes. ## (Optional) Issue link
No description provided.