Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions .github/workflows/claude_review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,13 @@ jobs:
- If the PR adds a new feature or significant functionality without corresponding tests, suggest adding tests
- If the PR fixes a bug that was not caught by an existing unit test, suggest adding a regression test to prevent recurrence
- Outdated or inaccurate documentation affected by the changes
- New direct global process group access in `megatron/core` production code
- Flag added calls to `parallel_state.get_*_group()` or directly imported
`get_*_group()` helpers unless they are in `parallel_state.py`,
`process_groups_config.py`, initialization/bootstrap code that materializes a
`ProcessGroupCollection`, tests, docs, or an explicitly documented migration fallback
- Prefer passing a `ProcessGroupCollection` or explicit
`torch.distributed.ProcessGroup` from the caller

Do NOT comment on:
- Style preferences or formatting
Expand Down Expand Up @@ -212,6 +219,18 @@ jobs:
- **API contract changes**: Changed function signatures, return types, or side effects in megatron/core/ without backward-compat shim
- **Model architecture changes**: Altered layer ordering, initialization, or normalization placement — existing pretrained weights become incompatible

### Megatron Core Process Group Usage
- In `megatron/core` production code, treat new direct reads of global process groups
from `parallel_state` as review findings unless they are clearly compatibility-only.
- Flag added calls to `parallel_state.get_*_group()` or directly imported
`get_*_group()` helpers when the surrounding code could instead receive a
`ProcessGroupCollection` or explicit `torch.distributed.ProcessGroup` from its caller.
- Do not flag `megatron/core/parallel_state.py`, `megatron/core/process_groups_config.py`,
tests, docs, initialization/bootstrap code that materializes a `ProcessGroupCollection`
from MPU globals, or explicitly documented migration fallbacks.
- This guidance is advisory and targets Megatron Core library code; do not apply it to
`megatron/training` or other training-loop code unless the PR opts into that migration.

### Mandatory Check: Unused New Variables / Arguments
- For each changed file, list newly added identifiers (function args, config fields, locals).
- Verify each has a meaningful read/use path — not just declaration/docstring or discard assignment (_ = new_arg).
Expand Down
19 changes: 19 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,22 @@ skill keyword — infer it from the artifact you read.
### Code Quality

- After editing imports in any Python files, always run `uv run isort` on those files to fix import order before committing.

### Megatron Core Process Groups

- In `megatron/core` production code, avoid adding new direct reads of global
process groups from `parallel_state` (for example,
`parallel_state.get_tensor_model_parallel_group()` or directly imported
`get_*_group()` helpers). Prefer accepting a `ProcessGroupCollection` or an
explicit `torch.distributed.ProcessGroup` from the caller and passing that
through.
- Allowed compatibility points include `megatron/core/parallel_state.py`,
`megatron/core/process_groups_config.py`, initialization/bootstrap code that
materializes a `ProcessGroupCollection` from MPU globals, tests, docs, and
migration fallbacks with an explicit comment.
- This guidance targets Megatron Core library code. Do not apply it to
`megatron/training` or other training-loop code unless the PR explicitly
opts into that migration.
- In reviews, flag new direct `parallel_state.get_*_group()` usage in
`megatron/core` unless it is one of the compatibility points above. This is
advisory guidance, not a CI gate.
Loading