Skip to content

[Fix] Restore per-batch device sync in IPC weight update send - #2104

Open
YanhuiDua wants to merge 1 commit into
InternLM:mainfrom
YanhuiDua:fix/ipc-restore-per-batch-device-sync
Open

YanhuiDua wants to merge 1 commit into
InternLM:mainfrom
YanhuiDua:fix/ipc-restore-per-batch-device-sync

Conversation

@YanhuiDua

@YanhuiDua YanhuiDua commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator

Root Cause

#2092 removed the per-batch empty_cache() from IPCWeightTransport._send, treating it as pure memory hygiene. It was load-bearing.

IPC weight updates reuse the same staging tensor across batches, and the rollout engine reads it through CUDA IPC (zero-copy). Buffer reuse is guarded by a cross-process event chain: train records "data ready" -> engine waits, copies, re-records "consumed" -> train waits before overwriting. The problem is that the rollout-side "consumed" re-record is not a reliable completion signal — it can fire before the engine's weight copies finish (verified experimentally: replacing the train-side event wait with a blocking event.synchronize() before the overwrite still produced torn weights). The per-batch empty_cache() is a full device synchronization (cudaFree is a synchronizing call) and was the only effective guard against the next batch overwriting the staging buffer while the engine is still copying the previous one.

Without it, the engine loads torn weights at the first weight sync. On autotest/config/rl_qwen3p5_vl_35B_grpo.py (qwen3.5-VL 35B, colocate IPC, lmdeploy tp2x4), step 1 stays healthy but step 2 (first sync + optimizer update) collapses: rollout entropy 0.23 -> 9.7, mismatch/mismatch_kl 5e-4 -> 2.07, rewards all <= -1 — while train loss / grad_norm stay normal, pinning the corruption to the weight transport, not the optimizer.

Fix

Restore DEVICE_MODULE.empty_cache() at the start of IPCWeightTransport._send (pre-#2092 behavior), with a comment explaining why it is load-bearing. The event-wait guard in build_flattened_tensor_data is kept as defense in depth.

The real fix belongs in LMDeploy: the "consumed" re-record must fire on the stream where the weight copies actually complete. Until then, the per-batch device sync must not be removed.

Test Plan

  • Full-run verification on 8xH200 (rl_qwen3p5_vl_35B_grpo, 2 rollout steps): Fix/empty cache and refine transport #2092 mismatch_kl=2.0689 (BAD); with this fix, step-2 mismatch_kl=5.499e-4, rollout entropy=0.2323 — identical to the healthy baseline (4.5e-4~6e-4 / ~0.23).
  • pre-commit run passes on the changed file.

Performance note

This restores the pre-#2092 per-batch empty_cache() on the IPC send path, so send-path cost returns to pre-#2092 levels. The sync is required for correctness on the staging-reuse path (see Root Cause); the cheaper alternatives were evaluated and rejected: event.synchronize() on the train side was measured BAD (mismatch_kl 2.0577) because the engine-side "consumed" event itself fires too early.

Root Cause: InternLM#2092 removed the per-batch empty_cache() from the IPC
weight-update send path, treating it as pure memory hygiene. It was
load-bearing: IPC batches reuse the same staging tensor that the
rollout engine reads through CUDA IPC, and the rollout-side
"consumed" event record is not a reliable ordering signal - an
event.synchronize() on the train side before the overwrite still
produced torn weights (measured). Without a device sync per batch,
the next batch overwrites the buffer while the engine is still
copying the previous one; the engine then loads torn weights and the
first post-sync rollout collapses (qwen3.5-VL 35B GRPO: rollout
entropy 0.23 -> 9.7, mismatch_kl 5e-4 -> 2.07).

Fix: restore DEVICE_MODULE.empty_cache() at the start of
IPCWeightTransport._send, as before InternLM#2092. Verified on 8xH200
(qwen3.5-VL 35B GRPO, colocate lmdeploy tp2x4): InternLM#2092 mismatch_kl
2.0689, InternLM#2092 plus this sync 5.44e-4 (healthy). The event-wait guard
in build_flattened_tensor_data is kept as defense in depth.

Test Plan: full-run verification on rl_qwen3p5_vl_35B_grpo (2
rollout steps on 8xH200): InternLM#2092 mismatch_kl=2.0689 (BAD); with this
fix, step-2 mismatch_kl=5.499e-4 and rollout entropy=0.2323, both
matching the healthy baseline. pre-commit passes on the changed
file.
@YanhuiDua
YanhuiDua force-pushed the fix/ipc-restore-per-batch-device-sync branch from 88ce2d2 to 2c5d9df Compare September 23, 2026 02:51
@PengchengShi00

Copy link
Copy Markdown
Collaborator

IPC buffer是一直复用的,empty_cache()不会回收这块IPC buffer。但是加了之后就没问题,应该是empty_cache() 会含有synchronize操作,这个同步操作起了作用。后面加 lmdeploy 的checkpoint-engine的时候,我测一下加同步是否可行。然后让LMdeploy加一个权重正确性校验的接口,增加一下单测。

This branch has not been deployed

No deployments
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