Skip to content

fix(data): handle all-empty BFD packing - #6971

Open
YZJF wants to merge 1 commit into
huggingface:mainfrom
YZJF:codex/fix-bfd-all-empty-sequences
Open

fix(data): handle all-empty BFD packing#6971
YZJF wants to merge 1 commit into
huggingface:mainfrom
YZJF:codex/fix-bfd-all-empty-sequences

Conversation

@YZJF

@YZJF YZJF commented Aug 29, 2026

Copy link
Copy Markdown

What does this PR do?

Fixes #6970.

pack_dataset(..., strategy="bfd") and "bfd_split" filter empty input sequences before binning. If every typed input sequence is empty, this leaves no bins. The empty Python list passed to pyarrow.compute.take is inferred as a null index array, which raises ArrowNotImplementedError instead of producing an empty packed dataset.

Construct the take indices as an explicit int64 Arrow array. This preserves the existing behavior for non-empty indices and lets both BFD strategies return an empty dataset with seq_lengths when all input sequences are empty.

Tests

  • RED before the production change: python -m pytest tests/test_data_utils.py -k 'all_empty_sequences' -vv failed for both bfd and bfd_split with ArrowNotImplementedError: Function 'array_take' has no kernel matching input types (list<item: int64>, null).
  • GREEN: the same focused command passed (2 passed).
  • Related suite: python -m pytest tests/test_data_utils.py -k 'TestPackDatasetBfd' -vv passed (9 passed).
  • pre-commit run --files trl/data_utils.py tests/test_data_utils.py, git diff --check, and python -m compileall -q trl/data_utils.py tests/test_data_utils.py passed.

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline, Pull Request section?
  • Was this discussed/approved via a GitHub issue? Fixes Bug: pack_dataset BFD fails when all sequences are empty #6970.
  • Did you make sure to update the documentation with your changes? No documentation change is needed for this internal empty-input fix.
  • Did you write any new necessary tests?

AI writing disclosure

  • No AI usage: the PR was written entirely by a human.
  • AI-assisted: some parts were suggested or improved by AI, but the PR was written and reviewed by a human.
  • AI-generated: the PR was mostly or fully generated by an AI tool.

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag members/contributors who may be interested in your PR.


Note

Low Risk
Narrow edge-case fix in dataset packing with explicit Arrow typing; behavior for non-empty inputs is unchanged and covered by existing BFD tests.

Overview
Fixes BFD packing (pack_dataset with strategy="bfd" or "bfd_split") when every input sequence is empty after filtering: it no longer raises ArrowNotImplementedError and instead returns an empty packed dataset with empty input_ids and seq_lengths.

The failure came from passing an empty Python list of row indices into pyarrow.compute.take, which PyArrow treated as a null index type. _pack_bfd now builds take indices as an explicit int64 Arrow array via pa.array(..., type=pa.int64()).

Tests add test_with_all_empty_sequences, parametrized over both BFD strategies, using typed Features for all-empty input_ids rows.

Reviewed by Cursor Bugbot for commit 49ebf23. Bugbot is set up for automated code reviews on this repo. Configure here.

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.

Bug: pack_dataset BFD fails when all sequences are empty

1 participant