Bump NMS version for avoiding regression in existing models#2348
Merged
wschin merged 3 commits intoonnx:masterfrom Sep 24, 2019
Merged
Bump NMS version for avoiding regression in existing models#2348wschin merged 3 commits intoonnx:masterfrom
wschin merged 3 commits intoonnx:masterfrom
Conversation
gramalingam
approved these changes
Sep 24, 2019
wschin
added a commit
to wschin/onnx
that referenced
this pull request
Sep 24, 2019
* Bump NMS version for avoiding regression in existing models * Bring old logic back
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
* Bump NMS version for avoiding regression in existing models * Bring old logic back
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.
In opset 10, NMS doesn't produce shape. With the newly added shape inference function, some existing models can be broken due to the interaction between NSM and IF nodes (IF's then branch and else branch produce different shapes). Thus, I feel it'd be better to bump the version of NMS so that old code can still work with old models.
Because shape inference produces side information for a model, if we consider side information a part of the spec, we also need to bump operator's version each time we add a shape inference function.