Skip to content

Commit 06785af

Browse files
EazyRealclaude
andauthored
fix(generators): compute batched rollout metrics from truncated responses (#1821)
## What In `SkyRLGymGenerator.generate_batched`, each engine response is truncated to `max_tokens` into `truncated_responses`, and `loss_masks` are built from those truncated lengths. The `GeneratorOutput` returns `truncated_responses` as `response_ids`. But the rollout metrics were computed from the **untruncated** `responses`: ```python rollout_metrics = get_rollout_metrics(responses, rewards, env_metrics, env_classes, loss_masks) ``` So `generate/{min,max,avg,std}_num_tokens` described sequences longer than the `response_ids` actually returned and trained, and disagreed with the assistant-token metrics derived from the truncated `loss_masks` passed into the same call. `generate/max_num_tokens` could exceed `max_generate_length`, violating the invariant added in #764. ## Fix Pass `truncated_responses` instead of `responses` — a one-token change that matches the async multi-turn path (`generate`), which feeds `get_rollout_metrics` the same `responses` it returns. ## Test `tests/train/generators/test_skyrl_gym_generator.py::test_generate_batched_metrics_use_truncated_responses` sets `max_generate_length=2` below the 4-token mock output and asserts `generate/max_num_tokens == 2`, matching the truncated `response_ids` and `loss_masks`. Fails on `main` (`4 == 2`), passes after the fix. All 24 tests in the file pass; black + ruff clean. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent fc829f8 commit 06785af

2 files changed

Lines changed: 35 additions & 1 deletion

File tree

skyrl/train/generators/skyrl_gym_generator.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -776,7 +776,7 @@ async def generate_batched(
776776
# Close the environment
777777
await self._run_in_executor_if_available(env.close)
778778

779-
rollout_metrics = get_rollout_metrics(responses, rewards, env_metrics, env_classes, loss_masks)
779+
rollout_metrics = get_rollout_metrics(truncated_responses, rewards, env_metrics, env_classes, loss_masks)
780780

781781
if self.generator_cfg.apply_overlong_filtering:
782782
# set loss mask to 0 if the stop reason is not "stop"

tests/train/generators/test_skyrl_gym_generator.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,40 @@ async def test_generate_batched(mock_make, mock_tokenizer, mock_llm, mock_env, g
358358
assert generator_output["loss_masks"][0] == [1] * len(MOCK_LLM_OUTPUT_IDS)
359359

360360

361+
@pytest.mark.asyncio
362+
@patch("skyrl_gym.make")
363+
async def test_generate_batched_metrics_use_truncated_responses(
364+
mock_make, mock_tokenizer, mock_llm, mock_env, generator_cfg, mock_env_cfg
365+
):
366+
"""Rollout metrics must describe the truncated responses that are actually
367+
returned/trained, not the raw engine output. With max_generate_length below
368+
the engine's output length, generate/max_num_tokens must equal the truncated
369+
length, matching response_ids and loss_masks."""
370+
generator_cfg.sampling_params.max_generate_length = 2 # < len(MOCK_LLM_OUTPUT_IDS) == 4
371+
mock_make.return_value = mock_env
372+
mock_env.init.return_value = ([{"role": "user", "content": "Initial input"}], {})
373+
374+
generator = SkyRLGymGenerator(
375+
generator_cfg=generator_cfg,
376+
skyrl_gym_cfg=mock_env_cfg,
377+
inference_engine_client=mock_llm,
378+
tokenizer=mock_tokenizer,
379+
)
380+
generator.base_conversation_token_ids = []
381+
382+
input_batch: GeneratorInput = {
383+
"prompts": [[{"role": "user", "content": "What is 3 + 5?"}]],
384+
"env_extras": [{"answer": "8"}],
385+
"env_classes": ["gsm8k"],
386+
}
387+
388+
generator_output: GeneratorOutput = await generator.generate(input_batch)
389+
390+
assert generator_output["response_ids"][0] == MOCK_LLM_OUTPUT_IDS[:2]
391+
assert generator_output["loss_masks"][0] == [1] * 2
392+
assert generator_output["rollout_metrics"]["generate/max_num_tokens"] == 2
393+
394+
361395
@pytest.mark.asyncio
362396
@pytest.mark.parametrize("batched", [True, False])
363397
@patch("skyrl_gym.make")

0 commit comments

Comments
 (0)