Document sub_group_size and correct its default to 1e9 - #8567
Merged
delock merged 1 commit intoSep 17, 2026
Merged
Conversation
sub_group_size had no description entry in the ZeRO config reference, and both the example JSON block and the ZERO_FORMAT deprecation warning claimed a default of 1e12 while the field default is Field(pp_int(1e9), ge=0). Adds the missing entry from @samyam's explanation in deepspeedai#1145 and corrects both 1e12 occurrences. Fixes deepspeedai#1145 Signed-off-by: Shravani Nikam <shravanibharatnikam@gmail.com>
shravanibnikam
requested review from
loadams,
tjruwase and
tohtana
as code owners
September 17, 2026 07:01
sfc-gh-truwase
approved these changes
Sep 17, 2026
sfc-gh-truwase
enabled auto-merge
September 17, 2026 11:00
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Sep 17, 2026
Merged
via the queue into
deepspeedai:master
with commit Sep 17, 2026
13e3609
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
sub_group_sizehas no description entry in the ZeRO section ofconfig-json.md. It appears only as a bare value in the example JSON block, so nothing tells a user what it does or when to change it. That is the original report in #1145.The default is also documented wrongly.
deepspeed/runtime/zero/config.py:182is authoritative:pp_int(deepspeed/runtime/config_utils.py:127) subclassesintand overrides only__repr__, so the default really is 1e9. @samyam's comment in #1145 says the same thing independently: "Most users can leave sub_group_size to its default value of 1e9."Two places said 1e12:
docs/_pages/config-json.md:381, the ZeRO example block.deepspeed/runtime/zero/config.py:39, insideZERO_FORMAT. That one is not a comment —ZERO_FORMATis interpolated into thelogger.warninginread_zero_config_deprecated()(config.py:58-66), so the wrong value was printed at runtime to anyone on the deprecated config format.docs/_tutorials/zero.md:139already says 1e9, which is the inconsistency #1145 points at.Fix
***sub_group_size***entry to the ZeRO section, betweenstage3_param_persistence_thresholdandstage3_gather_16bit_weights_on_model_save, matching the ordering in the example block above it. The***key***form matches every other entry in that section.config-json.md:381from 1e12 to 1e9.ZERO_FORMATinconfig.py:39from 1000000000000 to 1000000000, keeping the full-digit style of its neighbours.The prose is @samyam's explanation from #1145, relayed by @stas00, edited only for tense, to fix "each buckets" → "each bucket", and to drop the inline "1e9" now that the Default column carries it. It keeps the guidance on when to raise or lower the value, which seemed like the most useful part of that comment.
Where the 1e12 probably came from, and what I left alone
DeepSpeedZeroOptimizer_Stage3.__init__declaressub_group_size=1000000000000(deepspeed/runtime/zero/stage3.py:186). It is the only keyword default in that signature that disagrees with its config counterpart —reduce_bucket_size=500000000,max_reuse_distance=1000000000,max_live_parameters=1000000000andparam_persistence_threshold=100000all matchDeepSpeedZeroConfig. The sole in-tree caller,engine.py:2726, always passessub_group_size=self.zero_sub_group_size()from the config, so the 1e12 is only reachable by constructing the optimizer directly, and no released behaviour depends on it via the normal path.I have not touched it, because changing a public constructor's default is a behaviour change and does not belong in a docs fix. Flagging it in case you want it aligned separately — happy to send that as its own PR if so.
Also left alone for the same reason:
deepspeed/autotuning/config_templates/template_zero3.json:15andexamples/sdma_allgather/ds_config_zero3.json:39, both of which also say 1e12. Those are real configs rather than documentation, so their values may be deliberate.Test
No test. This is a documentation entry plus two literal corrections, so there is no contract to assert that a different correct implementation would not also satisfy. (Noting that #8242 added a docs/config consistency guard in this same file and @tohtana asked for it to be dropped as too fragile for CI.)
What was checked instead, on CPU:
pre-commit run --files docs/_pages/config-json.md deepspeed/runtime/zero/config.py— all hooks pass, includingyapf,flake8andcodespell.<em><strong>sub_group_size</strong></em>for the heading, a well-formed<table>withDescription/Defaultheaders and<code>1e9</code>in the default cell, and an<ol>with both tuning cases. I did not build the Jekyll site locally; the addition uses no Minimal Mistakes-specific syntax, so plain GFM rendering covers it.sub_group_size. After this change,docs/states the default in exactly one place and it agrees withconfig.py:182. The remaining occurrences are runtime code (zero/stage3.py,runtime/engine.py,superoffload/superoffload_stage3.py), tests, and the three non-doc configs noted above.config.py:182pluspp_int, and on @samyam's own statement in the issue.Fixes #1145