Skip to content
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

Reorg for converters elu and selu (FX Converter Refactor [7/N]) <Target: converter_reorg_proto> #1903

Merged
merged 1 commit into from
Jun 12, 2023

Conversation

apbose
Copy link
Collaborator

@apbose apbose commented May 10, 2023

No description provided.

Copy link

@github-actions github-actions bot left a 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

Copy link

@github-actions github-actions bot left a 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/converters/aten_ops_converters.py	2023-05-10 05:51:44.730358 +0000
+++ py/torch_tensorrt/fx/converters/aten_ops_converters.py	2023-05-10 05:52:04.953773 +0000
@@ -177,11 +177,11 @@
    args: Tuple[Argument, ...],
    kwargs: Dict[str, Argument],
    name: str,
) -> Union[TRTTensor, Sequence[TRTTensor]]:

-    if (len(args) > 2) :
+    if len(args) > 2:
        return activation.selu(
            network,
            target,
            SourceIR.ATEN,
            name,
--- py/torch_tensorrt/fx/converters/impl/activation.py	2023-05-10 05:51:44.730358 +0000
+++ py/torch_tensorrt/fx/converters/impl/activation.py	2023-05-10 05:52:05.422983 +0000
@@ -136,6 +136,6 @@
        source_ir,
        name,
        operation_type,
        input_val,
        dyn_range_fn=elu_dyn_range_fn,
-    )
\ No newline at end of file
+    )
--- py/torch_tensorrt/fx/converters/nn_ops_converters.py	2023-05-10 05:51:44.730358 +0000
+++ py/torch_tensorrt/fx/converters/nn_ops_converters.py	2023-05-10 05:52:05.556664 +0000
@@ -49,7 +49,7 @@
        network=network,
        target="torch.nn.functional.selu",
        source_ir=SourceIR.NN,
        name=layer_name,
        input_val=kwargs["input"],
-        alpha = kwargs["alpha"]
+        alpha=kwargs["alpha"],
    )
--- py/torch_tensorrt/fx/converters/acc_ops_converters.py	2023-05-10 05:51:44.730358 +0000
+++ py/torch_tensorrt/fx/converters/acc_ops_converters.py	2023-05-10 05:52:07.665077 +0000
@@ -1043,11 +1043,11 @@
    target: Target,
    args: Tuple[Argument, ...],
    kwargs: Dict[str, Argument],
    name: str,
) -> Union[TRTTensor, Sequence[TRTTensor]]:
-    
+
    return activation.elu(
        network,
        target,
        SourceIR.ACC,
        name,
@@ -1070,11 +1070,10 @@
        target,
        SourceIR.ACC,
        name,
        kwargs["input"],
    )
-    


@tensorrt_converter(acc_ops.softsign)
def acc_ops_softsign(
    network: TRTNetwork,
--- py/torch_tensorrt/fx/test/converters/aten_op/test_elu_aten.py	2023-05-10 05:51:44.734358 +0000
+++ py/torch_tensorrt/fx/test/converters/aten_op/test_elu_aten.py	2023-05-10 05:52:08.075263 +0000
@@ -46,6 +46,6 @@
            TestModule(), input_specs, expected_ops={torch.ops.aten.elu.default}
        )


if __name__ == "__main__":
-    run_tests()
\ No newline at end of file
+    run_tests()
--- py/torch_tensorrt/fx/test/converters/aten_op/test_selu_aten.py	2023-05-10 05:51:44.734358 +0000
+++ py/torch_tensorrt/fx/test/converters/aten_op/test_selu_aten.py	2023-05-10 05:52:08.233990 +0000
@@ -46,6 +46,6 @@
            TestModule(), input_specs, expected_ops={torch.ops.aten.elu.default}
        )


if __name__ == "__main__":
-    run_tests()
\ No newline at end of file
+    run_tests()

Copy link

@github-actions github-actions bot left a 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

Copy link

@github-actions github-actions bot left a 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

@apbose apbose requested review from narendasan, frank-wei and wushirong and removed request for yinghai May 12, 2023 16:58
Copy link

@github-actions github-actions bot left a 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

Copy link

@github-actions github-actions bot left a 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

Copy link

@github-actions github-actions bot left a 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

Copy link

@github-actions github-actions bot left a 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

Copy link

@github-actions github-actions bot left a 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

Copy link

@github-actions github-actions bot left a 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

@narendasan narendasan changed the title Reorg for converters elu and selu (FX Converter Refactor [2/N]) Reorg for converters elu and selu (FX Converter Refactor [7/N]) May 12, 2023
@apbose apbose changed the title Reorg for converters elu and selu (FX Converter Refactor [7/N]) Reorg for converters elu and selu (FX Converter Refactor [7/N]) <Target: converter_reorg_proto> May 15, 2023
@gs-olive gs-olive force-pushed the converter_reorg_proto branch from 1696cd2 to 12f545c Compare May 15, 2023 21:25
@gs-olive gs-olive force-pushed the converter_reorg_activation_elu branch from f87d034 to 84ff793 Compare May 15, 2023 22:21
Copy link

@github-actions github-actions bot left a 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

Copy link

@github-actions github-actions bot left a 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

@narendasan narendasan force-pushed the converter_reorg_activation_elu branch from 84ff793 to 67cac89 Compare May 30, 2023 22:08
@narendasan narendasan changed the base branch from converter_reorg_proto to main May 30, 2023 22:08
Comment on lines +182 to +189
if len(args) > 2:
return activation.selu(
network,
target,
SourceIR.ATEN,
name,
args[0],
)
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the purpose of this if logic?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The aten trace for both selu and elu comes as torch.ops.aten.elu.default, but with different args. elu has args[0] and args[1] for alpha, while selu has only one arg args[0]. Just a way to differentiate between the two.

Adding selu converter

Python linting correction
@narendasan narendasan force-pushed the converter_reorg_activation_elu branch from 67cac89 to 67ade93 Compare May 31, 2023 00:01
Copy link

@github-actions github-actions bot left a 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

Copy link

@github-actions github-actions bot left a 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

@narendasan narendasan merged commit ee24d82 into main Jun 12, 2023
@narendasan narendasan deleted the converter_reorg_activation_elu branch June 12, 2023 21:32
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.

4 participants