[Common] Default EP comm kernels to 32 SMs; keep shuffle/preprocess on all SMs - #3422
Conversation
…s on all SMs Signed-off-by: Phuong Nguyen <phuonguyen@nvidia.com>
|
/te-ci L1 |
Greptile SummaryThe PR changes the default NCCL EP communication-kernel budget to 32 SMs, clamped to the device SM count.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains. No blocking failure remains. Important Files Changed
Reviews (2): Last reviewed commit: "Merge branch 'main' into ep-sm-defaults" | Re-trigger Greptile |
| cfg.max_num_sms = group_config.num_comm_sms > 0 | ||
| ? static_cast<unsigned int>(group_config.num_comm_sms) | ||
| : NCCL_EP_AUTO; | ||
| : static_cast<unsigned int>(std::min(kDefaultCommSms, device_sms)); |
There was a problem hiding this comment.
The implementation now maps num_comm_sms == 0 to min(32, device_sms), but the native validation message still describes zero as automatic sizing, misleading users who are diagnosing or selecting the communication-kernel SM budget.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
jberchtold-nvidia
left a comment
There was a problem hiding this comment.
LGTM, thanks!
Description
Set a sane default SM budget for EP dispatch/combine (32 SMs) while leaving the local shuffle/preprocess kernels on all device SMs.
Type of change
Checklist: