[Feature] Add region-level recompute_cfg and per-model marker declarations (3/3) - #1981
Draft
HAOCHENYE wants to merge 5 commits into
Draft
[Feature] Add region-level recompute_cfg and per-model marker declarations (3/3)#1981HAOCHENYE wants to merge 5 commits into
recompute_cfg and per-model marker declarations (3/3)#1981HAOCHENYE wants to merge 5 commits into
Conversation
This was referenced Jul 28, 2026
HAOCHENYE
force-pushed
the
feat/sac-recompute-cfg-v2
branch
from
July 28, 2026 09:48
f6a8ac1 to
a5d7d96
Compare
… layer `checkpoint_record` is called from `xtuner/v1/module/decoder_layer/*.py`, and importing it from there pulls in `xtuner/v1/model/__init__.py`, which imports the decoder layers back. A module the `module/` layer depends on cannot live under `model/`. Split rather than move: the vocabulary and the marker session go to `xtuner/v1/utils`, below both packages, while the policy and the wrapping stay at the model layer, where knowing `nn.Module`, `CheckpointWrapper` and the DSA lifecycle predicate is legitimate. The session's API becomes public because it is now driven across a package boundary. `xtuner.v1.model.utils` keeps exporting the same names, so no import site outside these files changes.
…rations Lets a user keep named activation regions resident instead of recomputing them, inside the layers `fsdp_cfg.recompute_ratio` already selects. The two knobs stay orthogonal: `recompute_ratio` picks the layers, `recompute_cfg` picks the regions inside a selected layer. `XTunerBaseModelConfig.recompute_cfg` is tri-state. `None` and `False` keep today's behaviour of recomputing everything, `True` keeps every region the model declares, and an explicit list of `RecomputeUnit` keeps exactly those. Unlike `compile_cfg`, `None` is not "the model default": `default_recompute_cfg` declares what an architecture *can* keep, not what is worth keeping, so an unset config never changes a run's memory profile. `False` additionally propagates into nested sub-model configs, which is the walk `compile_cfg` already had, now shared between the two switches instead of duplicated. `MoEDecoderLayer` and `DenseDecoderLayer` record paired markers around the attention, router, dispatch, combine, shared-expert and MLP regions, in both the single-batch and the domino EP path. How much of that survives `torch.compile` depends on where the markers sit, which the per-model `default_recompute_cfg` docstrings spell out unit by unit.
…tion `test_micro_batch_path_records_the_same_markers` compared marker name sets, so it would have passed even if the domino path wrapped entirely different operations. It now compares which of the layer's own operations each region encloses, verified by mutation: moving `moe.dispatch.end` past the expert GEMMs fails the new assertion and passed the old one. Along the way the two paths are made to agree exactly -- `_forward` now reshapes outside the dispatch and combine regions, as the domino path already did. Intervals resolve in `BaseModel.__init__`, next to `compile_cfg` and by the same rule: `default_recompute_cfg` is answerable from the config alone, so there is nothing to wait for. A config naming a unit the model does not support now fails before the run spends anything on materializing and sharding weights. `recompute_cfg=False` reaching every nested sub-model config gains a test on a shipped compose config, which nests three of them; the previous probe nested one. Verified by mutation: stopping the walk at the outer config fails it. Record why regions use explicit `.begin` / `.end` marker pairs instead of ending each region at the next region's start marker, and warn instead of silently resolving to nothing when `recompute_cfg=True` meets a model that declares no units.
"Keep the expert dispatch / combine communication region" tells a user they are keeping the communication, which is the one thing the unit does not keep: the SAC policy forces every c10d collective to be recomputed, because keeping one would elide it from the recompute pass. What the unit trades memory for is the permutation, padding and unpermutation work on either side of the all-to-all.
Measured against the SAC policy at ep_size=2: only SAVE_MOE_DISPATCH keeps ops under compile (152), while SAVE_ATTN, SAVE_MOE_GATE and SAVE_MLP keep none. The docstring claimed the shared-expert half of SAVE_MLP stayed effective. The rule was stated as "markers must sit outside compiled regions", which is necessary but not sufficient. SAVE_MLP's markers do run in eager -- they sit in `_forward`, which EP does not compile -- but the region encloses `_shared_experts_forward`, which EP does compile, and ops inside a compiled region execute as fused kernels that never reach the per-op policy. A region is addressable only if its contents run in eager, not merely its endpoints. SAVE_MOE_DISPATCH survives because the dispatcher calls it wraps are the one part of the MoE path that stays uncompiled.
HAOCHENYE
force-pushed
the
feat/sac-recompute-cfg-v2
branch
from
July 28, 2026 10:43
a5d7d96 to
bdc1edb
Compare
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.
Stack (bottom to top):
feat/sac-nonlegacy-base→mainfeat/sac-engine→feat/sac-nonlegacy-baserecompute_cfgand per-model marker declarations (3/3) #1981feat/sac-recompute-cfg-v2→feat/sac-engine← you are here (top of stack)Summary
The user-facing half: a tri-state
recompute_cfg, per-modeldefault_recompute_cfgmapping semantic units to marker intervals, andcheckpoint_recordmarkers in the decoder-layer forwards. This is what makes the engine in PR2 reachable from a config.User surface
None/False— keep nothing (every selected layer recomputed whole)True— the model'sdefault_recompute_cfgNonedeliberately does not mean "model default", diverging fromcompile_cfg: keeping regions resident trades memory for speed, so an unset config must not silently change an existing run's peak memory.Orthogonal to
recompute_ratio, which continues to decide which layers are checkpointed;recompute_cfgdecides which regions inside a selected layer are kept.RecomputeUnitis aStrEnum, so configs round-trip readably (["save_attn", "save_mlp"]) and trainer resume preserves the setting — noField(exclude=True).Layering
checkpoint_recordis called fromxtuner/v1/module/decoder_layer/*.py, so the contract cannot live undermodel/— importing it from there pullsxtuner/v1/model/__init__.py, which imports the decoder layers back:The contract and marker session move to
xtuner/v1/utils/selective_checkpointing.py; the engine stays at the model layer, where it may importuses_dsa_topk_lifecycle.model/utils/__init__.pyre-exports, so no import site outside these files changes. A regression test pins the standalone import in a subprocess — in-process the check is vacuous, sincextuner.v1.modelis already imported by then.Markers
Explicit
.begin/.endpairs rather than "end = the next region's start"._micro_batch_forwardsplits the dispatch/combine chain across four stage loops, so "the next region" is a different region there than in_forward; under the implicit convention the same interval would mean two different things in the two paths. Both paths are instrumented, and a static guard checks every marker name referenced by adefault_recompute_cfgis actually recorded.What each unit actually keeps — measured, not asserted
Eager,
ep_size=2, kept-op identities from the real policy:save_attnsave_moe_gatesave_moe_dispatchsave_mlpLoss bit-identical to full recompute in every row, 48/48 live grads.
save_moe_dispatchkeeps the permutation/padding buffers either side of the all-to-all; the collective itself is always recomputed (PR2's invariant), and the docstring says so.Under EP +
torch.compilethe effective set issave_moe_dispatchalone. A region is addressable only if its contents run in eager:save_mlp's markers do run in eager, but its region encloses the EP-compiled_shared_experts_forward, so nothing is kept. Documented per unit with the measured numbers, and PR2's diagnostics report it at runtime.DSA models
DSA-lifecycle models route to reentrant (PR1) and cannot keep regions, so
default_recompute_cfgdeclares no units for them — otherwise the config validates and the trainer logs "keeps these regions resident: [6 intervals]" while nothing is kept. Verified: non-DSAunits=4 intervals=6, DSAunits=0 intervals=0. PR2's wrap-site fallback remains as an independent guard for a direct caller. The gate resolves lazily on first access because it must read the built layers, which subclasses create afterBaseModel.__init__returns — an eager gate would inspect a layerless model, never fire, and look correct.Test plan
Config tri-state resolution including recursive
Falsepropagation into nested compose configs (verified on a realQwen3VLMoE30BA3Config, not a synthetic probe); enum round-trip throughmodel_dump_json/model_validate; marker coverage across both MoE paths; the subprocess layering test, verified red by reintroducing an upward import. 33 passed / 4 skipped at the tip of the stack; every commit in this PR builds and passes independently (bisectable); pre-commit clean including mypy.