Repository navigation
docs(model): teach builder-backed model configs in the adding-model-support skill - #6402
Merged
Merged
Conversation
…upport skill The adding-model-support skill taught provider_bridge() and to_megatron_provider() as the way to configure a model. New and migrated families now use the builder-backed ModelConfig / ModelBuilder path (Qwen3, Muse Glimmer), and families that only override provider_bridge() get a silently incomplete get_model_config(). - SKILL.md: LLM bridges implement hf_config_to_model_config_kwargs(), delegate hf_config_to_provider_kwargs(), and set USE_MODEL_CONFIG_FOR_CONVERSION = True; custom configs and builders follow Muse Glimmer; recipes use get_model_config(); the smoke test builds through get_model(). - llm-patterns.md: model config pattern, the three defaults that differ from the legacy provider, layer-spec and YaRN guidance (replacing the stale YaRN provider-subclass example), and a checklist for migrating a provider-based bridge. - recipe-patterns.md, tests-and-examples.md, vlm-patterns.md: builder config recipes, strict recipe-test stubs, builder unit-test template, and the builder-backed multimodal pattern. Fixes the mapping-registry test template, which failed on dict-valued hf_param. - parity-testing/SKILL.md: point config checks at the builder mapping. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Kamran Jafari <kjafarisadeg@nvidia.com>
Contributor
Author
|
/ok to test a937f1e |
Contributor
…e it is filled Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Kamran Jafari <kjafarisadeg@nvidia.com>
Contributor
Author
|
/ok to test 57f5bd3 |
kamran-nvidia
marked this pull request as ready for review
October 13, 2026 15:54
cuichenx
approved these changes
Oct 14, 2026
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 ?
Updates the
adding-model-supportskill to teach the builder-backedModelConfig/ModelBuilderpath instead ofprovider_bridge()/to_megatron_provider(). Documentation only; no code changes.The skill still directs new bridges to override
provider_bridge(). For a family that only does that,get_model_config()returns a config built fromCONFIG_MAPPINGalone, and every attribute set inprovider_bridge()is silently dropped. Its YaRN example also describedyarn_*as fields missing from the provider; since #6365 they are declared on BridgeTransformerConfig.Changelog
SKILL.md:hf_config_to_model_config_kwargs(), makehf_config_to_provider_kwargs()return the same kwargs, and setUSE_MODEL_CONFIG_FOR_CONVERSION = True.BridgeGPTModelConfigcannot represent (multimodal, hybrid) get aModelConfigsubclass and aModelBuilder, following Muse Glimmer.provider_bridge()as legacy.get_model_config(), and the smoke test builds throughget_model().llm-patterns.md:position_embedding_type,masked_softmax_fusionandrope_scaling_factor. Missing one of these silently changes the model.recipe-patterns.md: recipes useget_model_config(); the recipe-test pattern uses a strict builder config and rejects the provider API.tests-and-examples.md: builder unit-test template covering config mapping, provider/builder parity, and the conversion flag.TypeErrorbecauseQKVMapping.hf_paramis a dict.Mock()HF config, which fails on both paths because unset attributes such ashidden_actcome back asMockobjects.vlm-patterns.md: adds the builder-backed multimodal pattern (Muse Glimmer) and labels the provider snippets as legacy.parity-testing/SKILL.md: config-mapping checks point tohf_config_to_model_config_kwargs()first.Verification
The snippets were run against
main(MCore 8949def81) in the NeMo 26.10 container, with Qwen3 standing in for the placeholder model:llm-patterns.mdwere extracted from the Markdown and executed. 16 tests passed.learned_absolute,False,8.0. Provider:rope,True,1.0.ValueError.AttributeError.AutoBridge.from_hf_config(...).get_model_config()works.SKILL.mdsmoke snippet (get_model_config()→get_model()→export_hf_weights()) passed on one GB200 GPU with a tiny Qwen3 checkpoint.Before your PR is "Ready for review"
Pre checks:
Additional Information
docs/adding-new-models.mdstill teachesprovider_bridge(); left for a separate docs PR.🤖 Generated with Claude Code