-
Notifications
You must be signed in to change notification settings - Fork 7.1k
diversify parameter types for a couple of prototype kernels #6635
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
Conversation
@@ -50,6 +50,13 @@ def maybe_skip(self, *, test_name, args_kwargs, device): | |||
features.BoundingBox: F.resize_bounding_box, | |||
features.Mask: F.resize_mask, | |||
}, | |||
skips=[ |
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.
These skips currently don't do anything since we don't have integer sizes, but will be necessary as soon that happens.
@@ -129,6 +129,9 @@ def sample_inputs_horizontal_flip_mask(): | |||
|
|||
def _get_resize_sizes(image_size): | |||
height, width = image_size | |||
length = max(image_size) | |||
# yield length |
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.
@datumbox uncomment this to see if your fix worked.
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.
# This is just a dummy value to avoid raising an error in `_affine_parse_args` although we don't have an | ||
# interpolation mode for bounding boxes. | ||
interpolation = InterpolationMode.NEAREST | ||
angle, translate, shear, center = _affine_parse_args(angle, translate, scale, shear, interpolation, center) |
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.
I have explicitly avoided calling this method on my other PR. See #6636 (comment). Thoughts?
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.
Well, I used it, since the old parsing, i.e.
vision/torchvision/prototype/transforms/functional/_geometry.py
Lines 291 to 306 in 2d92728
if translate is None: | |
translate = [0.0, 0.0] | |
if scale is None: | |
scale = 1.0 | |
if shear is None: | |
shear = [0.0, 0.0] | |
if center is None: | |
height, width = image_size | |
center_f = [width * 0.5, height * 0.5] | |
else: | |
center_f = [float(c) for c in center] | |
translate_f = [float(t) for t in translate] |
failed for some of the new sample inputs. Meaning, the bounding box kernel did not support some parameters that the other kernels supported. This is why I used this to make sure that every kernel does the same. If you have speed concerns, I can look into what exactly caused the failure and only fix this.
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.
I've reverted the changes in 6dfc965 and added skips for the tests. They should actually be xfails, but the test framework currently doesn't support that. #6653 adds support for xfails. @datumbox you need to remove the skips in your PR, because the tests otherwise will not start to fail, even if you did something wrong.
…6635) Summary: * add more size types for prototype resize sample inputs * add skip for dispatcher * add more sizes to resize kernel info * add more skips * add more diversity to gaussian_blur parameters * diversify affine parameters and fix bounding box kernel * fix center_crop dispatcher info * revert kernel fixes * add skips for scalar shears in affine_bounding_box Reviewed By: YosuaMichael Differential Revision: D39885425 fbshipit-source-id: 4262e95aae47790f0776e172782b04fc00f158dd
This is preparation for #6636. It diversifies the parameter type inputs for the kernels that are handled there.
This PR only skips some JIT tests for
int
orfloat
inputs while we have annotatedList[int]
andList[float]
. All inputs that otherwise provide a tuple instead of a list work.