Skip to content

Commit 518ad8d

Browse files
Merge pull request AI-Hypercomputer#4140 from AI-Hypercomputer:pr/fix-eval-reward-question-v2
PiperOrigin-RevId: 930602952
2 parents ef42536 + ad0a181 commit 518ad8d

2 files changed

Lines changed: 72 additions & 9 deletions

File tree

src/maxtext/trainers/post_train/rl/evaluate_rl.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ def score_responses(tmvp_config, question, responses, answers):
183183
raise ValueError(f"Unknown eval_mode: {eval_mode!r}")
184184

185185

186-
def _compute_row_reward(reward_fns, prompt, responses, answer, row_idx):
186+
def _compute_row_reward(reward_fns, prompt, question, responses, answer, row_idx):
187187
"""Sum the per-function reward scores across all sampled responses for one prompt.
188188
189189
Honors the sampling strategy `evaluate()` ran with: when `num_passes > 1`
@@ -193,6 +193,12 @@ def _compute_row_reward(reward_fns, prompt, responses, answer, row_idx):
193193
total by the number of (prompt, response) pairs to get the per-sample
194194
mean reward, mirroring tunix's GRPO per-rollout reward aggregation.
195195
196+
`question` is forwarded as a reward-fn kwarg because the built-in
197+
`check_numbers` reward (and any user reward that wants to inspect the
198+
raw question) reads it from `kwargs`. Training-time tunix passes the
199+
whole dataset row to each reward fn; we mirror the kwargs the trainer
200+
passes so eval-time and train-time reward calls are interchangeable.
201+
196202
Returns a tuple `(score_sum, n_responses)`. On any exception the
197203
failure is logged and `(0.0, 0)` is returned so the caller's running
198204
mean is not corrupted.
@@ -203,7 +209,7 @@ def _compute_row_reward(reward_fns, prompt, responses, answer, row_idx):
203209
score_sum = 0.0
204210
for resp in responses:
205211
for fn in reward_fns:
206-
scores = fn(prompts=[prompt], completions=[resp], answer=[answer])
212+
scores = fn(prompts=[prompt], completions=[resp], answer=[answer], question=question)
207213
if scores:
208214
score_sum += float(scores[0])
209215
return score_sum, len(responses)
@@ -283,7 +289,7 @@ def evaluate(
283289
# the actual per-(prompt, response) count at the end. See
284290
# `_compute_row_reward` for details.
285291
if use_reward:
286-
row_sum, row_count = _compute_row_reward(reward_fns, prompt, responses, answer, total)
292+
row_sum, row_count = _compute_row_reward(reward_fns, prompt, question, responses, answer, total)
287293
reward_sum += row_sum
288294
reward_count += row_count
289295

tests/post_training/unit/evaluate_rl_test.py

Lines changed: 63 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -216,25 +216,27 @@ class TestComputeRowReward(unittest.TestCase):
216216
def _two_fns(self):
217217
"""Return two reward functions whose per-response scores can be summed."""
218218

219-
# Each fn must accept prompts, completions, answer as keyword args and
220-
# return a list of per-completion scores. The helper calls fn once per
221-
# response with single-element lists, so the returned list has length 1.
222-
def fn1(prompts, completions, answer): # pylint: disable=unused-argument
219+
# Each fn must accept prompts, completions, answer, question as keyword
220+
# args and return a list of per-completion scores. The helper calls fn
221+
# once per response with single-element lists, so the returned list has
222+
# length 1.
223+
def fn1(prompts, completions, answer, question): # pylint: disable=unused-argument
223224
return [1.0 for _ in completions]
224225

225-
def fn2(prompts, completions, answer): # pylint: disable=unused-argument
226+
def fn2(prompts, completions, answer, question): # pylint: disable=unused-argument
226227
return [float(len(c)) for c in completions]
227228

228229
return [fn1, fn2]
229230

230231
@pytest.mark.cpu_only
231232
def test_single_response_single_fn(self):
232-
def fn(prompts, completions, answer): # pylint: disable=unused-argument
233+
def fn(prompts, completions, answer, question): # pylint: disable=unused-argument
233234
return [2.5 for _ in completions]
234235

235236
score_sum, count = evaluate_rl._compute_row_reward(
236237
reward_fns=[fn],
237238
prompt="p",
239+
question="q",
238240
responses=["abc"],
239241
answer="gold",
240242
row_idx=0,
@@ -247,6 +249,7 @@ def test_sums_across_reward_fns_for_single_response(self):
247249
score_sum, count = evaluate_rl._compute_row_reward(
248250
reward_fns=self._two_fns(),
249251
prompt="p",
252+
question="q",
250253
responses=["abcd"],
251254
answer="gold",
252255
row_idx=0,
@@ -261,6 +264,7 @@ def test_sums_across_passes_for_multiple_responses(self):
261264
score_sum, count = evaluate_rl._compute_row_reward(
262265
reward_fns=self._two_fns(),
263266
prompt="p",
267+
question="q",
264268
responses=["a", "bcd", "ef"],
265269
answer="gold",
266270
row_idx=0,
@@ -276,6 +280,7 @@ def test_empty_responses_returns_zero_and_zero_count(self):
276280
score_sum, count = evaluate_rl._compute_row_reward(
277281
reward_fns=self._two_fns(),
278282
prompt="p",
283+
question="q",
279284
responses=[],
280285
answer="gold",
281286
row_idx=0,
@@ -293,13 +298,65 @@ def _boom(**kwargs): # pylint: disable=unused-argument
293298
score_sum, count = evaluate_rl._compute_row_reward(
294299
reward_fns=[_boom],
295300
prompt="p",
301+
question="q",
296302
responses=["abc"],
297303
answer="gold",
298304
row_idx=0,
299305
)
300306
self.assertEqual(score_sum, 0.0)
301307
self.assertEqual(count, 0) # zero count so the caller's mean isn't biased
302308

309+
@pytest.mark.cpu_only
310+
def test_question_is_forwarded_to_reward_fn(self):
311+
"""Regression: helper must pass `question` through to reward fns.
312+
313+
The built-in `check_numbers` reward reads `kwargs["question"]`; if the
314+
helper omits it, every eval row produces `KeyError('question')` and
315+
`mean_reward` collapses to 0.0.
316+
"""
317+
received = {}
318+
319+
def fn(prompts, completions, answer, question): # pylint: disable=unused-argument
320+
received["question"] = question
321+
return [1.0 for _ in completions]
322+
323+
evaluate_rl._compute_row_reward(
324+
reward_fns=[fn],
325+
prompt="p",
326+
question="What is 2+2?",
327+
responses=["abc"],
328+
answer="gold",
329+
row_idx=0,
330+
)
331+
self.assertEqual(received["question"], "What is 2+2?")
332+
333+
@pytest.mark.cpu_only
334+
def test_integrates_with_real_check_numbers_reward(self):
335+
"""End-to-end: real `check_numbers` from utils_rl must not raise on the
336+
eval-time kwargs the helper passes (regression for the original
337+
`KeyError('question')` failure mode in production)."""
338+
from maxtext.trainers.post_train.rl import utils_rl # pylint: disable=import-outside-toplevel
339+
340+
config = _make_config(eval_mode="pass")
341+
342+
# `check_numbers` takes tmvp_config positionally via partial; mirror what
343+
# train_rl.py's make_reward_fn does.
344+
def wrapped_check_numbers(**kwargs):
345+
return utils_rl.check_numbers(tmvp_config=config, **kwargs)
346+
347+
# A correct-answer response should yield a non-zero score (proves the
348+
# kwargs all reached the inside of check_numbers).
349+
score_sum, count = evaluate_rl._compute_row_reward(
350+
reward_fns=[wrapped_check_numbers],
351+
prompt="solve: 2+2",
352+
question="What is 2+2?",
353+
responses=["<reasoning>2+2=4</reasoning><answer>4</answer>"],
354+
answer='["4"]', # json-encoded list of acceptable answers
355+
row_idx=0,
356+
)
357+
self.assertEqual(count, 1)
358+
self.assertGreater(score_sum, 0.0)
359+
303360

304361
if __name__ == "__main__":
305362
unittest.main()

0 commit comments

Comments
 (0)