-
Notifications
You must be signed in to change notification settings - Fork 53
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
Fix upsample_bilinear to respect align_corner argument #1254
Changes from all commits
04ccd06
60c9b97
2cb4245
7d14542
85fe17c
0ba5f40
ab8ec04
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2295,38 +2295,45 @@ def aten_upsample_bicubic2d_backward( | |
@torch_op("aten::upsample_bilinear2d", trace_only=True) | ||
def aten_upsample_bilinear2d( | ||
self: TReal, | ||
output_size: Optional[INT64] = None, | ||
output_size: INT64, | ||
align_corners: bool, | ||
scales_h: Optional[float] = None, | ||
scales_w: Optional[float] = None, | ||
align_corners: bool = True, # pylint: disable=unused-argument | ||
) -> TReal: | ||
"""upsample_bilinear2d(Tensor self, SymInt[2] output_size, bool align_corners, float? scales_h=None, float? scales_w=None) -> Tensor""" | ||
|
||
coordinate_transformation_mode = "align_corners" if align_corners else "pytorch_half_pixel" | ||
if output_size is not None: | ||
result = _aten_upsample_bilinear2d_output_size(self, output_size) | ||
result = _aten_upsample_bilinear2d_output_size( | ||
self, output_size, coordinate_transformation_mode | ||
) | ||
else: | ||
assert scales_h is not None | ||
assert scales_h == scales_w, f"scale_h({scales_h}) != scale_w({scales_w})" | ||
Comment on lines
2306
to
2312
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we reverse the check - omit output_size when the scales are not None? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you have link to spec whether which one takes precedence over the other? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The reference here shows scale is considered first if present: https://github.com/pytorch/pytorch/blob/f6767244cf292cb5e434303fe62e576b456e92eb/torch/_decomp/decompositions.py#L3311-L3315 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But testing suggested otherwise |
||
result = _aten_upsample_bilinear2d_scales(self, scales_h, scales_w) | ||
result = _aten_upsample_bilinear2d_scales( | ||
self, scales_h, scales_w, coordinate_transformation_mode | ||
) | ||
return result | ||
|
||
|
||
@torch_op("aten::upsample_bilinear2d.vec", trace_only=True) | ||
def aten_upsample_bilinear2d_vec( | ||
self: TReal, | ||
output_size: Optional[INT64] = None, | ||
align_corners: bool = True, | ||
scale_factors: Optional[Sequence[float]] = None, | ||
output_size: Optional[INT64], | ||
align_corners: bool, | ||
scale_factors: Optional[Sequence[float]], | ||
) -> TReal: | ||
"""upsample_bilinear2d.vec(Tensor input, SymInt[]? output_size, bool align_corners, float[]? scale_factors) -> Tensor""" | ||
scales_h = scale_factors[0] if scale_factors is not None else None | ||
scales_w = scale_factors[1] if scale_factors is not None else None | ||
return aten_upsample_bilinear2d(self, output_size, scales_h, scales_w, align_corners) | ||
return aten_upsample_bilinear2d(self, output_size, align_corners, scales_h, scales_w) | ||
|
||
|
||
@torch_op("aten::upsample_bilinear2d", private=True) | ||
def _aten_upsample_bilinear2d_output_size( | ||
self: TReal, | ||
output_size: INT64, | ||
coordinate_transformation_mode: str, | ||
) -> TReal: | ||
"""upsample_bilinear2d(Tensor self, SymInt[2] output_size, bool align_corners, float? scales_h=None, float? scales_w=None) -> Tensor""" | ||
|
||
|
@@ -2341,7 +2348,8 @@ def _aten_upsample_bilinear2d_output_size( | |
None, | ||
output_size, | ||
mode="linear", | ||
coordinate_transformation_mode="align_corners", | ||
coordinate_transformation_mode=coordinate_transformation_mode, | ||
nearest_mode="floor", | ||
) | ||
|
||
|
||
|
@@ -2350,6 +2358,7 @@ def _aten_upsample_bilinear2d_scales( | |
self: TReal, | ||
scales_h: float, | ||
scales_w: float, | ||
coordinate_transformation_mode: str, | ||
) -> TReal: | ||
"""upsample_bilinear2d(Tensor self, SymInt[2] output_size, bool align_corners, float? scales_h=None, float? scales_w=None) -> Tensor""" | ||
|
||
|
@@ -2366,7 +2375,8 @@ def _aten_upsample_bilinear2d_scales( | |
scales, # format should be: [1.0, 1.0, scale_h, scale_w] | ||
None, | ||
mode="linear", | ||
coordinate_transformation_mode="align_corners", | ||
coordinate_transformation_mode=coordinate_transformation_mode, | ||
nearest_mode="floor", | ||
) | ||
|
||
|
||
|
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.
Is this still valid?
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.
Unclear to me, looks like different scale values are not covered in tests.
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.
@xiaowuhu