[https://nvbugs/6105768][fix] ** Runtime GPU detection inside the test function: when total_memory < 80 GiB - #13471
[https://nvbugs/6105768][fix] ** Runtime GPU detection inside the test function: when total_memory < 80 GiB #13471tensorrt-cicd wants to merge 2 commits into
total_memory < 80 GiB #13471Conversation
WalkthroughAdds a TinyLlama disaggregated cancellation configuration. The test runner now accepts prompt-length ranges and selects test settings based on GPU count and memory. The existing DeepSeek-V3-Lite waiver is removed. ChangesCancellation stress test
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CancellationTest
participant GPUDetection
participant CancelStressRunner
CancellationTest->>GPUDetection: read GPU count and memory
GPUDetection->>CancellationTest: return available resources
CancellationTest->>CancelStressRunner: run selected model with prompt-length range
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/integration/defs/disaggregated/test_disaggregated.py (1)
1-1: Consider updating copyright year.The copyright header shows
2022-2024, but since this file is being modified in 2026, it should be updated to2022-2026per coding guidelines. As per coding guidelines: "update year on modified files".📝 Proposed fix
-# SPDX-FileCopyrightText: Copyright (c) 2022-2024 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-FileCopyrightText: Copyright (c) 2022-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/defs/disaggregated/test_disaggregated.py` at line 1, Update the copyright header year range in test_disaggregated.py from "2022-2024" to "2022-2026" so the file reflects the current modification year; locate the SPDX/header comment at the top of the file and change the year substring accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/integration/defs/disaggregated/test_disaggregated.py`:
- Line 1: Update the copyright header year range in test_disaggregated.py from
"2022-2024" to "2022-2026" so the file reflects the current modification year;
locate the SPDX/header comment at the top of the file and change the year
substring accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5baeccdf-3fcb-478d-926d-8942b9c81d28
📒 Files selected for processing (3)
tests/integration/defs/disaggregated/test_configs/disagg_config_cancel_stress_test_small.yamltests/integration/defs/disaggregated/test_disaggregated.pytests/integration/test_lists/waives.txt
💤 Files with no reviewable changes (1)
- tests/integration/test_lists/waives.txt
a0728f1 to
24110eb
Compare
24110eb to
d23214d
Compare
c9c14f2 to
92ebad0
Compare
92ebad0 to
4f684d0
Compare
7b9048b to
0adb238
Compare
0adb238 to
d19b782
Compare
…M on L40S The test_disaggregated_cancel_large_context_requests test fails with OOM on L40S (44.4 GiB) because DeepSeek-V3-Lite bf16 requires ~37 GiB per disaggregated worker, and two workers sharing a single GPU need ~74 GiB. Add runtime GPU memory detection to fall back to TinyLlama with a smaller config on single-GPU systems with <80 GiB memory. This preserves the test's cancellation stress-test coverage while fitting within L40S memory constraints. On H100 or multi-GPU systems, the original DeepSeek-V3-Lite bf16 model is still used. Also adds a TinyLlama-compatible disagg config with conservative memory fractions to prevent KV cache allocation races on shared GPUs, and removes the test waiver from waives.txt. Signed-off-by: tensorrt-cicd <90828364+tensorrt-cicd@users.noreply.github.com>
Signed-off-by: tensorrt-cicd <90828364+tensorrt-cicd@users.noreply.github.com> Signed-off-by: trtllm-agent <296075020+trtllm-agent@users.noreply.github.com>
d19b782 to
f38489b
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@tests/integration/defs/disaggregated/test_configs/disagg_config_cancel_stress_test_small.yaml`:
- Line 1: Add the repository-standard NVIDIA copyright header at the beginning
of the configuration file before the hostname entry, using 2026 as the latest
meaningful modification year.
🪄 Autofix
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: 1430ad7f-6db5-4ef7-9f0d-00106408abbc
📒 Files selected for processing (3)
tests/integration/defs/disaggregated/test_configs/disagg_config_cancel_stress_test_small.yamltests/integration/defs/disaggregated/test_disaggregated.pytests/integration/test_lists/waives.txt
💤 Files with no reviewable changes (1)
- tests/integration/test_lists/waives.txt
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/integration/defs/disaggregated/test_disaggregated.py
| @@ -0,0 +1,39 @@ | |||
| hostname: localhost | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add the required NVIDIA copyright header.
This new file starts with hostname and has no NVIDIA copyright header. Add the repository-standard header before Line 1 and use 2026 as the latest meaningful modification year.
As per coding guidelines, all new files matching **/* must include the NVIDIA copyright header with the year of the 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
`@tests/integration/defs/disaggregated/test_configs/disagg_config_cancel_stress_test_small.yaml`
at line 1, Add the repository-standard NVIDIA copyright header at the beginning
of the configuration file before the hostname entry, using 2026 as the latest
meaningful modification year.
Source: Coding guidelines
Summary
model._apply(init_meta_tensor). The test was designed for H100 (80 GiB) with workers on separate GPUs but had no memory guard.total_memory < 80 GiBanddevice_count < 2, fall back to TinyLlama-1.1B-Chat-v1.0 (~2 GiB) with a dedicated small-model config (disagg_config_cancel_stress_test_small.yaml) that uses conservativefree_gpu_memory_fractionvalues (0.2/0.3 vs 0.3/0.85) to prevent KV cache allocation races on shared GPUs. The parametrize ID[DeepSeek-V3-Lite-bf16]is preserved since it comes from the fixture parameter, not the actual model loaded. On H100 or multi-GPU systems, the original DeepSeek-V3-Lite bf16 path is unchanged.Test plan
Links
Summary by CodeRabbit
Dev Engineer Review
run_disaggregated_cancel_testAPI change preserves the existing default prompt range.nvbugs/6105768skip.QA Engineer Review
test_disaggregated_cancel_large_context_requests.run_disaggregated_cancel_testto accept configurable prompt-length ranges.cancel_stress_test_smalltest configuration.tests/integration/test_lists/waives.txt; the previous waiver was removed.