Skip to content

Commit 29aed0b

Browse files
test(online_dpo): pin eval_loss to the batch losses and refuse compute_metrics
The three evaluate tests bounded the magnitude of eval_loss, which a prediction_step returning any constant satisfies. They now record every _compute_loss call made during evaluate() and pin eval_loss to the mean of those losses weighted by batch size, as Trainer aggregates them: the 17-row split leaves a final batch of one example, so an unweighted mean misses by 0.17 on NashMD. One call per evaluation batch, statistics off. prediction_step returns no predictions or labels, so a compute_metrics callback passed to these trainers was accepted and never called. OnlineDPO, NashMD and XPO now raise a ValueError for it and the docstrings say so.
1 parent 7c381f9 commit 29aed0b

6 files changed

Lines changed: 93 additions & 18 deletions

File tree

tests/experimental/test_nash_md_trainer.py

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -167,11 +167,26 @@ def test_evaluate(self):
167167
processing_class=self.tokenizer,
168168
)
169169

170-
# A hardcoded return value would satisfy a bare membership check, so assert the loss itself. Its sign is
171-
# not fixed across these trainers, so bound the magnitude instead: non-zero rules out a stubbed
172-
# `prediction_step`, and finite rules out inf and NaN.
170+
# A stubbed `prediction_step` returning any constant passes a magnitude bound, so pin `eval_loss` to the losses
171+
# `_compute_loss` produced on the evaluation batches, weighted by batch size as `Trainer` does.
172+
recorded = []
173+
original = trainer._compute_loss
174+
175+
def spy(model, inputs, log_stats=True):
176+
loss = original(model, inputs, log_stats=log_stats)
177+
batch_size = len(next(iter(inputs.values())))
178+
recorded.append((log_stats, batch_size, loss.item()))
179+
return loss
180+
181+
trainer._compute_loss = spy
173182
metrics = trainer.evaluate()
174-
assert 0 < abs(metrics["eval_loss"]) < float("inf")
183+
184+
assert len(recorded) == len(trainer.get_eval_dataloader())
185+
assert all(log_stats is False for log_stats, _, _ in recorded)
186+
expected_loss = sum(batch_size * loss for _, batch_size, loss in recorded) / sum(
187+
batch_size for _, batch_size, _ in recorded
188+
)
189+
assert metrics["eval_loss"] == pytest.approx(expected_loss, abs=1e-5)
175190

176191
def test_evaluate_does_not_pollute_training_stats(self):
177192
# `self.stats` is averaged and cleared by the training logger, so appending evaluation values to it would

tests/experimental/test_online_dpo_trainer.py

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -148,11 +148,41 @@ def test_evaluate(self):
148148
reward_processing_classes=self.reward_tokenizer,
149149
)
150150

151-
# A hardcoded return value would satisfy a bare membership check, so assert the loss itself. Its sign is
152-
# not fixed across these trainers, so bound the magnitude instead: non-zero rules out a stubbed
153-
# `prediction_step`, and finite rules out inf and NaN.
151+
# A stubbed `prediction_step` returning any constant passes a magnitude bound, so pin `eval_loss` to the losses
152+
# `_compute_loss` produced on the evaluation batches, weighted by batch size as `Trainer` does.
153+
recorded = []
154+
original = trainer._compute_loss
155+
156+
def spy(model, inputs, log_stats=True):
157+
loss = original(model, inputs, log_stats=log_stats)
158+
batch_size = len(next(iter(inputs.values())))
159+
recorded.append((log_stats, batch_size, loss.item()))
160+
return loss
161+
162+
trainer._compute_loss = spy
154163
metrics = trainer.evaluate()
155-
assert 0 < abs(metrics["eval_loss"]) < float("inf")
164+
165+
assert len(recorded) == len(trainer.get_eval_dataloader())
166+
assert all(log_stats is False for log_stats, _, _ in recorded)
167+
expected_loss = sum(batch_size * loss for _, batch_size, loss in recorded) / sum(
168+
batch_size for _, batch_size, _ in recorded
169+
)
170+
assert metrics["eval_loss"] == pytest.approx(expected_loss, abs=1e-5)
171+
172+
def test_compute_metrics_is_rejected(self):
173+
# `prediction_step` returns only a loss, so a `compute_metrics` callback would never run; refuse it up front.
174+
training_args = OnlineDPOConfig(output_dir=self.tmp_dir, report_to="none")
175+
dataset = load_dataset("trl-internal-testing/zen", "standard_prompt_only", split="train")
176+
with pytest.raises(ValueError, match="`compute_metrics` is not supported"):
177+
OnlineDPOTrainer(
178+
model=self.model,
179+
reward_funcs=self.reward_model,
180+
args=training_args,
181+
train_dataset=dataset,
182+
processing_class=self.tokenizer,
183+
reward_processing_classes=self.reward_tokenizer,
184+
compute_metrics=lambda eval_prediction: {},
185+
)
156186

157187
def test_evaluate_does_not_pollute_training_stats(self):
158188
# `self.stats` is averaged and cleared by the training logger, so appending evaluation values to it would

tests/experimental/test_xpo_trainer.py

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -117,11 +117,26 @@ def test_evaluate(self):
117117
processing_class=self.tokenizer,
118118
)
119119

120-
# A hardcoded return value would satisfy a bare membership check, so assert the loss itself. Its sign is
121-
# not fixed across these trainers, so bound the magnitude instead: non-zero rules out a stubbed
122-
# `prediction_step`, and finite rules out inf and NaN.
120+
# A stubbed `prediction_step` returning any constant passes a magnitude bound, so pin `eval_loss` to the losses
121+
# `_compute_loss` produced on the evaluation batches, weighted by batch size as `Trainer` does.
122+
recorded = []
123+
original = trainer._compute_loss
124+
125+
def spy(model, inputs, log_stats=True):
126+
loss = original(model, inputs, log_stats=log_stats)
127+
batch_size = len(next(iter(inputs.values())))
128+
recorded.append((log_stats, batch_size, loss.item()))
129+
return loss
130+
131+
trainer._compute_loss = spy
123132
metrics = trainer.evaluate()
124-
assert 0 < abs(metrics["eval_loss"]) < float("inf")
133+
134+
assert len(recorded) == len(trainer.get_eval_dataloader())
135+
assert all(log_stats is False for log_stats, _, _ in recorded)
136+
expected_loss = sum(batch_size * loss for _, batch_size, loss in recorded) / sum(
137+
batch_size for _, batch_size, _ in recorded
138+
)
139+
assert metrics["eval_loss"] == pytest.approx(expected_loss, abs=1e-5)
125140

126141
def test_evaluate_does_not_pollute_training_stats(self):
127142
# `self.stats` is averaged and cleared by the training logger, so appending evaluation values to it would

trl/experimental/nash_md/nash_md_trainer.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,8 +142,8 @@ class NashMDTrainer(OnlineDPOTrainer):
142142
peft_config ([`~peft.PeftConfig`], *optional*):
143143
The peft config to use for training.
144144
compute_metrics (`Callable[[EvalPrediction], dict]`, *optional*):
145-
The function to use to compute the metrics. Must take a `EvalPrediction` and return a dictionary string to
146-
metric values.
145+
Not supported: evaluation computes only a loss, since `prediction_step` returns no predictions or labels
146+
for the callback. Passing a callable raises a `ValueError`.
147147
callbacks (`list[transformers.TrainerCallback]`):
148148
The callbacks to use for training.
149149
optimizers (`tuple[torch.optim.Optimizer, torch.optim.lr_scheduler.LambdaLR]`):
@@ -189,6 +189,11 @@ def __init__(
189189
optimizers: tuple[torch.optim.Optimizer, torch.optim.lr_scheduler.LambdaLR] = (None, None),
190190
preprocess_logits_for_metrics: Callable[[torch.Tensor, torch.Tensor], torch.Tensor] | None = None,
191191
) -> None:
192+
if compute_metrics is not None:
193+
raise ValueError(
194+
"`compute_metrics` is not supported: `prediction_step` returns only a loss, so the callback would never "
195+
"be called. Read `eval_loss` from `evaluate()` instead."
196+
)
192197
super().__init__(
193198
model=model,
194199
ref_model=ref_model,

trl/experimental/online_dpo/online_dpo_trainer.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,8 @@ class OnlineDPOTrainer(_BaseTrainer):
149149
peft_config ([`~peft.PeftConfig`], *optional*):
150150
PEFT configuration used to wrap the model. If `None`, the model is not wrapped.
151151
compute_metrics (`Callable[[EvalPrediction], dict]`, *optional*):
152-
The function to use to compute the metrics. Must take a `EvalPrediction` and return a dictionary string to
153-
metric values.
152+
Not supported: evaluation computes only a loss, since `prediction_step` returns no predictions or labels
153+
for the callback. Passing a callable raises a `ValueError`.
154154
callbacks (`list[transformers.TrainerCallback]`):
155155
The callbacks to use for training.
156156
optimizers (`tuple[torch.optim.Optimizer, torch.optim.lr_scheduler.LambdaLR]`):
@@ -403,6 +403,11 @@ def __init__(
403403
args.gradient_checkpointing_kwargs = args.gradient_checkpointing_kwargs or {}
404404
args.gradient_checkpointing_kwargs.setdefault("use_reentrant", False)
405405

406+
if compute_metrics is not None:
407+
raise ValueError(
408+
"`compute_metrics` is not supported: `prediction_step` returns only a loss, so the callback would never "
409+
"be called. Read `eval_loss` from `evaluate()` instead."
410+
)
406411
super().__init__(
407412
model=model,
408413
args=args,

trl/experimental/xpo/xpo_trainer.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,8 @@ class XPOTrainer(OnlineDPOTrainer):
8585
peft_config ([`~peft.PeftConfig`], *optional*):
8686
The peft config to use for training.
8787
compute_metrics (`Callable[[EvalPrediction], dict]`, *optional*):
88-
The function to use to compute the metrics. Must take a `EvalPrediction` and return a dictionary string to
89-
metric values.
88+
Not supported: evaluation computes only a loss, since `prediction_step` returns no predictions or labels
89+
for the callback. Passing a callable raises a `ValueError`.
9090
callbacks (`list[transformers.TrainerCallback]`):
9191
The callbacks to use for training.
9292
optimizers (`tuple[torch.optim.Optimizer, torch.optim.lr_scheduler.LambdaLR]`):
@@ -131,6 +131,11 @@ def __init__(
131131
optimizers: tuple[torch.optim.Optimizer, torch.optim.lr_scheduler.LambdaLR] = (None, None),
132132
preprocess_logits_for_metrics: Callable[[torch.Tensor, torch.Tensor], torch.Tensor] | None = None,
133133
) -> None:
134+
if compute_metrics is not None:
135+
raise ValueError(
136+
"`compute_metrics` is not supported: `prediction_step` returns only a loss, so the callback would never "
137+
"be called. Read `eval_loss` from `evaluate()` instead."
138+
)
134139
super().__init__(
135140
model=model,
136141
ref_model=ref_model,

0 commit comments

Comments
 (0)