Document that padding_free is currently disabled in DPO - #6972
Open
behroozazarkhalili wants to merge 1 commit into
Open
Document that padding_free is currently disabled in DPO#6972behroozazarkhalili wants to merge 1 commit into
behroozazarkhalili wants to merge 1 commit into
Conversation
DPOConfig documents padding_free as a working memory optimization and reducing_memory_usage.md shows DPO users how to switch it on. Neither says the flag has done nothing since the DPO refactor in #3906, which added a fallback at dpo_trainer.py:717-724 that warns and forces the value to False. The config also keeps reporting the requested value. The trainer copies args.padding_free into an instance attribute and overrides the copy, not the config, so training_args.padding_free stays True for the whole run and any serialized run config records a feature that never ran. Documentation only. The fallback, the warning message, and the field default are unchanged. Refs #2469
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Documents that
padding_freecurrently does nothing in DPO, at the three places a user would look:DPOConfig.padding_freedocstringhelptext, which is what the CLI printsdocs/source/reducing_memory_usage.mdWhy
padding_freeshipped for DPO in #2520 and was disabled by the DPO refactor in #3906.dpo_trainer.py:717-724warns and falls back to standard padding. Nothing outside that runtime warning says so, so the documented behavior and the actual behavior disagree.@albertvillanova described the same gap on #2469 on 2026-08-10: "a documented public config field is dead for every DPO user."
Measured on this branch against
trl-internal-testing/tiny-Qwen3ForCausalLM:The
Falsearm is the control. The first row shows what the runtime warning does not cover. The trainer overrides its own copy of the value, soDPOConfig.padding_freestill readsTrueafter construction. Anything reading the config rather than the trainer, including a serialized run config, records a feature that did not run.Scope
Documentation only. The fallback, the warning message, and the field default are unchanged, and the probe above prints the same two lines before and after the edit. SFT padding-free works and is untouched.
This describes a temporary state. @qgallouedec closed #5141 on 2026-08-27 saying he would redo padding-free DPO from a fresh branch, so the DPO tab and its example stay in place with a warning above them rather than being removed. That leaves the restoration PR one hunk to delete.
Verification
ruff checkandruff format --checkat the pinned v0.13.3 pass. A deliberately broken copy fails both, so the gate is live.0ab9ea0with--max_len 119passes. It rejected my first wrapping andorigin/mainpasses the same runner, so the failure was mine; the wrapping in this PR is the tool's own output.Refs #2469
Note
Low Risk
Documentation-only updates to config help and the memory-usage guide; training behavior is unchanged.
Overview
Aligns DPO
padding_freedocumentation with current trainer behavior after #3906: the option still exists on [DPOConfig], but settingpadding_free=Trueonly triggers a warning and standard padding is used.Adds matching notes in the
DPOConfig.padding_freeclass docstring and CLIhelptext, plus a WARNING on the DPO tab inreducing_memory_usage.md(with a link to #2469). The DPO example snippet is kept for when the feature returns. No runtime or default changes—SFT padding-free docs are unchanged.Reviewed by Cursor Bugbot for commit c8123a2. Bugbot is set up for automated code reviews on this repo. Configure here.