Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix ConvTranspose: enhance attribute check #3000

Merged
merged 13 commits into from
Sep 14, 2020
Merged

Fix ConvTranspose: enhance attribute check #3000

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
3cf1f0d
add check for using auto_pad and pads simultaneously
jcwchen Sep 4, 2020
e75e287
fix description for auto_pads == SAME_UPPER
jcwchen Sep 4, 2020
70952c0
update docs for operator
jcwchen Sep 4, 2020
8a0482d
fix the old one as well
jcwchen Sep 4, 2020
05af6f1
add a test
jcwchen Sep 5, 2020
80d6166
Merge branch 'master' into fix-convtranspose
jcwchen Sep 5, 2020
c319c96
Merge branch 'master' into fix-convtranspose
jcwchen Sep 9, 2020
6473362
Merge branch 'master' into fix-convtranspose
askhade Sep 9, 2020
f2cfb6a
Revert "fix description for auto_pads == SAME_UPPER"
jcwchen Sep 10, 2020
09bbe48
Revert "update docs for operator"
jcwchen Sep 10, 2020
c4a9bb0
Revert "fix the old one as well"
jcwchen Sep 10, 2020
4f0dc32
Merge branch 'master' into fix-convtranspose
jcwchen Sep 13, 2020
975f68b
Merge branch 'master' into fix-convtranspose
askhade Sep 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/Changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ This version of the operator has been available since version 1 of the default O
output_shape can also be explicitly specified in which case pads values are auto generated using these equations:

total_padding[i] = stride[i] * (input_size[i] - 1) + output_padding[i] + ((kernel_shape[i] - 1) * dilations[i] + 1) - output_shape[i]
If (auto_pads != SAME_UPPER): pads[start_i] = total_padding[i]/2; pads[end_i] = total_padding[i] - (total_padding[i]/2)
If (auto_pads == SAME_UPPER): pads[start_i] = total_padding[i]/2; pads[end_i] = total_padding[i] - (total_padding[i]/2)
Else: pads[start_i] = total_padding[i] - (total_padding[i]/2); pads[end_i] = (total_padding[i]/2).


Expand Down Expand Up @@ -10743,7 +10743,7 @@ This version of the operator has been available since version 11 of the default
output_shape can also be explicitly specified in which case pads values are auto generated using these equations:

total_padding[i] = stride[i] * (input_size[i] - 1) + output_padding[i] + ((kernel_shape[i] - 1) * dilations[i] + 1) - output_shape[i]
If (auto_pads != SAME_UPPER): pads[start_i] = total_padding[i]/2; pads[end_i] = total_padding[i] - (total_padding[i]/2)
If (auto_pads == SAME_UPPER): pads[start_i] = total_padding[i]/2; pads[end_i] = total_padding[i] - (total_padding[i]/2)
askhade marked this conversation as resolved.
Show resolved Hide resolved
Else: pads[start_i] = total_padding[i] - (total_padding[i]/2); pads[end_i] = (total_padding[i]/2).


Expand Down
2 changes: 1 addition & 1 deletion docs/Operators.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3360,7 +3360,7 @@ expect(convinteger_node_with_padding, inputs=[x, w, x_zero_point], outputs=[y_wi
output_shape can also be explicitly specified in which case pads values are auto generated using these equations:

total_padding[i] = stride[i] * (input_size[i] - 1) + output_padding[i] + ((kernel_shape[i] - 1) * dilations[i] + 1) - output_shape[i]
If (auto_pads != SAME_UPPER): pads[start_i] = total_padding[i]/2; pads[end_i] = total_padding[i] - (total_padding[i]/2)
If (auto_pads == SAME_UPPER): pads[start_i] = total_padding[i]/2; pads[end_i] = total_padding[i] - (total_padding[i]/2)
Else: pads[start_i] = total_padding[i] - (total_padding[i]/2); pads[end_i] = (total_padding[i]/2).


Expand Down
6 changes: 5 additions & 1 deletion onnx/defs/nn/defs.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
} else {
pads.assign(n_input_dims * 2, 0);
const auto* auto_pad_attr = ctx.getAttribute("auto_pad");
Expand Down Expand Up @@ -1300,7 +1304,7 @@ If the pads parameter is provided the shape of the output is calculated via the
output_shape can also be explicitly specified in which case pads values are auto generated using these equations:

total_padding[i] = stride[i] * (input_size[i] - 1) + output_padding[i] + ((kernel_shape[i] - 1) * dilations[i] + 1) - output_shape[i]
If (auto_pads != SAME_UPPER): pads[start_i] = total_padding[i]/2; pads[end_i] = total_padding[i] - (total_padding[i]/2)
If (auto_pads == SAME_UPPER): pads[start_i] = total_padding[i]/2; pads[end_i] = total_padding[i] - (total_padding[i]/2)
askhade marked this conversation as resolved.
Show resolved Hide resolved
Else: pads[start_i] = total_padding[i] - (total_padding[i]/2); pads[end_i] = (total_padding[i]/2).

)DOC";
Expand Down
2 changes: 1 addition & 1 deletion onnx/defs/nn/old.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1362,7 +1362,7 @@ If the pads parameter is provided the shape of the output is calculated via the
output_shape can also be explicitly specified in which case pads values are auto generated using these equations:

total_padding[i] = stride[i] * (input_size[i] - 1) + output_padding[i] + ((kernel_shape[i] - 1) * dilations[i] + 1) - output_shape[i]
If (auto_pads != SAME_UPPER): pads[start_i] = total_padding[i]/2; pads[end_i] = total_padding[i] - (total_padding[i]/2)
If (auto_pads == SAME_UPPER): pads[start_i] = total_padding[i]/2; pads[end_i] = total_padding[i] - (total_padding[i]/2)
Else: pads[start_i] = total_padding[i] - (total_padding[i]/2); pads[end_i] = (total_padding[i]/2).

)DOC";
Expand Down
10 changes: 10 additions & 0 deletions onnx/test/shape_inference_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1537,6 +1537,16 @@ def test_conv_transpose_with_group_and_output_shape(self): # type: () -> None
[])
self._assert_inferred(graph, [make_tensor_value_info('Y', TensorProto.FLOAT, (25, 64, 36, 36))])

def test_conv_transpose_with_pads_and_auto_pads(self): # type: () -> None
# This test should fail because pads cannot be used simultaneously with auto_pad
graph = self._make_graph(
[('X', TensorProto.FLOAT, (1, 1, 2, 2)),
('W', TensorProto.FLOAT, (1, 1, 3, 3)),
('B', TensorProto.FLOAT, (1, ))],
[make_node('ConvTranspose', ['X', 'W', 'B'], 'Y', auto_pad="SAME_UPPER", strides=[1, 1], pads=[0, 1, 1, 0])],
[])
self.assertRaises(RuntimeError, onnx.shape_inference.infer_shapes, helper.make_model(graph))

def test_mvn_function_output_shape(self): # type: () -> None
graph = self._make_graph(
[('X', TensorProto.FLOAT, (25, 48, 16, 16))],
Expand Down