-
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
feat: Add ExportedProgram as an IR #2191
Conversation
Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.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.
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:
--- /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/_compile.py 2023-08-10 23:36:35.545032+00:00
+++ /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/_compile.py 2023-08-10 23:39:45.559176+00:00
@@ -99,11 +99,13 @@
logging.Level.Warning,
"Input graph is a Torchscript module but the ir provided is default (dynamo). Please set ir=torchscript to suppress the warning. Compiling the module with ir=torchscript",
)
return _IRType.ts
elif module_is_exportable:
- raise ValueError("Input graph is an ExportedProgram which is not currently supported. Please provide torch.nn.Module or torch.fx.GraphModule as input.")
+ raise ValueError(
+ "Input graph is an ExportedProgram which is not currently supported. Please provide torch.nn.Module or torch.fx.GraphModule as input."
+ )
else:
raise ValueError("Module was provided in an unsupported format")
else:
raise ValueError("Unknown ir was requested")
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:
--- /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/_compile.py 2023-08-11 00:25:56.253605+00:00
+++ /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/_compile.py 2023-08-11 00:28:32.518873+00:00
@@ -101,11 +101,11 @@
)
return _IRType.ts
elif module_is_exportable:
raise ValueError(
"Input graph is an ExportedProgram which is not currently supported. Please provide torch.nn.Module or torch.fx.GraphModule as input."
- )
+ )
else:
raise ValueError("Module was provided in an unsupported format")
else:
raise ValueError("Unknown ir was requested")
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:
--- /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/_compile.py 2023-08-11 23:22:32.356641+00:00
+++ /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/_compile.py 2023-08-11 23:25:16.534788+00:00
@@ -101,11 +101,11 @@
)
return _IRType.ts
elif module_is_exportable:
raise ValueError(
"Input graph is an ExportedProgram which is not currently supported. Please provide torch.nn.Module or torch.fx.GraphModule as input."
- )
+ )
else:
raise ValueError("Module was provided in an unsupported format")
else:
raise ValueError("Unknown ir was requested")
Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.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.
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.
Overall, the addition looks good. Made comments on log messaging and docstrings.
elif ir == "exported_program": | ||
raise ValueError( | ||
"ir=exported_program is not currently supported. Supported ir options : ts|fx|dynamo" | ||
) |
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 for input type.
@@ -36,6 +37,7 @@ class _IRType(Enum): | |||
fx = 1 | |||
dynamo = 2 | |||
torch_compile = 3 | |||
exported_program = 4 |
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.
Unrelated to this change, but the docstring on this Enum
is not relevant to the content
Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.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.
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
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
Type of change
Please delete options that are not relevant and/or add your own.
Checklist: