fix(model): default MODEL_CONFIG_CLASS to None and opt in per family by jayzuccarelli · Pull Request #6305 · NVIDIA-NeMo/Megatron-Bridge

jayzuccarelli · 2026-10-03T02:35:20Z

What does this PR do ?

Makes the builder-backed config path opt-in per model family. Builder calls for families that have not migrated now raise instead of returning a wrong config, and the provider path no longer tells those families to switch.

Every bridge used to inherit MODEL_CONFIG_CLASS = BridgeGPTModelConfig, but only the migrated families map their HF config into it. For the rest, get_model_config() returned generic defaults and to_megatron_provider() warned users toward it.

Changelog

MegatronModelBridge.MODEL_CONFIG_CLASS now defaults to None. Migrated families opt in explicitly: LlamaBridge and Qwen3Bridge set BridgeGPTModelConfig; MuseGlimmerBridge already sets MuseGlimmerModelConfig. Subclasses of a migrated bridge inherit the opt-in.
hf_config_to_model_config() rejects unmigrated families through the existing MODEL_CONFIG_CLASS is None branch. The error names the bridge and points to AutoBridge.to_megatron_provider().
provider_bridge() emits the deprecation FutureWarning only when MODEL_CONFIG_CLASS is set. The provider path itself is unchanged.
Removed the _has_model_config_mapping() hook-override guard from the first revision.
Removed the explicit MODEL_CONFIG_CLASS = None on GLM5Bridge and BagelBridge, which is now the default.
Updated the class comment, docstrings and docs/bridge-guide.md to describe the opt-in default.
Tests: test_bridge_can_explicitly_disable_model_config is parametrized over every registered bridge (64). Unmigrated bridges must reject the builder path and have no config mapping; opted-in bridges must provide one; a subclass that sets None is rejected. Added an inheritance test and a provider test on Qwen2 (unmigrated) that asserts no FutureWarning. The Qwen3-based tests are gone since Qwen3 is migrated.

Testing

CPU only, no TransformerEngine (stubbed locally). Run against unmodified main, the updated test file gives 65 failed, 9 passed, 1 skipped; with this change it gives 74 passed, 1 skipped.

Unchanged between main and this branch: models/llama 44 passed, models/qwen 118 passed, models/glm_moe_dsa 25 passed, models/bagel conversion/checkpoint/dependencies 16 passed and 1 skipped, test_gpu_backend.py 21 passed, test_vlm_generation.py 7 passed, test_llama_recipes.py 105 passed, test_llama_perf_recipes.py 16 passed, test_qwen3_recipes.py 24 passed. test_auto_bridge.py: 97 passed, with the same 5 failures before and after locally (they need TE or a writable /tmp/output). The Muse Glimmer tests need real TransformerEngine and were not run locally; no Muse Glimmer file changed. ruff check and format clean.

Additional Information

Fixes get_model_config() returns incorrect configs for families without a config mapping #6242

AI-assisted (Claude Code); I reviewed the change and ran the tests.

…apping # What does this PR do ? Builder calls for families that have not migrated their config mapping now raise instead of returning a wrong config, and the provider path no longer tells those families to switch. Every bridge inherits MODEL_CONFIG_CLASS = BridgeGPTModelConfig, but only Llama and Muse Glimmer map their HF config into it. For the rest, get_model_config() returned generic defaults (Qwen3: learned_absolute position embeddings, linear bias on, no QK norm), and to_megatron_provider() warned users toward it. # Changelog - Add MegatronModelBridge._has_model_config_mapping(): true when a bridge overrides hf_config_to_model_config_kwargs() or hf_config_to_model_config(). - hf_config_to_model_config() raises ModelConfigNotSupportedError naming the bridge and pointing to AutoBridge.to_megatron_provider() when there is no mapping. - provider_bridge() only emits the deprecation FutureWarning for families with a mapping. The provider path itself is unchanged. - Two unit tests in test_gpt_model_config.py (Qwen3 as the unmigrated family). This is a guard on the hook overrides; the alternative is flipping the MODEL_CONFIG_CLASS default to None and opting in per family. Happy to switch if you prefer that. # Testing CPU only. Both new tests fail on main (DID NOT RAISE, and the FutureWarning) and pass with this change. test_gpt_model_config.py, llama/test_llama_bridge.py and qwen/test_qwen3_bridge.py: 74 passed, 1 skipped. Llama builder tests in test_auto_bridge.py still pass; the rest of that file has the same failures before and after locally (no TE/modelopt). ruff check and format clean. # Additional Information - Fixes NVIDIA-NeMo#6242 AI-assisted (Claude Code); I reviewed the change and ran the tests. Signed-off-by: Eugenio "Jay" Zuccarelli <11176606+jayzuccarelli@users.noreply.github.com>

copy-pr-bot · 2026-10-03T02:35:25Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

jayzuccarelli · 2026-10-11T00:24:15Z

@cuichenx when you have time, could you take a look at this one? #6347 (the follow-up to #6236 for the Mamba and GDN mappings) is also ready, and I just merged main into it to clear a conflict.

cuichenx

"flipping the MODEL_CONFIG_CLASS default to None", i think i prefer this approach slightly, one because in case there's a two level inheritance of model configs in the future (e.g. different muse glimmer model sizes) and two because it would make testing easier, the existing test_bridge_can_explicitly_disable_model_config can be modified to cover all models

…ct-unmigrated Signed-off-by: Eugenio "Jay" Zuccarelli <11176606+jayzuccarelli@users.noreply.github.com>

Make the builder-backed config path opt-in. MegatronModelBridge.MODEL_CONFIG_CLASS now defaults to None, and the migrated families (Llama, Qwen3, Muse Glimmer) set it explicitly. Unmigrated families reject get_model_config() through the existing MODEL_CONFIG_CLASS is None branch, and provider_bridge() only emits the deprecation FutureWarning for opted-in families. This replaces the _has_model_config_mapping() hook-override guard. The explicit MODEL_CONFIG_CLASS = None on GLM5Bridge and BagelBridge is now the default and is removed. test_bridge_can_explicitly_disable_model_config is parametrized over every registered bridge. Signed-off-by: Eugenio "Jay" Zuccarelli <11176606+jayzuccarelli@users.noreply.github.com>

…ge test Signed-off-by: Eugenio "Jay" Zuccarelli <11176606+jayzuccarelli@users.noreply.github.com>

jayzuccarelli · 2026-10-11T20:14:54Z

@cuichenx thanks, switched to that approach. MODEL_CONFIG_CLASS now defaults to None on MegatronModelBridge, and the migrated families opt in: LlamaBridge and Qwen3Bridge set BridgeGPTModelConfig, and MuseGlimmerBridge already sets its own config. The hook-override guard is removed, so unmigrated families are rejected by the existing None branch, and the provider deprecation warning is gated on the same attribute. Subclasses of a migrated bridge inherit the opt-in.

test_bridge_can_explicitly_disable_model_config is now parametrized over every registered bridge. It checks that unmigrated bridges reject get_model_config() and that opting in and providing a config mapping always go together. One exception: MuseGlimmerBridge overrides hf_config_to_model_config() entirely and does not read MODEL_CONFIG_CLASS, so the subclass-sets-None check skips it. I left that behavior unchanged.

I also removed the explicit MODEL_CONFIG_CLASS = None on GLM5Bridge and BagelBridge since it is now the default. I can restore those lines if you prefer them as documentation.

cuichenx

thanks, LGTM

cuichenx · 2026-10-12T04:12:59Z

/ok to test 0ef959a

github-actions · 2026-10-12T04:13:13Z

Automatic Claude reviews have been retired. To request a pull-request review, post a comment containing:

/review

Add model=claude to use a Claude reviewer (the default is model=codex). mode=light|strict selects the review depth; for example, /review model=claude mode=strict. Comment /review help for all options.

github-actions · 2026-10-12T04:14:10Z

🌿 Preview your docs: https://nvidia-preview-pr-6305-69e25210c18c.docs.buildwithfern.com/nemo/megatron-bridge

aroshanghias-nvd · 2026-10-12T17:36:06Z

The link check failed on retired documentation targets, which are now corrected on main. Could you bring in current main and rerun validation? The Qwen3 review thread also looks addressed and can be resolved.

…ct-unmigrated Signed-off-by: Eugenio "Jay" Zuccarelli <11176606+jayzuccarelli@users.noreply.github.com>

jayzuccarelli · 2026-10-12T20:02:07Z

@aroshanghias-nvd done: merged current main (no conflicts, same 8 files in the diff) and resolved the Qwen3 thread. CI is re-running.

aroshanghias-nvd · 2026-10-12T22:08:38Z

/ok to test 69e2521

…configs After #6305, MODEL_CONFIG_CLASS defaults to None and each family opts in. Set MODEL_CONFIG_CLASS = BridgeGPTModelConfig on the bridges this PR migrates, matching Qwen3 and Llama, and assert it in their tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Kamran Jafari <kjafarisadeg@nvidia.com>

nemo-automation-bot Bot added the community-request label Oct 3, 2026

jayzuccarelli marked this pull request as ready for review October 3, 2026 02:51

cuichenx added bug Something isn't working area:model Model implementations and HF bridge logic needs-review PR is ready for code review and waiting on a reviewer labels Oct 3, 2026

cuichenx reviewed Oct 11, 2026

View reviewed changes

Comment thread tests/unit_tests/models/test_gpt_model_config.py Outdated

jayzuccarelli added 3 commits October 11, 2026 19:38

Merge remote-tracking branch 'origin/main' into fix/model-config-reje…

3815dc6

…ct-unmigrated Signed-off-by: Eugenio "Jay" Zuccarelli <11176606+jayzuccarelli@users.noreply.github.com>

test(model): keep the explicit-disable check in the parametrized brid…

0ef959a

…ge test Signed-off-by: Eugenio "Jay" Zuccarelli <11176606+jayzuccarelli@users.noreply.github.com>

jayzuccarelli changed the title ~~fix(model): reject get_model_config() for families without a config mapping~~ fix(model): default MODEL_CONFIG_CLASS to None and opt in per family Oct 11, 2026

cuichenx approved these changes Oct 12, 2026

View reviewed changes

cuichenx added ready-to-merge PR is approved, current, and only waiting for CI to pass before merge and removed needs-review PR is ready for code review and waiting on a reviewer labels Oct 12, 2026

copy-pr-bot Bot deployed to test October 12, 2026 04:14 Active

Merge remote-tracking branch 'origin/main' into fix/model-config-reje…

69e2521

…ct-unmigrated Signed-off-by: Eugenio "Jay" Zuccarelli <11176606+jayzuccarelli@users.noreply.github.com>

copy-pr-bot Bot deployed to test October 12, 2026 22:09 Active

cuichenx enabled auto-merge (squash) October 13, 2026 00:16

cuichenx merged commit 1234500 into NVIDIA-NeMo:main Oct 13, 2026
84 checks passed

jayzuccarelli deleted the fix/model-config-reject-unmigrated branch October 13, 2026 13:08

jayzuccarelli mentioned this pull request Oct 13, 2026

perf(ckpt): build remaining split indices on the input device #6347

Merged

kamran-nvidia mentioned this pull request Oct 14, 2026

refactor(model): build Qwen2 and MiMo from builder-backed model configs #6379

Open

2 of 5 tasks

kamran-nvidia mentioned this pull request Oct 14, 2026

refactor(model): build OLMoE and GLM-4.5 from builder-backed model configs #6383

Open

2 of 5 tasks

kamran-nvidia mentioned this pull request Oct 14, 2026

refactor(model): build Hy V3 and MiniMax-M2 from builder-backed model configs #6392

Open

2 of 5 tasks

kamran-nvidia mentioned this pull request Oct 14, 2026

refactor(model): build Ling MoE V2 and EXAONE 4.0 from builder-backed model configs #6401

Open

2 of 5 tasks

Navigation Menu

Repository navigation

fix(model): default MODEL_CONFIG_CLASS to None and opt in per family - #6305

fix(model): default MODEL_CONFIG_CLASS to None and opt in per family#6305
cuichenx merged 5 commits into
NVIDIA-NeMo:mainfrom
jayzuccarelli:fix/model-config-reject-unmigrated

jayzuccarelli commented Oct 3, 2026 •

edited

Loading

Uh oh!

copy-pr-bot Bot commented Oct 3, 2026

Uh oh!

jayzuccarelli commented Oct 11, 2026

Uh oh!

cuichenx left a comment

Uh oh!

Uh oh!

jayzuccarelli commented Oct 11, 2026

Uh oh!

cuichenx left a comment

Uh oh!

cuichenx commented Oct 12, 2026

Uh oh!

github-actions Bot commented Oct 12, 2026

Uh oh!

github-actions Bot commented Oct 12, 2026 •

edited

Loading

Uh oh!

aroshanghias-nvd commented Oct 12, 2026

Uh oh!

jayzuccarelli commented Oct 12, 2026

Uh oh!

aroshanghias-nvd commented Oct 12, 2026

Uh oh!

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

jayzuccarelli commented Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do ?

Changelog

Testing

Additional Information

Uh oh!

copy-pr-bot Bot commented Oct 3, 2026

Uh oh!

jayzuccarelli commented Oct 11, 2026

Uh oh!

cuichenx left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jayzuccarelli commented Oct 11, 2026

Uh oh!

cuichenx left a comment

Choose a reason for hiding this comment

Uh oh!

cuichenx commented Oct 12, 2026

Uh oh!

github-actions Bot commented Oct 12, 2026

Uh oh!

github-actions Bot commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aroshanghias-nvd commented Oct 12, 2026

Uh oh!

jayzuccarelli commented Oct 12, 2026

Uh oh!

aroshanghias-nvd commented Oct 12, 2026

Uh oh!

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jayzuccarelli commented Oct 3, 2026 •

edited

Loading

github-actions Bot commented Oct 12, 2026 •

edited

Loading