Skip to content

Support Qwen3next - #939

Closed
yuzhongw-nvidia wants to merge 5 commits into
NVIDIA-NeMo:mainfrom
yuzhongw-nvidia:qwen3next
Closed

Support Qwen3next#939
yuzhongw-nvidia wants to merge 5 commits into
NVIDIA-NeMo:mainfrom
yuzhongw-nvidia:qwen3next

Conversation

@yuzhongw-nvidia

Copy link
Copy Markdown
Contributor

No description provided.

@copy-pr-bot

copy-pr-bot Bot commented Oct 14, 2025

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.

Comment thread src/megatron/bridge/models/conversion/param_mapping.py Outdated
Comment thread src/megatron/bridge/models/conversion/auto_bridge.py Outdated
Comment thread src/megatron/bridge/models/model_provider.py Outdated
@@ -2048,7 +1876,7 @@ def _build_sharded_state_dict_metadata(
else:
metadata["distrib_optim_sharding_type"] = "dp_zero_gather_scatter"
metadata["chained_optim_avoid_prefix"] = True
metadata["singleton_local_shards"] = False
metadata["singleton_local_shards"] = True

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.

why is this change needed?

@yuzhongw-nvidia yuzhongw-nvidia Oct 22, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to this line of code in megatron-lm, singleton_local_shards is set to False if and only if args.dist_ckpt_save_pre_mcore_014 or force_pre_mcore_014.

Honestly, I don't fully understand this flag, but in my actual runs, I've found that setting metadata["singleton_local_shards"] = False causes the tensor parallel sharding to be incorrect in MCore.

"output_layer.weight": "lm_head.weight",
"decoder.final_layernorm.weight": "model.norm.weight",
}
for megatron_param, hf_param in layer_mappings.items():

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.

can we just write down the mapping directly, not use layer_mappings. It's a bit inconsistent with other model.
We feel it is clearer just write the whole name together, though we need to duplicate the prefix.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I will do the change later.

@yuzhongw-nvidia yuzhongw-nvidia Oct 22, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Changes done. Does it look good now?

@yaoyu-33

yaoyu-33 commented Oct 14, 2025

Copy link
Copy Markdown
Contributor

Hi, thanks for contribution. unit tests/ functional tests need to be added. also it will be nice if you can attach your validation results.
https://github.com/NVIDIA-NeMo/Megatron-Bridge/blob/main/docs/adding-new-models.md#6-validate-with-examples

@yuzhongw-nvidia
yuzhongw-nvidia force-pushed the qwen3next branch 2 times, most recently from 1779982 to f388215 Compare October 22, 2025 09:12
@yaoyu-33

Copy link
Copy Markdown
Contributor

moved the PR to #1068

@yaoyu-33 yaoyu-33 closed this Oct 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants