-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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.
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
Allow user to specify huggingface link or local path to pretrained lora weights #3572
Changes from 11 commits
4ee94bd
e242ce8
1c00724
650e5e6
c18e9e8
d92190a
1fbd1b2
a8b674f
d3d66ef
4578552
4c63915
ac439cd
9b94d91
9e5dfec
f6695bb
4355353
1ab20cc
effcce0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -477,7 +477,7 @@ def check_llm_finetuning_output_feature_config(config: "ModelConfig"): # noqa: | |
if config.model_type != MODEL_LLM: | ||
return | ||
|
||
if config.trainer.type != "finetune": | ||
if config.trainer.type != "finetune" and config.adapter.pretrained_adapter_weights is not None: | ||
return | ||
|
||
if config.output_features[0].type != TEXT: | ||
|
@@ -493,6 +493,9 @@ def check_llm_finetuning_trainer_config(config: "ModelConfig"): # noqa: F821 | |
if config.model_type != MODEL_LLM: | ||
return | ||
|
||
if config.trainer.type == "none" and config.adapter.pretrained_adapter_weights is not None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be more simply if config.trainer.type == "none":
# The NoneTrainer for ZS is valid.
return There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But in this case, we would load in untrained LoRA weights if pretrained adapter weights weren't specified in the config, right? Would that be a problem? |
||
return | ||
|
||
if config.adapter is not None and config.trainer.type != "finetune": | ||
raise ConfigValidationError("LLM finetuning requires trainer type to be finetune.") | ||
|
||
|
@@ -508,7 +511,7 @@ def check_llm_finetuning_backend_config(config: "ModelConfig"): # noqa: F821 | |
return | ||
|
||
# LLM finetuning is only supported by the finetune trainer type | ||
if config.trainer.type != "finetune": | ||
if config.trainer.type != "finetune" and config.adapter.pretrained_adapter_weights is not None: | ||
return | ||
|
||
# Using local backend, so skip the checks below | ||
|
@@ -528,9 +531,8 @@ def check_llm_finetuning_backend_config(config: "ModelConfig"): # noqa: F821 | |
def check_llm_finetuning_adalora_config(config: "ModelConfig"): | ||
"""Checks that the adalora adapter is configured correctly. | ||
|
||
It requires a set of target_modules to be specified in the config for the model. If it isn't specified by the user, | ||
we also check against PEFT's predefined target module list for ADALORA to see if this key is present there. If | ||
neither is true, AdaloraModel will run into issues downstream. | ||
We check against PEFT's predefined target module list for ADALORA to see if this target_modules is present there. If | ||
not, AdaloraModel will run into issues downstream. | ||
""" | ||
if config.model_type != MODEL_LLM: | ||
return | ||
|
@@ -544,10 +546,7 @@ def check_llm_finetuning_adalora_config(config: "ModelConfig"): | |
from peft.utils import TRANSFORMERS_MODELS_TO_ADALORA_TARGET_MODULES_MAPPING | ||
|
||
model_config = _get_llm_model_config(config.base_model) | ||
if ( | ||
not config.adapter.target_modules | ||
and model_config.model_type not in TRANSFORMERS_MODELS_TO_ADALORA_TARGET_MODULES_MAPPING | ||
): | ||
if model_config.model_type not in TRANSFORMERS_MODELS_TO_ADALORA_TARGET_MODULES_MAPPING: | ||
raise ConfigValidationError( | ||
f"Adalora adapter is not supported for {model_config.model_type} model. " | ||
f"Supported model types are: {list(TRANSFORMERS_MODELS_TO_ADALORA_TARGET_MODULES_MAPPING.keys())}. " | ||
|
@@ -601,7 +600,9 @@ def check_llm_quantization_backend_incompatibility(config: "ModelConfig") -> Non | |
@register_config_check | ||
def check_qlora_requirements(config: "ModelConfig") -> None: # noqa: F821 | ||
"""Checks that all the necessary settings are in place for QLoRA.""" | ||
if config.model_type != MODEL_LLM or config.trainer.type == "none": | ||
if config.model_type != MODEL_LLM or ( | ||
config.trainer.type == "none" and config.adapter.pretrained_adapter_weights is not None | ||
): | ||
return | ||
|
||
if config.quantization and (not config.adapter or config.adapter.type != "lora"): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be an OR? Why does specifying
pretrained_adapter_weights
no longer require that the first output feature be TEXT?Or is it that we want to make it so that using the none trainer type doesn't require an output feature?
CC: @arnavgarg1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this was an oversight on my part. I was trying to go through the code to see where my change might break something down the line, and I might have gotten a little overzealous.