[None][test] DSv4 PR-6 coverage and import safety - #15710
Conversation
752a365 to
a64afb4
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #56362 [ run ] triggered by Bot. Commit: |
a64afb4 to
1f51d40
Compare
|
/bot kill |
|
PR_Github #56363 [ kill ] triggered by Bot. Commit: |
|
/bot run --disable-fail-fast |
|
PR_Github #56362 [ run ] completed with state |
|
PR_Github #56363 [ kill ] completed with state |
|
PR_Github #56367 [ run ] triggered by Bot. Commit: |
|
PR_Github #56367 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #56601 [ run ] triggered by Bot. Commit: |
|
PR_Github #56601 [ run ] completed with state
|
dccb092 to
fa860a3
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #57111 [ run ] triggered by Bot. Commit: |
|
PR_Github #57111 [ run ] completed with state
|
fa860a3 to
ce2bd21
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #57485 [ run ] triggered by Bot. Commit: |
|
PR_Github #57485 [ run ] completed with state |
|
/bot run --disable-fail-fast |
|
PR_Github #57536 [ run ] triggered by Bot. Commit: |
|
PR_Github #57536 [ run ] completed with state
|
956fd39 to
cebfb1e
Compare
Signed-off-by: Jiagan Cheng <jiaganc@nvidia.com>
6d6988d to
326087a
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #57995 [ run ] triggered by Bot. Commit: |
📝 WalkthroughWalkthroughThis PR guards CUDA capability queries behind ChangesGPU-free Import Guards
KV Cache Manager Cleanup
DeepSeek-V4 Accuracy and Smoke Tests
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (3)
tests/integration/defs/examples/test_deepseek_v4_pro.py (1)
72-108: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMissing return-type annotations on helper functions.
_get_llm_vocab_size,_make_token_id_prompt, and_run_token_id_smoke_wavelack return-type annotations. As per coding guidelines, "Always annotate functions with return types (useNoneif no return)".✏️ Suggested annotations
-def _get_llm_vocab_size(llm): +def _get_llm_vocab_size(llm) -> int: ... -def _make_token_id_prompt(length, vocab_size, salt): +def _make_token_id_prompt(length: int, vocab_size: int, salt: int) -> list[int]: ... -def _run_token_id_smoke_wave(llm, prompt_lengths, max_tokens, vocab_size): +def _run_token_id_smoke_wave(llm, prompt_lengths: list[int], max_tokens: int, vocab_size: int) -> None:Also applies to: 111-114, 117-135
🤖 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/examples/test_deepseek_v4_pro.py` around lines 72 - 108, The helper functions in this test module are missing required return-type annotations. Update _get_llm_vocab_size, _make_token_id_prompt, and _run_token_id_smoke_wave to include explicit return types, using the appropriate concrete type for _get_llm_vocab_size and _make_token_id_prompt and None for the function that only performs side effects. Keep the changes aligned with the existing function signatures so the annotations are easy to locate even if the surrounding code shifts.Source: Coding guidelines
tests/integration/defs/accuracy/test_llm_api_pytorch.py (1)
3853-3856: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMutable class attribute flagged by Ruff (RUF012).
EXTRA_EVALUATOR_KWARGSis a plaindict(...)class attribute. This mirrors the pre-existingEVALUATOR_KWARGSpattern inaccuracy_core.py'sGSM8Kclass, so it's consistent with established convention in this codebase, but it is worth confirming whether RUF012 is actually enforced for this file (Group A vs. legacy Group B toolchain) since the static analysis tool flagged it.🤖 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/accuracy/test_llm_api_pytorch.py` around lines 3853 - 3856, `EXTRA_EVALUATOR_KWARGS` in the `GSM8K` class is a mutable class attribute and is being flagged by Ruff RUF012; either annotate it as an intentional class-level constant in line with `EVALUATOR_KWARGS` in `accuracy_core.py` or move it to an immutable form if this file is under the enforced toolchain. Check the `GSM8K` definition and confirm whether this test module is subject to RUF012, then adjust the attribute definition accordingly so the lint rule is satisfied or explicitly avoided by the correct convention.Source: Linters/SAST tools
tests/integration/defs/accuracy/test_disaggregated_serving.py (1)
2577-2686: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSignificant config duplication between
test_auto_dtypeandtest_gen_first.
cache_transceiver_config,ctx_server_config, andgen_server_configare byte-for-byte identical between the two test methods (onlydisaggregated_server_config'sschedule_stylediffers). Consider extracting a small helper (e.g._build_dsv4_flash_configs(schedule_style=None)) to avoid drift between the two copies as this config evolves.♻️ Example refactor sketch
+ def _dsv4_flash_configs(self): + cache_transceiver_config = { + "backend": "NIXL", + "transceiver_runtime": "PYTHON", + "max_tokens_in_buffer": 4096, + } + ctx_server_config = { + "tensor_parallel_size": 2, + "moe_expert_parallel_size": 2, + "disable_overlap_scheduler": True, + "max_batch_size": DEEPSEEKV4_TEST_MAX_BATCH_SIZE, + "max_seq_len": 4096, + "kv_cache_config": {"free_gpu_memory_fraction": 0.5}, + "cache_transceiver_config": cache_transceiver_config, + } + gen_server_config = { + "tensor_parallel_size": 2, + "moe_expert_parallel_size": 2, + "enable_attention_dp": True, + "disable_overlap_scheduler": True, + "max_batch_size": DEEPSEEKV4_TEST_MAX_BATCH_SIZE, + "max_seq_len": 4096, + "moe_config": {"backend": "TRTLLM"}, + "kv_cache_config": {"free_gpu_memory_fraction": 0.5}, + "cache_transceiver_config": cache_transceiver_config, + } + return ctx_server_config, gen_server_config🤖 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/accuracy/test_disaggregated_serving.py` around lines 2577 - 2686, `test_auto_dtype` and `test_gen_first` duplicate the same `cache_transceiver_config`, `ctx_server_config`, and `gen_server_config` setup, which risks drift as these configs change. Extract the shared config construction into a small helper near these methods (for example, a builder for the disaggregated V4-Flash configs) and have both tests call it, varying only the `disaggregated_server_config` `schedule_style` in `test_gen_first`.
🤖 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/accuracy/test_llm_api_pytorch.py`:
- Around line 3846-3875: Register TestDeepSeekV4Pro::test_gsm8k_full_accuracy in
the corresponding l0_*.yml test list under the test-db integration config so
this 8-GPU accuracy gate is included in pre-merge CI; locate it by the
TestDeepSeekV4Pro class and the test_gsm8k_full_accuracy method, and add the new
test entry to the matching YAML tier alongside the other accuracy tests.
---
Nitpick comments:
In `@tests/integration/defs/accuracy/test_disaggregated_serving.py`:
- Around line 2577-2686: `test_auto_dtype` and `test_gen_first` duplicate the
same `cache_transceiver_config`, `ctx_server_config`, and `gen_server_config`
setup, which risks drift as these configs change. Extract the shared config
construction into a small helper near these methods (for example, a builder for
the disaggregated V4-Flash configs) and have both tests call it, varying only
the `disaggregated_server_config` `schedule_style` in `test_gen_first`.
In `@tests/integration/defs/accuracy/test_llm_api_pytorch.py`:
- Around line 3853-3856: `EXTRA_EVALUATOR_KWARGS` in the `GSM8K` class is a
mutable class attribute and is being flagged by Ruff RUF012; either annotate it
as an intentional class-level constant in line with `EVALUATOR_KWARGS` in
`accuracy_core.py` or move it to an immutable form if this file is under the
enforced toolchain. Check the `GSM8K` definition and confirm whether this test
module is subject to RUF012, then adjust the attribute definition accordingly so
the lint rule is satisfied or explicitly avoided by the correct convention.
In `@tests/integration/defs/examples/test_deepseek_v4_pro.py`:
- Around line 72-108: The helper functions in this test module are missing
required return-type annotations. Update _get_llm_vocab_size,
_make_token_id_prompt, and _run_token_id_smoke_wave to include explicit return
types, using the appropriate concrete type for _get_llm_vocab_size and
_make_token_id_prompt and None for the function that only performs side effects.
Keep the changes aligned with the existing function signatures so the
annotations are easy to locate even if the surrounding code shifts.
🪄 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: ad1f081b-f381-4d29-9bd3-ebd8c00fb53e
📒 Files selected for processing (11)
tensorrt_llm/_torch/attention_backend/flashinfer.pytensorrt_llm/_torch/attention_backend/triton_prefill.pytensorrt_llm/_torch/cuda_tile_utils.pytensorrt_llm/runtime/kv_cache_manager_v2/_core/_kv_cache.pytests/integration/defs/accuracy/accuracy_core.pytests/integration/defs/accuracy/references/gsm8k.yamltests/integration/defs/accuracy/test_disaggregated_serving.pytests/integration/defs/accuracy/test_llm_api_pytorch.pytests/integration/defs/examples/test_deepseek_v4_pro.pytests/integration/test_lists/test-db/l0_gb200_multi_gpus.ymltests/unittest/others/test_import_gpu_free.py
|
PR_Github #57995 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #58087 [ run ] triggered by Bot. Commit: |
|
PR_Github #58087 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #58160 [ run ] triggered by Bot. Commit: |
|
PR_Github #58160 [ run ] completed with state
|
brb-nv
left a comment
brb-nv
left a comment
There was a problem hiding this comment.
Approving with some questions.
|
/bot run --disable-fail-fast |
|
PR_Github #58355 [ run ] triggered by Bot. Commit: |
|
PR_Github #58355 [ run ] completed with state |
7 participants
Summary by CodeRabbit
Bug Fixes
New Features
Tests
Description
PR-6 from the DSv4 a7728aa+ split. This PR keeps the remaining coverage, CI metadata, and import-safety polish separate from the functional DSv4/runtime/router/MoE changes.
Changes included:
import tensorrt_llmremains GPU-free when no CUDA device is visible.Test Coverage
Passed:
test_reverted_disagg_gen_init_allocation_does_not_report_pending_statson GPU 0 using the modified worktree and freshly built bindings (1 passed in 1.12s).cmake --build cpp/build --config Release --parallel 16 --target bindingsfor the current worktree and B300 (sm_103).git diff --checkand strict changed-file conflict-marker checks.Not run:
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.