Skip to content

fix(dist): preserve new_group options across reload#1495

Open
EazyReal wants to merge 2 commits into
radixark:mainfrom
EazyReal:upstream-pr/reloadable-process-group-options
Open

fix(dist): preserve new_group options across reload#1495
EazyReal wants to merge 2 commits into
radixark:mainfrom
EazyReal:upstream-pr/reloadable-process-group-options

Conversation

@EazyReal

Copy link
Copy Markdown

Port of THUDM/slime#2095 for miles.

Summary:

  • capture original torch.distributed.new_group positional args and kwargs
  • replay that exact constructor contract when reloadable process groups are rebuilt
  • preserve timeout, pg_options, and future new_group options instead of hardcoding {ranks, backend=nccl}

Validation:

  • uv run --with pytest --with torch --with psutil --with numpy pytest --confcutdir=tests/fast/utils tests/fast/utils/test_reloadable_process_group.py -q -> 1 passed

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the process group reloading mechanism to preserve the original arguments and keyword arguments (such as timeout and pg_options) when recreating process groups. It extracts helper functions for identifying Gloo groups and retrieving group ranks, and adds a comprehensive unit test suite. The review feedback suggests two improvements to prevent potential TypeErrors: checking if old_new_group is None before calling it, and handling cases where group_args might be explicitly passed as None.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread miles/utils/reloadable_process_group.py
Comment thread miles/utils/reloadable_process_group.py Outdated
@EazyReal EazyReal force-pushed the upstream-pr/reloadable-process-group-options branch from 65d39dd to d07f362 Compare June 30, 2026 08:38
@EazyReal

Copy link
Copy Markdown
Author

@fzyzcjy @yueming-yuan @guapisolo @maocheng23 @jybsuper @Zhichenzzz @Shi-Dong Could I get a quick review on the PR? This preserves new_group args/kwargs across reloadable process-group reloads. That avoids silently dropping backend/timeout semantics etc.

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