Skip to content

Release the weight-checker snapshot once compare passes - #36561

Merged
yueming-yuan merged 2 commits into
sglang-milesfrom
yueming/weight-checker-release-snapshot
Aug 31, 2026
Merged

yueming-yuan merged 2 commits into
sglang-milesfrom
yueming/weight-checker-release-snapshot

Conversation

@yueming-yuan

@yueming-yuan yueming-yuan commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

The snapshot is a host copy of every weight, and compare is its only consumer. Keeping it after a passed check holds weights-sized host memory for the process lifetime — 35 GB per TP8 rank on DeepSeek-V4-284B, observed idle for a full 10h RL run. A failed compare keeps the snapshot for debugging.

Evidence: on 8×GB300 colocated RL, --check-weight-update-equal fires snapshot/reset/compare exactly once at init; the retained snapshot then contributes 4×35 GB/node to the host budget at every trainer sleep, and the job cgroup's memory.events records boundary oom_kills at the 898 GiB limit with the snapshot resident.

Semantics: a second compare without a fresh snapshot now raises, matching the existing test_without_snapshot_raises contract; two new unit tests pin release-on-success and keep-on-failure. Full test_weight_checker.py suite passes on GB300 (62 passed).


CI States

Latest PR Test (Base): ❌ Run #33426900003
Latest PR Test (Extra): ❌ Run #33426899826
Latest PR Test (AMD ROCm 7.2): ❌ Run #33426899896

@yueming-yuan
yueming-yuan force-pushed the yueming/weight-checker-release-snapshot branch from c751ed0 to f8e95be Compare August 27, 2026 16:24
# and freeing arena chunks does not shrink RSS (Grace/64K pages: even
# malloc_trim reclaims nothing).
named_params = [(name, param.data) for name, param in self._model_state()]
align = 64

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is fixed to 64? should it be optional for input?

@yueming-yuan yueming-yuan Aug 31, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is required, which is the same factor as torch allocator

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment added

@yueming-yuan
yueming-yuan force-pushed the yueming/weight-checker-release-snapshot branch from 893c325 to cb916b8 Compare August 31, 2026 18:46
@yueming-yuan
yueming-yuan merged commit 2f51d37 into sglang-miles Aug 31, 2026
80 of 90 checks passed
@yueming-yuan
yueming-yuan deleted the yueming/weight-checker-release-snapshot branch August 31, 2026 18:48
yueming-yuan added a commit that referenced this pull request Sep 2, 2026
Kimi-K2.5 / Kimi-K3-VL time_weight and MiniCPM-V pos_embed /
temporal_pos_embed are sincos tables built with torch.from_numpy, which
ignores the construction device context, so they stay on the CPU as
non-persistent buffers. No checkpoint and no weight sync carries them, yet
the weight checker resets and compares them; a randomized copy nothing
restores then fails every sync check on the model.

This stayed hidden while the snapshot was param.detach().cpu(): for a CPU
tensor .cpu() returns the tensor itself, so the snapshot aliased the live
buffer and reset moved both sides at once. #36561's arena copy removed the
alias and Kimi-K2.5's check has failed since.

Mark them with the checker's existing per-tensor _skip_weight_check flag at
the definition, the way kv-cache scales are marked. Weight-derived
non-persistent buffers (Inkling's _w2_lin / _a_cat / _w1_delta, GGUF
dequant caches) keep being checked: they are refreshed after every sync
and the check is what proves that refresh ran.
Kh4L pushed a commit to Kh4L/sglang that referenced this pull request Sep 8, 2026
Zhichenzzz pushed a commit that referenced this pull request Sep 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants