Offload routed expert of RL#1957
Conversation
|
@claude review |
| } | ||
| extra_info_dict = finalize_train_policy_metrics(extra_info_dict, DEVICE) |
There was a problem hiding this comment.
Claude: The SFT path resets peak stats before each step (DEVICE_MODULE.reset_peak_memory_stats() at line 831 of the current main), so per-step peaks are meaningful. The RL fit() path has no such reset — these values are monotonically non-decreasing across micro-batches and across fit() calls, so after the first peak they'll be identical in every subsequent log line.
Consider adding DEVICE_MODULE.reset_peak_memory_stats() at the top of the fit() method (or before the training loop) if you want per-rollout peak visibility.
If the intent is to track the overall peak across the entire worker lifetime, this is fine as-is — but then it only needs to be logged once (e.g. at the end of fit()), not per micro-batch.
| if seq_ctx.offload_rollout_routed_experts and rollout_routed_experts.device != hidden_states.device: | ||
| rollout_routed_experts = rollout_routed_experts.contiguous() | ||
| rollout_routed_experts = rollout_routed_experts.to(hidden_states.device) |
There was a problem hiding this comment.
Claude: Nit: since the tensor is unpinned, .to(device) is a synchronous copy regardless. If you later implement prefetch (as mentioned in the PR description), pin_memory() + to(device, non_blocking=True) on a CUDA stream would allow overlapping the transfer with prior-layer compute. Just flagging as a natural next step.
|
Claude: ## Summary Adds ProduceBatchResult impact: not affected IssuesWarning
Nit
VerdictAPPROVE — the core logic is correct and well-scoped. The offload flag cleanly integrates with the existing |
当输入给每个 trainworker 过多的数据后,原本不起眼的 routed expert 所占显存将会变得明显。本 PR 提供通过 offload_rollout_routed_experts 配置来将 routed expert 放置于 cpu 上,当需要时候再移到 gpu 上。
当前没有实现 prefetch 功能,原因是训练耗时占比本身不高。