Skip to content

[Bugfix] Fix InternVL/InternVL3 LoRA loading TypeError in adapter fallback#4684

Open
waynehacking8 wants to merge 1 commit into
InternLM:mainfrom
waynehacking8:wayne/fix-4105-internvl-lora-fallback
Open

[Bugfix] Fix InternVL/InternVL3 LoRA loading TypeError in adapter fallback#4684
waynehacking8 wants to merge 1 commit into
InternLM:mainfrom
waynehacking8:wayne/fix-4105-internvl-lora-fallback

Conversation

@waynehacking8

Copy link
Copy Markdown
Contributor

Summary

Fixes #4105. Serving OpenGVLab/InternVL3-14B (or any InternVL) with a LoRA adapter on the PyTorch backend crashes with:

TypeError: load_lora_weights() missing 1 required positional argument: 'adapter_id'

Root cause

Both InternVLChatModel.load_lora_weights (internvl.py) and InternVLForConditionalGeneration.load_lora_weights (internvl3_hf.py) fall back to the standalone lmdeploy.pytorch.adapter.adapter.load_lora_weights when the inner language model does not define load_lora_weights. Its signature is:

def load_lora_weights(model: nn.Module, weights, adapter_id): ...

but both call it as load_lora_weights(weights, adapter_id)weights binds to model, adapter_id binds to weights, and adapter_id is unbound → TypeError.

internvl3_hf.py has a second bug that makes the broken branch fire unconditionally: it gates on hasattr(self.model.language_model, 'load_lora_weights'), but the constructor stores self.language_model (there is no self.model). hasattr swallows the AttributeError and always takes the else branch.

Fix

Pass the top-level model (self) to the standalone loader, and use self.language_model for the hasattr/delegate path in internvl3_hf.py.

self is correct because the loader does dict(model.named_parameters()) and maps de-prefixed adapter keys like language_model.model.layers.0.mlp.down_proj.lora_adapters.down_proj.lora_A — i.e. names that start with language_model.. Only the top-level VLM (which has language_model as a child) produces that namespace. This mirrors the canonical caller internlm2.py:

load_lora_weights(self, weights_iter, adapter_id)

Relation to #4106

The earlier (closed) #4106 passed self.language_model (internvl.py) / self.model.language_model (internvl3_hf.py). self.language_model.named_parameters() yields model.layers... (no language_model. prefix), so the loader's params_dict[param_name] lookup misses — which is the KeyError/param-name mismatch the reporter hit after trying #4106. Passing self gives the loader the namespace it actually maps against, and also fixes the non-existent self.model reference.

Test

Added tests/pytorch/test_internvl_lora.py (GPU-free; mocks the standalone loader):

pytest tests/pytorch/test_internvl_lora.py
  • WITH fix: 3 passed — both classes pass (model, weights, adapter_id) to the loader, and internvl3_hf delegates correctly when the language model implements load_lora_weights.
  • WITHOUT fix: 3 failed (loader called with the wrong (weights, adapter_id) arity; delegation never happens).

ruff check clean on all three files.

Honesty / scope

The TypeError and self.model bugs are verified by code analysis + the GPU-free unit tests above. I do not have the InternVL3-14B + LoRA serving setup to run the full end-to-end here; the parameter-namespace analysis indicates passing self also resolves the downstream KeyError from #4106, but an e2e confirmation by someone with that model/adapter would be welcome. cc @windreamer (you triaged the original issue).


AI-assisted (Claude); every line human-reviewed.

…lback

InternVLChatModel.load_lora_weights and
InternVLForConditionalGeneration.load_lora_weights fall back to the
standalone load_lora_weights(model, weights, adapter_id) when the inner
language model has no load_lora_weights, but called it with only
(weights, adapter_id) -> "TypeError: load_lora_weights() missing 1 required
positional argument: 'adapter_id'", blocking every InternVL3/InternVL LoRA
adapter load.

Additionally, InternVLForConditionalGeneration gated the call on
hasattr(self.model.language_model, ...), but the constructor stores
self.language_model (there is no self.model). hasattr swallowed the
AttributeError, so the broken fallback fired unconditionally.

Pass the top-level model (self) to the standalone loader, matching the
canonical caller in internlm2.py -- its named_parameters() carry the
"language_model." prefix the loader maps against -- and use
self.language_model for the hasattr/delegate path.

Fixes InternLM#4105

Co-authored-by: Claude <noreply@anthropic.com>

Copilot AI left a comment

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.

Pull request overview

Fixes a PyTorch-backend LoRA adapter loading crash for InternVL/InternVL3 wrappers by correcting the standalone adapter-loader callsite and fixing an incorrect attribute reference that forced the fallback path.

Changes:

  • Fix InternVLChatModel fallback to call the standalone LoRA loader with the correct (model, weights, adapter_id) signature.
  • Fix InternVLForConditionalGeneration to (a) check self.language_model (not non-existent self.model.language_model) for delegation and (b) call the standalone loader with (self, weights, adapter_id).
  • Add a GPU-free unit test covering both the fallback arity and the InternVL3 delegation branch.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
lmdeploy/pytorch/models/internvl.py Fixes fallback LoRA loader call to pass self as the model argument.
lmdeploy/pytorch/models/internvl3_hf.py Fixes incorrect delegation target (self.language_model) and corrects fallback loader call signature.
tests/pytorch/test_internvl_lora.py Adds regression tests to ensure correct fallback arity and proper delegation behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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

Labels

Projects

None yet

3 participants