Skip to content

Fail fast when receiver topology is empty - #104

Closed
koriyoshi2041 wants to merge 1 commit into
MoonshotAI:mainfrom
koriyoshi2041:fix-empty-receiver-topology
Closed

Fail fast when receiver topology is empty#104
koriyoshi2041 wants to merge 1 commit into
MoonshotAI:mainfrom
koriyoshi2041:fix-empty-receiver-topology

Conversation

@koriyoshi2041

Copy link
Copy Markdown

Summary

  • fail fast when receiver assignment has non-empty buckets but no local receiver ranks
  • add a CPU regression for the zero-receiver topology

Problem

_gen_h2d_buckets() intersects targeted ranks with each participant's local topology before calling _assign_receiver_ranks(). A participant with no selected local rank can therefore reach receiver assignment with non-empty global buckets and an empty local topology.

In that state, receiver_list is empty while flattened_buckets is not. The outer assignment loop never advances assigned_cnt, so the update hangs indefinitely.

An isolated CPU reproduction calling _assign_receiver_ranks([(7, "bucket")], {}, {"mlx5_0": {7}}) remained alive after 750 ms and had to be terminated.

Fix

Raise a clear ValueError before entering the assignment loop when no local receiver can be selected. Returning an empty assignment would risk turning the collective mismatch into a false-success update.

Testing

pytest tests/test_assign_receiver_ranks.py -q
7 passed in 0.93s

ruff check checkpoint_engine/ps.py tests/test_assign_receiver_ranks.py
All checks passed!

ruff format --check checkpoint_engine/ps.py tests/test_assign_receiver_ranks.py
2 files already formatted

git diff --check

@koriyoshi2041

Copy link
Copy Markdown
Author

Closing after a deeper reachability audit. The update path returns non-target ranks before bucket generation, while gathered local topology includes each valid world rank, so an empty selected topology is not reachable for a valid non-empty target list. The helper can loop on a malformed direct input, but that does not justify the core-path claim in this PR. Thanks for the review bandwidth.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant