Skip to content

[BUG FIX] tuners/lora/eva: fix label mask guard using 'in' instead of hasattr for dict#3247

Open
Dev-X25874 wants to merge 2 commits into
huggingface:mainfrom
Dev-X25874:fix/eva-label-mask-dict-check-v2
Open

[BUG FIX] tuners/lora/eva: fix label mask guard using 'in' instead of hasattr for dict#3247
Dev-X25874 wants to merge 2 commits into
huggingface:mainfrom
Dev-X25874:fix/eva-label-mask-dict-check-v2

Conversation

@Dev-X25874
Copy link
Copy Markdown

@Dev-X25874 Dev-X25874 commented May 20, 2026

What

This is a clean re-open of #3233 (closed due to formatting issues across commits).
All review comments from @BenjaminBossan have been addressed from the
start in this PR.

In prepare_model_inputs_fn_language_modeling, the label mask guard uses
hasattr(model_input, "labels") to check whether labels are present. However,
model_input is a plain Python dict (enforced by the isinstance check three
lines above), and hasattr on a plain dict always returns False regardless of
its keys — dict keys are not exposed as object attributes.

This means the label masking branch (use_label_mask=True) has silently never
executed, even though it is the default in EvaConfig. As a result, ignored /
padding positions (label value −100) were included in the SVD computation
during EVA initialization.

Fix

Replace hasattr(model_input, "labels") with ("labels" in model_input), which
is the correct Python idiom for dict key membership. The body of the branch
already uses model_input["labels"] (subscript access), so only the guard was
wrong. No downstream side-effects for callers that do not include a "labels" key.

# Before
if peft_config.eva_config.use_label_mask and hasattr(model_input, "labels"):

# After
if peft_config.eva_config.use_label_mask and ("labels" in model_input):

Tests

Added test_eva_label_mask, parametrized over use_label_mask=True/False, which
runs get_eva_state_dict on a dataset augmented with a labels column and
asserts the returned state dict is non-empty in both cases. This directly exercises
the previously dead code path.

cc @BenjaminBossan @sirluk

@HuggingFaceDocBuilderDev
Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Copy link
Copy Markdown
Member

@BenjaminBossan BenjaminBossan left a comment

Choose a reason for hiding this comment

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

Okay, looks good from my side. Let's wait a week or so to see if Sirluk has time to check. Otherwise, I'll go ahead and merge.

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