-
Notifications
You must be signed in to change notification settings - Fork 355
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: Adding support for native int64 #2789
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.
There are some changes that do not conform to C++ style guidelines:
diff --git a/home/runner/work/TensorRT/TensorRT/core/util/trt_util.cpp b/tmp/changes.txt
index 503b88e..3ca5780 100644
--- a/home/runner/work/TensorRT/TensorRT/core/util/trt_util.cpp
+++ b/tmp/changes.txt
@@ -164,8 +164,8 @@ nvinfer1::Dims unsqueezeDims(const nvinfer1::Dims& d, int pos, int val, bool use
// Acceptable range for pos is [-d.nbDims - 1, d.nbDims]
TORCHTRT_ASSERT(
pos >= (-d.nbDims - 1) && pos <= d.nbDims,
- "ERROR: Index to unsqueeze is out of bounds. " << "Expected value in range [" << (-d.nbDims - 1) << ", "
- << d.nbDims << "], but got " << pos);
+ "ERROR: Index to unsqueeze is out of bounds. "
+ << "Expected value in range [" << (-d.nbDims - 1) << ", " << d.nbDims << "], but got " << pos);
// Unsqueeze with negative dimensions creates a new dimension at that index
pos = (pos < 0) ? (pos + d.nbDims + 1) : pos;
ERROR: Some files do not conform to 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 C++ style guidelines:
diff --git a/home/runner/work/TensorRT/TensorRT/core/util/trt_util.cpp b/tmp/changes.txt
index 503b88e..3ca5780 100644
--- a/home/runner/work/TensorRT/TensorRT/core/util/trt_util.cpp
+++ b/tmp/changes.txt
@@ -164,8 +164,8 @@ nvinfer1::Dims unsqueezeDims(const nvinfer1::Dims& d, int pos, int val, bool use
// Acceptable range for pos is [-d.nbDims - 1, d.nbDims]
TORCHTRT_ASSERT(
pos >= (-d.nbDims - 1) && pos <= d.nbDims,
- "ERROR: Index to unsqueeze is out of bounds. " << "Expected value in range [" << (-d.nbDims - 1) << ", "
- << d.nbDims << "], but got " << pos);
+ "ERROR: Index to unsqueeze is out of bounds. "
+ << "Expected value in range [" << (-d.nbDims - 1) << ", " << d.nbDims << "], but got " << pos);
// Unsqueeze with negative dimensions creates a new dimension at that index
pos = (pos < 0) ? (pos + d.nbDims + 1) : pos;
ERROR: Some files do not conform to 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 C++ style guidelines:
diff --git a/home/runner/work/TensorRT/TensorRT/core/util/trt_util.cpp b/tmp/changes.txt
index 503b88e..3ca5780 100644
--- a/home/runner/work/TensorRT/TensorRT/core/util/trt_util.cpp
+++ b/tmp/changes.txt
@@ -164,8 +164,8 @@ nvinfer1::Dims unsqueezeDims(const nvinfer1::Dims& d, int pos, int val, bool use
// Acceptable range for pos is [-d.nbDims - 1, d.nbDims]
TORCHTRT_ASSERT(
pos >= (-d.nbDims - 1) && pos <= d.nbDims,
- "ERROR: Index to unsqueeze is out of bounds. " << "Expected value in range [" << (-d.nbDims - 1) << ", "
- << d.nbDims << "], but got " << pos);
+ "ERROR: Index to unsqueeze is out of bounds. "
+ << "Expected value in range [" << (-d.nbDims - 1) << ", " << d.nbDims << "], but got " << pos);
// Unsqueeze with negative dimensions creates a new dimension at that index
pos = (pos < 0) ? (pos + d.nbDims + 1) : pos;
ERROR: Some files do not conform to 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 looks good! May need modification to this file as well:
TensorRT/py/torch_tensorrt/dynamo/conversion/converter_utils.py
Lines 300 to 315 in 1a4ffe4
if ( | |
isinstance(input_val, torch.Tensor) | |
and ctx.compilation_settings.truncate_long_and_double | |
): | |
if input_val.dtype == torch.int64: | |
input_val = input_val.to(torch.int32) | |
elif input_val.dtype == torch.float64: | |
input_val = input_val.to(torch.float32) | |
elif ( | |
isinstance(input_val, np.ndarray) | |
and ctx.compilation_settings.truncate_long_and_double | |
): | |
if input_val.dtype == np.int64: | |
input_val = input_val.astype(np.int32) | |
elif input_val.dtype == np.float64: | |
input_val = input_val.astype(np.float32) |
8cf85f1
to
a0b840b
Compare
7b6bdcd
to
ff2f9ae
Compare
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. Minor changes
9e839df
to
2d0fa75
Compare
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.
Looks good to me
warnings.warn( | ||
'Compiler option "truncate_long_and_double" is deprecated in favor of "truncate_double" as int64 is now natively supported, this option will be removed in the next version', | ||
DeprecationWarning, | ||
stacklevel=2, | ||
) |
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.
Any reason for not using logger
here?
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.
This is apparently the recommended way to handle deprecation warnings, iirc I configured the logger to pull these messages in in an earlier PR
Signed-off-by: Naren Dasan <naren@narendasan.com> Signed-off-by: Naren Dasan <narens@nvidia.com>
`truncate_long_and_double` has been deprecated in favor of `truncate_double` as int64 is natively supported Signed-off-by: Naren Dasan <naren@narendasan.com> Signed-off-by: Naren Dasan <narens@nvidia.com>
all layers Signed-off-by: Naren Dasan <naren@narendasan.com> Signed-off-by: Naren Dasan <narens@nvidia.com>
2d0fa75
to
c918cdb
Compare
|
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
Adds support for int64 as a native type in TensorRT
Fixes # (issue)
Type of change
Please delete options that are not relevant and/or add your own.
Checklist: