Conversation
When packing physical experts to GPUs, the `balanced_packing` function previously only considered load balancing, which could result in multiple physical replicas of the same logical expert being placed on the same GPU. This is inefficient and defeats the purpose of expert replication. Changes: - Add optional `logical_ids` parameter to `balanced_packing` function - When packing, prefer packs that don't already contain the same logical ID - Pass `phy2mlog` (physical-to-logical mapping) to balanced_packing in Step 3 - Add comprehensive test suite with regression test for issue deepseek-ai#22 The fix maintains backward compatibility - existing code without the logical_ids parameter continues to work as before. Fixes deepseek-ai#22 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes a bug where the expert packing algorithm could place multiple physical replicas of the same logical expert on the same GPU, which defeats the purpose of expert replication for load balancing.
Problem
As reported in #22, when distributing 256 logical experts + 32 redundant experts across 32 GPUs, the algorithm could produce mappings like:
where expert 236 appears twice on the same GPU.
Root Cause
The
balanced_packingfunction in Step 3 only considered load balancing when packing physical experts to GPUs. It did not check whether multiple physical experts mapping to the same logical expert would end up on the same GPU.Solution
logical_idsparameter tobalanced_packingfunctionphy2mlogmapping tobalanced_packingin Step 3 ofrebalance_experts_hierarchicalThe fix is backward compatible - existing code without the
logical_idsparameter continues to work.Changes
eplb.py: Modifiedbalanced_packingto accept and use logical ID constraintstest_eplb.py: Added comprehensive test suite including:Test Results
Test Plan
Fixes #22
🤖 Generated with Claude Code