Skip to content

FIX Accept layers_to_transform=0 together with layers_pattern - #3425

Merged
githubnemo merged 1 commit into
huggingface:mainfrom
winklemad:fix-layers-to-transform-index-zero
Aug 13, 2026
Merged

githubnemo merged 1 commit into
huggingface:mainfrom
winklemad:fix-layers-to-transform-index-zero

Conversation

@winklemad

Copy link
Copy Markdown
Contributor

No linked issue — found by reading the code.

What this fixes

Every tuner config validates the layers_pattern / layers_to_transform pairing in __post_init__ with:

if self.layers_pattern and not self.layers_to_transform:
    raise ValueError("When `layers_pattern` is specified, `layers_to_transform` must also be specified.")

Because 0 is falsy, passing the valid first-layer index layers_to_transform=0 together with a layers_pattern wrongly raises — even though layers_to_transform was specified:

from peft import LoraConfig

LoraConfig(target_modules=["q_proj"], layers_to_transform=0, layers_pattern="blocks")
# ValueError: When `layers_pattern` is specified, `layers_to_transform` must also be specified.

LoraConfig(target_modules=["q_proj"], layers_to_transform=1,  layers_pattern="blocks")  # OK
LoraConfig(target_modules=["q_proj"], layers_to_transform=[0], layers_pattern="blocks")  # OK

Only index 0 is affected — 1 and [0] (the same logical request) are accepted.

Why is None is the correct check

  • The consuming matcher already treats 0 as a valid index — check_target_module_exists in src/peft/tuners/tuners_utils.py gates on layer_indexes is not None, and the sibling check a few lines above in the same __post_init__ also uses self.layers_to_transform is not None.
  • The docstring states the pattern is "used only if layers_to_transform is different from None."

So the guard should compare against None, not truthiness. The check was added in #2159 and its message improved in #2169, but the falsy-0 case remained.

Scope

The identical guard is duplicated across all 19 tuner configs that support layer indexing (lora, adalora, boft, deft, delora, fourierft, frod, hra, lily, loha, lokr, oft, peanut, psoft, pvera, tinylora, vblora, vera, waveft), so the one-line fix is applied to every one.

Testing

Added test_config_layers_to_transform_index_zero, parametrized over every config in ALL_CONFIG_CLASSES that supports layers_to_transform (25 configs), asserting layers_to_transform=0 + layers_pattern is accepted. It fails before this change (19 of the configs raise) and passes after. ruff check / ruff format --check are clean and tests/test_config.py passes (1218 passed).

The config `__post_init__` guarded the `layers_pattern`/`layers_to_transform`
pairing with `if self.layers_pattern and not self.layers_to_transform:`. Since
0 is falsy, passing the valid first-layer index `layers_to_transform=0`
alongside a `layers_pattern` wrongly raised "When `layers_pattern` is
specified, `layers_to_transform` must also be specified." even though it was
specified. `layers_to_transform=1` or `[0]` were accepted, so only index 0 was
rejected.

The consuming matcher already treats 0 as a valid index
(`check_target_module_exists` in `tuners_utils.py` uses `layer_indexes is not
None`), and the docstring states the pattern is "used only if
`layers_to_transform` is different from None", so the guard should compare
against None. This check was duplicated verbatim across all tuner configs, so
it is fixed in every one.

Added a parametrized test covering every config that supports layer indexing.
ErenAta16

This comment was marked as spam.

@github-actions

Copy link
Copy Markdown

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

@githubnemo githubnemo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, thanks :)

@githubnemo
githubnemo merged commit 82f4c02 into huggingface:main Aug 13, 2026
9 of 10 checks 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.

3 participants