-
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
[BUG_FIX] Fix resize test #6298
Conversation
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.
Add another test case? I'm wondering about the combinations of the various modes, the check is missing any reference to half_pixel and bicubic.
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.
test_dynamic_to_static_resize
also needs to be updated, otherwise looks great
Frontend tests are failing really bad with this change. We need to fix hardcoded There is also bilinear + align_corners=False combination in tflite test, which translates to bilinear + asymmetric in Relay: I don't know what TF does with bilinear + align_corners=False. Also not sure if bilinear + asymmetric in Relay is a valid combination. Maybe @jwfromm @srkreddy1238 have thought on this? |
@masahi I changed the onnx frontend to set align_corners to False if the method is 'nearest_neighbor', otherwise set it to true. This got rid of the issue in the onnx importer. TFLite is not passing one test, though, because it's trying to use the bilinear + asymmetric combination... |
It looks like bilinear + asymmetric passes the resize test, but for some reason it is not tested in the topi test. I added it to the topi test and removed that check from the topi resize op. |
Yes this change now looks good to me. After CI pass we can merge. |
@masahi Thanks! |
oh CI is failing because there is nearest + half_pixel combination in pytorch frontend ... @electriclilies can you fix the condition above to
Options in resize is a hairy issue, sorry to have you go through this. |
@masahi Thanks! it looks like there is a similar condition in upsample3d, I also changed that for consistency. Let me know if you think that will cause a problem |
Thanks @electriclilies @mbrookhart |
* fix resize tests * add different scale to resize tests * fix dynamic to static resize test * fix error throwing in topi resize * fix topi and importer tests * fix lint * flakey test failed * make resize test less sensitive; had floating point rounding err on gpu * remove nearest_neighbor + half_pixel option from pytorch importer * remove nearest_neighbor + half_pixel in upsample3d
* fix resize tests * add different scale to resize tests * fix dynamic to static resize test * fix error throwing in topi resize * fix topi and importer tests * fix lint * flakey test failed * make resize test less sensitive; had floating point rounding err on gpu * remove nearest_neighbor + half_pixel option from pytorch importer * remove nearest_neighbor + half_pixel in upsample3d
* fix resize tests * add different scale to resize tests * fix dynamic to static resize test * fix error throwing in topi resize * fix topi and importer tests * fix lint * flakey test failed * make resize test less sensitive; had floating point rounding err on gpu * remove nearest_neighbor + half_pixel option from pytorch importer * remove nearest_neighbor + half_pixel in upsample3d
* fix resize tests * add different scale to resize tests * fix dynamic to static resize test * fix error throwing in topi resize * fix topi and importer tests * fix lint * flakey test failed * make resize test less sensitive; had floating point rounding err on gpu * remove nearest_neighbor + half_pixel option from pytorch importer * remove nearest_neighbor + half_pixel in upsample3d
* fix resize tests * add different scale to resize tests * fix dynamic to static resize test * fix error throwing in topi resize * fix topi and importer tests * fix lint * flakey test failed * make resize test less sensitive; had floating point rounding err on gpu * remove nearest_neighbor + half_pixel option from pytorch importer * remove nearest_neighbor + half_pixel in upsample3d
This PR fixes the bug in #6237. In resize, the method "nearest_neighbor" is not compatible with the coordinate_transform_mode "align_corners", and the method "bilinear" is not compatible with coordinate_transformation_modes other than "align_corners".
I also added checks to the topi resize method to prevent the user from passing incompatible method and coordinate_transformation_mode combinations into resize.
@mbrookhart @masahi please take a look!