[https://nvbugs/6433376][fix] Update the Dense test to mirror the MoE sibling — assert bfloat16 under… - #16203
Conversation
…h narrowed resolver PR NVIDIA#15249 added test_qwen35_dense_vl_resolves_mamba_ssm_cache_dtype asserting the old resolve_ssm_cache_dtype semantics that honored mamba_ssm_dtype as a cache-dtype fallback. PR NVIDIA#16065 (nvbugs/6422332) narrowed the resolver to honor only the explicit mamba_ssm_cache_dtype field for performance (fp32 SSM cache disables the FlashInfer bf16-state GDN decode kernel and doubles state memory traffic). The MoE sibling test was updated to match, but the dense test was missed. Mirror the MoE sibling: assert the "auto" path returns bfloat16 (weight dtype fallback) and add an explicit float32 opt-in assertion. Remove waives.txt entries added for this bug. Signed-off-by: trtllm-agent <296075020+trtllm-agent@users.noreply.github.com>
📝 WalkthroughWalkthroughThis PR updates the expected dtype for Mamba SSM KV-cache resolution in a Qwen3.5 dense VL unit test, changing "auto" resolution from float32 to bfloat16 and adding coverage for explicit float32 opt-in. Two corresponding test waiver entries are removed from the waivers file. ChangesMamba SSM Cache Dtype Test Update
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/unittest/_torch/modeling/test_modeling_qwen3_5_vl.py (1)
160-163: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winOpt-in coverage doesn't verify downstream propagation.
The "auto" case verifies both
quant_config.mamba_ssm_cache_dtypeand the downstreammamba_params.mamba_ssm_cache_dtypefromextract_mamba_kv_cache_params. The new float32 opt-in case only checksquant_config, leaving the downstream propagation path (which readsquant_config.mamba_ssm_cache_dtypefirst, perconfig_utils.py'sextract_mamba_kv_cache_params) unverified for the explicit opt-in.🧪 Suggested addition to close the coverage gap
opt_in_config = ModelConfig(pretrained_config=config) validate_and_set_mamba_ssm_cache_dtype(opt_in_config, "float32") assert opt_in_config.quant_config.mamba_ssm_cache_dtype is torch.float32 + + opt_in_mamba_params = extract_mamba_kv_cache_params( + config.text_config, + quant_config=opt_in_config.quant_config, + ) + assert opt_in_mamba_params.mamba_ssm_cache_dtype is torch.float32As per path instructions, coverage here is largely sufficient but this asymmetry between the auto/opt-in paths is a minor gap worth closing to mirror the auto-path assertions.
🤖 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/unittest/_torch/modeling/test_modeling_qwen3_5_vl.py` around lines 160 - 163, The explicit float32 opt-in test in the Qwen3_5 VL modeling suite only checks `quant_config.mamba_ssm_cache_dtype` and misses the downstream propagation verified in the auto path. Update the `test_modeling_qwen3_5_vl` case around `validate_and_set_mamba_ssm_cache_dtype` to also exercise `extract_mamba_kv_cache_params` on `opt_in_config` and assert the resulting `mamba_params.mamba_ssm_cache_dtype` is `torch.float32`, matching the coverage pattern used by the auto-case.Source: Path instructions
🤖 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.
Nitpick comments:
In `@tests/unittest/_torch/modeling/test_modeling_qwen3_5_vl.py`:
- Around line 160-163: The explicit float32 opt-in test in the Qwen3_5 VL
modeling suite only checks `quant_config.mamba_ssm_cache_dtype` and misses the
downstream propagation verified in the auto path. Update the
`test_modeling_qwen3_5_vl` case around `validate_and_set_mamba_ssm_cache_dtype`
to also exercise `extract_mamba_kv_cache_params` on `opt_in_config` and assert
the resulting `mamba_params.mamba_ssm_cache_dtype` is `torch.float32`, matching
the coverage pattern used by the auto-case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c8fc2534-78f3-4195-bb49-bf432aefa2ca
📒 Files selected for processing (2)
tests/integration/test_lists/waives.txttests/unittest/_torch/modeling/test_modeling_qwen3_5_vl.py
💤 Files with no reviewable changes (1)
- tests/integration/test_lists/waives.txt
|
/bot run --disable-fail-fast |
|
PR_Github #61250 [ run ] triggered by Bot. Commit: |
|
PR_Github #61250 [ run ] completed with state |
|
/bot run |
|
PR_Github #61800 [ run ] triggered by Bot. Commit: |
|
PR_Github #61800 [ run ] completed with state |
…nnahz/dep-1083-port-flashinfer-stable-va-lifecycle-for-native-all-reduce * 'main' of https://github.com/NVIDIA/TensorRT-LLM: (54 commits) [NVIDIA#15673][fix] Enable CUDA core fast path for SM89/SM120/SM121 (NVIDIA#12705) [None][test] Adjust timeout cases in QA perf test (NVIDIA#16894) [https://nvbugs/6157892][fix] Mistral format refactor (NVIDIA#15123) [None][feat] Add kimi_k2/glm_5 grouped routing and fused router to bench_moe (NVIDIA#16830) [https://nvbugs/6501376][fix] Test-only fix — drop the `if hidden_size % 2 != 0: with pytest.raises(...)`… (NVIDIA#16844) [TRTLLM-13642][feat] Add perf sanity tests for Llama-3.1-8B and Gemma-3-1B and verify cache transceiver V2 support (NVIDIA#16355) [https://nvbugs/6433376][fix] Update the Dense test to mirror the MoE sibling — assert `bfloat16` under… (NVIDIA#16203) [None][fix] Resolve NVFP4 mixed-precision base layers for the DSpark draft (NVIDIA#16831) [https://nvbugs/6479324][test] Remove waiver for fixed qwen3_5_4b_fp8_stress disaggregated stress test (NVIDIA#16878) [https://nvbugs/6507109][infra] Split slow DGX B300 attention unit tests (NVIDIA#16838) [None][infra] Waive 21 failed cases for main in post-merge 2862 (NVIDIA#16882) [None][perf] prepare_inputs: avoid O(seq_len) get_tokens(0) marshalling on the host (NVIDIA#16791) [None][perf] Optimize Blackwell fused MHC half-MMA kernel (NVIDIA#16799) [None][infra] Auto-update test durations from OpenSearch (last 7 days) [None][perf] Skip DeepGEMM clean_logits in DSA indexer prefill on custom top-k path (NVIDIA#16789) [None][feat] Support DeepSeek-V4 in layer_wise_benchmarks (NVIDIA#16774) [https://nvbugs/6465993][fix] use attention cache dtype for disaggregated transfer (NVIDIA#16505) [https://nvbugs/6463822][fix] Fix LTX2 CUDA graph test leak issue (NVIDIA#16775) [https://nvbugs/5948435][chore] Unwaive DeepSeekV3Lite test_nvfp4_4gpus CUTLASS ep4 fp8kv on RTXPro6000D (NVIDIA#16621) [TRTLLM-14417][fix] Exclude ADP/cuda-graph dummy requests from speculative-decode acceptance stats (NVIDIA#16571) ... Signed-off-by: Hannah Zhang <hannahz@nvidia.com>
…nnahz/dep-1082-shared-mnnvl-moe-lifecycle * 'main' of https://github.com/NVIDIA/TensorRT-LLM: (142 commits) [NVIDIA#15673][fix] Enable CUDA core fast path for SM89/SM120/SM121 (NVIDIA#12705) [None][test] Adjust timeout cases in QA perf test (NVIDIA#16894) [https://nvbugs/6157892][fix] Mistral format refactor (NVIDIA#15123) [None][feat] Add kimi_k2/glm_5 grouped routing and fused router to bench_moe (NVIDIA#16830) [https://nvbugs/6501376][fix] Test-only fix — drop the `if hidden_size % 2 != 0: with pytest.raises(...)`… (NVIDIA#16844) [TRTLLM-13642][feat] Add perf sanity tests for Llama-3.1-8B and Gemma-3-1B and verify cache transceiver V2 support (NVIDIA#16355) [https://nvbugs/6433376][fix] Update the Dense test to mirror the MoE sibling — assert `bfloat16` under… (NVIDIA#16203) [None][fix] Resolve NVFP4 mixed-precision base layers for the DSpark draft (NVIDIA#16831) [https://nvbugs/6479324][test] Remove waiver for fixed qwen3_5_4b_fp8_stress disaggregated stress test (NVIDIA#16878) [https://nvbugs/6507109][infra] Split slow DGX B300 attention unit tests (NVIDIA#16838) [None][infra] Waive 21 failed cases for main in post-merge 2862 (NVIDIA#16882) [None][perf] prepare_inputs: avoid O(seq_len) get_tokens(0) marshalling on the host (NVIDIA#16791) [None][perf] Optimize Blackwell fused MHC half-MMA kernel (NVIDIA#16799) [None][infra] Auto-update test durations from OpenSearch (last 7 days) [None][perf] Skip DeepGEMM clean_logits in DSA indexer prefill on custom top-k path (NVIDIA#16789) [None][feat] Support DeepSeek-V4 in layer_wise_benchmarks (NVIDIA#16774) [https://nvbugs/6465993][fix] use attention cache dtype for disaggregated transfer (NVIDIA#16505) [https://nvbugs/6463822][fix] Fix LTX2 CUDA graph test leak issue (NVIDIA#16775) [https://nvbugs/5948435][chore] Unwaive DeepSeekV3Lite test_nvfp4_4gpus CUTLASS ep4 fp8kv on RTXPro6000D (NVIDIA#16621) [TRTLLM-14417][fix] Exclude ADP/cuda-graph dummy requests from speculative-decode acceptance stats (NVIDIA#16571) ... Signed-off-by: Hannah Zhang <hannahz@nvidia.com>
Summary
bfloat16under"auto", add explicit"float32"opt-in check; remove bothwaives.txtentries.Test plan
Links
Summary by CodeRabbit