-
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
[Relay][Frontend][Onnx] MaxUnpool Operator #7036
Conversation
@masahi @mbrookhart can you guys take a look at this PR? |
Whoops, I tested this with a newer version of onnxruntime that can do lenient shape merges. I'll fix the test to specify the expected output shape. |
maybe it is time to upgrade ONNX and ORT on CI? We are at ONNX v1.6, while the latest is 1.8. Our ORT is hopelessly old: We are at 1.00 while the latest is 1.52. I can take a look at CI upgrade for ONNX this month @jwfromm. I did that for PyTorch last month. |
@masahi that would be nice, although in this case I was just being lazy and not calculating the output shape. I've fixed the test now if you want to take another look. |
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.
Interesting that we can support max unpool without modifying max pooling to return indices.
thanks @jwfromm |
@masahi you're right that most models will use MaxUnpool with MaxPool returning indices, and we probably want to add that option to MaxPool operators soon. |
* Added maxunpool test. * MaxUnpool implemented and tested. * Lint fix. * Add explicit output shape in tests.
* Added maxunpool test. * MaxUnpool implemented and tested. * Lint fix. * Add explicit output shape in tests.
* Added maxunpool test. * MaxUnpool implemented and tested. * Lint fix. * Add explicit output shape in tests.
I recently encountered a model with that uses the MaxUnpool op, a pretty interesting operator that is designed to reverse a maxpool. I've added support for it in our frontend through some sneaky use of
scatter
and found that it matches onnx's execution for each special case of the operator.