-
Notifications
You must be signed in to change notification settings - Fork 368
Selectively enable different frontends #2693
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
Conversation
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.
Overall looks great; overhaul of dtype system and frontend selection at install is very helpful. I just need to verify on local installs as well. Left a few small comments
| or torch_tensorrt.dtype.float in enabled_precisions | ||
| ): | ||
| precision = torch.float32 | ||
| if dtype.float16 in enabled_precisions or dtype.half in enabled_precisions: |
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.
dtype.float16 and dtype.half seem to point to the same enum object. Should this be:
if dtype.float16 in enabled_precisions or torch.float16 in enabled_precisions:| precision = torch.float32 | ||
| if dtype.float16 in enabled_precisions or dtype.half in enabled_precisions: | ||
| precision = dtype.float16 | ||
| elif dtype.float32 in enabled_precisions or dtype.float in enabled_precisions: |
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.
Same as above comment
| if dtype.float16 in enabled_precisions or dtype.half in enabled_precisions: | ||
| precision = dtype.float16 | ||
| elif dtype.float32 in enabled_precisions or dtype.float in enabled_precisions: | ||
| precision = dtype.float32 |
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.
Same as above
0045465 to
ae453fc
Compare
Signed-off-by: Naren Dasan <naren@narendasan.com> Signed-off-by: Naren Dasan <narens@nvidia.com>
…pes in the python package to decouple frontends Signed-off-by: Naren Dasan <naren@narendasan.com> Signed-off-by: Naren Dasan <narens@nvidia.com>
dynamo Signed-off-by: Naren Dasan <naren@narendasan.com> Signed-off-by: Naren Dasan <narens@nvidia.com>
Signed-off-by: Naren Dasan <naren@narendasan.com> Signed-off-by: Naren Dasan <narens@nvidia.com>
Signed-off-by: Naren Dasan <naren@narendasan.com> Signed-off-by: Naren Dasan <narens@nvidia.com>
Signed-off-by: Naren Dasan <naren@narendasan.com> Signed-off-by: Naren Dasan <narens@nvidia.com>
torch.fx.passes.splitter_base._SplitterBase Signed-off-by: Naren Dasan <naren@narendasan.com> Signed-off-by: Naren Dasan <narens@nvidia.com>
959f3e5 to
b21fb5f
Compare
py/torch_tensorrt/_compile.py
Outdated
| "Input is a torchscript module but the ir was not specified (default=dynamo), please set ir=torchscript to suppress the warning." | ||
| ) | ||
| return _IRType.ts | ||
| elif module_is_exportable: |
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.
Need to add feature check
py/torch_tensorrt/_compile.py
Outdated
| inputs: Optional[Sequence[Input | torch.Tensor]] = None, | ||
| ir: str = "default", | ||
| enabled_precisions: Optional[Set[torch.dtype | dtype]] = None, | ||
| enabled_precisions: Optional[Set[torch.dtype]] = None, |
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.
Correct type annotation
Signed-off-by: Naren Dasan <naren@narendasan.com> Signed-off-by: Naren Dasan <narens@nvidia.com>
Signed-off-by: Naren Dasan <naren@narendasan.com> Signed-off-by: Naren Dasan <narens@nvidia.com>
Signed-off-by: Naren Dasan <naren@narendasan.com> Signed-off-by: Naren Dasan <narens@nvidia.com>
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.
Overall looks great - added some small style/clarification/logging comments
|
Additionally, tested on Windows E2E models and appears to cleanly dispatch to the correct runtime without needing to modify the |
|
A few examples, such as the one below, use
|
Signed-off-by: Naren Dasan <naren@narendasan.com> Signed-off-by: Naren Dasan <narens@nvidia.com>
Signed-off-by: Naren Dasan <naren@narendasan.com> Signed-off-by: Naren Dasan <narens@nvidia.com>
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.
LGTM. Added minor comments
py/torch_tensorrt/_enums.py
Outdated
| use_default: bool, | ||
| ) -> Optional[Union[torch.dtype, trt.DataType, np.dtype, dtype]]: | ||
| try: | ||
| print(self) |
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.
can remove this statement
py/torch_tensorrt/_enums.py
Outdated
| elif t == DeviceType: | ||
| return self |
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'm curious when do we need to cast EngineCapability to DeviceType. Any examples ?
| use_fast_partitioner: bool = USE_FAST_PARTITIONER, | ||
| enable_experimental_decompositions: bool = ENABLE_EXPERIMENTAL_DECOMPOSITIONS, | ||
| enabled_precisions: Set[torch.dtype | dtype] | Tuple[torch.dtype | dtype] = ( | ||
| dtype.float32, |
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.
Shouldn't this be _defaults.ENABLED_PRECISIONS ?
|
|
||
| compilation_options = { | ||
| "precision": precision, | ||
| "enabled_precisions": enabled_precisions, |
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.
It seems like you've handled enabled_precisions to be empty scenario in compile function. We can use the same here
"enabled_precisions": (
enabled_precisions if enabled_precisions else _defaults.ENABLED_PRECISIONS
| [tool.setuptools] | ||
| package-dir = {"" = "py"} | ||
| include-package-data = false | ||
|
|
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.
Where would this file be used ?
|
Will docs be updated with the instructions as a different PR ? |
Signed-off-by: Naren Dasan <naren@narendasan.com> Signed-off-by: Naren Dasan <narens@nvidia.com>
Signed-off-by: Naren Dasan <naren@narendasan.com> Signed-off-by: Naren Dasan <narens@nvidia.com>
Signed-off-by: Naren Dasan <naren@narendasan.com> Signed-off-by: Naren Dasan <narens@nvidia.com>
Description
Allows users to configure which frontends/features they want to include in the
torch-tensorrtbuilds.Features can be enabled as so:
A builds feature set can be accessed via the following struct
where
ENABLED_FEATURESis anamedtupleFeatureSet:In order to support optional features, a number of core types have been abstracted:
torch_tensorrt.Devicehas no direct dependencies on the torchscript core and can be translated totorch_tensorrt.ts.Deviceto access those featurestorch_tensorrt.Inputbehaves the same waydtype,DeviceType,memory_formathave been defined and can translate fromnumpy,tensorrt,torchandtorch_tensorrt._C(assumingtorch_tensorrt.ENABLED_FEATURES.torchscript_frontendisTrue)Translating between different library enums now can take the form
Fixes #1943
Fixes #2379
Type of change
Please delete options that are not relevant and/or add your own.
Checklist: