Skip to content

diversify parameter types for a couple of prototype kernels#6635

Merged
pmeier merged 13 commits intopytorch:mainfrom
pmeier:resize-test
Sep 28, 2022
Merged

diversify parameter types for a couple of prototype kernels#6635
pmeier merged 13 commits intopytorch:mainfrom
pmeier:resize-test

Conversation

@pmeier
Copy link
Copy Markdown
Contributor

@pmeier pmeier commented Sep 23, 2022

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 or float inputs while we have annotated List[int] and List[float]. All inputs that otherwise provide a tuple instead of a list work.

features.BoundingBox: F.resize_bounding_box,
features.Mask: F.resize_mask,
},
skips=[
Copy link
Copy Markdown
Contributor Author

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.

def _get_resize_sizes(image_size):
height, width = image_size
length = max(image_size)
# yield length
Copy link
Copy Markdown
Contributor Author

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.

Copy link
Copy Markdown
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

@pmeier LGTM. Thanks for fixing the tests. It's going to be handy for checking #6636 more thoroughly.

@pmeier pmeier changed the title add more size types for prototype resize sample inputs diversify parameter types for a couple of prototype kernels Sep 27, 2022
Comment on lines +288 to +291
# 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)
Copy link
Copy Markdown
Contributor

@datumbox datumbox Sep 28, 2022

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?

Copy link
Copy Markdown
Contributor Author

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.

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.

Copy link
Copy Markdown
Contributor Author

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.

@pmeier pmeier merged commit 29b0831 into pytorch:main Sep 28, 2022
@pmeier pmeier deleted the resize-test branch September 28, 2022 11:50
facebook-github-bot pushed a commit that referenced this pull request Sep 29, 2022
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants