Skip to content

[training migration] Migrate GPT builder - #4741

Merged
maanug-nv merged 18 commits into
NVIDIA:mainfrom
maanug-nv:migrate-gpt-builder
Jun 1, 2026
Merged

[training migration] Migrate GPT builder#4741
maanug-nv merged 18 commits into
NVIDIA:mainfrom
maanug-nv:migrate-gpt-builder

Conversation

@maanug-nv

Copy link
Copy Markdown
Contributor

What does this PR do ?

Follow-up to #4550 for GPT Model.

⚠️ For major changes (either in lines of code or in its impact), please make sure to first share a design doc with the team. If you're unsure what's the best way to do so, contact the @mcore-oncall.

Issue tracking

For PRs from open-source community contributors:

  • New features: a linked issue is required. Please open a feature request and reference it here before submitting the PR.
  • Small updates (bug fixes, minor improvements): a linked issue is recommended and will accelerate the PR review process.

Linked issue:

Contribution process

Pre-checks

  • I have added relevant unit tests
  • I have added relevant functional tests
  • I have added proper typing to my code Typing guidelines
  • I have added relevant documentation
  • I have run the autoformatter.sh on my PR

Code review

Feel free to message or comment the @mcore-oncall to help accelerate your merge into main. The less complex your PR is, the faster it will be approved and merged!

All PRs start as draft. If you open a non-draft PR, it will be automatically converted to draft.

Step 1: Mark PR as "Ready for Review"

  1. When your PR is ready, click Ready for Review.
  2. An oncall reviewer is auto-assigned and expert reviewers are notified based on your changes.
    • Some PRs may jump straight to step 2. This is determined by .github/CODEOWNERS.

⚠️ Only mark as ready once merge-conflicts are resolved and the CI is passing.
Final Review might get declined if these requirements are not fulfilled.

Step 2: Final Review

For PRs that change megatron/core, once all expert reviewers have approved, the Final Review label is applied automatically and final reviewers are assigned.

For PRs outside megatron/core, this step is skipped.

Step 3: Approved

Once all required reviewers have approved, the Approved label is applied automatically.

Merge

Any member of mcore-engineers will be able to merge your PR.

For MRs into `dev` branch The proposed review process for `dev` branch is under active discussion.

MRs are mergable after one approval by either eharper@nvidia.com or zijiey@nvidia.com.

@copy-pr-bot

copy-pr-bot Bot commented May 11, 2026

Copy link
Copy Markdown

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.

@maanug-nv

maanug-nv commented May 11, 2026

Copy link
Copy Markdown
Contributor Author

TODO:

  • remove unnecessary spec function helpers in models/gpt.py
  • Ensure no gaps between gpt_builders.gpt_builder() and GPTModelBuilder.build_model()
  • implement gpt_config_from_args()
  • add unit tests

@maanug-nv

Copy link
Copy Markdown
Contributor Author

/claude review

Comment thread megatron/training/models/gpt.py Outdated
Comment thread megatron/training/models/__init__.py Outdated

@claude claude 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.

Good PR — clean migration of the GPT builder following the established HybridModelBuilder pattern, with solid unit test coverage.

Two inline comments posted:

  1. Bug — copy-paste error in GPTModelConfig.__getattr__: Both AttributeError messages say MambaModelConfig instead of GPTModelConfig. This will confuse anyone debugging an attribute lookup failure.

  2. Nit — missing trailing comma in __init__.py __all__ list on "GPTModelBuilder".

Everything else (config proxying, vocab padding, layer spec dispatch, MTP handling, pretrain_gpt.py integration, tests) looks correct.

maanug-nv added 3 commits June 1, 2026 13:22
Signed-off-by: Maanu Grover <maanug@nvidia.com>
This reverts commit bc2ea8f.
Signed-off-by: Maanu Grover <maanug@nvidia.com>
@maanug-nv
maanug-nv enabled auto-merge June 1, 2026 20:38
@svcnvidia-nemo-ci svcnvidia-nemo-ci added the Approved All necessary approvals have been made label Jun 1, 2026
@maanug-nv
maanug-nv added this pull request to the merge queue Jun 1, 2026
@svcnvidia-nemo-ci

Copy link
Copy Markdown
Contributor

🔄 Merge queue validation started!

You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/26784364022

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

Labels

Approved All necessary approvals have been made complexity: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants