Fix spec and shape inference for Unsqueeze op #2347
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
|
CC: @wschin @gramalingam |
hariharans29
commented
Sep 24, 2019
| []) | ||
| self._assert_inferred(graph, [make_tensor_value_info('y', TensorProto.FLOAT, (1, 3, 4, 5, 1))]) | ||
|
|
||
| def test_unsqueeze_negative_axes(self): # type: () -> None |
There was a problem hiding this comment.
won't work with existing logic.
wschin
approved these changes
Sep 24, 2019
spandantiwari
approved these changes
Sep 24, 2019
| Given an input tensor (`data`) of shape [3, 4, 5], then | ||
| Unsqueeze(data, axes=[0, 4]) outputs a tensor (`expanded`) containing same data as `data` but with shape [1, 3, 4, 5, 1]. | ||
|
|
||
| The attribute `axes` should not contain any duplicate entries. It is an error if it contains duplicates. |
There was a problem hiding this comment.
+1 for mentioning about duplicate entries.
|
Do we have tests for unsorted cases? |
I will add shape inference test for now. |
|
@spandantiwari @wschin - added backend test for unsorted axes too |
hariharans29
commented
Sep 24, 2019
wschin
pushed a commit
to wschin/onnx
that referenced
this pull request
Sep 24, 2019
* Fix spec for Unsqueeze * Update Changelog.md * Refine doc * Refine * PR comments * Update Changelog.md * Update shape inference test file
kevinch-nv
pushed a commit
that referenced
this pull request
Sep 25, 2019
* Relax IF's shape inference rule (#2345) * Relax If's shape inference rule * Make shape inference tests ok and move code to the right place * Add document changes * Update onnx/defs/controlflow/defs.cc * Update onnx/defs/controlflow/defs.cc * Address comments * Address comments * Address comments * Fix shape inference test * Disable a type check * Address a comment * Update defs.cc * Update Changelog.md * Update Operators.md * Bump NMS version for avoiding regression in existing models (#2348) * Bump NMS version for avoiding regression in existing models * Bring old logic back * Fix spec and shape inference for Unsqueeze op (#2347) * Fix spec for Unsqueeze * Update Changelog.md * Refine doc * Refine * PR comments * Update Changelog.md * Update shape inference test file
jcwchen
pushed a commit
to jcwchen/onnx
that referenced
this pull request
Sep 23, 2020
* Fix spec for Unsqueeze * Update Changelog.md * Refine doc * Refine * PR comments * Update Changelog.md * Update shape inference test file
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.
Unsqueeze op is unclear on how it deals with values in
axesand what are the acceptable values foraxesFix shape inference logic. In current state, it cannot handle negative axes properly (the way it is intended to)