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

[ONNX] [Relay] Resize Opset 13 #9265

Merged
merged 4 commits into from
Oct 14, 2021
Merged

[ONNX] [Relay] Resize Opset 13 #9265

merged 4 commits into from
Oct 14, 2021

Conversation

CircleSpin
Copy link
Contributor

AndrewZhaoLuo
AndrewZhaoLuo approved these changes Oct 12, 2021
View reviewed changes
Copy link
Contributor

@AndrewZhaoLuo AndrewZhaoLuo left a comment

Choose a reason for hiding this comment

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

LGTM

tests/python/frontend/onnx/test_forward.py Outdated
@@ -3970,6 +3970,12 @@ def verify(ishape, oshape, scales, mode, coord_trans="asymmetric", alpha=0.5, ex
make_constant_node("scales", onnx.TensorProto.FLOAT, (len(scales),), scales),
]
input_names = ["X", "roi", "scales"]

# if scales == []:
Copy link
Contributor

Choose a reason for hiding this comment

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

Just remove this if you don't need it.

mbrookhart
mbrookhart approved these changes Oct 12, 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.

Remove the commented code, otherwise LGTM, thanks!

emijiayw
emijiayw reviewed Oct 13, 2021
View reviewed changes
python/tvm/relay/frontend/onnx.py
@classmethod
def v11_13_common(cls, inputs, size, attr, params):
"""
Resize v11 and Resize v13 are identical except in how
Copy link

Choose a reason for hiding this comment

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

Resize v11 and resize

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi emijiayw, Thanks for your feedback! Resize v13 is specified for clarity since just resize could be misleading. :)

python/tvm/relay/frontend/onnx.py Outdated
def v11_13_common(cls, inputs, size, attr, params):
"""
Resize v11 and Resize v13 are identical except in how
they handle passing in scale and size. This utility
Copy link

Choose a reason for hiding this comment

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

they handle the passing

python/tvm/relay/frontend/onnx.py
scale = inputs[2]
size = inputs[3]
if size is not None:
assert scale is None, "One of scale or size should be passed, not both."
Copy link

Choose a reason for hiding this comment

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

One of the scales or sizes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

both scale and size are singular (there is only one of them) and adding an 's' makes them plural. It's only one dog or cat, not dogs or cats 🐶 🐈 vs 🐶 🐶 🐶 🐈 🐈 🐈

python/tvm/relay/frontend/onnx.py
), "One of scale or size should be passed, not both."
size = inputs[3]
else:
assert len(scale_shape) != 0, "One of scale or size should be passed."
Copy link

Choose a reason for hiding this comment

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

One of the scales or sizes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above!

@masahi masahi merged commit 7e014a4 into apache:main Oct 14, 2021
masahi pushed a commit to Laurawly/tvm-1 that referenced this pull request Oct 14, 2021
@CircleSpin @jwfromm @masahi
[ONNX] [Relay] Resize Opset 13 (apache#9265)
8b94d93 
* Fix handling of optional inputs.

* Missed one test in the ignore list.

* split 11 and 13

* removed comments, adjusted for git review

Co-authored-by: Josh Fromm <jwfromm@uw.edu>
Co-authored-by: Matthew <mbrookhart@octoml.ai>
Co-authored-by: CircleSpin <jocelyn@pop-os.localdomain>
@junrushao junrushao mentioned this pull request Nov 2, 2021
Closed
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 7, 2022
@CircleSpin @jwfromm @ylc
[ONNX] [Relay] Resize Opset 13 (apache#9265)
9eb05a8 
* Fix handling of optional inputs.

* Missed one test in the ignore list.

* split 11 and 13

* removed comments, adjusted for git review

Co-authored-by: Josh Fromm <jwfromm@uw.edu>
Co-authored-by: Matthew <mbrookhart@octoml.ai>
Co-authored-by: CircleSpin <jocelyn@pop-os.localdomain>
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
@CircleSpin @jwfromm @ylc
[ONNX] [Relay] Resize Opset 13 (apache#9265)
05c125b 
* Fix handling of optional inputs.

* Missed one test in the ignore list.

* split 11 and 13

* removed comments, adjusted for git review

Co-authored-by: Josh Fromm <jwfromm@uw.edu>
Co-authored-by: Matthew <mbrookhart@octoml.ai>
Co-authored-by: CircleSpin <jocelyn@pop-os.localdomain>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emijiayw emijiayw emijiayw left review comments

@mbrookhart mbrookhart mbrookhart approved these changes

@AndrewZhaoLuo AndrewZhaoLuo AndrewZhaoLuo approved these changes

@areusch areusch Awaiting requested review from areusch

@comaniac comaniac Awaiting requested review from comaniac

@Huyuwei Huyuwei Awaiting requested review from Huyuwei

@jroesch jroesch Awaiting requested review from jroesch

@junrushao junrushao Awaiting requested review from junrushao

@jwfromm jwfromm Awaiting requested review from jwfromm

@kazum kazum Awaiting requested review from kazum

@merrymercy merrymercy Awaiting requested review from merrymercy

@siju-samuel siju-samuel Awaiting requested review from siju-samuel

@srkreddy1238 srkreddy1238 Awaiting requested review from srkreddy1238

@tqchen tqchen Awaiting requested review from tqchen

@yzhliu yzhliu Awaiting requested review from yzhliu

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

Successfully merging this pull request may close these issues.

None yet
6 participants
@CircleSpin @mbrookhart @AndrewZhaoLuo @emijiayw @masahi @jwfromm