Skip to content

compat peft 0.19#80

Merged
Jintao-Huang merged 2 commits into
modelscope:mainfrom
Jintao-Huang:compat_peft_019_
May 15, 2026
Merged

compat peft 0.19#80
Jintao-Huang merged 2 commits into
modelscope:mainfrom
Jintao-Huang:compat_peft_019_

Conversation

@Jintao-Huang
Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces compatibility for peft version 0.19.0 and above by refactoring the update_layer method in src/mcore_bridge/tuners/lora.py to extract configuration parameters from a config object within kwargs. A critical issue was identified where the code could raise a KeyError if the config object is missing from kwargs while the peft_019 flag is enabled, and a suggestion was provided to safely check for the key's existence.

Comment thread src/mcore_bridge/tuners/lora.py Outdated
@Jintao-Huang
Copy link
Copy Markdown
Collaborator Author

/gemini review

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces compatibility for PEFT 0.19.0 in the LoRA tuner by modifying the update_layer method to handle configuration parameters passed via a config object. The reviewer identified a potential TypeError because lora_alpha is still defined as a mandatory keyword argument, which conflicts with how PEFT 0.19.0 provides arguments. Furthermore, the feedback points out that lora_alpha is missing from the extraction logic and recommends using .get() for safer dictionary access to ensure backward compatibility.

Comment thread src/mcore_bridge/tuners/lora.py
@Jintao-Huang Jintao-Huang merged commit 3848780 into modelscope:main May 15, 2026
1 check passed
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