Fail fast when GRPO combines Liger with the MoE auxiliary loss - #7161
Open
YeonwooSung wants to merge 1 commit into
Open
Fail fast when GRPO combines Liger with the MoE auxiliary loss#7161YeonwooSung wants to merge 1 commit into
YeonwooSung wants to merge 1 commit into
Conversation
Liger fuses the GRPO loss without materializing router logits, so the MoE load-balancing term was silently dropped. Raise the same error DPO and KTO already use.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
GRPOTraineralready setsaux_loss_enabledfor Mixture-of-Experts models, butuse_liger_kernel=Trueroutescompute_lossthroughcompute_liger_loss, which never sees router logits. The load-balancing term was dropped with no error.DPO and KTO already raise in this combination. This copies that fail-fast into GRPO (
Liger GRPO loss does not support...) and addstest_init_fails_with_moe_aux_loss_and_ligerontiny-Qwen3MoeForCausalLM.Fixes #7009
Before submitting
AI writing disclosure
We welcome the use of AI tools to help with contributions. For transparency and to help us improve our review process, please indicate the level of AI involvement in this PR.
Who can review?
Anyone in the community is free to review the PR once the tests have passed.
Note
Low Risk
Init-time validation only; no change to training paths except blocking an already-unsupported silent misconfiguration.
Overview
GRPO now rejects Liger + MoE aux loss at init, matching DPO/KTO behavior. When a MoE model has load-balancing enabled (
router_aux_loss_coefnon-zero) anduse_liger_kernel=True,GRPOTrainerraises aValueErrorexplaining that the fused Liger path cannot use router logits, with guidance to disable the aux loss or turn off Liger.Adds
test_init_fails_with_moe_aux_loss_and_liger(Liger-gated) usingtiny-Qwen3MoeForCausalLMto assert that error on construction.Reviewed by Cursor Bugbot for commit 7566981. Bugbot is set up for automated code reviews on this repo. Configure here.