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

[Relay] Support resize in the ONNX conversion #8455

Merged
merged 5 commits into from
Jul 21, 2021
Merged

[Relay] Support resize in the ONNX conversion #8455

merged 5 commits into from
Jul 21, 2021

Conversation

schilkunda-amba
Copy link
Contributor

  • Added support for resize2d op
  • Added unit test

@schilkunda-amba
[Relay to Onnx]
0e80fa1 
* Added support for resize2d op
* Added unit test
@schilkunda-amba
[Relay to Onnx][Resize]
43324c3 
* Fixed formatting errors
@comaniac comaniac changed the title [Relay to Onnx conversion][Resize] [Relay] Support resize in the ONNX conversion Jul 12, 2021
comaniac
comaniac reviewed Jul 12, 2021
View reviewed changes
Copy link
Contributor

@comaniac comaniac left a comment

Choose a reason for hiding this comment

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

Can we also support resize in other dimensions?

trevor-m
trevor-m requested changes Jul 12, 2021
View reviewed changes
Copy link
Contributor

@trevor-m trevor-m left a comment

Choose a reason for hiding this comment

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

Thanks @schilkunda-amba! Some small comments

python/tvm/contrib/target/onnx.py Show resolved Hide resolved
tests/python/contrib/test_onnx.py Show resolved Hide resolved
@schilkunda-amba
[Relay to Onnx][Resize]
34ae7ab 
* Fixed issue in resize conversion: round maps to round_preferc_ceil
* Updated resize unit test to test for coordinate transform mode and
round
* Known issue: Does not match for (NN, align_corners) and Cubic
@schilkunda-amba
Copy link
Contributor Author

Can we also support resize in other dimensions?

Sure, I will look into it.

@schilkunda-amba
Copy link
Contributor Author

@comaniac @trevor-m
I have made the changes (catch error using else and updating the test for coord trans and rounding method). I see that it the conversion fails (as in not identical) for certain cases.

I had to add the following:
if (i == "nearest_neighbor" and j == "align_corners") or (i == "cubic" and j in ["half_pixel", "align_corners"]):

Should I use the discussion forum to debug this?

@comaniac
Copy link
Contributor

@mbrookhart could you help take a look if you got a chance?

mbrookhart
mbrookhart approved these changes Jul 13, 2021
View reviewed changes
Copy link
Contributor

@mbrookhart mbrookhart left a comment

Choose a reason for hiding this comment

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

This looks fine to me. I am curious what the use case for this pass is :)

@schilkunda-amba
Copy link
Contributor Author

This looks fine to me. I am curious what the use case for this pass is :)

@mbrookhart Here is the PR for this: https://discuss.tvm.apache.org/t/rfc-relay-to-onnx/6101

The issue I am facing is relay operator resize2d doesn't faithfully convert to onnx operator Resize when method = nearest_neighbor and coordinate_transformation_mode = half_pixel

@mbrookhart
Copy link
Contributor

Interesting. We run this onnx file in CI and make sure it imports correctly: https://github.com/onnx/onnx/blob/v1.6.0/onnx/backend/test/data/node/test_resize_upsample_sizes_nearest_ceil_half_pixel/model.onnx

Maybe you could import it, and then re-export the relay, and compare the protobuf you get with the original?

@schilkunda-amba
Copy link
Contributor Author

Interesting. We run this onnx file in CI and make sure it imports correctly: https://github.com/onnx/onnx/blob/v1.6.0/onnx/backend/test/data/node/test_resize_upsample_sizes_nearest_ceil_half_pixel/model.onnx

Maybe you could import it, and then re-export the relay, and compare the protobuf you get with the original?

Thanks. Let me give it a try.

comaniac
comaniac approved these changes Jul 21, 2021
View reviewed changes
Copy link
Contributor

@comaniac comaniac left a comment

Choose a reason for hiding this comment

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

LGTM. cc @trevor-m

trevor-m
trevor-m approved these changes Jul 21, 2021
View reviewed changes
Copy link
Contributor

@trevor-m trevor-m left a comment

Choose a reason for hiding this comment

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

Thanks @schilkunda-amba!

@trevor-m trevor-m merged commit 789ab1f into apache:main Jul 21, 2021
@schilkunda-amba
Copy link
Contributor Author

Thanks @trevor-m @comaniac

@schilkunda-amba schilkunda-amba deleted the relay_to_onnx_resize branch July 21, 2021 21:19
ylc pushed a commit to ylc/tvm that referenced this pull request Sep 29, 2021
@schilkunda-amba @ylc
[Relay] Support resize in the ONNX conversion (apache#8455)
9d58481 
* [Relay to Onnx]

* Added support for resize2d op
* Added unit test

* [Relay to Onnx][Resize]

* Fixed formatting errors

* [Relay to Onnx][Resize]

* Fixed issue in resize conversion: round maps to round_preferc_ceil
* Updated resize unit test to test for coordinate transform mode and
round
* Known issue: Does not match for (NN, align_corners) and Cubic

* * Fixed formatting errors

* * Fixed some more formatting errors
@junrushao junrushao mentioned this pull request Nov 1, 2021
Closed
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
@schilkunda-amba @ylc
[Relay] Support resize in the ONNX conversion (apache#8455)
12793a4 
* [Relay to Onnx]

* Added support for resize2d op
* Added unit test

* [Relay to Onnx][Resize]

* Fixed formatting errors

* [Relay to Onnx][Resize]

* Fixed issue in resize conversion: round maps to round_preferc_ceil
* Updated resize unit test to test for coordinate transform mode and
round
* Known issue: Does not match for (NN, align_corners) and Cubic

* * Fixed formatting errors

* * Fixed some more formatting errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trevor-m trevor-m trevor-m approved these changes

@comaniac comaniac comaniac approved these changes

@mbrookhart mbrookhart mbrookhart approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@schilkunda-amba @comaniac @mbrookhart @trevor-m