- Notifications
You must be signed in to change notification settings - Fork371
feat: Adding support for native int64#2789
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
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.txtindex 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.txtindex 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.txtindex 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
gs-olive left a comment
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 in1a4ffe4
| if ( | |
| isinstance(input_val,torch.Tensor) | |
| andctx.compilation_settings.truncate_long_and_double | |
| ): | |
| ifinput_val.dtype==torch.int64: | |
| input_val=input_val.to(torch.int32) | |
| elifinput_val.dtype==torch.float64: | |
| input_val=input_val.to(torch.float32) | |
| elif ( | |
| isinstance(input_val,np.ndarray) | |
| andctx.compilation_settings.truncate_long_and_double | |
| ): | |
| ifinput_val.dtype==np.int64: | |
| input_val=input_val.astype(np.int32) | |
| elifinput_val.dtype==np.float64: | |
| input_val=input_val.astype(np.float32) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
8cf85f1 toa0b840bCompare This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
7b6bdcd toff2f9aeCompare
peri044 left a comment
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
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
9e839df to2d0fa75Comparegs-olive left a comment
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 usinglogger 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 supportedSigned-off-by: Naren Dasan <naren@narendasan.com>Signed-off-by: Naren Dasan <narens@nvidia.com>
all layersSigned-off-by: Naren Dasan <naren@narendasan.com>Signed-off-by: Naren Dasan <narens@nvidia.com>
2d0fa75 toc918cdbComparegs-olive commentedApr 30, 2024
|
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 Dasannaren@narendasan.com
Signed-off-by: Naren Dasannarens@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: