Skip to content

Fix PEFT ensure_weight_tying warning in liger + PEFT GRPO tests#6188

Merged
albertvillanova merged 2 commits into
mainfrom
fix-warnings-peft-ensure_weight_tying
Jun 30, 2026
Merged

Fix PEFT ensure_weight_tying warning in liger + PEFT GRPO tests#6188
albertvillanova merged 2 commits into
mainfrom
fix-warnings-peft-ensure_weight_tying

Conversation

@albertvillanova

@albertvillanova albertvillanova commented Jun 26, 2026

Copy link
Copy Markdown
Member

This PR fixes two UserWarnings emitted by PEFT in CI when running the liger + PEFT tests in test_grpo_trainer.py.

Motivation

PEFT ≥ 0.18.0 warns when a model with tie_word_embeddings=True has a tied layer (e.g. lm_head) included in the adapter via modules_to_save or target_modules, but ensure_weight_tying is not set to True on the LoraConfig. Both affected tests deliberately include lm_head in the adapter config on a model with tied embeddings, so the warning fires as a side effect.

Solution

For PEFT ≥ 0.18.0 (modules_to_save case) and ≥ 0.19.0 (target_modules case), pass ensure_weight_tying=True to LoraConfig to suppress the warning at the source. For older PEFT versions that predate the flag, fall back to pytest.warns to explicitly acknowledge the expected warning.

Changes

Updated tests to handle PEFT version differences by conditionally setting ensure_weight_tying and using contextlib.nullcontext() or pytest.warns as appropriate, ensuring correct handling of warnings and errors across PEFT versions:

  • In test_liger_kernel_with_peft_lm_head_raises: set ensure_weight_tying=True on LoraConfig for PEFT ≥ 0.19.0, with a pytest.warns fallback for older versions
  • In test_liger_kernel_with_peft_modules_to_save_lm_head_allowed: set ensure_weight_tying=True on LoraConfig for PEFT ≥ 0.18.0, with a pytest.warns fallback for older versions
  • Add import contextlib and import peft (under the is_peft_available() guard) to support the version-gated logic

Note

Low Risk
Test-only changes to LoRA config setup; no production trainer or library logic is modified.

Overview
Silences PEFT weight-tying UserWarnings in CI for two existing Liger + PEFT GRPO tests that intentionally put lm_head in the adapter on tied-embedding models.

Adds import peft (behind is_peft_available()) and builds LoraConfig with ensure_weight_tying=True when peft >= 0.19.0 in test_liger_kernel_with_peft_lm_head_raises and test_liger_kernel_with_peft_modules_to_save_lm_head_allowed. Test behavior (expected ValueError vs successful construction) is unchanged.

Reviewed by Cursor Bugbot for commit 80ac694. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 402bfcf. Configure here.

Comment thread tests/test_grpo_trainer.py Outdated
@bot-ci-comment

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.

@qgallouedec qgallouedec left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

thks

@albertvillanova albertvillanova merged commit a277092 into main Jun 30, 2026
13 checks passed
@albertvillanova albertvillanova deleted the fix-warnings-peft-ensure_weight_tying branch June 30, 2026 06:05
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