Fix ConvTranspose: enhance attribute check #3000

jcwchen · 2020-09-05T00:10:02Z

Description

Enhance the attribute check for ConvTranspose: pads and auto_pads can not be used at the same time.
Add a unit test for it
(move it to another PR) Update the document for ConvTranspos: make the example be consistent as the doc description

Motivation and Context
#2787

docs/Changelog.md

onnx/defs/nn/defs.cc

This reverts commit e75e287.

This reverts commit 70952c0.

This reverts commit 8a0482d.

jcwchen · 2020-09-10T21:20:12Z

Updating the document for ConvTranspose will move to another PR because the modification needs further review. Just removed it from this PR.

askhade · 2020-09-14T16:59:20Z

@gramalingam @ebarsoum @postrational : This PR enforces a condition which is already part of the spec. I do not think we need a version bump here. Thoughts?

gramalingam · 2020-09-14T22:08:20Z

onnx/defs/nn/defs.cc

@@ -1191,6 +1191,10 @@ void convTransposeShapeInference(InferenceContext& ctx) {
    if (pads.size() != n_input_dims * 2) {
      fail_shape_inference("Attribute pads has incorrect size");
    }
+    const auto* auto_pad_attr = ctx.getAttribute("auto_pad");
+    if (nullptr != auto_pad_attr) {
+      fail_shape_inference("The pads attribute cannot be used simultaneously with auto_pad attribute");


Just a minor nit: this is neither a shape_inference error nor a type_inference error. Unfortunately, we have only "fail_type_inference" and "fail_shape_inference". This is okay, but it might be good to have a consistent categorization of errors (may be adding another category of errors) ... not necessarily in this PR, could be later also.

gramalingam · 2020-09-14T22:11:22Z

@gramalingam @ebarsoum @postrational : This PR enforces a condition which is already part of the spec. I do not think we need a version bump here. Thoughts?

Agree

* add check for using auto_pad and pads simultaneously * fix description for auto_pads == SAME_UPPER * update docs for operator * fix the old one as well * add a test * Revert "fix description for auto_pads == SAME_UPPER" This reverts commit e75e287. * Revert "update docs for operator" This reverts commit 70952c0. * Revert "fix the old one as well" This reverts commit 8a0482d. Co-authored-by: Ashwini Khade <askhade@microsoft.com> Signed-off-by: daquexian <daquexian566@gmail.com>

* add check for using auto_pad and pads simultaneously * fix description for auto_pads == SAME_UPPER * update docs for operator * fix the old one as well * add a test * Revert "fix description for auto_pads == SAME_UPPER" This reverts commit e75e287. * Revert "update docs for operator" This reverts commit 70952c0. * Revert "fix the old one as well" This reverts commit 8a0482d. Co-authored-by: Ashwini Khade <askhade@microsoft.com>

* add check for using auto_pad and pads simultaneously * fix description for auto_pads == SAME_UPPER * update docs for operator * fix the old one as well * add a test * Revert "fix description for auto_pads == SAME_UPPER" This reverts commit e75e287. * Revert "update docs for operator" This reverts commit 70952c0. * Revert "fix the old one as well" This reverts commit 8a0482d. Co-authored-by: Ashwini Khade <askhade@microsoft.com> Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* remove wrong description for pow Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * Add size check to make_tensor (#2987) Co-authored-by: Ke Zhang <linkerzhang@yeah.net> Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * Fix float16 data convert issue in numpy_helper.to_array (#3002) * handle f16 case for to_array * fix flake8 * nit: comment Co-authored-by: Ashwini Khade <askhade@microsoft.com> Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * Deprecate Travis CI (#2773) * adding gtests for c++ api test * deprecate travis * adding the build badges for the new pipelines in Azure, deprecating travis build badge, renaming circleCI badge * updating badge label * removing - in badge names * c++ api changes for linux * update environment variables * update env variables, setup tools call * Update Linux-CI.yml for Azure Pipelines * revert changes to Linux and Mac CIs * delete last travis file Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * Fix shape inference of scalar ConstantOfShape (#3005) When input shape is (0), we do not add any dim to inferred shape but we should initialize tensor_type.shape by calling mutable_shape(). Co-authored-by: Ashwini Khade <askhade@microsoft.com> Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * fix shape inference for loop (#3014) Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * Fix ConvTranspose: enhance attribute check (#3000) * add check for using auto_pad and pads simultaneously * fix description for auto_pads == SAME_UPPER * update docs for operator * fix the old one as well * add a test * Revert "fix description for auto_pads == SAME_UPPER" This reverts commit e75e287. * Revert "update docs for operator" This reverts commit 70952c0. * Revert "fix the old one as well" This reverts commit 8a0482d. Co-authored-by: Ashwini Khade <askhade@microsoft.com> Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * Schema change to support dynamic shapes in ORT (#2955) * Changes to schema and python tests * Modify test * Remove attribute that is input also * Changes to optimizers, adapters and tests * Run flake8 * undo unrequired comit files, fix formatting, review changes * Fix ci test, cleanup * Fix narrowing conversion error * add missed test model Co-authored-by: G. Ramalingam <grama@microsoft.com> Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * fix loop shape inference for ver 11 (#3023) Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * Update softmax family ops behavior to align with other frameworks (fix #2289) (#2879) * Update softmax family ops behavior to align with other frameworks * Update logsoftmax, hardmax tests, regenerate docs and test data * fix wrong input name in function * regenerate test data * fix flake8 error * regenerate docs * regenerate docs * add missing type annotation for hardmax * add the math for softmax family operators * remove the 'description' field in docs as it is covered by the math * fix wrong format in axis attr * replace name with description * restore the name field for axis attr * regenerate docs * regenerate docs * add the missing name * regenerate docs * update reducesum to align with master * regenerate tests Co-authored-by: Wei-Sheng Chin <wschin@outlook.com> Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> Co-authored-by: G. Ramalingam <grama@microsoft.com> Co-authored-by: Ke Zhang <linkerzhang@yeah.net> Co-authored-by: Ashwini Khade <askhade@microsoft.com> Co-authored-by: Vinitra Swamy <vinitras@gmail.com> Co-authored-by: Shinichiro Hamaji <shinichiro.hamaji@gmail.com> Co-authored-by: ashbhandare <ash.bhandare@gmail.com> Co-authored-by: daquexian <daquexian566@gmail.com> Co-authored-by: Wei-Sheng Chin <wschin@outlook.com>

* add check for using auto_pad and pads simultaneously * fix description for auto_pads == SAME_UPPER * update docs for operator * fix the old one as well * add a test * Revert "fix description for auto_pads == SAME_UPPER" This reverts commit e75e287. * Revert "update docs for operator" This reverts commit 70952c0. * Revert "fix the old one as well" This reverts commit 8a0482d. Co-authored-by: Ashwini Khade <askhade@microsoft.com>

…x#2999) * remove wrong description for pow Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * Add size check to make_tensor (onnx#2987) Co-authored-by: Ke Zhang <linkerzhang@yeah.net> Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * Fix float16 data convert issue in numpy_helper.to_array (onnx#3002) * handle f16 case for to_array * fix flake8 * nit: comment Co-authored-by: Ashwini Khade <askhade@microsoft.com> Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * Deprecate Travis CI (onnx#2773) * adding gtests for c++ api test * deprecate travis * adding the build badges for the new pipelines in Azure, deprecating travis build badge, renaming circleCI badge * updating badge label * removing - in badge names * c++ api changes for linux * update environment variables * update env variables, setup tools call * Update Linux-CI.yml for Azure Pipelines * revert changes to Linux and Mac CIs * delete last travis file Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * Fix shape inference of scalar ConstantOfShape (onnx#3005) When input shape is (0), we do not add any dim to inferred shape but we should initialize tensor_type.shape by calling mutable_shape(). Co-authored-by: Ashwini Khade <askhade@microsoft.com> Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * fix shape inference for loop (onnx#3014) Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * Fix ConvTranspose: enhance attribute check (onnx#3000) * add check for using auto_pad and pads simultaneously * fix description for auto_pads == SAME_UPPER * update docs for operator * fix the old one as well * add a test * Revert "fix description for auto_pads == SAME_UPPER" This reverts commit e75e287. * Revert "update docs for operator" This reverts commit 70952c0. * Revert "fix the old one as well" This reverts commit 8a0482d. Co-authored-by: Ashwini Khade <askhade@microsoft.com> Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * Schema change to support dynamic shapes in ORT (onnx#2955) * Changes to schema and python tests * Modify test * Remove attribute that is input also * Changes to optimizers, adapters and tests * Run flake8 * undo unrequired comit files, fix formatting, review changes * Fix ci test, cleanup * Fix narrowing conversion error * add missed test model Co-authored-by: G. Ramalingam <grama@microsoft.com> Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * fix loop shape inference for ver 11 (onnx#3023) Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * Update softmax family ops behavior to align with other frameworks (fix onnx#2289) (onnx#2879) * Update softmax family ops behavior to align with other frameworks * Update logsoftmax, hardmax tests, regenerate docs and test data * fix wrong input name in function * regenerate test data * fix flake8 error * regenerate docs * regenerate docs * add missing type annotation for hardmax * add the math for softmax family operators * remove the 'description' field in docs as it is covered by the math * fix wrong format in axis attr * replace name with description * restore the name field for axis attr * regenerate docs * regenerate docs * add the missing name * regenerate docs * update reducesum to align with master * regenerate tests Co-authored-by: Wei-Sheng Chin <wschin@outlook.com> Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> Co-authored-by: G. Ramalingam <grama@microsoft.com> Co-authored-by: Ke Zhang <linkerzhang@yeah.net> Co-authored-by: Ashwini Khade <askhade@microsoft.com> Co-authored-by: Vinitra Swamy <vinitras@gmail.com> Co-authored-by: Shinichiro Hamaji <shinichiro.hamaji@gmail.com> Co-authored-by: ashbhandare <ash.bhandare@gmail.com> Co-authored-by: daquexian <daquexian566@gmail.com> Co-authored-by: Wei-Sheng Chin <wschin@outlook.com>

jcwchen added 5 commits September 4, 2020 16:35

add check for using auto_pad and pads simultaneously

3cf1f0d

fix description for auto_pads == SAME_UPPER

e75e287

update docs for operator

70952c0

fix the old one as well

8a0482d

add a test

05af6f1

jcwchen requested review from a team as code owners September 5, 2020 00:10

jcwchen and others added 3 commits September 4, 2020 17:10

Merge branch 'master' into fix-convtranspose

80d6166

Merge branch 'master' into fix-convtranspose

c319c96

Merge branch 'master' into fix-convtranspose

6473362

askhade reviewed Sep 10, 2020

View reviewed changes

docs/Changelog.md Outdated Show resolved Hide resolved

askhade reviewed Sep 10, 2020

View reviewed changes

onnx/defs/nn/defs.cc Outdated Show resolved Hide resolved

jcwchen added 3 commits September 10, 2020 14:15

Revert "fix description for auto_pads == SAME_UPPER"

f2cfb6a

This reverts commit e75e287.

Revert "update docs for operator"

09bbe48

This reverts commit 70952c0.

Revert "fix the old one as well"

c4a9bb0

This reverts commit 8a0482d.

jcwchen changed the title ~~Fix ConvTranspose: enhance attribute check and update docs~~ Fix ConvTranspose: enhance attribute check Sep 10, 2020

Merge branch 'master' into fix-convtranspose

4f0dc32

askhade approved these changes Sep 14, 2020

View reviewed changes

gramalingam reviewed Sep 14, 2020

View reviewed changes

gramalingam approved these changes Sep 14, 2020

View reviewed changes

Merge branch 'master' into fix-convtranspose

975f68b

askhade merged commit f421b68 into onnx:master Sep 14, 2020

jcwchen mentioned this pull request Sep 15, 2020

ConvTranspose auto_pad and explicit pads combination should be invalid #2787

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix ConvTranspose: enhance attribute check #3000

Fix ConvTranspose: enhance attribute check #3000

jcwchen commented Sep 5, 2020 •

edited

jcwchen commented Sep 10, 2020 •

edited

askhade commented Sep 14, 2020

gramalingam Sep 14, 2020

gramalingam commented Sep 14, 2020

Fix ConvTranspose: enhance attribute check #3000

Fix ConvTranspose: enhance attribute check #3000

Conversation

jcwchen commented Sep 5, 2020 • edited

jcwchen commented Sep 10, 2020 • edited

askhade commented Sep 14, 2020

gramalingam Sep 14, 2020

Choose a reason for hiding this comment

gramalingam commented Sep 14, 2020

jcwchen commented Sep 5, 2020 •

edited

jcwchen commented Sep 10, 2020 •

edited