-
Notifications
You must be signed in to change notification settings - Fork 360
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
fx2trt converters transformer encoder #1814
fx2trt converters transformer encoder #1814
Conversation
- Add backend adapted from previous `fx2trt_compiler` provided by Dynamo - Currently, the TRTSplitter needs work to fully support the `aten` path - Additionally, the existing `aten` pass was reworked to exclude the `torch._dynamo.export` call, which may be necessary here
…onverters_transformer_encoder
…e, aten::rsub, aten::rsqrt
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.
Code conforms to C++ style guidelines
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.
Code conforms to Python style guidelines
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.
Code conforms to C++ style guidelines
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.
Code conforms to Python style guidelines
|
||
|
||
@tensorrt_converter(torch.nn.functional.relu) | ||
@tensorrt_converter(torch.nn.modules.activation.ReLU) |
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.
why do we have the torch.nn.modules.xxx?
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.
The idea was to separate out the torch.nn, torch.ops.aten and torch.nn into separate files and register them in separate files. Then the common implementation would be in operator.py and activation.py. So they are reusing the existing acc_ops implementation in many cases, just that it is placed in operator.py and activation.py.
At present we do not need these converters perse, and since these do not have tests, I will remove them.
@@ -1,76 +1,197 @@ | |||
import numpy as np | |||
import operator |
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.
why don't consolidate this file with operator.py
?
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.
Just a design choice, to separate activation.py operators from operator.py operators. In activation.py I have kept the function add_activation_layer which is the same as common_activation.py.
|
||
|
||
def layer_norm( | ||
def acc_ops_layer_norm( |
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 think we need a discussion on the development plan for this aten2ait. We hope that acc_ops_converters.py are not changed fundamentally unless it is a feature support.
I suggest a good way as Apurba can work on the aten_ops_converters.py
and call the converters defined in operators.py. If you can reuse any existing implementations in acc_ops_converters.py
, that is good and saves your efforts. If you have a better idea to reuse or need to restructure the code, you can create the utility functions in operator.py
So in way, Apurba's development will lay in aten_ops_converters.py
and 'operator.py'
cc @narendasan @wushirong @yinghai
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.
Sure, we can sync up and discuss, that would be good!
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.
Code conforms to Python style guidelines
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.
Code conforms to C++ style guidelines
…onverters_transformer_encoder
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.
Code conforms to Python style guidelines
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.
Code conforms to C++ style guidelines
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.
There are some changes that do not conform to Python style guidelines:
--- py/torch_tensorrt/fx/test/converters/aten_op/test_rsub_aten.py 2023-04-20 19:31:00.210035 +0000
+++ py/torch_tensorrt/fx/test/converters/aten_op/test_rsub_aten.py 2023-04-20 19:31:16.056961 +0000
@@ -13,11 +13,11 @@
]
)
def test_rsub(self, _, x, alpha):
class rsub(nn.Module):
def forward(self, input):
- return torch.rsub(input, input, alpha = alpha)
+ return torch.rsub(input, input, alpha=alpha)
inputs = [torch.randn(x)]
self.run_test(
rsub(),
inputs,
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.
Code conforms to C++ style guidelines
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.
Code conforms to Python style guidelines
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.
Code conforms to C++ style guidelines
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.
Code conforms to Python style guidelines
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.
Code conforms to C++ style guidelines
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.
Code conforms to C++ style guidelines
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.
Code conforms to Python style guidelines
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.
Code conforms to C++ style guidelines
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.
Code conforms to Python style guidelines
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.
Code conforms to C++ style guidelines
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.
There are some changes that do not conform to Python style guidelines:
--- py/torch_tensorrt/fx/test/converters/aten_op/test_layer_norm_aten.py 2023-04-21 05:45:09.441819 +0000
+++ py/torch_tensorrt/fx/test/converters/aten_op/test_layer_norm_aten.py 2023-04-21 05:45:25.842503 +0000
@@ -15,11 +15,10 @@
inputs = [torch.randn(1, 3, 224, 224)]
self.run_test(
TestModule(), inputs, expected_ops={torch.ops.aten.layer_norm.default}
)
-
def test_layernorm_with_dynamic_shape(self):
class TestModule(torch.nn.Module):
def __init__(self):
super().__init__()
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.
Code conforms to C++ style guidelines
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.
Code conforms to Python style guidelines
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.
Code conforms to C++ style guidelines
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.
Code conforms to Python style guidelines
Which release version has merged this pr? |
Description
Converters- aten::rsub, aten::rsqrt, aten::chunk(further improvements), aten::softmax, aten::layer_norm, aten::where
Fixes #1752 Transformer encoder
Fixes #1769
Fixes #1794