Fix spec and shape inference for Unsqueeze op#2347
Merged
wschin merged 11 commits intoonnx:masterfrom Sep 24, 2019
Merged
Conversation
Contributor
Author
|
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 |
Contributor
Author
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
Contributor
spandantiwari
left a comment
There was a problem hiding this comment.
LGTM. A clarifying comment about the order of axes in axes would be good to have.
| 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. |
Contributor
There was a problem hiding this comment.
+1 for mentioning about duplicate entries.
Collaborator
|
Do we have tests for unsorted cases? |
Contributor
Author
I will add shape inference test for now. |
Contributor
Author
|
@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
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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)