support Pad(18) #14219
Merged
support Pad(18) #14219
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Liqun Fu <[email protected]>
Signed-off-by: Liqun Fu <[email protected]>
skottmckay
added a commit
that referenced
this pull request
Jan 12, 2023
…ing pipeline. Slice(was checked in today. Resize: #13890 Col2Im: #12311 ScatterND and ScatterElement: #14224 Pad (should also fix CenterCropPad failures): #14219 Bitwise ops: #14197 Optional: Unknown if we're intending to support this in 1.14 Not sure about SoftPlus as that failing due to `Could not find an implementation for Exp(1)`. ORT supports opset 6 and on, so it seems incorrect for the test model created in opset 18 to be using a version of Exp that is so old.
Signed-off-by: Liqun Fu <[email protected]>
skottmckay
added a commit
that referenced
this pull request
Jan 12, 2023
…ing pipeline (#14259) ### Description <!-- Describe your changes. --> Skip tests for opset18 models that we haven't implemented kernels for yet. Slice was checked in today so those failures should go away. Resize: #13890 (all resize failures are fixed by this PR as confirmed in output [here](https://dev.azure.com/aiinfra/530acbc4-21bc-487d-8cd8-348ff451d2ff/_apis/build/builds/264725/logs/729)) Col2Im: #12311 ScatterND and ScatterElement: #14224 Pad (should also fix CenterCropPad failures): #14219 Bitwise ops: #14197 Optional: Unknown if we're intending to support this in 1.14 Not sure about SoftPlus as that is failing due to `Could not find an implementation for Exp(1)`. ORT supports Exp from opset 6 and on, and it seems incorrect for the test model created for opset 18 to be using a version of Exp that is so old. Would have expected it to use the latest - Exp(13). @liqunfu is this something that requires a fix to the ONNX model? ### Motivation and Context <!-- - Why is this change required? What problem does it solve? - If it fixes an open issue, please link to the issue here. --> Fix pipeline
skottmckay
reviewed
Jan 17, 2023
Signed-off-by: Liqun Fu <[email protected]>
Signed-off-by: Liqun Fu <[email protected]>
Signed-off-by: Liqun Fu <[email protected]>
skottmckay
reviewed
Jan 19, 2023
skottmckay
reviewed
Jan 19, 2023
Signed-off-by: Liqun Fu <[email protected]>
Signed-off-by: Liqun Fu <[email protected]>
| @@ -805,5 +805,28 @@ TEST(PadOpTest, BoolType) { | |||
| test.Run(OpTester::ExpectResult::kExpectSuccess, "", {kTensorrtExecutionProvider}); | |||
| } | |||
|
|
|||
| TEST(PadOpTest, ConstantPadAxes) { | |||
There was a problem hiding this comment.
Can we please add more tests?
There was a problem hiding this comment.
The feature added to Pad(18) is with pad axes input. One new test case was added in ONNX for this new feature. Here the similar unit test is added to test the new feature.
| const Tensor* axes_tensor = ctx->Input<Tensor>(3); | ||
| if (axes_tensor) { | ||
| const auto& axes_tensor_dims = axes_tensor->Shape().GetDims(); | ||
| ORT_ENFORCE(axes_tensor_dims.size() == 1, "Axes tensor should be a 1D tensor "); |
There was a problem hiding this comment.
I know some of the code is old here and it has established a practice that it's ok to use ORT_ENFORCE in a Compute method. Since Compute already returns a Status we should try to return a Status with INVALID_ARGUMENT code.
pranavsharma
approved these changes
Jan 23, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Liqun Fu [email protected]
Description
Support Pad(18)
Motivation and Context
for Ort release 1.14