[None][infra] upgrade dlfw 2604 but downgrade cuda from 13.2 to 13.1 - #14543
Closed
EmmaQiaoCh wants to merge 53 commits into
Closed
[None][infra] upgrade dlfw 2604 but downgrade cuda from 13.2 to 13.1#14543EmmaQiaoCh wants to merge 53 commits into
EmmaQiaoCh wants to merge 53 commits into
Conversation
Signed-off-by: EmmaQiaoCh <qqiao@nvidia.com>
Signed-off-by: EmmaQiaoCh <qqiao@nvidia.com>
Signed-off-by: Emma Qiao <qqiao@nvidia.com>
Signed-off-by: EmmaQiaoCh <qqiao@nvidia.com>
Signed-off-by: EmmaQiaoCh <qqiao@nvidia.com>
Signed-off-by: Emma Qiao <qqiao@nvidia.com>
Signed-off-by: EmmaQiaoCh <qqiao@nvidia.com>
Signed-off-by: EmmaQiaoCh <qqiao@nvidia.com>
… 2.11.0a0 Signed-off-by: EmmaQiaoCh <qqiao@nvidia.com>
Signed-off-by: EmmaQiaoCh <qqiao@nvidia.com>
Signed-off-by: EmmaQiaoCh <qqiao@nvidia.com>
Signed-off-by: EmmaQiaoCh <qqiao@nvidia.com>
Signed-off-by: Emma Qiao <qqiao@nvidia.com>
Signed-off-by: Emma Qiao <qqiao@nvidia.com>
Signed-off-by: Emma Qiao <qqiao@nvidia.com>
Signed-off-by: EmmaQiaoCh <qqiao@nvidia.com>
Signed-off-by: EmmaQiaoCh <qqiao@nvidia.com>
Signed-off-by: EmmaQiaoCh <qqiao@nvidia.com>
Signed-off-by: EmmaQiaoCh <qqiao@nvidia.com>
Signed-off-by: EmmaQiaoCh <qqiao@nvidia.com>
Signed-off-by: Emma Qiao <qqiao@nvidia.com>
Signed-off-by: EmmaQiaoCh <qqiao@nvidia.com>
Signed-off-by: EmmaQiaoCh <qqiao@nvidia.com>
Signed-off-by: EmmaQiaoCh <qqiao@nvidia.com>
Signed-off-by: Emma Qiao <qqiao@nvidia.com>
Signed-off-by: EmmaQiaoCh <qqiao@nvidia.com>
Signed-off-by: Emma Qiao <qqiao@nvidia.com>
Signed-off-by: EmmaQiaoCh <qqiao@nvidia.com>
Signed-off-by: Emma Qiao <qqiao@nvidia.com>
Signed-off-by: Emma Qiao <qqiao@nvidia.com>
Signed-off-by: EmmaQiaoCh <qqiao@nvidia.com>
Signed-off-by: EmmaQiaoCh <qqiao@nvidia.com>
Signed-off-by: Emma Qiao <qqiao@nvidia.com>
Signed-off-by: Emma Qiao <qqiao@nvidia.com>
Signed-off-by: EmmaQiaoCh <qqiao@nvidia.com>
Signed-off-by: EmmaQiaoCh <qqiao@nvidia.com>
EmmaQiaoCh
requested review from
a team,
QiJune,
hyukn,
kaiyux,
tburt-nv and
yiqingy0
May 26, 2026 03:39
Collaborator
Author
|
/bot run --stage-list "Build-Docker-Images" |
Collaborator
|
PR_Github #50266 [ run ] triggered by Bot. Commit: |
Contributor
📝 WalkthroughWalkthroughThis PR upgrades the TensorRT-LLM environment to use CUDA 13.2.1, PyTorch 2.11.0, and TensorRT 10.16.1, refactors CUDA virtual memory API initialization, fixes C++ compilation flags for ARM64/GCC scenarios, updates PyTorch compilation backends, and expands test waivers for known failing configurations. ChangesEnvironment and dependency version updates
CUDA virtual memory allocation API refactoring
C++ compilation flag compatibility fixes
PyTorch compilation backend updates
Test waiver and SKIP configuration updates
Sequence DiagramsNo sequence diagrams generated. The PR contains primarily configuration, build system, and dependency updates alongside refactoring of initialization patterns and test waivers—changes that do not introduce multi-component sequential workflows or data flows suitable for sequence visualization. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The changes span multiple file categories (configuration, build, C++, Python, Docker) with moderate heterogeneity. Environment version bumps are straightforward but pervasive; CUDA API refactoring is consistent initialization pattern changes across header/implementation/tests; C++ CMake flag handling requires build-system understanding; PyTorch backend switching is localized; test waivers are configuration additions. Most changes are low-density logic updates with good localization, though the breadth and variety of affected areas (Docker, Jenkins, build system, C++, Python, tests) adds some review complexity. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/rename_docker_images.py (1)
1-7:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd the required NVIDIA copyright header.
This modified Python source file still lacks the required NVIDIA header. The repo policy requires TensorRT-LLM source files to carry that header and update the year on meaningful edits.
As per coding guidelines "All TensorRT-LLM source files should contain an NVIDIA copyright header with year of latest meaningful modification."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/rename_docker_images.py` around lines 1 - 7, Add the required NVIDIA copyright header at the top of the Python file (above the existing shebang "#!/usr/bin/env python3"), using the standard TensorRT-LLM header text and update the year to reflect the latest meaningful modification; ensure the header is a block comment that precedes imports (import argparse, datetime, os, pathlib, subprocess) and matches the repo's canonical header format and wording exactly.
🧹 Nitpick comments (1)
docker/common/install_cuda_toolkit.sh (1)
38-75: ⚡ Quick winQuote variable in rm command for shell safety.
The
${OLD_CUDA_VER}variable should be quoted in therm -rfcommand to prevent word splitting and globbing issues if the version string ever contains spaces or special characters.🛡️ Suggested fix
reinstall_ubuntu_cuda() { export DEBIAN_FRONTEND=noninteractive apt-get update # Remove existing CUDA toolkit and related libraries. `|| true` keeps the # script resilient when a pattern matches no installed package. apt-get remove --purge -y "cuda*" "libcublas*" "libcufft*" "libcufile*" "libcurand*" \ "libcusolver*" "libcusparse*" "libnpp*" "libnvjpeg*" "nsight*" "libnvvm*" \ "gds-tools*" || true apt-get autoremove --purge -y || true - rm -rf /usr/local/cuda-${OLD_CUDA_VER} + rm -rf "/usr/local/cuda-${OLD_CUDA_VER}" # Install prerequisites for the .run installer. apt-get install -y --no-install-recommends wget ca-certificates build-essential wget --retry-connrefused --timeout=180 --tries=10 --continue https://developer.download.nvidia.com/compute/cuda/${CUDA_VER_SHORT}/local_installers/cuda_${CUDA_VER}_linux.run sh cuda_${CUDA_VER}_linux.run --silent --override --toolkit rm -f cuda_${CUDA_VER}_linux.run apt-get clean rm -rf /var/lib/apt/lists/* }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker/common/install_cuda_toolkit.sh` around lines 38 - 75, In reinstall_ubuntu_cuda(), make the rm invocation safe by quoting the OLD_CUDA_VER variable to prevent word-splitting/globbing; update the line that removes the old CUDA directory (the rm -rf /usr/local/cuda-${OLD_CUDA_VER} call) to use a quoted expansion like "/usr/local/cuda-${OLD_CUDA_VER}" or '/usr/local/cuda-"${OLD_CUDA_VER}"' so the path is treated as a single token.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@jenkins/L0_Test.groovy`:
- Around line 4381-4388: The CI installs cuda-toolkit-13-2 but the runtime/NVRTC
pin in docker/common/install_tensorrt.sh is 13.1; update the sanity-path
installation to match the runtime by changing the apt package in the block that
calls trtllm_utils.llmExecStepWithRetry (the script that runs "apt-get update &&
apt-get install -y cuda-toolkit-13-2") to install the 13.1 toolkit (e.g.,
cuda-toolkit-13-1 or the exact package name that corresponds to CUDA 13.1), and
update the surrounding echo/comment to reflect CUDA 13.1 so the userspace
matches NVRTC_CUDA_VER="13.1" and CUDA_RUNTIME="13.1.80-1".
In `@README.md`:
- Line 11: The README CUDA badge currently claims "13.2.1" but the install
script install_cuda_toolkit.sh downgrades toolkits to 13.1.1; update the badge
in README.md to reflect the actual toolkit version used for compilation (change
the displayed/version text and shields.io URL to 13.1.1), or alternatively add a
short note in the installation docs explaining the mixed-version strategy and
why the base image and compiled toolkit differ so readers aren’t misled.
In `@scripts/rename_docker_images.py`:
- Around line 51-66: The code currently uses dst_mr when constructing the source
image tag, which can pull the wrong image; add a new CLI option --src-mr
(dest="src_mr", type=str or int, default=None) alongside --commit and
--src-branch in the same source_group, and change the code that builds
src_suffix to prefer src_mr when provided (fall back to dst_mr only if src_mr is
None); ensure the new flag has clear help text ("source MR/PR id embedded in the
source image tag; if omitted, dst_mr is used") so callers can explicitly supply
a different PR/MR for the source image.
---
Outside diff comments:
In `@scripts/rename_docker_images.py`:
- Around line 1-7: Add the required NVIDIA copyright header at the top of the
Python file (above the existing shebang "#!/usr/bin/env python3"), using the
standard TensorRT-LLM header text and update the year to reflect the latest
meaningful modification; ensure the header is a block comment that precedes
imports (import argparse, datetime, os, pathlib, subprocess) and matches the
repo's canonical header format and wording exactly.
---
Nitpick comments:
In `@docker/common/install_cuda_toolkit.sh`:
- Around line 38-75: In reinstall_ubuntu_cuda(), make the rm invocation safe by
quoting the OLD_CUDA_VER variable to prevent word-splitting/globbing; update the
line that removes the old CUDA directory (the rm -rf
/usr/local/cuda-${OLD_CUDA_VER} call) to use a quoted expansion like
"/usr/local/cuda-${OLD_CUDA_VER}" or '/usr/local/cuda-"${OLD_CUDA_VER}"' so the
path is treated as a single token.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d51c5a2f-9033-4a09-929c-c76de6916abd
📒 Files selected for processing (22)
README.mdconstraints.txtcpp/include/tensorrt_llm/runtime/virtualMemory.hcpp/tensorrt_llm/deep_ep/CMakeLists.txtcpp/tensorrt_llm/flash_mla/CMakeLists.txtcpp/tensorrt_llm/runtime/virtualMemory.cppcpp/tests/unit_tests/common/cudaDriverWrapperTest.cppcpp/tests/unit_tests/runtime/virtualMemoryTest.cppdocker/Dockerfile.multidocker/Makefiledocker/common/install_cuda_toolkit.shdocker/common/install_pytorch.shdocker/common/install_tensorrt.shdocs/source/legacy/reference/support-matrix.mdjenkins/Build.groovyjenkins/L0_Test.groovyjenkins/current_image_tags.propertiesrequirements.txtscripts/rename_docker_images.pytensorrt_llm/_torch/compilation/backend.pytensorrt_llm/_torch/compilation/piecewise_optimizer.pytests/integration/test_lists/waives.txt
💤 Files with no reviewable changes (1)
- constraints.txt
Comment on lines
+4381
to
+4388
| trtllm_utils.llmExecStepWithRetry(pipeline, script: "apt-get update && apt-get install -y cuda-toolkit-13-2") | ||
| } | ||
| // Extra PyTorch CUDA 13.0 install for all bare-metal environments (Default PyTorch is for CUDA 12.8) | ||
| // Extra PyTorch CUDA 13.2 install for all bare-metal environments (Default PyTorch is for CUDA 12.8) | ||
| if (values[6]) { | ||
| echo "###### Extra PyTorch CUDA 13.0 install Start ######" | ||
| echo "###### Extra PyTorch CUDA 13.2 install Start ######" | ||
| // Use internal mirror instead of https://download.pytorch.org/whl/cu130 for better network stability. | ||
| // PyTorch CUDA 13.0 package and torchvision package can be installed as expected. | ||
| trtllm_utils.llmExecStepWithRetry(pipeline, script: "pip3 install torch==2.10.0+cu130 torchvision==0.25.0+cu130 --extra-index-url https://urm.nvidia.com/artifactory/api/pypi/pytorch-cu128-remote/simple --extra-index-url https://download.pytorch.org/whl/cu130") | ||
| trtllm_utils.llmExecStepWithRetry(pipeline, script: "pip3 install torch==2.11.0+cu130 torchvision==0.26.0+cu130 --extra-index-url https://urm.nvidia.com/artifactory/api/pypi/pytorch-cu128-remote/simple --extra-index-url https://download.pytorch.org/whl/cu130") |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the Groovy lines around the reported snippet
echo "== Groovy: jenkins/L0_Test.groovy (lines ~4381-4388) =="
sed -n '4370,4400p' jenkins/L0_Test.groovy
# Verify CUDA pin in docker/common/install_tensorrt.sh
echo
echo "== Shell: docker/common/install_tensorrt.sh (search CUDA/NVRTC pins) =="
ls -la docker/common/install_tensorrt.sh
rg -n --context 3 "(cuda|NVRTC|nvrtc|13\.|toolkit)" docker/common/install_tensorrt.sh
# Also search repo-wide for cuda-toolkit-13-2 and CUDA 13.1 runtime pins to assess scope
echo
echo "== Repo-wide occurrences =="
rg -n --context 2 "cuda-toolkit-13-2" .
rg -n --context 2 "(cuda-toolkit-13-1|13\.1|NVRTC|nvrtc)" docker/common/install_tensorrt.shRepository: NVIDIA/TensorRT-LLM
Length of output: 12022
Align CUDA sanity-check userspace with the shipped TensorRT/NVRTC runtime
jenkins/L0_Test.groovyinstallscuda-toolkit-13-2(CUDA 13.2) on the bare-metal sanity path.docker/common/install_tensorrt.shintentionally pins NVRTC and CUDA runtime/driver to CUDA 13.1 (NVRTC_CUDA_VER="13.1",CUDA_RUNTIME="13.1.80-1").- This mismatch can let the sanity job pass on a newer (13.2) userspace than the runtime components that will ship (13.1).
Make the sanity-check CUDA toolkit selection match the runtime/NVRTC pin (either install the 13.1 toolkit in CI or update the runtime pin to 13.2 consistently).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@jenkins/L0_Test.groovy` around lines 4381 - 4388, The CI installs
cuda-toolkit-13-2 but the runtime/NVRTC pin in docker/common/install_tensorrt.sh
is 13.1; update the sanity-path installation to match the runtime by changing
the apt package in the block that calls trtllm_utils.llmExecStepWithRetry (the
script that runs "apt-get update && apt-get install -y cuda-toolkit-13-2") to
install the 13.1 toolkit (e.g., cuda-toolkit-13-1 or the exact package name that
corresponds to CUDA 13.1), and update the surrounding echo/comment to reflect
CUDA 13.1 so the userspace matches NVRTC_CUDA_VER="13.1" and
CUDA_RUNTIME="13.1.80-1".
| [](https://www.python.org/downloads/release/python-31012/) | ||
| [](https://developer.nvidia.com/cuda-downloads) | ||
| [](https://pytorch.org) | ||
| [](https://developer.nvidia.com/cuda-downloads) |
Contributor
There was a problem hiding this comment.
CUDA badge version inconsistent with actual toolkit version.
The badge displays CUDA 13.2.1, but docker/common/install_cuda_toolkit.sh downgrades the toolkit (nvcc/cudart/nvrtc) to 13.1.1. The badge should reflect the CUDA toolkit version developers actually use for compilation, not the base image version.
📝 Suggested fix
-[](https://developer.nvidia.com/cuda-downloads)
+[](https://developer.nvidia.com/cuda-downloads)Alternatively, if you want to communicate the dual-version setup, consider:
-[](https://developer.nvidia.com/cuda-downloads)
+[](https://developer.nvidia.com/cuda-downloads)with a note in the installation docs explaining the mixed-version strategy.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| [](https://developer.nvidia.com/cuda-downloads) | |
| [](https://developer.nvidia.com/cuda-downloads) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` at line 11, The README CUDA badge currently claims "13.2.1" but
the install script install_cuda_toolkit.sh downgrades toolkits to 13.1.1; update
the badge in README.md to reflect the actual toolkit version used for
compilation (change the displayed/version text and shields.io URL to 13.1.1), or
alternatively add a short note in the installation docs explaining the
mixed-version strategy and why the base image and compiled toolkit differ so
readers aren’t misled.
Comment on lines
+51
to
+66
| source_group = parser.add_mutually_exclusive_group() | ||
| source_group.add_argument( | ||
| "--commit", | ||
| type=str, | ||
| default=None, | ||
| help= | ||
| "GitHub mode: short commit SHA embedded in the source image tag (default: `git rev-parse --short HEAD`)." | ||
| ) | ||
| source_group.add_argument( | ||
| "--src-branch", | ||
| dest="src_branch", | ||
| type=str, | ||
| default=None, | ||
| help="GitLab mode: source branch name embedded in the source image tag. " | ||
| "When set, the tag is built as `<prefix>-<src_branch>-<src_build_id>` instead of the GitHub-PR layout." | ||
| ) |
Contributor
There was a problem hiding this comment.
Don't use the destination MR to resolve the source tag.
GitHub mode builds src_suffix with dst_mr, but the CLI still describes that number as belonging to the destination image. If source and destination PR/MR identifiers ever differ, the script will pull the wrong source image. Please either add a separate source-PR argument or rename the parameter/help text to make the same-PR-only contract explicit.
Also applies to: 204-219
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/rename_docker_images.py` around lines 51 - 66, The code currently
uses dst_mr when constructing the source image tag, which can pull the wrong
image; add a new CLI option --src-mr (dest="src_mr", type=str or int,
default=None) alongside --commit and --src-branch in the same source_group, and
change the code that builds src_suffix to prefer src_mr when provided (fall back
to dst_mr only if src_mr is None); ensure the new flag has clear help text
("source MR/PR id embedded in the source image tag; if omitted, dst_mr is used")
so callers can explicitly supply a different PR/MR for the source image.
Collaborator
|
PR_Github #50266 [ run ] completed with state
|
Collaborator
Author
|
We decide to give up this change, close this pr. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
Infrastructure Updates
Bug Fixes
New Features
Tests
Description
Test Coverage
PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
If PR introduces API changes, an appropriate PR label is added - either
api-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin the PR title.Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.