[https://nvbugs/6079901][fix] Avoid divide-by-zero in KVCacheTransfer…#13618
Conversation
|
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 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
|
/bot run --disable-fail-fast |
|
PR_Github #46207 [ run ] triggered by Bot. Commit: |
|
PR_Github #46207 [ run ] completed with state
|
|
/bot help |
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. |
|
/bot run --disable-fail-fast |
|
PR_Github #47012 [ run ] triggered by Bot. Commit: |
|
PR_Github #47012 [ run ] completed with state
|
b948b0e to
a40cfce
Compare
|
/bot run |
|
PR_Github #48210 [ run ] triggered by Bot. Commit: |
|
PR_Github #48210 [ run ] completed with state
|
a40cfce to
3dc73e4
Compare
…Manager::computeBlockTransferBytes for empty pools The recent kv-cache statistics monitoring commit (580c7a6, "[TRTLLM-11421][feat] Support better kv cache statistics monitoring") derives bytesPerElement as getSizeInBytes()/getSize() inside KVCacheTransferManager::computeBlockTransferBytes. Pools whose primaryPtr is non-null but has zero elements (e.g. layers that contribute no KV cache in NAS-style heterogeneous models such as Llama-3_3-Nemotron-Super-49B-v1) trigger a SIGFPE / integer divide-by-zero on the first onboard transfer that touches such a pool. Skip pools with zero elements: they legitimately contribute zero transfer bytes, so the stats are unchanged and the divide-by-zero is avoided. Repro before fix: post-merge L0 stage RTXPro6000D-PyTorch-Post-Merge-1 (test_e2e.py::test_ptp_quickstart_advanced[Nemotron-Super-49B-v1-FP8-...]) crashes with "Floating point exception (8) / Integer divide-by-zero (1)" inside computeBlockTransferBytes during KVCacheManager::addSequenceBatch. After fix the test passes locally on RTX Pro 6000 (SM120) in 73s. Signed-off-by: Faraz Khoubsirat <[email protected]>
3dc73e4 to
48a9ea9
Compare
|
/bot run |
|
PR_Github #50065 [ run ] triggered by Bot. Commit: |
|
PR_Github #50065 [ run ] completed with state |
…Manager::computeBlockTransferBytes for empty pools
The recent kv-cache statistics monitoring commit (580c7a6, "[TRTLLM-11421][feat] Support better kv cache statistics monitoring") derives bytesPerElement as getSizeInBytes()/getSize() inside KVCacheTransferManager::computeBlockTransferBytes. Pools whose primaryPtr is non-null but has zero elements (e.g. layers that contribute no KV cache in NAS-style heterogeneous models such as Llama-3_3-Nemotron-Super-49B-v1) trigger a SIGFPE / integer divide-by-zero on the first onboard transfer that touches such a pool.
Skip pools with zero elements: they legitimately contribute zero transfer bytes, so the stats are unchanged and the divide-by-zero is avoided.
Repro before fix: post-merge L0 stage RTXPro6000D-PyTorch-Post-Merge-1 (test_e2e.py::test_ptp_quickstart_advanced[Nemotron-Super-49B-v1-FP8-...]) crashes with "Floating point exception (8) / Integer divide-by-zero (1)" inside computeBlockTransferBytes during KVCacheManager::addSequenceBatch. After fix the test passes locally on RTX Pro 6000 (SM120) in 73s.
Summary by CodeRabbit
Description
Test Coverage
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)
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.