fix(peft_utils): raise actionable ValueError when rank_dict is empty in get_peft_kwargs#13759
Open
dparikh79 wants to merge 1 commit into
Open
Conversation
…in get_peft_kwargs
`get_peft_kwargs` is reached after callers (e.g.
`_load_lora_into_text_encoder` and the various
`load_lora_into_{unet,transformer}` flows) walk the model's
`named_modules` and probe the state_dict for
`{module}.lora_B.weight` keys. When the saved LoRA state_dict
carries an extra or missing prefix versus the target model
(e.g. `text_model.encoder.*` vs `encoder.*`), an adapter-name
infix between `lora_B` and `weight` (e.g. `.default_0.`), or a
non-diffusers serialization format that was not converted
upstream, none of the probed keys are present in the state_dict
and `rank_dict` arrives here empty.
The first statement of the function was
`r = lora_alpha = list(rank_dict.values())[0]`, which then
raised an unhelpful `IndexError: list index out of range` with
no signal about which mismatch was actually responsible (see
huggingface/peft#3238 for a real-world report against SDXL +
peft 0.19; the report was filed against peft but the failure
path is in diffusers).
Raise a `ValueError` whose message names the common causes and
shows a sample of the state_dict keys, so the next user with a
key-mismatch can diagnose their specific case from the
exception alone rather than from a 5-frame deep IndexError.
Adds focused tests for the new error path, the safe-on-None
behavior, and the unchanged fast-path.
Refs: huggingface/peft#3238
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refs huggingface/peft#3238 (the issue was filed against peft, but the failure path is here in diffusers).
get_peft_kwargsis reached from every diffusers LoRA loader (_load_lora_into_text_encoder,load_lora_into_unet,load_lora_into_transformer, etc.). Each caller walksmodel.named_modules()and probes the state_dict for{module_name}.lora_B.weightkeys, building arank_dict. When that loop matches nothing (typical when the saved keys carry an extra/missing prefix vs the target model, an adapter-name infix betweenlora_Bandweight, or a non-diffusers serialization format that was not converted upstream),rank_dictarrives atget_peft_kwargsempty.The first statement of the function is:
which raises
IndexError: list index out of rangewith no signal about which mismatch is actually responsible. The traceback ends insideget_peft_kwargswith no way to tell whether you have a prefix problem, an adapter-name infix problem, a format-conversion problem, or just a state_dict that genuinely has no LoRA weights at all.This PR raises a
ValueErrorahead of the indexing, with a message that names the common causes and shows a sample of the state_dict keys. Future bug reports surface the actual mismatch shape in the user's report instead of a cryptic IndexError.Scope
src/diffusers/utils/peft_utils.py, functionget_peft_kwargs, ~20 added lines (a guard + diagnostic message).rank_dictis non-empty, the existing first line runs unchanged.tests/others/test_peft_utils.pywith three focused unit tests (emptyrank_dictraises,Nonestate_dict is safe, fast-path unchanged).This is intentionally a defensive surface-area fix only. The underlying SDXL/SD3.5 key-mismatch in #3238 is a separate concern (likely in
convert_state_dict_to_peftor the per-pipeline_load_lora_into_text_encoderrank-discovery loops); diagnosing it requires reproducing the user's specific state_dict layout. Surfacing the actionable error here is the prerequisite for collecting that data from future reporters.Test plan
tests/others/test_peft_utils.py(new):test_empty_rank_dict_raises_actionable_value_error; exercises the new path; assertsValueError, asserts message names the underlying mismatch and includes a state_dict key sample.test_empty_rank_dict_with_none_state_dict_is_safe; covers thepeft_state_dict=Nonedefensive branch.test_non_empty_rank_dict_unchanged; smoke-tests the happy path (r=4,lora_alpha=4, norank_pattern, expectedtarget_modules).sys.pathimport without needing the full diffusers install (the function is pure utility code).AI Assistance Disclosure
Investigation, the patch, and the tests were drafted with Claude assistance. I read the call graph from each caller (
_load_lora_into_text_encoder,load_lora_into_unet) intoget_peft_kwargsto confirmrank_dictcan legitimately be empty under valid-input + key-mismatch conditions, and that the change preserves the happy path bitwise. I am the human contributor accountable for this PR.