Repository navigation
Default pad_to_multiple_of to "auto": pad to a multiple of 128 on Neuron - #7724
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ecf90b4e0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4ecf90b. Configure here.
…, like the other optional fields

Part of #7658.
Neuron compiles once per input shape, so without
pad_to_multiple_ofevery new sequence length stalls training for about a minute. This PR makes"auto"the defaultpad_to_multiple_ofin SFT, DPO, KTO, Reward, GRPO, RLOO and Distillation: 128 on Neuron,Noneelsewhere, so nothing changes on GPU. An explicit value is kept as is.pad_to_multiple_of=None.Why not a warning
We already know the right value, and warnings get missed (agents launching runs never read them).
Why not 128 everywhere
It only helps SDPA on Hopper (cuDNN builds a graph for each new sequence length), and less as the model grows. Elsewhere, and with flash attention on every GPU, it costs 2-6%.
Data
SFT on
trl-lib/Capybara,max_length=4096, 100 steps, steady-state samples/s, torch 2.13 unless noted. Change withpad_to_multiple_of=128vsNone:Full H100 sweep (192 runs). Peak allocated memory doesn't depend on the pad; the fragmentation OOMs at batch 8 go away with
expandable_segments, not with padding:cc @michaelbenayoun
Note
Medium Risk
Cross-cutting default change on all major trainer configs affects batch padding and sequence lengths on Neuron (intended); off Neuron
"auto"still resolves to no padding, but anyone depending on the old implicit default should confirm explicit overrides.Overview
pad_to_multiple_ofnow defaults to"auto"(instead ofNone) on SFT, DPO, KTO, Reward, GRPO, RLOO, and Distillation configs. The field type acceptsint | str | None, and__post_init__normalizes CLI strings ("128","none"/"null") and resolves"auto"to 128 whendevice.type == "neuron", otherwiseNone—so GPU training behavior stays unpadded unless overridden.Docs/help text for
pad_to_multiple_ofare updated accordingly.tests/test_cli_utils.pyadds parametrized coverage thatTrlParserparses--pad_to_multiple_ofcorrectly across those config classes (including"auto"→Noneoff Neuron).Reviewed by Cursor Bugbot for commit 93c6e25. Bugbot is set up for automated code reviews on this repo. Configure here.