Skip to content

Fix Qwen2.5-VL huggingface conversion issue (#2107)#2156

Merged
meatybobby merged 6 commits intomainfrom
bobchen/fix_qwen25
Feb 4, 2026
Merged

Fix Qwen2.5-VL huggingface conversion issue (#2107)#2156
meatybobby merged 6 commits intomainfrom
bobchen/fix_qwen25

Conversation

@meatybobby
Copy link
Copy Markdown
Contributor

@meatybobby meatybobby commented Jan 31, 2026

What does this PR do ?

Fix Qwen2.5-VL huggingface conversion issue (#2107)
Moving decoder assigning out of model initialization to avoid mcore recognize it as LLM and causing the error.

Changelog

  • Add specific line by line info of high level changes in this PR.

GitHub Actions CI

See the CI sectionin 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?

If you haven't finished some of the above items you can still open "Draft" PR.

Additional Information

  • Related to # (issue)

Summary by CodeRabbit

  • Refactor
    • Reorganized decoder accessibility in the VLM inference engine for improved MCore compatibility.

✏️ Tip: You can customize this high-level summary in your review settings.

@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot bot commented Jan 31, 2026

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.

@meatybobby meatybobby requested a review from huvunvidia January 31, 2026 00:07
@meatybobby
Copy link
Copy Markdown
Contributor Author

/ok to test a999acf

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Jan 31, 2026

📝 Walkthrough

Walkthrough

The decoder alias for Qwen25VLModel is relocated from the model definition to the MCore inference engine wrapper, maintaining compatibility functionality while reorganizing where the aliasing occurs.

Changes

Cohort / File(s) Summary
Decoder Alias Relocation
src/megatron/bridge/models/qwen_vl/modeling_qwen25_vl.py, src/megatron/bridge/inference/vlm/base.py
Removed decoder alias assignment from Qwen25VLModel definition; added equivalent alias on MCore inference engine side via mcore_model.module.decoder to mcore_model.module.language_model.decoder mapping.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix Qwen2.5-VL huggingface conversion issue (#2107)' directly relates to the main change in the PR, which is fixing a Hugging Face conversion issue by moving the decoder assignment out of model initialization.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Test Results For Major Changes ✅ Passed PR contains minor changes (5 lines modified) focused on fixing a HuggingFace conversion issue for Qwen2.5-VL. Per check criteria, minor changes are acceptable without explicit test result documentation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch bobchen/fix_qwen25

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/megatron/bridge/inference/vlm/base.py`:
- Around line 132-135: The code assumes mcore_model is DDP-wrapped and directly
uses mcore_model.module.decoder causing AttributeError when wrap_with_ddp=False;
change to safely resolve the underlying model (e.g., base = getattr(mcore_model,
"module", mcore_model)) before assigning the decoder so that for config of type
Qwen25VLModelProvider you set base.decoder = base.language_model.decoder; update
the block that references mcore_model.module.decoder to use this safe accessor
(referencing symbols: Qwen25VLModelProvider, mcore_model, module, decoder,
language_model).

@meatybobby
Copy link
Copy Markdown
Contributor Author

/ok to test 23c68ad

@huvunvidia
Copy link
Copy Markdown
Contributor

Looks good!

Copy link
Copy Markdown
Contributor

@huvunvidia huvunvidia left a comment

Choose a reason for hiding this comment

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

Looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

r0.3.0 Cherry-pick label for r0.3.0 release branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants