Skip to content

[bugfix] fix tie_word_embeddings #150

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Jintao-Huang merged 1 commit into from
Jul 17, 2026
+0 −4
Merged

[bugfix] fix tie_word_embeddings #150

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions src/mcore_bridge/config/parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,9 @@ def _convert_config(config, _internal_call=False) -> Dict[str, Any]:
k = 'llm_model_type'
megatron_config[k] = hf_v
break
# fix Qwen/Qwen3-VL-30B-A3B-Thinking
untie_embeddings_and_output_weights = megatron_config.get('untie_embeddings_and_output_weights')
for key in ['text_config', 'llm_config', 'thinker_config']:
if hasattr(config, key):
megatron_config.update(_convert_config(getattr(config, key), _internal_call=True))
Comment on lines 109 to 111

@gemini-code-assist gemini-code-assist Bot Jul 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Removing the preservation of untie_embeddings_and_output_weights entirely can lead to issues when hf_config.name_or_path is not set (e.g., during programmatic initialization or testing). In such cases, the sub-config's default tie_word_embeddings = True (which maps to untie_embeddings_and_output_weights = False) will overwrite the parent config's explicit or class-default tie_word_embeddings = False (which maps to untie_embeddings_and_output_weights = True).

Instead of removing the preservation logic completely, we can make it smarter: only restore untie_embeddings_and_output_weights if the parent config had it set to True (untied). This prevents the sub-config's default False (tied) from overwriting the parent's explicit True (untied), while still allowing a sub-config's explicit True (untied) to override the parent's default False (tied).

    untie_embeddings_and_output_weights = megatron_config.get('untie_embeddings_and_output_weights')
    for key in ['text_config', 'llm_config', 'thinker_config']:
        if hasattr(config, key):
            megatron_config.update(_convert_config(getattr(config, key), _internal_call=True))
    if untie_embeddings_and_output_weights is True:
        megatron_config['untie_embeddings_and_output_weights'] = untie_embeddings_and_output_weights

if untie_embeddings_and_output_weights is not None:
megatron_config['untie_embeddings_and_output_weights'] = untie_embeddings_and_output_weights
# compat llama3
if getattr(config, 'rope_scaling', None) is not None:
if isinstance(config.rope_scaling, int):
Expand Down
Loading