- 
                Notifications
    You must be signed in to change notification settings 
- Fork 368
TRT-LLM loading mechanism tool #3398
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
57dbb3f    to
    3e38e87      
    Compare
  
    | f"Ensure the path is correct and the library is compatible", | ||
| exc_info=e_os_error, | ||
| else: | ||
| py_version = f"cp{sys.version_info.major}{sys.version_info.minor}" | 
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.
Why do we restrict to cp310 and cp312, It shouldnt matter if we are pulling the whl and unzipping ourselves
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.
https://pypi.nvidia.com/tensorrt-llm/ In this since I see the tags for only cp310 and cp312 I added the check
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/tests/py/dynamo/conversion/test_nccl_ops.py	2025-02-27 20:03:00.014038+00:00
+++ /home/runner/work/TensorRT/TensorRT/tests/py/dynamo/conversion/test_nccl_ops.py	2025-02-27 20:03:24.885031+00:00
@@ -22,11 +22,11 @@
from .harness import DispatchTestCase
class TestGatherNcclOpsConverter(DispatchTestCase):
-    @parameterized.expand([(8)])
+    @parameterized.expand([8])
    def test_nccl_ops(self, linear_layer_dim):
        class DistributedGatherModel(nn.Module):
            def __init__(self, input_dim):
                super().__init__()
                self.fc = torch.nn.Linear(input_dim, input_dim)9ba407b    to
    5f3fdac      
    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.
There are some changes that do not conform to Python style guidelines:
--- /home/runner/work/TensorRT/TensorRT/tests/py/dynamo/conversion/test_nccl_ops.py	2025-02-27 20:05:38.023287+00:00
+++ /home/runner/work/TensorRT/TensorRT/tests/py/dynamo/conversion/test_nccl_ops.py	2025-02-27 20:06:02.662188+00:00
@@ -22,11 +22,11 @@
from .harness import DispatchTestCase
class TestGatherNcclOpsConverter(DispatchTestCase):
-    @parameterized.expand([(8)])
+    @parameterized.expand([8])
    def test_nccl_ops(self, linear_layer_dim):
        class DistributedGatherModel(nn.Module):
            def __init__(self, input_dim):
                super().__init__()
                self.fc = torch.nn.Linear(input_dim, input_dim)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/tests/py/dynamo/conversion/test_nccl_ops.py	2025-02-27 20:05:54.405311+00:00
+++ /home/runner/work/TensorRT/TensorRT/tests/py/dynamo/conversion/test_nccl_ops.py	2025-02-27 20:06:21.454993+00:00
@@ -22,11 +22,11 @@
from .harness import DispatchTestCase
class TestGatherNcclOpsConverter(DispatchTestCase):
-    @parameterized.expand([(8)])
+    @parameterized.expand([8])
    def test_nccl_ops(self, linear_layer_dim):
        class DistributedGatherModel(nn.Module):
            def __init__(self, input_dim):
                super().__init__()
                self.fc = torch.nn.Linear(input_dim, input_dim)5f3fdac    to
    b66350e      
    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.
There are some changes that do not conform to Python style guidelines:
--- /home/runner/work/TensorRT/TensorRT/tests/py/dynamo/conversion/test_nccl_ops.py	2025-04-15 19:58:05.267724+00:00
+++ /home/runner/work/TensorRT/TensorRT/tests/py/dynamo/conversion/test_nccl_ops.py	2025-04-15 19:58:36.145897+00:00
@@ -22,11 +22,11 @@
from .harness import DispatchTestCase
class TestGatherNcclOpsConverter(DispatchTestCase):
-    @parameterized.expand([(8)])
+    @parameterized.expand([8])
    def test_nccl_ops(self, linear_layer_dim):
        class DistributedGatherModel(nn.Module):
            def __init__(self, input_dim):
                super().__init__()
                self.fc = torch.nn.Linear(input_dim, input_dim)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/tests/py/dynamo/conversion/test_nccl_ops.py	2025-04-15 21:00:13.719714+00:00
+++ /home/runner/work/TensorRT/TensorRT/tests/py/dynamo/conversion/test_nccl_ops.py	2025-04-15 21:00:40.093669+00:00
@@ -22,11 +22,11 @@
from .harness import DispatchTestCase
class TestGatherNcclOpsConverter(DispatchTestCase):
-    @parameterized.expand([(8)])
+    @parameterized.expand([8])
    def test_nccl_ops(self, linear_layer_dim):
        class DistributedGatherModel(nn.Module):
            def __init__(self, input_dim):
                super().__init__()
                self.fc = torch.nn.Linear(input_dim, input_dim)6e893ed    to
    77f2145      
    Compare
  
    f30acb7    to
    9c238ae      
    Compare
  
    89d621d    to
    27aa2f2      
    Compare
  
    dbfd7ee    to
    15d681a      
    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.
Seems like its close are there any tests for the downloader? like verifying the correct file is downloaded and available?
| The test t | 
c8b8337    to
    340182b      
    Compare
  
    1502865    to
    7c99c11      
    Compare
  
    | install_cuda_aarch64 | ||
| fi | ||
|  | ||
| if [[ "$(uname -s)" == "Linux" && "$(uname -m)" == "x86_64" ]]; then | 
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 this just be called in the test script instead installing it for all tests?
        
          
                py/torch_tensorrt/dynamo/utils.py
              
                Outdated
          
        
      | "TensorRT-LLM plugins for NCCL backend are not supported on Windows" | ||
| ) | ||
| return False | ||
| if "aarch64" in platform: | 
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.
What about SBSA?
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.
aarch64 should take care of it, right? I should change the log message to maybe "TensorRT-LLM plugins for NCCL backend are not supported on Jetson and sbsa devices (aarch64)"?
6550195    to
    76f7068      
    Compare
  
            
          
                py/torch_tensorrt/dynamo/utils.py
              
                Outdated
          
        
      | ) | ||
| return False | ||
| if torch.cuda.is_available(): | ||
| device_name = torch.cuda.get_device_name().lower() | 
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.
Did you test this, I was recommending using this: https://docs.python.org/ro/3/library/platform.html#platform.release
We use it here:
Line 54 in e760c3c
| "torch>=2.9.0.dev,<2.10.0; platform_machine != 'aarch64' or (platform_machine == 'aarch64' and 'tegra' not in platform_release)", | 
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.
ok changed to the above
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.
add the test setup for sbsa
8ea4010    to
    8e59794      
    Compare
  
    d656160    to
    86d39ad      
    Compare
  
    86d39ad    to
    2f6ff54      
    Compare
  
    …ing in dynamo.compile TRT-LLM installation utilities and adding test cases adding the option in _compiler.py changes in the TRT-LLM loading tool- removing install_wget, install_unzip, install_mpi Further changes in error logging of the TRT-LLM installation tool moving the load_tensorrt_llm to dynamo/utils.py correcting misprint for TRT LLM load Using python lib for download to make it platform agnostic dll file path update for windows correcting the non critical lint error Including version in versions.txt linting error fixes and rebase fix removing Platform enum from converter_utils.py Addressing review comments- tmp dir for wheel download and wheel extraction, variable for py_version checks for windows where NCCL backend is not supported adding checks for windows and jetson devices Keeping the extracted and deleting download file, restructuring test modifying the error warning of missing libmpi libs removing the redundant initializations adding tests in CI correcting the skip test condition install MPI libs for linux x86 adding SBSA to the supported platform of TRT-LLM libs and installing MPI libs for the distributed tests Using python package for platform detection using python platform
2f6ff54    to
    68b3346      
    Compare
  
    | Closing in favor of #3829 | 
TRT-LLM download utility