Skip to content

fix(rollout): guard zero-length response against x[-0:] returning the whole sequence - #2255

Open
hobostay wants to merge 1 commit into
THUDM:mainfrom
hobostay:fix-zero-length-response-slice
Open

fix(rollout): guard zero-length response against x[-0:] returning the whole sequence#2255
hobostay wants to merge 1 commit into
THUDM:mainfrom
hobostay:fix-zero-length-response-slice

Conversation

@hobostay

@hobostay hobostay commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Problem

Python slicing makes seq[-0:] return the entire sequence, not an empty one. Two rollout post-processing paths slice with [-response_length:] and break when response_length == 0:

  1. slime/rollout/sft_rollout.py — for a fully-masked SFT sample (no trainable tokens, e.g. no assistant turn or a chat-template mismatch), get_response_lengths returns 0 and loss_mask[-0:] assigns the whole prompt-length mask to sample.loss_mask while sample.response_length == 0. This violates the downstream invariant len(loss_mask) == response_length, asserted in slime/ray/rollout.py and slime/utils/types.py, so one bad sample crashes the entire rollout. On any path where the assertion is not reached, the prompt tokens would silently be treated as trainable.

  2. slime/rollout/on_policy_distillation.py — for an empty generation (e.g. immediate EOS), t_log_prob[-0:] keeps the full prompt+response-length teacher log-prob tensor instead of an empty one, misaligning the OPD KL computation with response_length.

Minimal reproduction:

loss_mask = [0] * 50
response_length = 0
len(loss_mask[-response_length:])  # 50, expected 0

Fix

Guard the zero-length case explicitly in both places (seq[-n:] if n > 0 else empty). Behavior for response_length > 0 is unchanged.

Tests

Added tests/test_zero_length_response.py (CPU, @pytest.mark.unit):

  • SFT rollout with a fully-masked sample → loss_mask == [], invariant holds (fails without the fix).
  • SFT rollout with a normal sample → tail mask unchanged.
  • OPD with response_length == 0 → empty teacher_log_probs (fails without the fix).
  • OPD with a normal response → tail log-probs unchanged.

All 4 tests pass locally; the two zero-length tests fail on current main, confirming they pin the bug.

… whole sequence

Python slicing makes seq[-0:] return the entire sequence, not an empty
one. Two rollout post-processing paths slice with [-response_length:]
and broke when response_length == 0:

* sft_rollout: a fully-masked SFT sample (no trainable tokens) ended up
  with loss_mask covering the whole prompt while response_length == 0,
  violating the downstream invariant len(loss_mask) == response_length
  asserted in slime/ray/rollout.py and slime/utils/types.py, crashing
  the rollout (or, where unasserted, training on the prompt tokens).

* on_policy_distillation: an empty generation (e.g. immediate EOS)
  kept the full-length teacher log-prob tensor instead of an empty
  one, misaligning the OPD KL computation with response_length.

Add explicit guards for the zero-length case in both places, plus CPU
unit tests that fail without the fix.
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.

1 participant