Aliases for qwen3-vl name map#625
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the _infer_prefix_and_remap function in unsloth_zoo/saving_utils.py to handle specific key aliasing for LoRA weights by checking for predefined source and target prefix mappings. The review feedback suggests moving the hardcoded prefix mapping tuple outside the loop to improve efficiency by avoiding redundant object creation in each iteration.
| for source_prefix, target_prefix in ( | ||
| ("model.visual.", "visual."), | ||
| ("model.language_model.", "model."), | ||
| ("language_model.", "model."), | ||
| ): |
There was a problem hiding this comment.
The tuple of prefix mappings is defined inside the loop, which means it is re-created for every iteration of the lora_weights loop. While this is not a functional bug, it is inefficient. Consider moving this tuple definition outside the loop to avoid unnecessary object creation.
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9fbeea5e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if aliased_key is not None: | ||
| remapped[aliased_key] = v | ||
| changed = True | ||
| continue |
There was a problem hiding this comment.
When a Qwen3-VL-MoE adapter has regular layers that hit this alias path (for example model.language_model...q_proj -> model...q_proj) plus fused expert LoRA entries that do not have a direct .weight safetensor counterpart, this sets changed but never records any inferred namespace for unmatched_keys. The fallback below only prefixes deferred keys when inferred_prefixes is populated, so those expert keys remain under the old model.language_model...experts name and _merge_moe_experts_file looks for the safetensor-side model...experts prefix instead, silently skipping the MoE LoRA merge for the model this change is meant to support.
Useful? React with 👍 / 👎.
1 participant
Fixes: unslothai/unsloth#5290