-
-
Notifications
You must be signed in to change notification settings - Fork 11.8k
[Bugfix] EPLB load statistics problem #22167
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
Conversation
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.
Code Review
The pull request addresses a bug in the expert load balancing (EPLB) mechanism for MoE kernels. The change to count the load for all physical experts activated by each token aims to make the statistics independent of dispatch timing. The shape adjustments in vllm/distributed/eplb/eplb_state.py and changes in vllm/model_executor/layers/fused_moe/layer.py seem correct. However, there's a critical concern regarding the normalization of the expert load in vllm/distributed/eplb/eplb_state.py that might affect the "naive dispatch" code path.
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add 🚀 |
|
@abmfy please review, thanks |
abmfy
left a comment
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.
Overall LGTM.
Could you please confirm if my understanding aligns with yours here:
- Under naive all-to-all dispatch settings (i.e., performing DP without using dedicated kernels that handle DP dispatch, such as DeepEP high-throughput mode), each DP rank will contribute the same token set to the expert load. As a result, the recorded expert load will be multiplied by
dp_size(this also applies in TP+DP settings).
If this is correct, could you please add it to the comments ofexpert_load_windowas aNOTE:? Since we plan to expose expert load metrics through interfaces, this clarification will help us divide the metrics bydp_sizelater, ensuring that the reported figures have a clear meaning. - When using communication kernels such as DeepEP or
pplx-kernels, which handle dispatch and combine within theprepareandfinalizemethods of the modular kernel, the case should be the same as above because the expert load is collected before the modular kernel is invoked.
Also, could you please add some additional tests (manual tests are fine) to verify correctness under different settings, such as TP and DP+TP? It would also be great to confirm that under these settings, the balancedness after rearrangement remains as good as in the previous implementation.
Thanks so much for the contribution!
Thanks for the review! Yes, we have the same understanding. I will add the test and note soon. |
|
I conducted several manual test locally, the result looks correct. Just divide the metrics by Here are brief parameters and results: sampling_params = SamplingParams(
temperature=0.8, top_p=0.95, max_tokens=[16, 20][global_dp_rank % 2]
)
llm = LLM(
model=model,
tensor_parallel_size=GPUs_per_dp_rank,
enforce_eager=enforce_eager,
enable_expert_parallel=True,
enable_eplb=True,
eplb_window_size=1000,
eplb_step_interval=100,
trust_remote_code=trust_remote_code,
max_num_seqs=max_num_seqs,
gpu_memory_utilization=gpu_memory_utilization,
)
Also output the proportion between expert loads ( if not is_profile:
layer_idx = 0
loads = global_expert_load_window[layer_idx]
loads = loads.float()
ratios = loads / loads[0]
print(f"Expert load ratios (relative to expert 0) for layer {layer_idx}:")
print([f"expert_{i}: {ratio:.3f}" for i, ratio in enumerate(ratios.tolist())])The output of expert load ratios, which looks consistent: |
|
@abmfy Notes and tests have been added, please review, thanks! |
|
@DarkLight1337 @hmellor PTAL, thank you. |
|
This pull request has merge conflicts that must be resolved before it can be |
Signed-off-by: ycyaw66 <[email protected]>
Signed-off-by: David Chen <[email protected]>
f1132da to
afcbb12
Compare
|
Thanks for tracking this down 🙏 |
Signed-off-by: ycyaw66 <[email protected]> Signed-off-by: David Chen <[email protected]> Co-authored-by: ycyaw66 <[email protected]> Signed-off-by: Jinzhen Lin <[email protected]>
Signed-off-by: ycyaw66 <[email protected]> Signed-off-by: David Chen <[email protected]> Co-authored-by: ycyaw66 <[email protected]> Signed-off-by: Noam Gat <[email protected]>
Signed-off-by: ycyaw66 <[email protected]> Signed-off-by: David Chen <[email protected]> Co-authored-by: ycyaw66 <[email protected]> Signed-off-by: Paul Pak <[email protected]>
Signed-off-by: ycyaw66 <[email protected]> Signed-off-by: David Chen <[email protected]> Co-authored-by: ycyaw66 <[email protected]> Signed-off-by: Diego-Castan <[email protected]>
Signed-off-by: ycyaw66 <[email protected]> Signed-off-by: David Chen <[email protected]> Co-authored-by: ycyaw66 <[email protected]>
Signed-off-by: ycyaw66 <[email protected]> Signed-off-by: David Chen <[email protected]> Co-authored-by: ycyaw66 <[email protected]> Signed-off-by: Xiao Yu <[email protected]>
Signed-off-by: ycyaw66 <[email protected]> Signed-off-by: David Chen <[email protected]> Co-authored-by: ycyaw66 <[email protected]>
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.Purpose
Fix the bug of #21883.
When using naive dispatch, there will be no problem with the statistics of expert workload. However, when using the pplx-kernel or DeepEP, which follow a different code path, the dispatch might not have been executed yet at that point. The load of experts routed to other nodes is not counted.
Previously, the logic for calculating expert load was to only count the load of activated experts by the current rank and aggregate the global load in
eplc_state.py. The logic for modifying expert statistics in this PR is to directly count the load of all physical experts activated by each token.After this modification, the time point of dispatch is no longer important, and load statistics can work normally when activating the pplx-kernel or DeepEP. When using naive dispatch, due to the modification of logic, each token's activated expert will be counted multiple times. However, since the load between experts is still proportional, this modification will not affect the results of the EPLB algorithm.
Test Plan
Output the sum of expert load in the function
rearrange()ineplb_state.pyafter expert load calculation:Test with/wo pplx:
VLLM_ALL2ALL_BACKEND=pplx CUDA_VISIBLE_DEVICES=0,1 python examples/offline_inference/data_parallel.py \ --model="/workspace/models/DeepSeek-V2-Lite" \ --trust-remote-code \ --dp-size=2 \ --tp-size=1CUDA_VISIBLE_DEVICES=0,1 python examples/offline_inference/data_parallel.py \ --model="/workspace/models/DeepSeek-V2-Lite" \ --trust-remote-code \ --dp-size=2 \ --tp-size=1Test Result
with pplx:
wo pplx, twice as much as using pplx when
dp=2andtp=1(because each token's activated expert count twice) , which is expected:(Optional) Documentation Update