Add axes (optional) input to Pad #2178
Merged
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
attila-dusnoki-htec
force-pushed
the
onnx_parse_pad_axes
branch
from
ccbab8b to
a16c25a
Compare
umangyadav
reviewed
Sep 18, 2023
umangyadav
reviewed
Sep 18, 2023
umangyadav
reviewed
Sep 18, 2023
attila-dusnoki-htec
force-pushed
the
onnx_parse_pad_axes
branch
from
a16c25a to
f72e7ba
Compare
Codecov Report
@@ Coverage Diff @@
## develop #2178 +/- ##
===========================================
+ Coverage 91.33% 91.35% +0.01%
===========================================
Files 438 438
Lines 16435 16465 +30
===========================================
+ Hits 15011 15041 +30
Misses 1424 1424
|
attila-dusnoki-htec
force-pushed
the
onnx_parse_pad_axes
branch
2 times, most recently
from
b9f0a7a to
0363676
Compare
umangyadav
approved these changes
Oct 10, 2023
CharlieL7
approved these changes
Oct 10, 2023
src/onnx/parse_pad.cpp
Outdated
| if(args.size() >= pos) | ||
| { | ||
| auto axes_arg = args.at(pos - 1)->eval(); | ||
| check_arg_empty(axes_arg, "PARSE_PAD: pad input must be constant"); |
There was a problem hiding this comment.
Suggested change
| check_arg_empty(axes_arg, "PARSE_PAD: pad input must be constant"); | |
| check_arg_empty(axes_arg, "PARSE_PAD: variable `axes` input not supported"); |
src/onnx/parse_pad.cpp
Outdated
| if(args.size() >= 2) | ||
| { | ||
| auto pad_arg = args.at(1)->eval(); | ||
| check_arg_empty(pad_arg, "PARSE_PAD: pad input must be constant"); |
There was a problem hiding this comment.
Suggested change
| check_arg_empty(pad_arg, "PARSE_PAD: pad input must be constant"); | |
| check_arg_empty(pad_arg, "PARSE_PAD: `pads` input must be constant"); |
Pad version 18 introduced axes, as a way to make pads easier to use. With that, you only need to define the actually changing pads and the axes that they will be used on. This change only affects the way the parsing works, no modification on the migraphx pad op needed.
attila-dusnoki-htec
force-pushed
the
onnx_parse_pad_axes
branch
from
0363676 to
1788665
Compare
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.
Pad version 18 introduced axes, as a way to make pads easier to use. With that, you only need to define the actually changing pads and the axes that they will be used on.
This change only affects the way the parsing works, no modification on the migraphx pad op needed.
Note: no onnx python unittest to enable, because they fail with
PARSE_PAD: pad input must be constantResolves migraphx-benchmark#71