-
-
Notifications
You must be signed in to change notification settings - Fork 15.6k
Support nccl fp8 communication #32760
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,7 +37,8 @@ def __init__( | |
| use_overlapped | ||
| and not ( | ||
| (self.enable_eplb and backend != "allgather_reducescatter") | ||
| or (self.moe_config.use_flashinfer_cutlass_kernels and self.dp_size > 1) | ||
| # TODO: Is this correct? | ||
| or self.moe_parallel_config.use_fi_all2allv_kernels | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed DP check changes overlap disabling behaviorMedium Severity The condition for disabling shared expert overlap changed from checking |
||
| ) | ||
| and self._shared_experts is not None | ||
| ) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
TODO: Is this correct?comment indicates uncertainty about the logic for disabling shared expert overlap whenuse_fi_all2allv_kernelsis true. If this condition is incorrect, it could lead to improper disabling of shared expert overlap, potentially impacting performance or correctness. Please verify this logic and either remove theTODOcomment with a clarifying explanation or correct the condition if it's found to be wrong.