Keep the elastic batch size within max_train_batch_size - #8237
Merged
tohtana merged 1 commit intoAug 12, 2026
Merged
Conversation
_get_compatible_gpus_v01 checks every micro batch against max_acceptable_batch_size, but the first heuristic scales their LCM, and the LCM is never checked. get_candidate_batch_sizes then took the >= branch for it and offered it unscaled, so a base the caller already ruled out became a candidate, and because the LCM divides every micro batch it tends to win the most-valid-GPU-counts vote. With micro_batch_sizes [8, 10, 12] and max_train_batch_size 100 the returned train_batch_size is 120. DeepSpeedConfig writes that straight into train_batch_size, so the job runs 20 percent over the limit the user set. The existing test_proper_mbsz shows the same thing: its config caps at 32 and gets 42 back. Skip a base larger than the cap. Scaling one can only grow it further, so it can never yield a legal candidate, and every micro batch is already validated against the cap so the candidate list cannot end up empty. test_proper_mbsz asked for world_size 7, which was only reachable because 42 was over the cap. It now asks for 4, where the batch per GPU is 6, so 7 is still ruled out and the assertion that 3 is chosen is unchanged. Signed-off-by: Vineeth Sai <vineethsai4444@gmail.com>
vineethsaivs
requested review from
loadams,
tjruwase and
tohtana
as code owners
August 9, 2026 01:04
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a66f83037c
ℹ️ 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".
tohtana
approved these changes
Aug 11, 2026
tohtana
left a comment
Collaborator
There was a problem hiding this comment.
Thank you @vineethsaivs for your contribution! Looks good to me.
tohtana
enabled auto-merge
August 11, 2026 23:54
Merged
via the queue into
deepspeedai:master
with commit Aug 12, 2026
2cfebbd
13 of 15 checks passed
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.
Problem
_get_compatible_gpus_v01validates every micro batch againstmax_acceptable_batch_size:but the first heuristic scales the LCM of the micro batches, and the LCM is never checked. It goes into
base_listand reachesget_candidate_batch_sizes, where thebase >= max_acceptable_batch_sizebranch appends it unscaled. So a batch size the caller already said was too large becomes a candidate, and since the LCM divides every micro batch it tends to win the most-valid-GPU-counts vote inget_best_candidates.The docstring says the heuristic produces "the largest batch size less than the max_acceptable batch size", and
config-json.mddocumentsmax_train_batch_sizeas "Max acceptable batch size can be used in training", so the returned value is not supposed to exceed it.Repro
DeepSpeedConfig.__init__writes that return value straight intoself._param_dict[TRAIN_BATCH_SIZE], so the job runs 20 percent over the limit the user set, with the matching effect on the LR schedule and step count.It is not an exotic corner.
[8, 12]with a cap of 16 gives 24, and a brute-force sweep over micro batch sets of size 2 and 3 drawn from 1 to 32, against every cap up to 400, finds it in 946105 configurations.The clearest evidence is in this repo:
tests/unit/elasticity/test_elastic.py::test_proper_mbszsetsmax_train_batch_sizeto 32 with micro batches[1, 2, 3, 7], whose LCM is 42, and gets 42 back today.Fix
Skip a base larger than the cap. Scaling one can only make it bigger, so it can never yield a legal candidate, and every micro batch is already validated against the cap, so the candidate list cannot end up empty.
What this changes for the existing tests
test_basic_10kis unaffected: still 9792, still 23 valid GPU counts.test_proper_mbszneeded one number changed, and I want to be upfront about it rather than bury it. Itsworld_size=7was only reachable because the batch size came back as 42, over its own cap of 32; at any legal batch size for that config, 7 is not a valid GPU count. I changed it to 4, where the batch per GPU is 6, so 7 is still correctly ruled out and the assertion that 3 is chosen is unchanged. That keeps the test doing what it was written to do, which is check the micro batch picked for a given world size.If you would rather keep
world_size=7working, then the LCM overshoot is load-bearing rather than a bug, and this PR is the wrong change; I would want to hear that before going further. I could not find a config for those micro batches that makes 7 valid without exceeding the cap.test_batch_size_within_maxis new and pins the actual contract.Test
Run on CPU by driving the test bodies against the real
compute_elastic_config, once againstmasterand once against this branch; this path is pure Python and needs no GPU.yapf --style .style.yapfandflake8 --config .flake8are clean on both changed files, and clean on the unmodified tree as a control.There is one other open PR touching this file, #8162, in
compute_elastic_config'sreturn_microbatchtail. It does not overlap these lines.