Skip to content

fix(recipe): pin expert tensor parallelism on Qwen3.5-VL MoE pretrain recipes - #5754

Merged
cuichenx merged 1 commit into
mainfrom
pruprakash/fix_recipe_qwen35_vl_moe_pretrain_expert_tp
Oct 6, 2026
Merged

cuichenx merged 1 commit into
mainfrom
pruprakash/fix_recipe_qwen35_vl_moe_pretrain_expert_tp

Conversation

@pruprakash

Copy link
Copy Markdown
Contributor

What does this PR do ?

Fixes three Qwen3.5-VL MoE pretrain recipes raising RuntimeError at initialize_model_parallel, by pinning the expert tensor parallel size they left unset.

Changelog

  • src/megatron/bridge/recipes/qwen_vl/h100/qwen35_vl.py: set expert_tensor_parallel_size = 1 on the 35B-A3B/8gpu, 122B-A10B/128gpu and 397B-A17B/512gpu pretrain mock recipes.
  • tests/unit_tests/recipes/qwen_vl/test_qwen35_vl_recipes.py: 1 parametrized test (3 cases) asserting the dense and expert grids divide each recipe's named world size, built through a provider fake that leaves ETP unset.

GitHub Actions CI

See the CI section in the Contributing doc for how to trigger the CI. A Nvidia developer will need to approve and trigger the CI for external contributors.

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

  • Root cause: ETP unset, so Megatron-Core resolves it to the dense TP (model_parallel_config.py:506, parallel_state.py:871) and the expert grid needs 4x the ranks each recipe name advertises.
  • Blast radius: tutorials/data/datacomp/train_qwen36_example.sh and both qwen3.6-35b-a3b card leaves launch the 35B-A3B recipe on 8 GPUs with no overrides; all fail at init.
  • Regression? No, latent since 4d7ce790 (refactor(recipes): add h100 recipe namespace #4622). Masked in unit tests because the shared _FakeModelCfg pre-sets ETP=1.
  • Verification: 8xH100, nvcr.io/nvidian/nemo:nightly. initialize_model_parallel at the recipe topology: ETP unset gives world_size (8) is not divisible by expert_tensor_model_pipeline_parallel size (32); ETP=1 gives OK (dp=1 etp=1 ep=4). Unit red-green 3 failed to 3 passed; pytest tests/unit_tests/recipes/qwen_vl/ 225 passed.
  • Not included: the card leaves' GBS 512 vs recipe GBS 32 mismatch is a separate card-side defect.
  • Related to # (issue)

… recipes

The three MoE pretrain mock recipes left expert_tensor_parallel_size unset,
so Megatron-Core resolved it to the dense tensor parallel size of 4 and the
expert grid demanded four times the ranks each recipe name advertises.
initialize_model_parallel raised before any model was built.

Adds a unit test covering the change (red-green verified).

Signed-off-by: Pruthviraj Prakash <pruprakash@nvidia.com>
@pruprakash pruprakash added bug Something isn't working area:recipe Training recipes and launch configs needs-review PR is ready for code review and waiting on a reviewer qa_rcca_done labels Aug 23, 2026
@copy-pr-bot

copy-pr-bot Bot commented Aug 23, 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.

@claude

claude Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Light review — LGTM

The fix pins expert_tensor_parallel_size = 1 on the three Qwen3.5-VL MoE pretrain mock recipes. Verified that with the fix every expert grid (ETPEPPP) divides the named world size:

  • 35b_a3b_8gpu: dense 412=8 (ok), expert 142=8 (ok)
  • 122b_a10b_128gpu: dense 428=64 -> 128 (ok), expert 188=64 -> 128 (ok)
  • 397b_a17b_512gpu: dense 4216=128 -> 512 (ok), expert 11616=256 -> 512 (ok)

Without the fix an unset value resolves to TP=4, yielding expert grids of 32 / 256 / 1024 that do not divide the world size, so the change is both correct and necessary. The new parametrized unit test (test_qwen35_vl_moe_pretrain_mock_grids_divide_their_named_world_size) is the right GPU-free coverage and correctly simulates the unset case via _FakeModelCfgWithoutExpertTensorParallel.

Related (out of scope, follow-up): the sibling qwen3_vl MoE mock configs — qwen3_vl_30b_a3b_pretrain_8gpu_h100_bf16_mock_config and qwen3_vl_235b_a22b_pretrain_256gpu_h100_bf16_mock_config — set expert_model_parallel_size but omit expert_tensor_parallel_size, the identical latent issue. Consider extending the same fix and test to them.

Suggested test cases

  • No perf tests impacted. (No scripts/performance/configs/ files changed.)
  • Relevant unit coverage is included: tests/unit_tests/recipes/qwen_vl/test_qwen35_vl_recipes.py::test_qwen35_vl_moe_pretrain_mock_grids_divide_their_named_world_size (parametrized over the 8/128/512-GPU recipes).

@yaoyu-33 yaoyu-33 added the needs-more-tests Requires additional L0 and L1 test coverage before merge label Aug 24, 2026
@pruprakash
pruprakash requested a review from yaoyu-33 August 24, 2026 18:08
@pruprakash pruprakash self-assigned this Aug 24, 2026
@pruprakash

Copy link
Copy Markdown
Contributor Author

/ok to test 1403572

@cuichenx
cuichenx merged commit dbafd7e into main Oct 6, 2026
131 checks passed
@cuichenx
cuichenx deleted the pruprakash/fix_recipe_qwen35_vl_moe_pretrain_expert_tp branch October 6, 2026 23:29
@qiaochuz-nv

Copy link
Copy Markdown

Retrospective QA-fix tracking issue: #6394. It records the fix authored by @pruprakash and delivered by this merged PR.

This branch was previously deployed

2 inactive deployments
public — 14035726 Deployed Aug 24, 2026 by copy-pr-bot[bot] via release / finalize / publish-docs #4960
test — 14035726 Deployed Aug 24, 2026 by copy-pr-bot[bot] via cicd-wait-in-queue #20939
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:recipe Training recipes and launch configs bug Something isn't working needs-more-tests Requires additional L0 and L1 test coverage before merge needs-review PR is ready for code review and waiting on a reviewer qa_rcca_done

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants