feat: Adding support for native int64 by narendasan · Pull Request #2789 · pytorch/TensorRT

narendasan · 2024-04-27T01:14:05Z

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.

New feature (non-breaking change which adds functionality)

Checklist:

My code follows the style guidelines of this project (You can use the linters)
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas and hacks
I have made corresponding changes to the documentation
I have added tests to verify my fix or my feature
New and existing unit tests pass locally with my changes
I have added the relevant labels to my PR in so that relevant reviewers are notified

github-actions

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

github-actions

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

github-actions

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

gs-olive

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)

peri044

LGTM. Minor changes

tests/py/dynamo/models/test_dtype_support.py

gs-olive

Looks good to me

gs-olive · 2024-04-30T22:06:51Z

py/torch_tensorrt/dynamo/_compiler.py

+            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,
+            )


Any reason for not using logger here?

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>

gs-olive · 2024-04-30T22:12:10Z

Verified functional compilation on Stable Diffusion example

Signed-off-by: Naren Dasan <naren@narendasan.com> Signed-off-by: Naren Dasan <narens@nvidia.com>

facebook-github-bot added the cla signed label Apr 27, 2024

github-actions bot requested a review from bowang007 April 27, 2024 01:14

github-actions bot requested changes Apr 27, 2024

View reviewed changes

narendasan requested review from gs-olive and peri044 and removed request for bowang007 April 27, 2024 01:18

github-actions bot requested changes Apr 27, 2024

View reviewed changes

gs-olive reviewed Apr 29, 2024

View reviewed changes

github-actions bot added the component: converters Issues re: Specific op converters label Apr 29, 2024

This comment was marked as outdated.

Sign in to view

narendasan force-pushed the native_i64_support branch from 8cf85f1 to a0b840b Compare April 29, 2024 20:42

This comment was marked as outdated.

Sign in to view

narendasan force-pushed the native_i64_support branch 3 times, most recently from 7b6bdcd to ff2f9ae Compare April 30, 2024 02:03

peri044 requested changes Apr 30, 2024

View reviewed changes

narendasan force-pushed the native_i64_support branch 2 times, most recently from 9e839df to 2d0fa75 Compare April 30, 2024 21:04

gs-olive approved these changes Apr 30, 2024

View reviewed changes

gs-olive reviewed Apr 30, 2024

View reviewed changes

narendasan added 3 commits April 30, 2024 15:08

feat: Adding support for native int64

dcf1d01

Signed-off-by: Naren Dasan <naren@narendasan.com> Signed-off-by: Naren Dasan <narens@nvidia.com>

chore: Deprecate truncate_long_and_double for the dynamo frontend

e0488fe

`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>

fix: add explicit cast for i64 outputs as they may not be supported in

c918cdb

all layers Signed-off-by: Naren Dasan <naren@narendasan.com> Signed-off-by: Naren Dasan <narens@nvidia.com>

narendasan force-pushed the native_i64_support branch from 2d0fa75 to c918cdb Compare April 30, 2024 22:08

narendasan merged commit 717e11b into main Apr 30, 2024

narendasan deleted the native_i64_support branch April 30, 2024 22:57

narendasan added a commit that referenced this pull request Apr 30, 2024

feat: Adding support for native int64 (#2789)

68ecb29

Signed-off-by: Naren Dasan <naren@narendasan.com> Signed-off-by: Naren Dasan <narens@nvidia.com>

narendasan mentioned this pull request Apr 30, 2024

2.3 cherry pick feat: Adding support for native int64 (#2789) #2802

Merged

7 tasks

narendasan added a commit that referenced this pull request May 1, 2024

2.3 cherry pick feat: Adding support for native int64 (#2789) (#2802)

0499493

Signed-off-by: Naren Dasan <naren@narendasan.com> Signed-off-by: Naren Dasan <narens@nvidia.com>

laikhtewari pushed a commit that referenced this pull request May 24, 2024

feat: Adding support for native int64 (#2789)

61b5280

Signed-off-by: Naren Dasan <naren@narendasan.com> Signed-off-by: Naren Dasan <narens@nvidia.com>

	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)

Conversation

narendasan commented Apr 27, 2024

Description

Type of change

Checklist:

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

gs-olive left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

peri044 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gs-olive left a comment

Choose a reason for hiding this comment

Uh oh!

gs-olive Apr 30, 2024

Choose a reason for hiding this comment

Uh oh!

narendasan Apr 30, 2024

Choose a reason for hiding this comment

Uh oh!

gs-olive commented Apr 30, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants