[Feature] Add the region-level selective checkpointing engine (2/3) - #1980
Draft
HAOCHENYE wants to merge 6 commits into
Draft
[Feature] Add the region-level selective checkpointing engine (2/3)#1980HAOCHENYE wants to merge 6 commits into
HAOCHENYE wants to merge 6 commits into
Conversation
This was referenced Jul 28, 2026
HAOCHENYE
force-pushed
the
feat/sac-engine
branch
from
July 28, 2026 09:48
c51f915 to
5003f04
Compare
Turn the whole-layer checkpoint into a per-op decision: a contextvars marker session records which `checkpoint_record` intervals are open, and the policy behind `create_selective_checkpoint_contexts` keeps the ops inside them while recomputing the rest. The sharding paths call one entry point per selected layer, which also owns the reentrant fallback for DSA layers. Two op classes are never kept, whatever the markers say: ops with a mutable schema, whose cached tensor can be overwritten before backward reads it, and collectives, whose destination buffer may be allocated outside the interval.
Every layer `recompute_ratio` selects now goes through one entry point, which picks the recompute strategy the layer supports: region-level selective checkpointing, or the legacy reentrant path for layers carrying the DSA top-k lifecycle. `BaseModel.recompute_intervals` is the whole surface the config layer needs; keeping nothing reproduces today's whole-layer recompute. Add the regression tests: kept regions reproduce full-recompute gradients bit for bit and keep them alive, unbalanced and overlapping intervals are safe, in-place writes inside a kept region are rejected rather than silently doubled, and a kept region matches full recompute under domino EP both eager and compiled.
The warnings exist so that a `recompute_cfg` which keeps nothing does not look like a silent no-op, and they are deduplicated because every layer of a model reaches the same diagnosis. Keyed globally on the interval or the layer name, though, the second model in a process -- an RL reference model, a compose model's other tower -- was silenced by the first, which is the same silent no-op wearing a different hat. Key on the diagnosis instead.
The compile-granularity rule was wrong: a region is addressable only if its contents run in eager, not merely its endpoints. `SAVE_MLP` under EP falsifies the old rule -- both markers fire in the uncompiled layer body while the region encloses the compiled shared-expert forward, so nothing is kept. Neither diagnostic could fire for it: the markers ran, and the layer is not compiled as one region. That is a silent no-op of the same shape as the three this stack already found. The session now learns from the policy whether anything was actually kept while an interval was open, which is a statement about contents rather than endpoints, and reports intervals that opened and kept nothing. Diagnostics also aggregate over the owning model instead of firing per layer. Interval maps legitimately span dense and MoE layers, so a marker missing from one layer type is normal, and warning per layer fired on every correctly configured model -- which teaches users to ignore the warnings that matter.
The legacy reentrant path no longer exists, so a layer carrying the DSA top-k lifecycle takes the ordinary selective checkpointing path like any other layer. The test asserted a fallback that was removed with the path itself.
The two comments named `_MarkerSession.report_unreached`, a method that never existed under that name: unreached markers are reported from `_report_pass`, once the owner's layers have all completed a pass.
HAOCHENYE
force-pushed
the
feat/sac-engine
branch
from
July 28, 2026 10:43
5003f04 to
4d14cb3
Compare
HAOCHENYE
commented
Jul 29, 2026
HAOCHENYE
left a comment
Collaborator
Author
There was a problem hiding this comment.
There is a major design flaw: compile cannot be compatible with selective checkpointing. This is completely unacceptable. We need to discuss again.
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-base← you are hererecompute_cfgand per-model marker declarations (3/3) #1981feat/sac-recompute-cfg-v2→feat/sac-engineSummary
The selective-checkpointing engine: a marker session, a per-op policy, and one seam that the FSDP sharding paths call. Whole
DecoderLayergets onecheckpoint(..., use_reentrant=False, context_fn=...);create_selective_checkpoint_contexts(policy_fn)then decides per op whether to keep or recompute. Default isMUST_RECOMPUTE; ops inside an active marker interval areMUST_SAVE.The seam
One unconditional call at all five wrap sites (MoE layers, MTP, dense, both vision towers). Empty
intervals== today's full-layer recompute, so this PR alone is behaviour-preserving. The strategy choice lives inside: DSA-lifecycle layers → legacy reentrant, empty intervals → plain checkpointing, otherwise SAC.Nesting: FSDP outermost →
CheckpointWrapper.forward→ checkpoint boundary → marker session →module.__call__→ hooks →module.forward. The session is opened inside the region so the recompute pass rebuilds its own state, andmodule.__call__(not.forward) keeps module hooks inside the region — an earlier in-placeforwardreplacement moved them out and broke the DSA top-k cache lifecycle.Policy invariants
Two rules apply regardless of marker state, because they are properties of the mechanism:
outargument are rejected inside a kept region. "Never keep mutating ops" is not sufficient: if a region keepszeros_likeand recomputesadd_, recompute pops the forward's already-summed buffer and adds again — finite loss, no error, wrong gradients. Out-variants are exempt on the schema propertyalias_info.is_write, not on op name, because inductor'saten.mm.outis what the policy sees on every compiled layer.c10d/_c10d_functionalcollectives are always recomputed. Keeping a collective is only correct while its destination buffer's allocating op sits in the same interval; an interval boundary splitting them would return garbage silently.Measured on a real 2-layer MoE,
ep_size=2: keeping only the expert GEMMs givesMUST_SAVE 18 / MUST_RECOMPUTE 235; spanning the all2all gives247 / 6with forced recomputes{'c10d:collective': 2, '_c10d_functional:collective': 4}. These were obtained by injecting markers by hand — this PR adds no markers to model code, so the numbers are not reachable from a config until PR #1981 lands.Compile behaviour
contextvarsis untraceable by Dynamo, so bothcheckpoint_recordand the session bind early-return undertorch.compiler.is_compiling(). Consequences, all verified:Diagnostics
A configuration that silently does nothing is the failure mode this feature is most prone to, so three cases are reported, once per model (deduped on the whole diagnosis, held in a
WeakKeyDictionary):Test plan
tests/model/test_selective_checkpointing.py(new): kept-region output and gradients bit-identical to full recompute (atol=rtol=0) plus gradient liveness; unbalanced interval safe; overlapping intervals union; marker outside a session is a no-op; in-place-in-kept-region rejected; DSA layer falls back to reentrant; two GPU cases (2 ranks,ep=2,intra_layer_micro_batch=2) eager and compiled. Diagnostic tests verified red-before-green — the first version passed vacuously becausecaplognever sees loguru output.Plus
tests/model/test_recompute.pyandtests/module/attention/test_dsa_mla.pygreen; pre-commit clean including mypy.