feat: expose IResizeLayer in dynamo.conversion.impl #2488

bowang007 · 2023-11-28T06:17:00Z

Description

#2219

Type of change

Expose IResizeLayer in dynamo.conversion.impl

Checklist:

My code follows the style guidelines of this project (You can use the linters)
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas and hacks
I have made corresponding changes to the documentation
I have added tests to verify my fix or my feature
New and existing unit tests pass locally with my changes
I have added the relevant labels to my PR in so that relevant reviewers are notified

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

github-actions

Code conforms to Python style guidelines

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

py/torch_tensorrt/dynamo/conversion/aten_ops_converters.py

gs-olive · 2023-11-29T21:06:05Z

py/torch_tensorrt/dynamo/conversion/impl/interpolate.py

+    if out_shape is not None:
+        resize_layer.shape = list(input.shape)[:2] + list(out_shape)
+    else:
+        resize_layer.scales = [1, 1] + list(scale_factor)


Should both branches of the conditional ensure that both shape and scales are set, or can TRT handle cases where only one is set?

@gs-olive
Looks like there are some internal assertions in Pytorch that checks the arguments for shape and scales.
So pytorch will throws out an error if none of them is set or both of them are set.
Since this is a converter so we assumes that the arguments are valid or pytorch will throw out error right?
Please correct me if I'm wrong, thanks!

py/torch_tensorrt/dynamo/conversion/impl/interpolate.py

zewenli98

Our converters come from the schema. The decorator's key should be one of the scheme's functions.

py/torch_tensorrt/dynamo/conversion/aten_ops_converters.py

tests/py/dynamo/conversion/test_interpolate.py

gs-olive

Added a few suggestions, but overall looks good! Could you also add support for the non-vec variants, such as upsample_bilinear2d.

py/torch_tensorrt/dynamo/conversion/impl/upsample.py

gs-olive

Looks good overall - added minor comment about mypy linting. Also needs a rebase onto the latest main to resolve merge conflicts

py/torch_tensorrt/dynamo/conversion/aten_ops_converters.py

bowang007 · 2023-12-06T01:14:25Z

Added a few suggestions, but overall looks good! Could you also add support for the non-vec variants, such as upsample_bilinear2d.

@gs-olive Any idea why this happens?

AssertionError: Detected converter for OpOverloadPacket aten.upsample_nearest2d. We do not support OpOverloadPacket-keyed converters with multiple overloads. Make sure to explicitly specify each converter overload. For instance aten.mean is not a valid key, but aten.mean.default is.

gs-olive · 2023-12-06T01:36:07Z

@bowang007 - actually, I realized the upsample_bilinear2d op is not in the Core ATen IR set, so it is okay to omit it. That error arises when the decorator specified for the converter is something like aten.upsample_bilinear2d instead of aten.upsample_bilinear2d.default

bowang007 · 2023-12-06T22:06:03Z

@bowang007 - actually, I realized the upsample_bilinear2d op is not in the Core ATen IR set, so it is okay to omit it. That error arises when the decorator specified for the converter is something like aten.upsample_bilinear2d instead of aten.upsample_bilinear2d.default

In that case, how can we add converter for aten.upsample_bilinear2d?

gs-olive · 2023-12-07T18:35:57Z

We can add this converter using the decorator @torch.ops.aten.upsample_bilinear2d.default as opposed to @torch.ops.aten.upsample_bilinear2d.

bowang007 · 2023-12-08T23:30:11Z

Our converters come from the schema. The decorator's key should be one of the scheme's functions.

yeah that makes sense! Thanks!

gs-olive

Just needs a rebase, then looks good to me

github-actions

Code conforms to C++ style guidelines

github-actions

There are some changes that do not conform to Python style guidelines:

--- /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/dynamo/conversion/aten_ops_converters.py	2023-12-09 01:40:38.396894+00:00
+++ /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/dynamo/conversion/aten_ops_converters.py	2023-12-09 01:42:26.080943+00:00
@@ -2502,6 +2502,6 @@
        input=args[0],
        out_shape=args_bounds_check(args, 1),
        scale_factors=args_bounds_check(args, 3),
        resize_mode="bilinear",
        align_corners=args_bounds_check(args, 2),
-    )
\ No newline at end of file
+    )

github-actions

Code conforms to C++ style guidelines

github-actions

There are some changes that do not conform to Python style guidelines:

--- /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/dynamo/conversion/aten_ops_converters.py	2023-12-09 01:41:43.828878+00:00
+++ /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/dynamo/conversion/aten_ops_converters.py	2023-12-09 01:43:31.367921+00:00
@@ -2502,6 +2502,6 @@
        input=args[0],
        out_shape=args_bounds_check(args, 1),
        scale_factors=args_bounds_check(args, 3),
        resize_mode="bilinear",
        align_corners=args_bounds_check(args, 2),
-    )
\ No newline at end of file
+    )

outdated.

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

bowang007 requested a review from gs-olive November 28, 2023 06:17

facebook-github-bot added the cla signed label Nov 28, 2023

bowang007 requested review from zewenli98 and apbose November 28, 2023 06:17

bowang007 requested a review from narendasan November 28, 2023 06:17

bowang007 marked this pull request as draft November 28, 2023 06:18

github-actions bot approved these changes Nov 28, 2023

View reviewed changes

bowang007 self-assigned this Nov 28, 2023

github-actions bot approved these changes Nov 28, 2023

View reviewed changes

peri044 mentioned this pull request Nov 28, 2023

Expose IResizeLayer in dynamo.conversion.impl #2219

Closed

bowang007 marked this pull request as ready for review November 29, 2023 06:01

gs-olive reviewed Nov 29, 2023

View reviewed changes

zewenli98 previously requested changes Nov 29, 2023

View reviewed changes

py/torch_tensorrt/dynamo/conversion/aten_ops_converters.py Outdated Show resolved Hide resolved

tests/py/dynamo/conversion/test_interpolate.py Outdated Show resolved Hide resolved

bowang007 force-pushed the dynamo_resize_converter branch from 9e2cc48 to f538a97 Compare December 4, 2023 22:50

bowang007 requested review from gs-olive and zewenli98 December 4, 2023 22:55

gs-olive reviewed Dec 5, 2023

View reviewed changes

py/torch_tensorrt/dynamo/conversion/impl/upsample.py Outdated Show resolved Hide resolved

py/torch_tensorrt/dynamo/conversion/impl/upsample.py Outdated Show resolved Hide resolved

py/torch_tensorrt/dynamo/conversion/impl/upsample.py Show resolved Hide resolved

gs-olive reviewed Dec 6, 2023

View reviewed changes

py/torch_tensorrt/dynamo/conversion/aten_ops_converters.py Outdated Show resolved Hide resolved

bowang007 force-pushed the dynamo_resize_converter branch from aeb73f3 to a2680d0 Compare December 8, 2023 23:34

bowang007 requested a review from gs-olive December 8, 2023 23:34

gs-olive approved these changes Dec 9, 2023

View reviewed changes

bowang007 force-pushed the dynamo_resize_converter branch from a2680d0 to f08e370 Compare December 9, 2023 01:40

bowang007 requested review from zewenli98 and removed request for zewenli98 December 9, 2023 01:41

github-actions bot approved these changes Dec 9, 2023

View reviewed changes

github-actions bot requested changes Dec 9, 2023

View reviewed changes

github-actions bot approved these changes Dec 9, 2023

View reviewed changes

github-actions bot requested changes Dec 9, 2023

View reviewed changes

feat: expose IResizeLayer in dynamo

960458f

bowang007 force-pushed the dynamo_resize_converter branch from f08e370 to 960458f Compare December 9, 2023 01:50

github-actions bot approved these changes Dec 9, 2023

View reviewed changes

bowang007 merged commit 623d2f7 into main Dec 9, 2023

feat: expose IResizeLayer in dynamo.conversion.impl #2488

feat: expose IResizeLayer in dynamo.conversion.impl #2488

Uh oh!

Conversation

Description

Type of change

Checklist:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!