Skip to content

Fix dict iteration bug in get_peft_state_maybe_zero_3#153

Open
Mr-Neutr0n wants to merge 1 commit intoBAAI-DCAI:mainfrom
Mr-Neutr0n:fix/peft-state-dict-iteration
Open

Fix dict iteration bug in get_peft_state_maybe_zero_3#153
Mr-Neutr0n wants to merge 1 commit intoBAAI-DCAI:mainfrom
Mr-Neutr0n:fix/peft-state-dict-iteration

Conversation

@Mr-Neutr0n
Copy link

Summary

Fixes two bugs in the lora_only branch of get_peft_state_maybe_zero_3 in bunny/train/train.py:

  1. Missing .items() on dict iteration: for k, t in maybe_lora_bias iterates over dict keys (strings), causing Python to unpack each key string into individual characters rather than yielding key-value pairs. This would raise a ValueError at runtime if any key is longer than 2 characters. Fixed by using maybe_lora_bias.items().

  2. Stale variable reference: bias_name in the second loop referenced the last value assigned during the first loop's iteration, rather than the current key k. This meant only the last bias name computed in the first loop was ever checked/stored, regardless of which bias parameter was actually being processed. Fixed by replacing bias_name with k.

Before

for k, t in maybe_lora_bias:
    if bias_name in lora_bias_names:
        to_return[bias_name] = t

After

for k, t in maybe_lora_bias.items():
    if k in lora_bias_names:
        to_return[k] = t

Test plan

  • Verified the fix matches the equivalent logic in LLaVA's train.py where this function was originally sourced from
  • The corrected code properly iterates over the bias dict items and checks each key individually against the collected lora bias names

Fix two bugs in the lora_only bias handling branch:

1. `for k, t in maybe_lora_bias` iterates over dict keys (strings),
   causing unpacking to split a key string into individual characters
   instead of yielding key-value pairs. Fixed by using `.items()`.

2. `bias_name` referenced the last value assigned in the previous
   loop rather than the current key `k`, so the wrong bias name was
   being checked and stored. Fixed by replacing `bias_name` with `k`.
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.

1 participant