-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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.
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
[Torch] Upsampling op support and enable registering a user defined op conversion map #4961
Conversation
@tqchen @comaniac I get an error from sphinx issues which is not related to this PR, can you take a look? It seems this comes from |
I saw these errors before but have no idea how were they resolved. Could you rebase to upstream master to see if errors persist? |
It is already rebased to the latest commit. The error is still there. |
On a quick look, it seems it is behaving as expected by
The warnings are not introduced by your change, so it seems we need a pre-patch fixing docs, in order to unblock the build for this new check, introduced recently. The relevant lines are: |
Discussion about fixing docs is happening in #4908. |
Yes. It is due to the doc checker introduced yesterday. I'm looking into the problem now. |
Left minor comments but otherwise looks good. Having a custom convert map is a neat idea that I'd like to see added to other frontends. |
docs error should be fixed by #4967 |
07c63dd
to
d610392
Compare
@alexwong @jwfromm I removed the dependency on |
@vinx13 @anijain2305 @icemelon9 comments are addressed and tests passed, I think it is ready for merge. A custom convert map could be of interest to other frontends. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, with a minor suggestion.
4bd9cdb
to
e410221
Compare
Thanks @jwfromm @alexwong @anijain2305 |
…p conversion map (apache#4961) * add custom conversion map * add roi align test using custom convert map * refactor test * add support for upsampling op and test on segmentation models * remove redundant no_grad * add upsampling test case * make the default custom map None, instead of empty dict * updated tests, remove packaging and drop PT 1.2 support * add better support for aten::to and tests * add a note on dilation in x86
…p conversion map (apache#4961) * add custom conversion map * add roi align test using custom convert map * refactor test * add support for upsampling op and test on segmentation models * remove redundant no_grad * add upsampling test case * make the default custom map None, instead of empty dict * updated tests, remove packaging and drop PT 1.2 support * add better support for aten::to and tests * add a note on dilation in x86
This adds support for torch upsample op. It enables running image segmentation models from torchvision (see the test case).
I also added a simple mechanism to register a custom op conversion map, that can be used with the predefined
_convert_map
we already have. The motivation is that torch has too many ops to cover, and it also allows defining custom operators. This makes implementing all op conversion in a single map inside Relay frontend infeasible, and it would be very tedious if we have to modify TVM source each time we need to add a new conversion. Also in torch there are many esoteric operators that doesn't justify for being added to the "official" conversion map in the frontend.For example of how to use the custom conversion map, see the test case
test_custom_conversion_map()
where I show how to convert torchvision's custom optorchvision::roi_align
using Relay'svision.roi_align
op.Please review @jwfromm @alexwong @vinx13 @anijain2305 @icemelon9