Skip to content

Raise ValueError instead of NotImplementedError for Liger DPO with PEFT#6225

Merged
albertvillanova merged 2 commits into
mainfrom
dpo-peft-liger-error
Jul 1, 2026
Merged

Raise ValueError instead of NotImplementedError for Liger DPO with PEFT#6225
albertvillanova merged 2 commits into
mainfrom
dpo-peft-liger-error

Conversation

@albertvillanova

@albertvillanova albertvillanova commented Jun 30, 2026

Copy link
Copy Markdown
Member

This PR changes the exception raised by DPOTrainer when use_liger_kernel=True is combined with a PEFT model, from NotImplementedError to ValueError.

Motivation

NotImplementedError (a subclass of RuntimeError) is meant for abstract methods that subclasses must override, or as a development placeholder. Rejecting an invalid combination of constructor arguments is not that case: the user passed arguments of the right type whose combination is unsupported, which is idiomatically a ValueError.

This also restores consistency. The two sibling Liger guards in the same block (compute_metrics and precompute_ref_log_probs) already raise ValueError, as does the analogous Liger + PEFT guard in GRPOTrainer.

Changes

  • Raise ValueError instead of NotImplementedError when use_liger_kernel=True is used with a PEFT model in DPOTrainer.
  • Reword the error message to state the unsupported combination and the actionable remedy (use_liger_kernel=False).
  • Update the corresponding test to expect ValueError and the new message.

Note

Low Risk
Only the exception type and message change at init for an already-unsupported configuration; training behavior is unchanged for valid setups.

Overview
When DPOTrainer is constructed with use_liger_kernel=True and a PEFT model, it now raises ValueError instead of NotImplementedError, matching the other Liger configuration guards in the same block and the style used in GRPOTrainer.

The error text now states that the flag combination is unsupported and tells callers to set use_liger_kernel=False to train with PEFT. test_init_fails_with_peft_and_liger expects ValueError and the updated message.

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

@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.

@albertvillanova albertvillanova merged commit 61b9ba0 into main Jul 1, 2026
13 of 14 checks passed
@albertvillanova albertvillanova deleted the dpo-peft-liger-error branch July 1, 2026 05:13
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