Skip to content

fix(tuners): guard child.weight access in _replace_module for non-linear modules#3213

Open
Chessing234 wants to merge 1 commit into
huggingface:mainfrom
Chessing234:fix/replace-module-guard-child-weight
Open

fix(tuners): guard child.weight access in _replace_module for non-linear modules#3213
Chessing234 wants to merge 1 commit into
huggingface:mainfrom
Chessing234:fix/replace-module-guard-child-weight

Conversation

@Chessing234
Copy link
Copy Markdown
Contributor

Bug: _replace_module in tuners_utils.py unconditionally assigns new_module.weight = child.weight without checking whether child actually has a weight attribute:

if not hasattr(new_module, "base_layer"):
    new_module.weight = child.weight   # crashes if child has no weight
    if hasattr(child, "bias"):
        new_module.bias = child.bias

When modules_to_save contains non-linear module blocks (e.g. Siglip2EncoderLayer, full transformer encoder layers, or any module without a weight parameter), calling merge_and_unload() raises:

AttributeError: 'Siglip2EncoderLayer' object has no attribute 'weight'

Root cause: The bias assignment already uses hasattr(child, "bias") as a guard, but the weight assignment has no equivalent guard. Non-linear modules in modules_to_save legitimately lack weight.

Fix: Add the same hasattr guard for weight that already exists for bias:

if not hasattr(new_module, "base_layer"):
    if hasattr(child, "weight"):
        new_module.weight = child.weight
    if hasattr(child, "bias"):
        new_module.bias = child.bias

This is a one-line addition matching the existing convention in the same block.

…ear modules

When modules_to_save contains non-linear modules (e.g. encoder blocks
without weight attributes), _replace_module unconditionally assigns
child.weight, raising AttributeError. Add hasattr guard to match the
existing pattern used for bias.
@BenjaminBossan
Copy link
Copy Markdown
Member

Thanks for providing this PR. It would be great to add a unit test to catch this error.

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