Skip to content

docs(model): teach builder-backed model configs in the adding-model-support skill - #6402

Merged
cuichenx merged 2 commits into
mainfrom
kjafarisadeg/skills-model-config
Oct 14, 2026
Merged

cuichenx merged 2 commits into
mainfrom
kjafarisadeg/skills-model-config

Conversation

@kamran-nvidia

Copy link
Copy Markdown
Contributor

What does this PR do ?

Updates the adding-model-support skill to teach the builder-backed ModelConfig / ModelBuilder path instead of provider_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 from CONFIG_MAPPING alone, and every attribute set in provider_bridge() is silently dropped. Its YaRN example also described yarn_* as fields missing from the provider; since #6365 they are declared on Bridge TransformerConfig.

Changelog

  • SKILL.md:
    • LLM bridges implement hf_config_to_model_config_kwargs(), make hf_config_to_provider_kwargs() return the same kwargs, and set USE_MODEL_CONFIG_FOR_CONVERSION = True.
    • Models that BridgeGPTModelConfig cannot represent (multimodal, hybrid) get a ModelConfig subclass and a ModelBuilder, following Muse Glimmer.
    • The hook table marks provider_bridge() as legacy.
    • Recipes use get_model_config(), and the smoke test builds through get_model().
    • The LLM reference moves from Qwen2 to Qwen3 (Qwen2 is not migrated yet).
  • llm-patterns.md:
    • Model config pattern and a full bridge example.
    • Table of the three defaults that differ between the builder config and the legacy provider: position_embedding_type, masked_softmax_fusion and rope_scaling_factor. Missing one of these silently changes the model.
    • Layer-spec guidance, including that MCore's builder default handles dense-first MoE.
    • YaRN guidance that replaces the stale provider-subclass example.
    • A checklist for migrating an existing provider-based bridge.
  • recipe-patterns.md: recipes use get_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.
    • Also fixes the existing mapping-registry template, which raised TypeError because QKVMapping.hf_param is a dict.
    • Replaces the Mock() HF config, which fails on both paths because unset attributes such as hidden_act come back as Mock objects.
  • 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 to hf_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:

  • Snippets: the unit-test template, the recipe-test stub, and the bridge example from llm-patterns.md were extracted from the Markdown and executed. 16 tests passed.
  • Defaults table: measured on a bridge with no overrides. Builder: learned_absolute, False, 8.0. Provider: rope, True, 1.0.
  • Other claims checked:
    • Unknown kwargs raise ValueError.
    • Undeclared attribute assignment raises AttributeError.
    • Flat assignment routes to the nested transformer config.
    • AutoBridge.from_hf_config(...).get_model_config() works.
    • Qwen3 fills the HF YaRN defaults.
  • Smoke test: the SKILL.md smoke 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:

  • Make sure you read and followed Contributor guidelines
  • Did you write any new necessary tests?
  • Did you add or update any necessary documentation?
  • Does the PR affect components that are optional to install? (Ex: Numba, Pynini, Apex etc)
    • Reviewer: Does the PR have correct import guards for all optional libraries?

Additional Information

🤖 Generated with Claude Code

…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>
@kamran-nvidia kamran-nvidia added docs-only With great power comes great responsibility. docs Documentation-only updates or documentation debt area:model Model implementations and HF bridge logic labels Oct 9, 2026
@copy-pr-bot

copy-pr-bot Bot commented Oct 9, 2026

Copy link
Copy Markdown

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.

@kamran-nvidia

Copy link
Copy Markdown
Contributor Author

/ok to test a937f1e

@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

…e it is filled

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Kamran Jafari <kjafarisadeg@nvidia.com>
@kamran-nvidia

Copy link
Copy Markdown
Contributor Author

/ok to test 57f5bd3

@kamran-nvidia
kamran-nvidia marked this pull request as ready for review October 13, 2026 15:54
@cuichenx cuichenx added the needs-review PR is ready for code review and waiting on a reviewer label Oct 13, 2026
@cuichenx
cuichenx merged commit e05ec7a into main Oct 14, 2026
42 checks passed
@cuichenx
cuichenx deleted the kjafarisadeg/skills-model-config branch October 14, 2026 03:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:model Model implementations and HF bridge logic docs Documentation-only updates or documentation debt docs-only With great power comes great responsibility. needs-review PR is ready for code review and waiting on a reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants