Price HybridEP recompute outputs at the live communication extent - #971
Conversation
|
Review requested from McCarthy and Minsky for exact head This narrow Author checks: 128 relevant CPU tests plus type/lint/format. Independent source review: 10 new focused CPU cases pass. Automatic CI is pending; neither the regression nor broader GPU CI proves the historical full backward fits. Prior broad estimator work is substantially superseded by current main and is not included. Please review correctness and explicitly assess eligibility under Brad's standing three-agent rule: small/medium scope, no API change, no |
|
Routing update: McCarthy and Minsky are unavailable in the active agent directory. The three independent reviewers for this exact head are the existing Schulman delegates |
|
Independent review — Schulman delegate CLEAR on exact head The allocation chain is consistent: Validation: independently ran only the ten new targeted CPU cases; 10 passed, 57 deselected, no skips. They cover zero buffer growth with a live larger extent, the 896,008,192-byte output floor, resulting admission refusal, expired/absent/smaller extents, profile precedence, and excluded modes/topologies. CUDA was hidden; fixtures use CPU/meta tensors and no model forward. The positive regression asserts CUDA remains uninitialized. All function signatures/decorators are unchanged, and the complete Eligibility assessment: this can split or refuse calls that the old estimate admitted. That is the direct, already requested correction for the planner omitting a necessary allocation before an OOM; I do not classify it as a separate significant behavior requiring fresh approval under the standing rule. It introduces no extra collective, allocator/cache release, execution-state mutation, public API, or The component floor is not a complete backward-memory bound, a fix for CP2/EP1, or native fit evidence. I ran no native/GPU test, do not claim green hosted GPU CI, and did not merge. The remaining exact-head reviews and required CI disposition stay separate. |
|
Independent review — Schulman delegate CLEAR on exact head I traced the actual supported source path: live prior graph → restored communication high-water → checkpoint reruns forward → Independent validation: 10 focused CPU tests passed, 57 deselected, no skips. CUDA was hidden; CPU/meta fixtures ran without a model forward, and the positive regression asserts CUDA remains uninitialized. The tests cover zero buffer growth with the 896,008,192-byte fresh-output floor, changed admission, profile precedence, live/expired/absent/smaller extents and excluded modes. All 344 function signatures/decorators in Eligibility: this can split or refuse work admitted by the former underestimate. I consider that the direct, already requested correction for a necessary allocation omitted before OOM, within TrainerRank. It introduces no separate significant behavior needing fresh approval under the standing rule: no extra collective, cache release, allocation action, routing change, public API or This component floor is not a complete backward bound, CP2/EP1 coverage, or native proof that the historical workload fits. I ran no native/GPU test, did not merge, and do not claim green GPU CI. Remaining exact-head reviews and required CI disposition remain separate. |
|
Independent delegate review by The 14 production lines price the fresh HybridEP recompute output at the applicable communication extent within the existing workspace maximum. Prior high-water extent applies only while a pending weak graph is live; the check does not prune references, change routing, add collectives or release caches. I independently checked the allocation/source chain and ran the 10 new focused CPU cases: all passed. This does not prove that every admitted backward fits. No public API or Required CI and native qualification remain separate. This review does not turn a capacity failure before test execution into a pass, waive CI, establish native runtime qualification or authorize merge by itself. The three exact-head CLEAR reviews and required checks still apply. Local retained receipts (paths are workstation evidence, not public downloads):
|
|
Local qualification completed for exact head
This used the pinned local Python/runtime, private test dependency supplement, owned caches and an offline model snapshot; it is not hosted-image equivalence. The first local attempt's pytest temporary-symlink guard failure is preserved; the fresh reviewed guard correction changed no test or ART source. Required hosted GPU CI remains failed: run 36123275999, attempt 2 could not acquire H200×2 before remote setup/tests. Local qualification does not waive that gate. The high-water-specific native fixture remains separate; this suite does not prove that regression, complete backward fit or memory savings. No merge performed. Independent retained-result audit SHA256: |
|
Native validation on exact head Whole target-logprob hashes, admitted plans and all14 recorded selected-adapter gradient tensors per small condition matched between plain and observed passes on both ranks. The measured calls added no Dynamo counter changes after warmup. The run and exact registered processes closed successfully. This confirms the additional recompute-output workspace component in this fixture. Other/profile terms kept the total admission estimate unchanged, so it does not demonstrate a changed admission decision or prevented OOM. It is not an exhaustive size/topology test, allocator-peak proof or full-model memory bound; no unpatched-head native comparison was run. |
Retain the reviewed CP2/EP2 combine-output floor beside current TP4 sequence-parallel checkpoint pricing. The HybridEP guard and regression additions are unchanged from 819dda8; preserve the current GDN segment accounting and fixture signature.
819dda8 to
dffe286
Compare
Picks up main (0e0c31b) through the updated #963 and #978 branches. _checkpoint_floor_decoder stays TP1 by default, so the dense widths and their discounts remain TP1/CP2 only; with sequence_parallel it applies main's #988 checks for the traced TP x SP floor. _checkpoint_memory_floor prices TP > 1 with #988's floor, then the layout and generic floors, and #971's combine-extent floor after all three. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A small gradient-enabled call can be admitted while an earlier, larger HybridEP graph is still live. Checkpoint recomputation uses the restored communication extent: combine allocates a fresh BF16
[P, H]output before cropping it to the smaller logical result. Existing buffer capacity does not cover this new allocation.For supported CP2/EP2 full recomputation, include that output as a conservative floor in checkpoint workspace. Read the prior extent only while its graph is live and take a maximum with existing stages. This can split or refuse a previously admitted call. Public APIs,
art.megatron, routing and execution are unchanged.The current revision preserves main's TP4/SP/GDN checkpoint accounting. Its 14 production lines and 167 added test lines are byte-identical to the previous candidate; removing them reconstructs the two current-main files exactly.
Validation for exact head
dffe2869c0806212c0d76c34413ccbb2362f5119:Earlier hosted GPU acquisition failure is retained as historical evidence, superseded only for current-head CI by the successful run above.
Related to #848 and #949. CP2/EP1 remains outside this correction. Draft remains held for Brad's decision on the admission behavior change. No deployment or frozen-run adoption is included.