fix/feat: Add Dynamo-only converter registry #1944
Conversation
gs-olive
added
component: dynamo
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
So this is the file where fx and dynamo converters are getting concatenated. The new converters could be registered using dynamo_tensorrt_converter, so wouldn't it be better to move the new converters to py/torch_tensorrt/dynamo instead of changing the existing converters in py/torch_tensorrt/fx from tensorrt_converter to dynamo_tensorrt_converter?
There was a problem hiding this comment.
Yes, I agree - no existing converters will need to be moved for this PR. New converters can be placed in the py/torch_tensorrt/dynamo directory and decorated with the @dynamo_tensorrt_converter decorator. If they are to be included in FX as well, we can just switch out the decorator for @tensorrt_converter (and optionally move the converter to py/torch_tensorrt/fx)
gs-olive
force-pushed
the
dynamo_converter_registry
branch
from
eece393 to
78b5826
Compare
gs-olive
force-pushed
the
dynamo_converter_registry
branch
from
78b5826 to
5125c00
Compare
gs-olive
force-pushed
the
dynamo_converter_registry
branch
from
5125c00 to
eaeaa46
Compare
gs-olive
force-pushed
the
dynamo_converter_registry
branch
from
eaeaa46 to
2893d3e
Compare
gs-olive
force-pushed
the
dynamo_converter_registry
branch
from
2893d3e to
846ec17
Compare
gs-olive
force-pushed
the
dynamo_converter_registry
branch
from
90e8b8d to
3057909
Compare
There was a problem hiding this comment.
In the new structure, does the converter_registry.py file belong in conversion/ @peri044?
gs-olive
force-pushed
the
dynamo_converter_registry
branch
from
3057909 to
8f518ab
Compare
gs-olive
force-pushed
the
dynamo_converter_registry
branch
from
8f518ab to
fe094db
Compare
gs-olive
force-pushed
the
dynamo_converter_registry
branch
from
fe094db to
5377e43
Compare
gs-olive
force-pushed
the
dynamo_converter_registry
branch
from
5377e43 to
170bcd6
Compare
There was a problem hiding this comment.
Sample implementation of a capability validator which disallows dynamic shapes. It could potentially also check the Dynamo config as an alternative way to verify no Dynamic shapes. This may be too restrictive, however, since static-only converters can still operate within Dynamic-enabled graphs, as long as their inputs are static/constant
- Add Dynamo converter registry which functions as a superset of the standard FX converter registry - For use with new + experimental converters - Uses custom decorator `dynamo_tensorrt_converter` - Update references within Dynamo functions to use the converter registry `DYNAMO_CONVERTERS`
- Add custom class overriding default Dictionary class to access converters from various registries - Add new dictionary type `Dict[Target, Sequence[ConverterSupport]]` as well as ConverterSupport class which stores a converter and its validation implementation - Add unified `DYNAMO_CONVERTERS` dictionary which coalesces both the FX and Dynamo converter dictionaries and acts as a single unified dictionary - Streamline dictionary accesses via get/contains accessors - Add priority converter decorator enum to prioritize user-provided converters and name argument checking "capability validation" to clarify utility - Add boilerplate `no_dynamic` converter capability validator for easy use in specifying converters as not-able to handle dynamic shapes
- Add debug statements for converter registrations in both FX and Dynamo - Print operator counts along with registry support message
gs-olive
force-pushed
the
dynamo_converter_registry
branch
from
170bcd6 to
4d14ae8
Compare
Description
Add Dynamo converter registry which functions as a superset of the standard FX converter registry
For use with new + experimental converters
Uses custom decorator
dynamo_tensorrt_converterUpdate references within Dynamo functions to use the converter registry
DYNAMO_CONVERTERSAdd custom class overriding default Dictionary class to access converters from various registries
Add new dictionary type
Dict[Target, Sequence[ConverterSupport]]as well as ConverterSupport class which stores a converter and its validation implementationAdd unified
DYNAMO_CONVERTERSdictionary which coalesces both the FX and Dynamo converter dictionaries and acts as a single unified dictionaryFixes #1942
Fixes #2071
Type of change
Checklist: