[None][test] Assert MTP acceptance length in ADP + LM-head-TP accuracy tests#16521
[None][test] Assert MTP acceptance length in ADP + LM-head-TP accuracy tests#16521qiaoxj07 wants to merge 1 commit into
Conversation
…y tests Wrong MTP draft tokens are invisible to accuracy scores: the target model rejects and regenerates them, so GSM8K/MMLU stay intact and the only observable symptom is a collapsed acceptance length (~1.0). This is how https://nvbugs/6460072 (wrong draft argmax in the ADP + LM-head-TP greedy path) shipped through green CI. Enable iteration stats for the latency_adp_lmtp, latency_trtllmgen_adp_lmtp and latency_adp_lmtp_tp4 parametrizations of TestDeepSeekR1::test_nvfp4_multi_gpus and assert the mean acceptance length over the profiled iterations stays >= 1.5 (healthy MTP3 is ~2.5, a broken draft path gives ~1.0). Signed-off-by: Xianjie <5410381+qiaoxj07@users.noreply.github.com>
|
Merge-order dependency: #16440 must land first. The motivating bug is still live on |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe integration test now conditionally enables iteration statistics and validates MTP speculative-decoding acceptance length for configurations using MTP with LM-head tensor parallelism in ADP. ChangesMTP acceptance validation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
| # stays intact and the only observable symptom is a collapsed | ||
| # acceptance length (https://nvbugs/6460072). Assert on it for the | ||
| # ADP + LM-head-TP cases, whose draft sampling takes a dedicated path. | ||
| check_acceptance = mtp_nextn > 0 and enable_lm_head_tp_in_adp |
There was a problem hiding this comment.
@mikeiovine for viz. @qiaoxj07 contributed one such AL protection test. Is it suitable for test suite? I'm not sure if this is connected to any CI testing system already.
BowenFu
left a comment
There was a problem hiding this comment.
LGTM — test-only; the MTP acceptance-length assertion and iter-perf-stats are scoped to the 3 ADP+LM-head-TP parametrizations, no behavior change elsewhere.
Background
https://nvbugs/6460072 (fix in #16440): on main, the ADP +
enable_lm_head_tp_in_adp+ greedy draft-sampling path produced wrong MTP draft tokens (per-rank argmax on the wrong all-gather rows / a local vocab shard).The bug shipped through green CI even though three parametrizations of
TestDeepSeekR1::test_nvfp4_multi_gpus(latency_adp_lmtp,latency_trtllmgen_adp_lmtp,latency_adp_lmtp_tp4) exercise exactly this configuration — because accuracy scores cannot see this bug class: rejected drafts are regenerated by the target model, so outputs (and GSM8K/MMLU scores) stay correct and the only observable symptom is the acceptance length collapsing to ~1.0, i.e. a pure perf regression. In one production benchmark this manifested as a hidden +13% gen-only iteration-time regression (force-accepted wrong drafts degrading MoE routing balance).Summary
Add an acceptance-length guard to the three ADP + LM-head-TP parametrizations:
enable_iter_perf_statsfor those cases only (all other parametrizations keep the defaultFalse— zero behavior change),llm.get_stats()and assert the meanspecDecodingStats.acceptanceLengthover the retained iterations is>= 1.5.Healthy DeepSeek-R1 + MTP3 greedy acceptance length is ~2.5–3.0; a broken draft path gives ~1.0, so the 1.5 threshold has wide margins on both sides. The stats-reading pattern follows the existing
test_eagle3_spec_decoding_statsunit test.With this assertion, a regression of the same class as https://nvbugs/6460072 fails these tests instead of silently passing.
Impact
Test-only change. No runtime behavior change for any other parametrization.
Related PRs
use_lm_head_tp_in_adp and is_all_greedy_sample, keep… #16440 (the fix for the motivating bug)Summary by CodeRabbit