Repository navigation
Make the global process-group usage check report-only - #7986
Merged
Merged
Conversation
The check added for the parallel_state migration blocks merges through two paths: its own workflow step, and TestCommittedBaseline, which compares the real megatron/core tree with the allowlist and runs in the required unit-test job. That makes every new read of parallel_state, including a fallback for callers that do not pass a ProcessGroupCollection, a hard error before there is agreement on whether such fallbacks are allowed. The repository's agent guidance already treats this as advisory, not a CI gate. Keep the checker and its stdlib tests. List new reads in the workflow log without failing the job, and drop the test that compares the tree with the allowlist. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Yan Xu <yxu1@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
chtruong814
approved these changes
Oct 8, 2026
jaredcasper
enabled auto-merge
October 8, 2026 18:59
Contributor
Author
|
/ok to test 6a5c2f1 |
7 tasks
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/37965685307 |
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/37965726558 |
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/37965761935 |
This branch was previously deployed
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.
Why. #6258 added a ratchet against new reads of global process-group state in
megatron/core. It fails CI in two places: the workflow's checker step, which is not a required check, andTestCommittedBaseline.test_matches_current_tree, which compares the real tree with the allowlist. The H100 catch-all unit-test bucket collects that test, and the bucket feeds the requiredNemo_CICD_Test, so the ratchet is in effect a merge gate. It rejects any new read, including apg_collection=Nonefallback toparallel_statefor callers that don't pass aProcessGroupCollection. It also rejects refactors that move a grandfathered read, or remove one without refreshing the allowlist. Whether new code may add such fallbacks hasn't been agreed yet. The repository's agent guidance (CLAUDE.md, "Megatron Core Process Groups") also treats the rule as advisory rather than a CI gate. This PR makes the check report-only until the policy is settled in #6307.What.
continue-on-error: true. New and stale entries are still listed in the job log, but the job passes.TestCommittedBaselineis removed. The remaining 29 tests run the checker on synthetic sources in temporary directories and don't depend on the tree.--update/--statsare unchanged.Tests.
megatron/core, and (b) removing a grandfathered read without running--update. In both, the test file from main fails and this version passes. The checker still reports both cases and exits 1.Related to #6307.
🤖 Generated with Claude Code