Skip to content

Reject use_liger_kernel in GMPOTrainer - #7163

Open
YeonwooSung wants to merge 1 commit into
huggingface:mainfrom
YeonwooSung:fix/6808-gmpo-reject-liger
Open

Reject use_liger_kernel in GMPOTrainer#7163
YeonwooSung wants to merge 1 commit into
huggingface:mainfrom
YeonwooSung:fix/6808-gmpo-reject-liger

Conversation

@YeonwooSung

@YeonwooSung YeonwooSung commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

GMPOTrainer only overrides _compute_loss. Inherited GRPOTrainer.compute_loss still routes use_liger_kernel=True through the GRPO fused loss, so training silently skips the geometric-mean objective.

This raises in GMPOTrainer.__init__ before super().__init__, so the Liger path never starts. A Liger GMPO kernel is not added. gmpo.md notes that the inherited flag is unsupported.

Adds test_init_fails_with_liger_kernel.

Fixes #6808

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline, Pull Request section?
  • Was this discussed/approved via a GitHub issue? Please add a link to it if that's the case.
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?

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.

  • No AI usage: the PR was written entirely by a human.
  • AI-assisted: some parts were suggested or improved by AI, but the PR was written and reviewed by a human.
  • AI-generated: the PR was mostly or fully generated by an AI tool.

Who can review?

Anyone in the community is free to review the PR once the tests have passed.


Note

Low Risk
Small guard at trainer init plus docs and a unit test; no change to the GMPO loss path when use_liger_kernel is false (default).

Overview
GMPOTrainer now fails fast when use_liger_kernel=True, because the inherited GRPO/Liger training path would optimize the GRPO fused loss instead of GMPO’s geometric-mean objective.

A ValueError is raised in GMPOTrainer.__init__ before GRPOTrainer.__init__, so the Liger path never starts. gmpo.md documents that use_liger_kernel is inherited from GRPOConfig but unsupported. test_init_fails_with_liger_kernel asserts construction raises with the expected message.

Fixes #6808.

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

The inherited Liger path runs the GRPO fused loss and skips the
geometric-mean objective. Fail at init instead of training the wrong loss.
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.

GMPO: use_liger_kernel=True silently bypasses the GMPO loss; MoE aux loss and entropy bonus dropped (gaps not covered by #6095)

1 participant