feat: support many elementwise dynamo converters #2263

zewenli98 · 2023-08-25T00:37:49Z

Description

Support many elementwise dynamo converters, including add, mul, maximum, minimum, sub, div(already implemented), pow, floor_divide, logical_and, logical_or, logical_xor, eq, gt, lt

Fixes #2208

Type of change

New feature (non-breaking change which adds functionality)

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

zewenli98 · 2023-08-29T21:16:18Z

@gs-olive The error seems not to be related to this PR. Could you take a look? Thanks!

gs-olive · 2023-08-29T22:18:20Z

@zewenli98 - taking a look now! Also, could you rebase this one to main to resolve the merge conflicts?

zewenli98 · 2023-08-29T23:27:33Z

@gs-olive Rebased! thanks!

gs-olive

See the comments below to fix the Dynamo errors appearing for this PR.

py/torch_tensorrt/dynamo/conversion/impl/elementwise/ops.py

gs-olive

Two additional changes are needed to fix this CI issue. In convert_binary_elementwise, we need to change Frameworks.TORCH to Frameworks.NUMPY:

TensorRT/py/torch_tensorrt/dynamo/conversion/impl/elementwise/base.py

Lines 77 to 82 in e49ef6d

    
           if isinstance(lhs_val, TRTTensor): 
        
               lhs_dtype = unified_dtype_converter(lhs_val.dtype, Frameworks.TORCH) 
        
               is_lhs_trt_tensor = True 
        
           if isinstance(rhs_val, TRTTensor): 
        
               rhs_dtype = unified_dtype_converter(rhs_val.dtype, Frameworks.TORCH) 
        
               is_rhs_trt_tensor = True

As well as torch.tensor to np.array:

TensorRT/py/torch_tensorrt/dynamo/conversion/impl/elementwise/base.py

Lines 105 to 108 in e49ef6d

    
           if is_lhs_trt_tensor and isinstance(rhs_val, (float, int)): 
        
               rhs_val = torch.tensor([rhs_val], dtype=lhs_dtype) 
        
           if is_rhs_trt_tensor and isinstance(lhs_val, (float, int)): 
        
               lhs_val = torch.tensor([lhs_val], dtype=rhs_dtype)

The above are fixed in #2265, so let me try to merge that + rebase to resolve the above

zewenli98 · 2023-08-31T22:10:20Z

@gs-olive Thanks for the suggestions and detailed explanations! I updated and rebased!

gs-olive

Changes look great - added some comments on schemas, converter support, and the div, add, and sub operators, which have special cases

py/torch_tensorrt/dynamo/conversion/aten_ops_converters.py

zewenli98 · 2023-09-01T23:42:35Z

@gs-olive Thanks for the review! Resolved issues above.

py/torch_tensorrt/dynamo/conversion/converter_utils.py

gs-olive · 2023-09-05T22:25:46Z

@zewenli98 - Seeing the following error in both test failures for this PR:

[TRT] [E] 4: [network.cpp::inferOutputTypes::2063] Error Code 4: Internal Error (Output tensor output0 of type Int32 produced from output of incompatible type Float)

I assume it is because the forward function in those tests is like so:

        class Tensor0DInput(torch.nn.Module):
            def forward(self, x):
                return x * 7

The input tensor x is an integer Tensor, so Torch provides an Int32 output (meaning we also expect an Int32 output), but due to the new converters, we are casting all integers for these elementwise ops to floats. I know that some elementwise converters require float inputs, but do all of the converters require this? As in - could aten.mul work without this cast?

gs-olive · 2023-09-05T22:28:09Z

For instance - in TorchScript, we don't have float casts for mul or add, only div.

gs-olive

New changes look great! See comments above on narrowing the usage of float casting.

zewenli98 · 2023-09-06T02:48:46Z

py/torch_tensorrt/dynamo/conversion/impl/elementwise/base.py

@gs-olive Thanks for your review! I just found a similar error. For some ops, such as eq, we have to specify output_dtypes=[torch.bool] in tests, otherwise, we will get error: [TRT] [E] 4: [network.cpp::inferOutputTypes::2063] Error Code 4: Internal Error (Output tensor output0 of type Int32 produced from output of incompatible type Bool).

However, the weird is that if we don't specify output_dtypes=[torch.bool], instead, just add output.dtype (do nothing) before return output in convert_binary_elementwise. The error disappeared and it can pass the test. So, I was wondering if this is kind of like lazy mode, requiring running output.dtype to get right type?

That is definitely strange, calling output.dtype shouldn't have an effect on its own, to my knowledge. The way we currently determine output data types in our torch.compile backend is we run the graph in Torch and see the output types, then we set the TRT engine outputs according to this. This becomes problematic if TRT requires a float cast where Torch does not (for instance, on Int, Int adds or multiplies). For this reason, we do not need float casting in add or mul, as there is currently not float casting in our TorchScript path. On the other hand, we do need bool type specification for eq, as you observed, since the output could otherwise be Int32.

gs-olive

See the cast_int_int_div_trt_tensor function and cross-validate with the C++ implementations to verify where casts are needed for the elementwise operators. I think very few of these need trt_cast_int_to_float. Please let me know if there are any resources that indicate otherwise.

gs-olive · 2023-09-07T00:31:49Z

py/torch_tensorrt/dynamo/conversion/impl/elementwise/ops.py

+    if isinstance(lhs_val, TRTTensor):
+        lhs_val = trt_cast_int_to_float(network, name, lhs_val)
+
+    if isinstance(rhs_val, TRTTensor):
+        rhs_val = trt_cast_int_to_float(network, name, rhs_val)


This can be removed, unless there is reason to cast the add operator to a float.

gs-olive · 2023-09-07T00:32:01Z

py/torch_tensorrt/dynamo/conversion/impl/elementwise/ops.py

+    if isinstance(lhs_val, TRTTensor):
+        lhs_val = trt_cast_int_to_float(network, name, lhs_val)
+
+    if isinstance(rhs_val, TRTTensor):
+        rhs_val = trt_cast_int_to_float(network, name, rhs_val)


This can also be removed

zewenli98 · 2023-09-07T03:04:21Z

yes! Thanks for the details! I just wanted to check if adding output.dtype is useful (control variables) in this commit. I'll let you know after resolving all the issues!

zewenli98 · 2023-09-07T21:20:21Z

@gs-olive Let me give some explanations here. At first, I referred to this doc https://docs.nvidia.com/deeplearning/tensorrt/operators/docs/ElementWise.html#data-types where it says int32 is not supported for all elementwise ops. However, after some tests, I found only div and pow don't support int32. Accordingly, I made these changes in this commit. But it looks like there's an error about pip in the smoke test.

gs-olive · 2023-09-07T21:30:42Z

@zewenli98 Understood - thanks for this resource. In general, I would recommend using the C++ TRT documentation for input restrictions. This link for the C++ API suggests that POW is restricted to float, half, and bool, and AND, OR, and XOR only support bool, whereas all other ops can also have int32 (though div is an outlier, as you mentioned). Regarding the pip error, it is resolved in #2298 which will be merged soon.

py/torch_tensorrt/dynamo/conversion/converter_utils.py

py/torch_tensorrt/dynamo/conversion/impl/elementwise/ops.py

gs-olive · 2023-09-07T23:11:12Z

Thanks for the changes - #2298 was just merged, so could you rebase to main

zewenli98 · 2023-09-07T23:26:41Z

Rebased! Thanks George!

gs-olive

Looks great to me! Well tested and very useful utilities + converters added. This will greatly improve our operator coverage and correctness - thanks @zewenli98!

gs-olive · 2023-09-08T18:06:50Z

@zewenli98 - Could you rebase to the latest main for TorchScript testing

add output_dtypes in test add util func and fix bugs add overloads, update tests, and fix a bug fix arg bug delete int2float conversion for some ops update type conversion

facebook-github-bot added the cla signed label Aug 25, 2023

github-actions bot requested a review from gs-olive August 25, 2023 00:38

zewenli98 force-pushed the elementwise_dynamo_converters branch from 8dfba80 to a187bb9 Compare August 29, 2023 23:24

gs-olive requested changes Aug 31, 2023

View reviewed changes

py/torch_tensorrt/dynamo/conversion/impl/elementwise/ops.py Outdated Show resolved Hide resolved

gs-olive reviewed Aug 31, 2023

View reviewed changes

zewenli98 force-pushed the elementwise_dynamo_converters branch from a187bb9 to da66673 Compare August 31, 2023 22:07

zewenli98 requested a review from gs-olive August 31, 2023 22:10

gs-olive requested changes Aug 31, 2023

View reviewed changes

zewenli98 mentioned this pull request Sep 1, 2023

feat: support linear (fully connected layer) dynamo converter #2253

Merged

7 tasks

zewenli98 requested a review from gs-olive September 1, 2023 23:41

zewenli98 force-pushed the elementwise_dynamo_converters branch from a4075ca to b51c121 Compare September 2, 2023 00:27

gs-olive requested changes Sep 5, 2023

View reviewed changes

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

zewenli98 requested a review from gs-olive September 5, 2023 22:21

gs-olive requested changes Sep 5, 2023

View reviewed changes

zewenli98 commented Sep 6, 2023

View reviewed changes

gs-olive mentioned this pull request Sep 6, 2023

↔ [Converter] Decouple aten.div from acc_ops_converters #2297

Closed

gs-olive requested changes Sep 7, 2023

View reviewed changes

zewenli98 force-pushed the elementwise_dynamo_converters branch from 1e65a53 to 25c1ff4 Compare September 7, 2023 20:47

gs-olive requested changes Sep 7, 2023

View reviewed changes

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

py/torch_tensorrt/dynamo/conversion/impl/elementwise/ops.py Outdated Show resolved Hide resolved

zewenli98 force-pushed the elementwise_dynamo_converters branch from 8dd9a33 to 6175423 Compare September 7, 2023 23:24

zewenli98 requested a review from gs-olive September 8, 2023 00:11

gs-olive approved these changes Sep 8, 2023

View reviewed changes

feat: support many elementwise dynamo converters

07d823b

add output_dtypes in test add util func and fix bugs add overloads, update tests, and fix a bug fix arg bug delete int2float conversion for some ops update type conversion

zewenli98 force-pushed the elementwise_dynamo_converters branch from 6175423 to 07d823b Compare September 8, 2023 18:11

gs-olive merged commit 40f8064 into pytorch:main Sep 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support many elementwise dynamo converters #2263

feat: support many elementwise dynamo converters #2263

zewenli98 commented Aug 25, 2023

zewenli98 commented Aug 29, 2023 •

edited

Loading

gs-olive commented Aug 29, 2023

zewenli98 commented Aug 29, 2023 •

edited

Loading

gs-olive left a comment

gs-olive left a comment •

edited

Loading

zewenli98 commented Aug 31, 2023

gs-olive left a comment

zewenli98 commented Sep 1, 2023

gs-olive commented Sep 5, 2023

gs-olive commented Sep 5, 2023 •

edited

Loading

gs-olive left a comment

zewenli98 Sep 6, 2023

gs-olive Sep 6, 2023 •

edited

Loading

gs-olive left a comment

gs-olive Sep 7, 2023

gs-olive Sep 7, 2023

zewenli98 commented Sep 7, 2023

zewenli98 commented Sep 7, 2023

gs-olive commented Sep 7, 2023 •

edited

Loading

gs-olive commented Sep 7, 2023

zewenli98 commented Sep 7, 2023

gs-olive left a comment

gs-olive commented Sep 8, 2023

	if isinstance(lhs_val, TRTTensor):
	lhs_dtype = unified_dtype_converter(lhs_val.dtype, Frameworks.TORCH)
	is_lhs_trt_tensor = True
	if isinstance(rhs_val, TRTTensor):
	rhs_dtype = unified_dtype_converter(rhs_val.dtype, Frameworks.TORCH)
	is_rhs_trt_tensor = True

	if is_lhs_trt_tensor and isinstance(rhs_val, (float, int)):
	rhs_val = torch.tensor([rhs_val], dtype=lhs_dtype)
	if is_rhs_trt_tensor and isinstance(lhs_val, (float, int)):
	lhs_val = torch.tensor([lhs_val], dtype=rhs_dtype)

feat: support many elementwise dynamo converters #2263

feat: support many elementwise dynamo converters #2263

Conversation

zewenli98 commented Aug 25, 2023

Description

Type of change

Checklist:

zewenli98 commented Aug 29, 2023 • edited Loading

gs-olive commented Aug 29, 2023

zewenli98 commented Aug 29, 2023 • edited Loading

gs-olive left a comment

Choose a reason for hiding this comment

gs-olive left a comment • edited Loading

Choose a reason for hiding this comment

zewenli98 commented Aug 31, 2023

gs-olive left a comment

Choose a reason for hiding this comment

zewenli98 commented Sep 1, 2023

gs-olive commented Sep 5, 2023

gs-olive commented Sep 5, 2023 • edited Loading

gs-olive left a comment

Choose a reason for hiding this comment

zewenli98 Sep 6, 2023

Choose a reason for hiding this comment

gs-olive Sep 6, 2023 • edited Loading

Choose a reason for hiding this comment

gs-olive left a comment

Choose a reason for hiding this comment

gs-olive Sep 7, 2023

Choose a reason for hiding this comment

gs-olive Sep 7, 2023

Choose a reason for hiding this comment

zewenli98 commented Sep 7, 2023

zewenli98 commented Sep 7, 2023

gs-olive commented Sep 7, 2023 • edited Loading

gs-olive commented Sep 7, 2023

zewenli98 commented Sep 7, 2023

gs-olive left a comment

Choose a reason for hiding this comment

gs-olive commented Sep 8, 2023

zewenli98 commented Aug 29, 2023 •

edited

Loading

zewenli98 commented Aug 29, 2023 •

edited

Loading

gs-olive left a comment •

edited

Loading

gs-olive commented Sep 5, 2023 •

edited

Loading

gs-olive Sep 6, 2023 •

edited

Loading

gs-olive commented Sep 7, 2023 •

edited

Loading