[Common] Default EP comm kernels to 32 SMs; keep shuffle/preprocess on all SMs - #3422
[Common] Default EP comm kernels to 32 SMs; keep shuffle/preprocess on all SMs#3422phu0ngng wants to merge 1 commit into
Conversation
…s on all SMs Signed-off-by: Phuong Nguyen <phuonguyen@nvidia.com>
|
/te-ci L1 |
Greptile SummaryThe PR changes the default NCCL EP dispatch/combine budget from automatic sizing to 32 SMs, clamped to the active device, while defaulting local shuffle and preprocessing kernels to all device SMs.
Confidence Score: 4/5The PR appears safe to merge, with one non-blocking diagnostic inconsistency that should be corrected. The implementation and public API documentation consistently apply the new 32-SM default, but invalid native configurations can still produce an error message claiming that zero enables automatic sizing. Files Needing Attention: transformer_engine/common/ep/ep_backend.cpp Important Files Changed
Reviews (1): Last reviewed commit: "[Common] Default EP comm kernels to 32 S..." | 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!
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: