ONNX Export Interpolate (Resize) for opset version 10 #21434

lara-hdr · 2019-06-05T21:53:49Z

No description provided.

lara-hdr · 2019-06-05T22:07:39Z

waiting on #20533 to be merged (tests will fail until merged with the referenced PR)

houseroad · 2019-06-07T16:33:16Z

torch/onnx/symbolic_opset9.py

+    return symbolic_fn
+
+
+upsample_nearest1d = _interpolate('upsample_nearest1d', 3, "nearest")


fuse upsamples together is great!

maybe we should have some convention on the prefix of the helper functions, such as _interpolate, slice_op, they are not real symbolics, just some helper functions.

I renamed slice to follow the same convention as _interpolate and other ops (_max_pool, _avg_pool).

houseroad

Rebase to the master?

lara-hdr · 2019-06-07T18:42:07Z

torch/nn/functional.py

-            return [(torch.floor(input.size(i + 2) * torch.tensor(float(scale_factors[i])))) for i in range(dim)]
+            return [(torch.floor(float(input.size(i + 2)) * torch.tensor(float(scale_factors[i])))) for i in range(dim)]
        else:
-            return [int(math.floor(int(input.size(i + 2)) * scale_factors[i])) for i in range(dim)]


This cast was making the input of the operator Floor set as "integer", which is not supported (Floor only accepts float and double as input types), and fails in onnxruntime.

I am a bit confused here, seems the tracer won't go with the else branch, why will it affect onnx's export? do you test the script module? But it's equivalent change, so i am okay with it.

lara-hdr · 2019-06-07T18:48:49Z

torch/onnx/symbolic_opset10.py

+upsample_nearest1d = _interpolate('upsample_nearest1d', 3, "nearest")
+upsample_nearest2d = _interpolate('upsample_nearest2d', 4, "nearest")
+upsample_nearest3d = _interpolate('upsample_nearest3d', 5, "nearest")
+


Removed support for linear mode in both opsets 9 and 10.
Linear mode behaves differently in onnx and pytorch (with both align_coner enabled and disabled).
The exported model was not matching numbers with onnxruntime.
(the difference is explained here : microsoft/onnxruntime#1179).

Will try to add support to linear mode in a different PR.

lara-hdr · 2019-06-10T16:17:01Z

@pytorchbot rebase this please

torch/onnx/symbolic_opset10.py

facebook-github-bot

@houseroad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

lara-hdr · 2019-06-13T00:11:56Z

@houseroad feedback on this?

houseroad · 2019-06-13T17:12:30Z

I think the failing tests are related. Could you take a look?

facebook-github-bot

@houseroad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

houseroad

Looks good. Please address the questions before merge.

houseroad · 2019-06-19T17:19:24Z

torch/nn/functional.py

-            return [(torch.floor(input.size(i + 2) * torch.tensor(float(scale_factors[i])))) for i in range(dim)]
+            return [(torch.floor(float(input.size(i + 2)) * torch.tensor(float(scale_factors[i])))) for i in range(dim)]
        else:
-            return [int(math.floor(int(input.size(i + 2)) * scale_factors[i])) for i in range(dim)]


I am a bit confused here, seems the tracer won't go with the else branch, why will it affect onnx's export? do you test the script module? But it's equivalent change, so i am okay with it.

houseroad · 2019-06-19T17:19:54Z

torch/nn/functional.py

        # make scale_factor a tensor in tracing so constant doesn't get baked in
        if torch._C._get_tracing_state():
-            return [(torch.floor(input.size(i + 2) * torch.tensor(float(scale_factors[i])))) for i in range(dim)]
+            return [(torch.floor((input.size(i + 2) * torch.tensor(float(scale_factors[i]))).float())) for i in range(dim)]


seems the old code should generate the float tensor as well, is .float() conversion necessary?

Without this, the generated ONNX graph fails in ORT and complains that the input of the "Floor" node is an Int.
I could submit an example of the ONNX file and the error if required.

For the following line, I did the same for consistency and out of precaution.

Yea, i am wondering how the onnx model looke like? could you attach the exported model?

lara-hdr · 2019-06-19T18:51:30Z

Resize.zip

@houseroad I attached the model generated without the changes in functional.py, for the code below.
I also added a screenshot of the error I mentioned above.
(for opset9 I also get a similar model (with Upsample node instead of Resize) and error)

def Test_Upsample_op():
class Test_Upsample(nn.Module):
def forward(self, input):
return nn.functional.interpolate(input, mode="nearest", scale_factor=2)
model = Test_Upsample()
input = torch.tensor([[[[1., 2],[3, 4],]]])
output = model(input)
onnx_helper.Save('upsample', 'upsample', model, input, output, 10)

facebook-github-bot · 2019-06-19T22:10:35Z

@houseroad merged this pull request in 34aee93.

Lara added 2 commits May 16, 2019 09:24

onnx export upsample for opset 10

ca072c1

merge, refactor and add tests

4a317cb

pytorchbot added module: nn Related to torch.nn module: onnx Related to torch.onnx labels Jun 5, 2019

ezyang added the open source label Jun 6, 2019

ezyang requested a review from houseroad June 6, 2019 16:46

ezyang added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Jun 6, 2019

houseroad reviewed Jun 7, 2019

View reviewed changes

merge slice, fix name convention and remove linear mode

e1b195a

lara-hdr commented Jun 7, 2019

View reviewed changes

Merge remote-tracking branch 'origin/master' into HEAD

64a497e

houseroad reviewed Jun 11, 2019

View reviewed changes

torch/onnx/symbolic_opset10.py Outdated Show resolved Hide resolved

lara-hdr added 2 commits June 11, 2019 16:46

Update symbolic_opset10.py

306f784

Update symbolic_opset10.py

fabae72

facebook-github-bot reviewed Jun 12, 2019

View reviewed changes

fix tensor float cast

026b4cd

facebook-github-bot reviewed Jun 19, 2019

View reviewed changes

houseroad approved these changes Jun 19, 2019

View reviewed changes

facebook-github-bot closed this in 34aee93 Jun 19, 2019

facebook-github-bot added the merged label Jun 19, 2019

spandantiwari mentioned this pull request Jul 17, 2019

Upsample ONNX gives INVALID_GRAPH onnx/onnx#2118

Closed

mruberry added the Merged label Oct 28, 2020

		return symbolic_fn


		upsample_nearest1d = _interpolate('upsample_nearest1d', 3, "nearest")

ONNX Export Interpolate (Resize) for opset version 10 #21434

ONNX Export Interpolate (Resize) for opset version 10 #21434

Uh oh!

Conversation

lara-hdr commented Jun 5, 2019

Uh oh!

lara-hdr commented Jun 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

houseroad left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lara-hdr commented Jun 10, 2019

Uh oh!

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

lara-hdr commented Jun 13, 2019

Uh oh!

houseroad commented Jun 13, 2019

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

houseroad left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lara-hdr commented Jun 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Jun 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

lara-hdr commented Jun 5, 2019 •

edited

Loading

lara-hdr commented Jun 19, 2019 •

edited

Loading