Skip to content

policy_loss is never logged by GMPOTrainer, gspo_token's GRPOTrainer, or RLOOTrainer #7005

Description

@behroozazarkhalili

GRPOTrainer logs a policy_loss metric. Three trainers that expose the same training interface never log it, so a consumer that reads policy_loss out of log_history gets a KeyError on all three.

Which trainers, and why

Each of the three overrides _compute_loss with a self-contained implementation. None calls super()._compute_loss(), so the parent's metric block is never reached, and none appends policy_loss of its own. The table below comes from resolving the method against the imported classes at runtime:

Trainer supplies _compute_loss calls super()._compute_loss() logs policy_loss
trl.trainer.grpo_trainer.GRPOTrainer itself no yes
trl.experimental.gmpo.gmpo_trainer.GMPOTrainer itself no no
trl.experimental.gspo_token.grpo_trainer.GRPOTrainer itself no no
trl.trainer.rloo_trainer.RLOOTrainer itself no no

AsyncGRPOTrainer is a separate case and falls outside this report. Its MRO is [AsyncGRPOTrainer, _BaseTrainer, Trainer, object], so it does not inherit from GRPOTrainer and has no _compute_loss anywhere to override.

The gap predates #6861

On that PR's merge base the three trainers contain zero occurrences of policy_loss, and they still contain zero. What #6861 changes is GRPO's own logging, from conditional on an entropy bonus to unconditional. After it lands, a GRPO run always reports policy_loss and a run on any of the three above still reports nothing.

docs/source/grpo_trainer.md documents policy_loss in the GRPO metric list.

What a fix has to decide

The capture point depends on how each trainer builds its loss, so the append cannot be copied across unchanged:

  • GMPOTrainer aggregates a geometric mean over sequences and then divides by the accumulation normalizer before returning. It also has no entropy bonus block at all, so "the loss before the entropy bonus" has no direct counterpart there.
  • RLOOTrainer builds its loss its own way and needs its own answer to the same question.

Each trainer therefore needs a capture point chosen deliberately, and a test that pins it.

How the table was produced

I resolved _compute_loss through each class's real MRO, disassembled the resolved function to look for a super() call naming it, and scanned its constants for the metric name. One detail matters for anyone repeating this: two different modules are named grpo_trainer.py and both define a class called GRPOTrainer, so identifying them by basename collapses trl/experimental/gspo_token/grpo_trainer.py onto trl/trainer/grpo_trainer.py and reports the override as an inheritance.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions