Skip to content

Conversation

@djsaunde
Copy link
Collaborator

@djsaunde djsaunde commented Nov 7, 2025

This PR patches the SFTTrainer class' constructor to auto-enable sample packing when applicable (i.e., when the user is not training a multi-modal model or passing in a custom data collator). Sample packing is a good default in SFT since we reduce the amount of zero-padding we see and increase the training token/s throughput as a result.

Followup to #3525; can merge after that PR is. I closed that PR, I think we should just merge this one.

Needs to be extensively tested so we don't break any existing notebooks / scripts.

@djsaunde djsaunde self-assigned this Nov 7, 2025
@djsaunde djsaunde removed the request for review from danielhanchen November 7, 2025 19:03
@djsaunde djsaunde changed the title auto-enable sample packing auto-enable SFT sample packing Nov 7, 2025
@djsaunde djsaunde force-pushed the auto-packing branch 2 times, most recently from 072de80 to efe4424 Compare November 10, 2025 20:23
@djsaunde djsaunde force-pushed the auto-packing branch 3 times, most recently from f352814 to 9cd0b26 Compare November 19, 2025 19:14
@djsaunde djsaunde mentioned this pull request Nov 20, 2025
1 task
@djsaunde djsaunde marked this pull request as ready for review November 20, 2025 13:47
@djsaunde
Copy link
Collaborator Author

I tested all the SFT notebooks on the main page README. I'd like to test a few more notebooks, including ones that we don't expect to use packing (multimodal training, RL training, ...).

@djsaunde
Copy link
Collaborator Author

Okay, I ran through all the notebooks displayed in the README. As expected, non of the non-text-only SFT notebooks utilized sample packing, and none of them hit any errors when switching to this branch.

@djsaunde djsaunde changed the title auto-enable SFT sample packing SFT sample packing Nov 21, 2025
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