Give gloo process groups a home in ProcessGroupCollection - #6259
Draft
Connor-XY wants to merge 1 commit into
Draft
Give gloo process groups a home in ProcessGroupCollection#6259Connor-XY wants to merge 1 commit into
Connor-XY wants to merge 1 commit into
Conversation
Proposed resolution for D3, scoped to the gloo groups the distributed optimizer needs. Currently setup_process_groups_for_optimizer *raises* when a collection is supplied and use_gloo_process_groups is true, so explicit process-group passing and gloo groups are mutually exclusive -- and the optimizer path is on every training run. Verified: hoisting the optimizer fallback without this fails 7/95 against a pristine 102/0, every failure that ValueError. - add check_initialized to get_data_parallel_group_gloo and get_expert_data_parallel_group_gloo, matching every other accessor, so a collection can be materialised in create_gloo_process_groups=False jobs - add intra_dp_cp_gloo / intra_expt_dp_gloo to ProcessGroupCollection and populate them in use_mpu_process_groups - read them in setup_process_groups_for_optimizer instead of refusing With that, the optimizer fallback hoists to the edge as NVIDIA#6099 asks: three resolution sites inside private helpers become one at get_megatron_optimizer. Global process-group reads in megatron/core: 112 -> 110. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Yan Xu <yxu1@nvidia.com>
Contributor
Author
|
Part of #6307 — tracking issue for the |
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.
What does this PR do?
Today, explicit process groups and gloo process groups are mutually exclusive — and the distributed optimizer is on every training run.
ProcessGroupCollection.setup_process_groups_for_optimizerbranches onpg_collection is None:Nonebranch builds gloo groups fromparallel_state.get_data_parallel_group_gloo()/get_expert_data_parallel_group_gloo()elsebranch raises (process_groups_config.py):So any job passing an explicit collection cannot use gloo groups at all. This blocks the
parallel_statemigration for the optimizer path.Root cause
get_data_parallel_group_glooandget_expert_data_parallel_group_gloowere the only accessors inuse_mpu_process_groups's mapping without acheck_initializedparameter, so they assert rather than returningNonewhen gloo groups were never created (initialize_model_parallel(create_gloo_process_groups=False)). A collection therefore could not carry them.Changes
check_initializedto both gloo accessors, matching every other accessor in that mapping.intra_dp_cp_gloo/intra_expt_dp_glootoProcessGroupCollection, named for their existingintra_dp_cp/intra_expt_dpsiblings, and populate them inuse_mpu_process_groups.setup_process_groups_for_optimizerreads them instead of refusing. Absent →None(the legitimate no-gloo case); present → passed through.get_megatron_optimizer.That last part is the shape proposed in #6099: "move all global-process-group fallbacks to the edge, around
get_megatron_optimizer, and make all internal code require explicit process groups."Test change worth reviewing
test_optimizer.py"Test 6" asserted the old refusal (pytest.raises(ValueError, match="Gloo process groups are not supported")). It now asserts the new contract: a collection without gloo groups yieldsNone; one carrying them passes them through.Verification
Paired runs on 4×GPU against unmodified
mainat the same commit, same container:mainSuites:
test_optimizer.py,test_emerging_optimizers.py,test_process_groups_config.py.Scope
This covers only the gloo groups the optimizer needs. Virtual-pipeline rank/size,
GlobalMemoryBufferand NCCL options are not process groups and still have no home inProcessGroupCollection; they need a separate design.Contribution process
Pre-checks