[None][chore] Add set_segment arg to slurm scripts - #9731
Conversation
📝 WalkthroughWalkthroughThis pull request introduces GB200 GPU support and configuration updates for disaggregated benchmark testing. Changes include conditional Slurm segment parameter handling in job submission, new test configurations for Qwen3, DeepSeek, and Kimi models with GPU type specifications and environment variables, dataset path updates, utility enhancements for environment and configuration management, and expanded test coverage with new benchmark entries. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/integration/defs/perf/disagg/utils/config_validator.py (1)
82-87: Fix ctx_max_seq_len assertion message to reflect the failure conditionThe error message
"config error: ctx_max_seq_len > isl"describes the success condition and is inconsistent with the gen assertion message, which correctly describes the failure condition. Error messages should clarify what went wrong:- assert ctx_max_seq_len > isl, "config error: ctx_max_seq_len > isl" + assert ctx_max_seq_len > isl, "config error: ctx_max_seq_len <= isl"
♻️ Duplicate comments (12)
tests/integration/defs/perf/disagg/test_configs/disagg/perf/Qwen3-235B-A22B-FP4_1k1k_ctx1_gen4_tep8_bs32_eplb0_mtp0_ccb-UCX.yaml (1)
5-7: Duplicate issue:supported_gpusincludes GB300 butgpu_typeis hardcoded to GB200.Same inconsistency as the NIXL variant: metadata lists GB200 and GB300 as supported (lines 5-7), but
gpu_typeis hardcoded to GB200 (line 33). See the review comment from the NIXL file for context.Also applies to: 33-33
tests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_1k1k_ctx2_gen1_dep16_bs128_eplb0_mtp3_ccb-UCX.yaml (1)
4-4: Same GPU type/supported_gpus mismatch and environment variable concerns as preceding config files.This file has identical changes to the previous DeepSeek config (case-normalized model_dir_name, slurm gres, hardcoded GB200 gpu_type, and environment variables). The same concerns apply: verify the supported_gpus metadata is intentional, and confirm that GB300 handling (if needed) is implemented elsewhere.
Also applies to: 17-17, 33-33, 41-42
tests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_1k1k_ctx1_gen4_tep8_bs32_eplb0_mtp0_ccb-UCX.yaml (1)
4-4: Same concerns apply: GPU type configuration mismatch and environment variables.Also applies to: 17-17, 33-33, 41-42
tests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_8k1k_ctx8_gen1_dep32_bs16_eplb0_mtp3_ccb-UCX.yaml (1)
4-4: Same GPU type configuration concerns.Also applies to: 17-17, 33-33, 41-42
tests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_1k1k_ctx2_gen1_dep16_bs128_eplb0_mtp3_ccb-NIXL.yaml (1)
4-4: Same GPU type configuration concerns.Also applies to: 17-17, 33-33, 41-42
tests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_1k1k_ctx1_gen4_tep8_bs32_eplb0_mtp3_ccb-UCX.yaml (1)
4-4: Same GPU type configuration concerns.Also applies to: 17-17, 33-33, 41-42
tests/integration/defs/perf/disagg/test_configs/wideep/perf/deepseek-r1-fp4_8k1k_ctx8_gen1_dep32_bs16_eplb288_mtp3_ccb-NIXL.yaml (1)
18-18: Same GB300 compatibility concern as other configs.This config has the same hardcoded
--gres=gpu:4issue that may conflict with GB300 support. See the comment indeepseek-r1-fp4_8k1k_ctx6_gen1_dep16_bs64_eplb0_mtp0_ccb-NIXL.yamlfor details.tests/integration/defs/perf/disagg/test_configs/wideep/perf/Qwen3-235B-A22B-FP4_1k1k_ctx1_gen1_dep16_bs64_eplb288_mtp3_ccb-UCX.yaml (1)
18-18: Same GB300 compatibility concern.This Qwen3 config exhibits the same hardcoded
--gres=gpu:4issue. The pattern is repeated across all modified configs in this PR.tests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_8k1k_ctx8_gen1_dep32_bs16_eplb0_mtp3_ccb-NIXL.yaml (1)
17-17: Same GB300 compatibility concern.This config has the same hardcoded
--gres=gpu:4issue as noted in other configs.tests/integration/defs/perf/disagg/test_configs/wideep/perf/kimi-k2-thinking-fp4_8k1k_ctx8_gen1_dep32_bs256_eplb416_mtp0_ccb-UCX.yaml (1)
18-18: Same GB300 compatibility concern.This new Kimi-K2 config has the same hardcoded
--gres=gpu:4issue noted in other configs.tests/integration/defs/perf/disagg/test_configs/wideep/perf/deepseek-r1-fp4_8k1k_ctx6_gen1_dep16_bs64_eplb288_mtp0_ccb-NIXL.yaml (1)
18-18: Same GB300 compatibility concern.This wideep config has the same hardcoded
--gres=gpu:4issue as noted in other configs.tests/integration/defs/perf/disagg/test_configs/wideep/accuracy/kimi-k2-thinking-fp4_1k1k_ctx3_gen1_dep32_bs1024_eplb384_mtp0_ccb-UCX.yaml (1)
24-24: Same GB300 compatibility concern.This accuracy config has the same hardcoded
--gres=gpu:4issue noted in other configs.
🧹 Nitpick comments (6)
tests/integration/defs/perf/disagg/test_configs/disagg/perf/Qwen3-235B-A22B-FP4_1k1k_ctx1_gen1_dep32_bs16_eplb0_mtp3_ccb-UCX.yaml (1)
6-7: Verify GPU type consistency across metadata and configuration.The metadata declares both
GB200andGB300assupported_gpus, buthardware.gpu_typeis hardcoded toGB200. Per the PR title stating GB300 lacks segment support, confirm whether:
- This config is intended for GB200 only, with separate GB300-variant configs elsewhere.
- The
supported_gpuslist should be narrowed to match the specific GPU type.- The
gpu_typefield should be dynamic or conditionally set based on runtime selection.Given the context that similar configurations exist across the PR, ensure this pattern is consistent across all disagg/perf configs.
Also applies to: 33-33
tests/integration/defs/perf/disagg/test_configs/wideep/accuracy/deepseek-r1-fp4_1k1k_ctx2_gen1_dep16_bs128_eplb288_mtp3_ccb-NIXL.yaml (1)
24-24: Document the purpose of--gres=gpu:4in Slurm args.The
extra_argsfield now includes--gres=gpu:4, which explicitly requests 4 GPU resources in Slurm. Per the PR objectives, this may be a workaround for GB300's lack of segment support. Add an inline comment to explain the relationship between this Slurm flag and the GPU type constraints, especially if different GPU types require different handling.tests/integration/defs/perf/disagg/execution/executor.py (1)
252-253: Consider conditional cleanup based on debug mode.Preserving the temp config file on exception aids debugging, but this introduces a few concerns:
- File accumulation: Repeated failures could accumulate temp files over time without cleanup.
- Inconsistency: Normal submission failure (lines 240-241) still cleans up the temp file, but the exception path doesn't.
- Missing documentation: No comment explains why cleanup is disabled here.
Consider one of these approaches:
Option 1 (preferred): Make cleanup conditional on debug mode:
except Exception as e: error_msg = str(e) # Extract stderr from CalledProcessError if available if hasattr(e, "stderr") and e.stderr: error_msg = e.stderr.decode() if isinstance(e.stderr, bytes) else e.stderr logger.error(f"Job submission exception: {error_msg}") - # Clean up temporary file on exception temp_config_path = test_config.temp_config_path - # if os.path.exists(temp_config_path): - # os.remove(temp_config_path) + # Preserve temp config in debug mode for troubleshooting, otherwise clean up + if not EnvManager.get_debug_mode() and os.path.exists(temp_config_path): + os.remove(temp_config_path) return False, error_msgOption 2: Add a comment explaining the intent:
temp_config_path = test_config.temp_config_path + # Preserve temp config file on exception to aid debugging of submission failures # if os.path.exists(temp_config_path): # os.remove(temp_config_path)Based on learnings, test infrastructure should prioritize debugging capability, but a conditional approach balances this with resource management.
tests/integration/defs/perf/disagg/execution/subprocess_utils.py (2)
1-15: Add standard NVIDIA copyright header at top of fileThis Python file lacks the NVIDIA copyright header required for TensorRT‑LLM OSS files. Since you’re already modifying it, please add the standard current‑year NVIDIA header block at the top to align with project guidelines.
60-63: stderr logging misses failing commands due tocheck=True; log in exception path as wellWith
check=True,subprocess.runraisesCalledProcessErroron non‑zero exit codes, so the new stderr logging only runs for successful commands that still write to stderr, and not for the failures where stderr is most useful. Consider logging stderr in the exception path while preserving the existing API (still raising on failure).You can refactor
exec_cmd_with_outputlike this:def exec_cmd_with_output(*popenargs, timeout: Optional[float] = None, **kwargs) -> str: @@ - result = subprocess.run( - *popenargs, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - timeout=timeout, - check=True, - **kwargs, - ) - - # Log stderr if it exists - if result.stderr: - stderr_output = result.stderr.decode() - logger.error(f"Command stderr: {stderr_output}") - - return result.stdout.decode() + try: + result = subprocess.run( + *popenargs, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + timeout=timeout, + check=True, + **kwargs, + ) + except subprocess.CalledProcessError as e: + # Log stderr on failure, then re-raise to preserve behavior + if e.stderr: + stderr_output = e.stderr.decode() + logger.error(f"Command stderr (failed): {stderr_output}") + raise + + # For successful commands, optionally log stderr if present (e.g., warnings) + if result.stderr: + stderr_output = result.stderr.decode() + logger.error(f"Command stderr: {stderr_output}") + + return result.stdout.decode()tests/integration/defs/perf/disagg/utils/common.py (1)
106-134: LGTM: Model-aware container mount with Kimi-K2 special handling.The updated signature adds model-aware mounting logic. The Kimi-K2 shared memory requirement (640G) is well-documented and addresses a specific host memory constraint. The conditional dataset_dir and repo_dir mounting logic properly handles placeholder values.
Note: Line 124 has an unnecessary f-string prefix (no placeholders present). While functionally correct, removing the
fprefix would address the static analysis warning.Apply this diff if you wish to address the static analysis hint:
- mounts.append(f"tmpfs:/dev/shm:size=640G") + mounts.append("tmpfs:/dev/shm:size=640G")
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (59)
examples/disaggregated/slurm/benchmark/config.yaml(1 hunks)examples/disaggregated/slurm/benchmark/submit.py(2 hunks)tests/integration/defs/perf/disagg/compare_backends.py(2 hunks)tests/integration/defs/perf/disagg/envs/ENV.md(2 hunks)tests/integration/defs/perf/disagg/execution/executor.py(1 hunks)tests/integration/defs/perf/disagg/execution/subprocess_utils.py(2 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/Qwen3-235B-A22B-FP4_1k1k_ctx1_gen1_dep16_bs64_eplb0_mtp3_ccb-NIXL.yaml(2 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/Qwen3-235B-A22B-FP4_1k1k_ctx1_gen1_dep16_bs64_eplb0_mtp3_ccb-UCX.yaml(2 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/Qwen3-235B-A22B-FP4_1k1k_ctx1_gen1_dep32_bs16_eplb0_mtp3_ccb-NIXL.yaml(2 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/Qwen3-235B-A22B-FP4_1k1k_ctx1_gen1_dep32_bs16_eplb0_mtp3_ccb-UCX.yaml(2 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/Qwen3-235B-A22B-FP4_1k1k_ctx1_gen4_tep8_bs32_eplb0_mtp0_ccb-NIXL.yaml(2 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/Qwen3-235B-A22B-FP4_1k1k_ctx1_gen4_tep8_bs32_eplb0_mtp0_ccb-UCX.yaml(2 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/Qwen3-235B-A22B-FP4_1k1k_ctx2_gen1_dep16_bs128_eplb0_mtp1_ccb-NIXL.yaml(2 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/Qwen3-235B-A22B-FP4_1k1k_ctx2_gen1_dep16_bs128_eplb0_mtp1_ccb-UCX.yaml(2 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/Qwen3-235B-A22B-FP8_1k1k_ctx1_gen1_tep8_bs32_eplb0_mtp0_ccb-NIXL.yaml(1 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/Qwen3-235B-A22B-FP8_1k1k_ctx1_gen1_tep8_bs32_eplb0_mtp0_ccb-UCX.yaml(1 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_1k1k_ctx1_gen1_dep32_bs32_eplb0_mtp0_ccb-NIXL.yaml(4 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_1k1k_ctx1_gen1_dep32_bs32_eplb0_mtp0_ccb-UCX.yaml(3 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_1k1k_ctx1_gen4_tep8_bs32_eplb0_mtp0_ccb-NIXL.yaml(3 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_1k1k_ctx1_gen4_tep8_bs32_eplb0_mtp0_ccb-UCX.yaml(3 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_1k1k_ctx1_gen4_tep8_bs32_eplb0_mtp3_ccb-NIXL.yaml(3 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_1k1k_ctx1_gen4_tep8_bs32_eplb0_mtp3_ccb-UCX.yaml(3 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_1k1k_ctx2_gen1_dep16_bs128_eplb0_mtp3_ccb-NIXL.yaml(3 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_1k1k_ctx2_gen1_dep16_bs128_eplb0_mtp3_ccb-UCX.yaml(3 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_8k1k_ctx1_gen3_tep8_bs16_eplb0_mtp3_ccb-NIXL.yaml(3 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_8k1k_ctx1_gen3_tep8_bs16_eplb0_mtp3_ccb-UCX.yaml(3 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_8k1k_ctx1_gen3_tep8_bs32_eplb0_mtp0_ccb-NIXL.yaml(3 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_8k1k_ctx1_gen3_tep8_bs32_eplb0_mtp0_ccb-UCX.yaml(3 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_8k1k_ctx6_gen1_dep16_bs64_eplb0_mtp0_ccb-NIXL.yaml(3 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_8k1k_ctx6_gen1_dep16_bs64_eplb0_mtp0_ccb-UCX.yaml(3 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_8k1k_ctx8_gen1_dep32_bs16_eplb0_mtp3_ccb-NIXL.yaml(3 hunks)tests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_8k1k_ctx8_gen1_dep32_bs16_eplb0_mtp3_ccb-UCX.yaml(3 hunks)tests/integration/defs/perf/disagg/test_configs/wideep/accuracy/deepseek-r1-fp4_1k1k_ctx2_gen1_dep16_bs128_eplb288_mtp3_ccb-NIXL.yaml(3 hunks)tests/integration/defs/perf/disagg/test_configs/wideep/accuracy/kimi-k2-thinking-fp4_1k1k_ctx3_gen1_dep32_bs1024_eplb384_mtp0_ccb-UCX.yaml(1 hunks)tests/integration/defs/perf/disagg/test_configs/wideep/perf/Qwen3-235B-A22B-FP4_1k1k_ctx1_gen1_dep16_bs64_eplb288_mtp3_ccb-NIXL.yaml(2 hunks)tests/integration/defs/perf/disagg/test_configs/wideep/perf/Qwen3-235B-A22B-FP4_1k1k_ctx1_gen1_dep16_bs64_eplb288_mtp3_ccb-UCX.yaml(2 hunks)tests/integration/defs/perf/disagg/test_configs/wideep/perf/Qwen3-235B-A22B-FP4_1k1k_ctx1_gen1_dep32_bs16_eplb288_mtp3_ccb-NIXL.yaml(2 hunks)tests/integration/defs/perf/disagg/test_configs/wideep/perf/Qwen3-235B-A22B-FP4_1k1k_ctx1_gen1_dep32_bs16_eplb288_mtp3_ccb-UCX.yaml(2 hunks)tests/integration/defs/perf/disagg/test_configs/wideep/perf/Qwen3-235B-A22B-FP4_1k1k_ctx2_gen1_dep16_bs128_eplb288_mtp1_ccb-NIXL.yaml(2 hunks)tests/integration/defs/perf/disagg/test_configs/wideep/perf/Qwen3-235B-A22B-FP4_1k1k_ctx2_gen1_dep16_bs128_eplb288_mtp1_ccb-UCX.yaml(2 hunks)tests/integration/defs/perf/disagg/test_configs/wideep/perf/deepseek-r1-fp4_1k1k_ctx1_gen1_dep32_bs32_eplb288_mtp0_ccb-NIXL.yaml(2 hunks)tests/integration/defs/perf/disagg/test_configs/wideep/perf/deepseek-r1-fp4_1k1k_ctx1_gen1_dep32_bs32_eplb288_mtp0_ccb-UCX.yaml(2 hunks)tests/integration/defs/perf/disagg/test_configs/wideep/perf/deepseek-r1-fp4_1k1k_ctx2_gen1_dep16_bs128_eplb288_mtp3_ccb-NIXL.yaml(2 hunks)tests/integration/defs/perf/disagg/test_configs/wideep/perf/deepseek-r1-fp4_1k1k_ctx2_gen1_dep16_bs128_eplb288_mtp3_ccb-UCX.yaml(2 hunks)tests/integration/defs/perf/disagg/test_configs/wideep/perf/deepseek-r1-fp4_1k1k_ctx2_gen1_dep48_bs16_eplb288_mtp3_ccb-DEFAULT.yaml(1 hunks)tests/integration/defs/perf/disagg/test_configs/wideep/perf/deepseek-r1-fp4_8k1k_ctx2_gen1_dep32_bs128_eplb288_mtp3_ccb-DEFAULT.yaml(2 hunks)tests/integration/defs/perf/disagg/test_configs/wideep/perf/deepseek-r1-fp4_8k1k_ctx6_gen1_dep16_bs64_eplb288_mtp0_ccb-NIXL.yaml(2 hunks)tests/integration/defs/perf/disagg/test_configs/wideep/perf/deepseek-r1-fp4_8k1k_ctx6_gen1_dep16_bs64_eplb288_mtp0_ccb-UCX.yaml(2 hunks)tests/integration/defs/perf/disagg/test_configs/wideep/perf/deepseek-r1-fp4_8k1k_ctx8_gen1_dep32_bs16_eplb288_mtp3_ccb-NIXL.yaml(2 hunks)tests/integration/defs/perf/disagg/test_configs/wideep/perf/deepseek-r1-fp4_8k1k_ctx8_gen1_dep32_bs16_eplb288_mtp3_ccb-UCX.yaml(2 hunks)tests/integration/defs/perf/disagg/test_configs/wideep/perf/kimi-k2-thinking-fp4_1k1k_ctx3_gen1_dep32_bs1024_eplb384_mtp0_ccb-UCX.yaml(1 hunks)tests/integration/defs/perf/disagg/test_configs/wideep/perf/kimi-k2-thinking-fp4_8k1k_ctx8_gen1_dep32_bs256_eplb416_mtp0_ccb-UCX.yaml(1 hunks)tests/integration/defs/perf/disagg/testlist/disagg.txt(1 hunks)tests/integration/defs/perf/disagg/testlist/disagg_gb300.txt(1 hunks)tests/integration/defs/perf/disagg/testlist/wideep.txt(1 hunks)tests/integration/defs/perf/disagg/utils/common.py(5 hunks)tests/integration/defs/perf/disagg/utils/config_loader.py(4 hunks)tests/integration/defs/perf/disagg/utils/config_validator.py(1 hunks)tests/integration/defs/pytest.ini(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (CODING_GUIDELINES.md)
**/*.py: The code developed for TensorRT-LLM should conform to Python 3.8+
Indent Python code with 4 spaces; do not use tabs
Always maintain the namespace when importing in Python, even if only one class or function from a module is used (e.g., usefrom package.subpackage import fooand thenfoo.SomeClass()instead offrom package.subpackage.foo import SomeClass)
Python filenames should use snake_case (e.g.,some_file.py)
Python class names should use PascalCase (e.g.,class SomeClass)
Python function and method names should use snake_case (e.g.,def my_awesome_function():)
Python local variable names should use snake_case, with prefixkfor variable names that start with a number (e.g.,k_99th_percentile = ...)
Python global variables should use upper snake_case with prefixG(e.g.,G_MY_GLOBAL = ...)
Python constants should use upper snake_case (e.g.,MY_CONSTANT = ...)
Avoid shadowing variables declared in an outer scope in Python
Initialize all externally visible members of a Python class in the constructor
For Python interfaces that may be used outside a file, prefer docstrings over comments
Python comments should be reserved for code within a function, or interfaces that are local to a file
Use Google style docstrings for Python classes and functions, which can be parsed by Sphinx
Python attributes and variables can be documented inline with type and description (e.g.,self.x = 5followed by"""<type>: Description of 'x'""")
Avoid using reflection in Python when functionality can be easily achieved without reflection
When using try-except blocks in Python, limit the except clause to the smallest set of specific errors possible instead of catching all exceptions
When using try-except blocks in Python to handle multiple possible variable types (duck-typing), keep the body of the try as small as possible and use the else block to implement the logic
Files:
tests/integration/defs/perf/disagg/execution/subprocess_utils.pytests/integration/defs/perf/disagg/compare_backends.pytests/integration/defs/perf/disagg/utils/config_validator.pytests/integration/defs/perf/disagg/execution/executor.pyexamples/disaggregated/slurm/benchmark/submit.pytests/integration/defs/perf/disagg/utils/config_loader.pytests/integration/defs/perf/disagg/utils/common.py
**/*.{cpp,h,cu,py}
📄 CodeRabbit inference engine (CODING_GUIDELINES.md)
All TensorRT-LLM Open Source Software code files should contain an NVIDIA copyright header that includes the current year at the top
Files:
tests/integration/defs/perf/disagg/execution/subprocess_utils.pytests/integration/defs/perf/disagg/compare_backends.pytests/integration/defs/perf/disagg/utils/config_validator.pytests/integration/defs/perf/disagg/execution/executor.pyexamples/disaggregated/slurm/benchmark/submit.pytests/integration/defs/perf/disagg/utils/config_loader.pytests/integration/defs/perf/disagg/utils/common.py
🧠 Learnings (10)
📚 Learning: 2025-08-26T09:49:04.956Z
Learnt from: pengbowang-nv
Repo: NVIDIA/TensorRT-LLM PR: 7192
File: tests/integration/test_lists/test-db/l0_dgx_b200.yml:56-72
Timestamp: 2025-08-26T09:49:04.956Z
Learning: In TensorRT-LLM test configuration files, the test scheduling system handles wildcard matching with special rules that prevent duplicate test execution even when the same tests appear in multiple yaml files with overlapping GPU wildcards (e.g., "*b200*" and "*gb200*").
Applied to files:
tests/integration/defs/perf/disagg/test_configs/disagg/perf/Qwen3-235B-A22B-FP8_1k1k_ctx1_gen1_tep8_bs32_eplb0_mtp0_ccb-NIXL.yamltests/integration/defs/perf/disagg/test_configs/wideep/perf/deepseek-r1-fp4_1k1k_ctx2_gen1_dep16_bs128_eplb288_mtp3_ccb-UCX.yamltests/integration/defs/perf/disagg/test_configs/wideep/perf/deepseek-r1-fp4_1k1k_ctx2_gen1_dep48_bs16_eplb288_mtp3_ccb-DEFAULT.yamltests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_8k1k_ctx1_gen3_tep8_bs32_eplb0_mtp0_ccb-NIXL.yamltests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_1k1k_ctx1_gen4_tep8_bs32_eplb0_mtp0_ccb-NIXL.yamltests/integration/defs/perf/disagg/test_configs/wideep/perf/deepseek-r1-fp4_8k1k_ctx6_gen1_dep16_bs64_eplb288_mtp0_ccb-UCX.yamltests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_8k1k_ctx8_gen1_dep32_bs16_eplb0_mtp3_ccb-NIXL.yamltests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_8k1k_ctx1_gen3_tep8_bs16_eplb0_mtp3_ccb-NIXL.yamltests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_1k1k_ctx2_gen1_dep16_bs128_eplb0_mtp3_ccb-NIXL.yamltests/integration/defs/perf/disagg/test_configs/disagg/perf/Qwen3-235B-A22B-FP4_1k1k_ctx2_gen1_dep16_bs128_eplb0_mtp1_ccb-NIXL.yamltests/integration/defs/perf/disagg/test_configs/disagg/perf/Qwen3-235B-A22B-FP8_1k1k_ctx1_gen1_tep8_bs32_eplb0_mtp0_ccb-UCX.yamltests/integration/defs/perf/disagg/test_configs/wideep/perf/deepseek-r1-fp4_8k1k_ctx8_gen1_dep32_bs16_eplb288_mtp3_ccb-UCX.yamltests/integration/defs/perf/disagg/test_configs/disagg/perf/Qwen3-235B-A22B-FP4_1k1k_ctx1_gen1_dep32_bs16_eplb0_mtp3_ccb-UCX.yamltests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_1k1k_ctx1_gen1_dep32_bs32_eplb0_mtp0_ccb-UCX.yaml
📚 Learning: 2025-09-09T09:40:45.658Z
Learnt from: fredricz-20070104
Repo: NVIDIA/TensorRT-LLM PR: 7645
File: tests/integration/test_lists/qa/llm_function_core.txt:648-648
Timestamp: 2025-09-09T09:40:45.658Z
Learning: In TensorRT-LLM test lists, it's common and intentional for the same test to appear in multiple test list files when they serve different purposes (e.g., llm_function_core.txt for comprehensive core functionality testing and llm_function_core_sanity.txt for quick sanity checks). This duplication allows tests to be run in different testing contexts.
Applied to files:
tests/integration/defs/perf/disagg/test_configs/disagg/perf/Qwen3-235B-A22B-FP8_1k1k_ctx1_gen1_tep8_bs32_eplb0_mtp0_ccb-NIXL.yamltests/integration/defs/perf/disagg/test_configs/disagg/perf/Qwen3-235B-A22B-FP8_1k1k_ctx1_gen1_tep8_bs32_eplb0_mtp0_ccb-UCX.yaml
📚 Learning: 2025-07-28T17:06:08.621Z
Learnt from: moraxu
Repo: NVIDIA/TensorRT-LLM PR: 6303
File: tests/integration/test_lists/qa/examples_test_list.txt:494-494
Timestamp: 2025-07-28T17:06:08.621Z
Learning: In TensorRT-LLM testing, it's common to have both CLI flow tests (test_cli_flow.py) and PyTorch API tests (test_llm_api_pytorch.py) for the same model. These serve different purposes: CLI flow tests validate the traditional command-line workflow, while PyTorch API tests validate the newer LLM API backend. Both are legitimate and should coexist.
Applied to files:
tests/integration/defs/perf/disagg/test_configs/disagg/perf/Qwen3-235B-A22B-FP8_1k1k_ctx1_gen1_tep8_bs32_eplb0_mtp0_ccb-NIXL.yaml
📚 Learning: 2025-08-20T07:43:36.447Z
Learnt from: ChristinaZ
Repo: NVIDIA/TensorRT-LLM PR: 7068
File: cpp/tensorrt_llm/kernels/moeTopKFuncs.cuh:169-172
Timestamp: 2025-08-20T07:43:36.447Z
Learning: In TensorRT-LLM MOE kernels, when processing up to 128 experts across 32 threads, each thread handles at most 4 experts (N < 5 constraint), where N represents candidates per thread rather than total system capacity.
Applied to files:
tests/integration/defs/perf/disagg/test_configs/disagg/perf/Qwen3-235B-A22B-FP8_1k1k_ctx1_gen1_tep8_bs32_eplb0_mtp0_ccb-NIXL.yaml
📚 Learning: 2025-08-18T08:42:02.640Z
Learnt from: samuellees
Repo: NVIDIA/TensorRT-LLM PR: 6974
File: tensorrt_llm/serve/scripts/benchmark_dataset.py:558-566
Timestamp: 2025-08-18T08:42:02.640Z
Learning: In TensorRT-LLM's RandomDataset (tensorrt_llm/serve/scripts/benchmark_dataset.py), when using --random-token-ids option, sequence length accuracy is prioritized over semantic correctness for benchmarking purposes. The encode/decode operations should use skip_special_tokens=True and add_special_tokens=False to ensure exact target token lengths.
Applied to files:
tests/integration/defs/perf/disagg/test_configs/disagg/perf/Qwen3-235B-A22B-FP8_1k1k_ctx1_gen1_tep8_bs32_eplb0_mtp0_ccb-NIXL.yamltests/integration/defs/perf/disagg/test_configs/wideep/perf/deepseek-r1-fp4_1k1k_ctx2_gen1_dep48_bs16_eplb288_mtp3_ccb-DEFAULT.yaml
📚 Learning: 2025-08-20T06:56:02.889Z
Learnt from: eopXD
Repo: NVIDIA/TensorRT-LLM PR: 6768
File: cpp/tensorrt_llm/batch_manager/kvCacheManager.cpp:577-579
Timestamp: 2025-08-20T06:56:02.889Z
Learning: In cpp/tensorrt_llm/batch_manager/kvCacheManager.cpp, maxSequenceLength is now enforced as a non-optional argument in the BlockManager constructor, so concerns about std::nullopt defaulting to 0 are not applicable. When windowSize > maxSequenceLength, a warning should be added instead of handling optional parameter cases.
Applied to files:
tests/integration/defs/perf/disagg/utils/config_validator.py
📚 Learning: 2025-11-14T11:22:03.729Z
Learnt from: nzmora-nvidia
Repo: NVIDIA/TensorRT-LLM PR: 9163
File: tensorrt_llm/_torch/auto_deploy/custom_ops/quant.py:107-113
Timestamp: 2025-11-14T11:22:03.729Z
Learning: In TensorRT-LLM AutoDeploy custom ops, when adding hardware capability checks to select between kernel implementations (e.g., cuBLAS vs. CUDA kernel), use descriptive variable names that identify the specific GPU architectures or families being targeted (e.g., `is_blackwell_geforce_or_ada`) rather than generic names like `enable_cuda_core`. This makes it clear that the code is selecting an implementation path based on hardware capabilities, not enabling/disabling hardware features.
Applied to files:
tests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_1k1k_ctx1_gen4_tep8_bs32_eplb0_mtp0_ccb-NIXL.yamltests/integration/defs/perf/disagg/test_configs/disagg/perf/Qwen3-235B-A22B-FP4_1k1k_ctx2_gen1_dep16_bs128_eplb0_mtp1_ccb-NIXL.yaml
📚 Learning: 2025-09-17T06:01:01.836Z
Learnt from: fredricz-20070104
Repo: NVIDIA/TensorRT-LLM PR: 7785
File: tests/integration/defs/perf/utils.py:321-333
Timestamp: 2025-09-17T06:01:01.836Z
Learning: In test infrastructure code for disaggregated serving tests, prefer logging errors and continuing execution rather than raising exceptions on timeout, to avoid disrupting test cleanup and causing cascading failures.
Applied to files:
tests/integration/defs/perf/disagg/execution/executor.py
📚 Learning: 2025-08-14T06:36:40.701Z
Learnt from: timlee0212
Repo: NVIDIA/TensorRT-LLM PR: 6886
File: tensorrt_llm/_torch/models/modeling_deepseekv3.py:0-0
Timestamp: 2025-08-14T06:36:40.701Z
Learning: In DeepSeek V3 model (tensorrt_llm/_torch/models/modeling_deepseekv3.py), the disagreement between AllReduce.__init__ guard and _compute_mlp_tp_size logic for MNNVL usage is expected by design. The AllReduce component and MLP TP-size computation intentionally use different criteria for MNNVL availability decisions.
Applied to files:
tests/integration/defs/perf/disagg/test_configs/disagg/perf/deepseek-r1-fp4_8k1k_ctx1_gen3_tep8_bs16_eplb0_mtp3_ccb-NIXL.yaml
📚 Learning: 2025-08-29T14:07:45.863Z
Learnt from: EmmaQiaoCh
Repo: NVIDIA/TensorRT-LLM PR: 7370
File: tests/unittest/trt/model_api/test_model_quantization.py:24-27
Timestamp: 2025-08-29T14:07:45.863Z
Learning: In TensorRT-LLM's CI infrastructure, pytest skip markers (pytest.mark.skip) are properly honored even when test files have __main__ blocks that call test functions directly. The testing system correctly skips tests without requiring modifications to the __main__ block execution pattern.
Applied to files:
tests/integration/defs/pytest.ini
🧬 Code graph analysis (1)
tests/integration/defs/perf/disagg/execution/subprocess_utils.py (1)
tests/integration/defs/perf/disagg/utils/logger.py (1)
error(89-91)
🪛 Ruff (0.14.7)
tests/integration/defs/perf/disagg/utils/common.py
124-124: f-string without any placeholders
Remove extraneous f prefix
(F541)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Pre-commit Check
|
LGTM |
3f68bc3 to
db619e5
Compare
fredricz-20070104
changed the title
fredricz-20070104
changed the title
|
/bot run --skip-test |
|
PR_Github #27295 [ run ] triggered by Bot. Commit: |
|
PR_Github #27295 [ run ] completed with state |
2128613 to
4e6c9e3
Compare
5d691d9 to
b937612
Compare
|
/bot reuse-pipeline |
|
PR_Github #27431 [ reuse-pipeline ] triggered by Bot. Commit: |
|
PR_Github #27422 [ run ] completed with state |
|
PR_Github #27431 [ reuse-pipeline ] completed with state |
Signed-off-by: FredricZ-2007 <226039983+fredricz-20070104@users.noreply.github.com>
Signed-off-by: FredricZ-2007 <226039983+fredricz-20070104@users.noreply.github.com>
Signed-off-by: FredricZ-2007 <226039983+fredricz-20070104@users.noreply.github.com>
Signed-off-by: FredricZ-2007 <226039983+fredricz-20070104@users.noreply.github.com>
Signed-off-by: FredricZ-2007 <226039983+fredricz-20070104@users.noreply.github.com>
Signed-off-by: FredricZ-2007 <226039983+fredricz-20070104@users.noreply.github.com>
Signed-off-by: FredricZ-2007 <226039983+fredricz-20070104@users.noreply.github.com>
Signed-off-by: FredricZ-2007 <226039983+fredricz-20070104@users.noreply.github.com>
Signed-off-by: FredricZ-2007 <226039983+fredricz-20070104@users.noreply.github.com>
Signed-off-by: FredricZ-2007 <226039983+fredricz-20070104@users.noreply.github.com>
Signed-off-by: FredricZ-2007 <226039983+fredricz-20070104@users.noreply.github.com>
Signed-off-by: FredricZ-2007 <226039983+fredricz-20070104@users.noreply.github.com>
Signed-off-by: FredricZ-2007 <226039983+fredricz-20070104@users.noreply.github.com>
Signed-off-by: FredricZ-2007 <226039983+fredricz-20070104@users.noreply.github.com>
07c61d2 to
cb8ba85
Compare
|
/bot reuse-pipeline - |
|
/bot --resuse-pipeline |
GitHub Bot Help
Provide a user friendly way for developers to interact with a Jenkins server. Run See details below for each supported subcommand. Details
Launch build/test pipelines. All previously running jobs will be killed.
kill
Kill all running builds associated with pull request. skip
Skip testing for latest commit on pull request. reuse-pipeline
Reuse a previous pipeline to validate current commit. This action will also kill all currently running builds associated with the pull request. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break. |
|
PR_Github #27771 Bot args parsing error: usage: /bot [-h] |
|
/bot reuse-pipeline |
|
PR_Github #27773 [ reuse-pipeline ] triggered by Bot. Commit: |
|
PR_Github #27773 [ reuse-pipeline ] completed with state |
…IDIA#9731) Signed-off-by: FredricZ-2007 <226039983+fredricz-20070104@users.noreply.github.com>
…IDIA#9731) Signed-off-by: FredricZ-2007 <226039983+fredricz-20070104@users.noreply.github.com>
…IDIA#9731) Signed-off-by: FredricZ-2007 <226039983+fredricz-20070104@users.noreply.github.com>
…IDIA#9731) Signed-off-by: FredricZ-2007 <226039983+fredricz-20070104@users.noreply.github.com>
5 participants
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.
Add GB300 dlcluster support since it doesn't support segment