Skip to content

fix(sft): truncate overlong collator inputs - #520

Open
Hang-get wants to merge 1 commit into
alibaba:mainfrom
Hang-get:fix/sft-collator-max-length
Open

Hang-get wants to merge 1 commit into
alibaba:mainfrom
Hang-get:fix/sft-collator-max-length

Conversation

@Hang-get

@Hang-get Hang-get commented Nov 8, 2026

Copy link
Copy Markdown

Problem

The SFT pipeline can pass examples longer than sequence_length to DataCollatorForSFT. A mixed-length batch then fails during tensorization, while a uniformly overlong batch produces inputs and labels with different lengths.

Fixes #519.

Cause

Hugging Face padding does not truncate sequence fields. The collator capped labels at max_length but passed overlong input_ids and attention_mask through unchanged.

Fix

Truncate every padded sequence field to max_length before padding. The existing label truncation and shift then stay aligned with the model inputs.

The regression test covers one overlong and one shorter example in the same batch.

Tests

  • env -u LD_LIBRARY_PATH ROLL_LOG_DIR=/tmp/t4-roll-logs PYTHONPATH=/tmp/t4-ROLL /tmp/t4-ROLL-venv/bin/python -m pytest -q tests/datasets/test_collator.py — 2 passed
  • env -u LD_LIBRARY_PATH /tmp/t4-ROLL-venv/bin/pre-commit run --files tests/datasets/test_collator.py — all hooks passed
  • git diff --check — passed

The changed production file was also passed to pre-commit. Its formatter hooks modify unrelated existing sections, and flake8 reports the existing E266 at roll/datasets/collator.py:471; those unrelated changes are not included.

Not run

  • GPU or distributed SFT training; the failure and regression test are CPU-only.

AI assistance (Cursor) was used for this change.

SFT tokenization can produce sequences longer than sequence_length, while the collator previously truncated only labels. Truncate all padded sequence fields before padding so inputs and labels remain aligned.

Fixes alibaba#519

AI assistance (Cursor) was used for this change.

Co-authored-by: Cursor <cursoragent@cursor.com>
@CLAassistant

CLAassistant commented Nov 8, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

This branch has not been deployed

No deployments
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.

SFT collator crashes when a sample exceeds sequence_length

2 participants