Conversation
|
/bot run |
|
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 (2)
Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review. WalkthroughDFlash validation now warns when disaggregated generation uses a cache transceiver backend. The configuration remains accepted. Tests cover supported backends, dictionary inputs, backend-less configurations, and non-DFlash decoding. ChangesDFlash disaggregation handling
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Feature Suggested reviewers: Merge Risk: ⚪ Minimal · up to The warning behavior correctly covers configured DFlash transceivers without changing accepted configurations or output correctness. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
PR_Github #74119 [ run ] triggered by Bot. Commit: |
|
PR_Github #74119 [ run ] completed with state
|
brnguyen2
added
the
api-compatible
DFlash builds the draft model's cross-attention context from target hidden states captured during prefill. A disaggregated generation server does not prefill: it receives the target KV cache through the transceiver, which does not carry the drafter's context. The generation server therefore drafts without the prompt, and its draft positions are based at zero rather than the request's true sequence positions. This path already runs: DFlashSpecMetadata.prepare assigns each disaggregated generation request its own drafter slot, and drafts are verified against the target, so output is unaffected. Only the acceptance rate suffers, silently. Log a warning from TorchLlmArgs.validate_speculative_config when DFlash is combined with a configured cache transceiver backend, so the cost is visible at startup. The configuration is not rejected; the predicate matches the existing disaggregation test used by validate_early_first_token_response. Add configuration tests covering all five transceiver backends in both object and dict form, plus cases asserting the warning is not emitted for DFlash without a transceiver backend or for disaggregated serving with other speculative algorithms. Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>
979eed5 to
0088128
Compare
|
/bot run |
|
PR_Github #74152 [ run ] triggered by Bot. Commit: |
|
PR_Github #74152 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #74182 [ run ] triggered by Bot. Commit: |
|
/bot run |
|
PR_Github #74217 [ run ] triggered by Bot. Commit: |
|
PR_Github #74182 [ run ] completed with state |
|
PR_Github #74217 [ run ] completed with state
|
2 participants
Description
DFlash builds the draft model's cross-attention context from target hidden states captured during prefill. A disaggregated generation server does not prefill: it receives the target KV cache through the cache transceiver, which does not carry the drafter's context. The generation server therefore drafts without the prompt, and its draft positions are based at zero rather than the request's true sequence positions.
This path already runs today:
DFlashSpecMetadata.prepareassigns each disaggregated generation request its own drafter slot, and drafts are verified against the target, so output is unaffected — only the acceptance rate suffers, silently.This PR logs a warning from
TorchLlmArgs.validate_speculative_configwhen DFlash is combined with a configured cache transceiver backend, so the cost is visible at startup. The configuration is not rejected; the predicate matches the existing disaggregation test used byvalidate_early_first_token_response.Test Coverage
tests/unittest/llmapi/test_llm_args.py(cpu_only): the warning fires for all five transceiver backends in both object and dict config form; it does not fire for DFlash without a transceiver backend, or for disaggregated serving with other speculative algorithms.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.
Dev Engineer Review
TorchLlmArgs.validate_speculative_confignow warns when DFlash uses a cache transceiver backend. The configuration remains valid, but draft acceptance may decrease because generation servers lack prompt context. Other speculative algorithms and configurations without a transceiver backend remain unaffected.QA Engineer Review
tests/unittest/llmapi/test_llm_args.pyadds parameterized coverage for five transceiver backends in object and dictionary forms. Tests cover warning, acceptance, and non-warning paths. No integration test-list change applies to this unit test file. Coverage is sufficient, but the downstreamL0_MergeRequest_PRpipeline failed and needs follow-up.Per-File QA Perspective
tensorrt_llm/llmapi/llm_args.py: Verify that the warning triggers only for DFlash with a cache transceiver backend. Verify that validation still accepts the configuration.tests/unittest/llmapi/test_llm_args.py: Covers supported backends, configuration representations, and regression paths. This unit test file does not require an integration test-list entry.