fix: Raise error when registering Packet-keyed converter #2285

gs-olive · 2023-09-05T17:09:03Z

Description

Packet-keyed converters are too error-prone, since new overloads could be added or existing overloads may have been overlooked

Type of change

Improved error-checking

Checklist:

[ x ] My code follows the style guidelines of this project (You can use the linters)
[ x ] I have performed a self-review of my own code
[ x ] I have commented my code, particularly in hard-to-understand areas and hacks
[ x ] I have made corresponding changes to the documentation
[ - ] I have added tests to verify my fix or my feature
- CI-validated
[ x ] New and existing unit tests pass locally with my changes
[ x ] 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 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

gs-olive · 2023-09-06T05:10:06Z

tests/py/dynamo/backend/test_specialized_models.py

+        class PacketAsOperator(torch.nn.Module):
+            def forward(self, x):
+                return torch.ops.aten.tanh(x)
+
+        # Operations expected to be removed in the traced graph
+        expected_ops = {torch.ops.aten.tanh.default}
+        unexpected_ops = {
+            torch.ops.aten.tanh,
+        }


Validates that the Packet callable torch.ops.aten.tanh is transformed into torch.ops.aten.tanh.default by AOT autograd.

github-actions

Code conforms to Python style guidelines

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

py/torch_tensorrt/dynamo/conversion/converter_registry.py

- Packet-keyed converters are too error-prone, since new overloads could be added or existing overloads may have been overlooked

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

gs-olive requested a review from apbose September 5, 2023 17:09

gs-olive self-assigned this Sep 5, 2023

facebook-github-bot added the cla signed label Sep 5, 2023

github-actions bot added component: api [Python] Issues re: Python API component: conversion Issues re: Conversion stage component: dynamo Issues relating to the `torch.compile` or `torch._dynamo.export` paths labels Sep 5, 2023

github-actions bot approved these changes Sep 5, 2023

View reviewed changes

gs-olive force-pushed the converter_registry_op_overload branch from bcd3511 to fcd6913 Compare September 5, 2023 18:11

github-actions bot approved these changes Sep 5, 2023

View reviewed changes

gs-olive force-pushed the converter_registry_op_overload branch from fcd6913 to 894eb67 Compare September 6, 2023 05:09

github-actions bot added the component: tests Issues re: Tests label Sep 6, 2023

gs-olive commented Sep 6, 2023

View reviewed changes

github-actions bot approved these changes Sep 6, 2023

View reviewed changes

gs-olive force-pushed the converter_registry_op_overload branch from 894eb67 to 1255f71 Compare September 9, 2023 06:45

github-actions bot approved these changes Sep 9, 2023

View reviewed changes

gs-olive force-pushed the converter_registry_op_overload branch from 1255f71 to 8f39c94 Compare September 19, 2023 17:49

apbose reviewed Sep 20, 2023

View reviewed changes

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

fix: Raise error when registering Packet-keyed converter

ff0c7d5

- Packet-keyed converters are too error-prone, since new overloads could be added or existing overloads may have been overlooked

gs-olive force-pushed the converter_registry_op_overload branch from 8f39c94 to ff0c7d5 Compare September 20, 2023 17:33

github-actions bot approved these changes Sep 20, 2023

View reviewed changes

gs-olive merged commit b50290d into main Sep 20, 2023

gs-olive deleted the converter_registry_op_overload branch September 20, 2023 18:39

fix: Raise error when registering Packet-keyed converter #2285

fix: Raise error when registering Packet-keyed converter #2285

Uh oh!

Conversation

gs-olive commented Sep 5, 2023

Description

Type of change

Checklist:

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

gs-olive Sep 6, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants