[optim]--rematerialize-param-from-master-weight: save the bf16 weight backup in colocate#1572
[optim]--rematerialize-param-from-master-weight: save the bf16 weight backup in colocate#1572yueming-yuan wants to merge 13 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces the --restore-weights-from-fp32-main optimization, which reconstructs low-precision model weights from the distributed optimizer's fp32 main parameters instead of maintaining a full pinned CPU copy during rollout pauses, thereby reducing host memory usage. Feedback on these changes highlights several robustness improvements: first, frozen parameters (requires_grad=False) should be included in the extras backup to prevent startup assertion failures; second, direct attribute access on model modules for buffers should use getattr to avoid potential AttributeErrors; and third, defensive checks should be added before invoking optimizer and model chunk synchronization methods to handle edge cases like disabled optimizers.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| def named_restore_extras(model: Sequence[torch.nn.Module]) -> Iterator[tuple[str, torch.Tensor]]: | ||
| """Tensors NOT reconstructable from the distributed optimizer's fp32 main | ||
| params: expert_bias buffers (the only buffers synced today, see | ||
| _named_params_and_buffers_vanilla) and fp32-dtype parameters, whose | ||
| optimizer "main" is a view of the param itself (shard_fp32_groups).""" | ||
| for vp_stage, model_module in enumerate(model): | ||
| for name, buffer in model_module.named_buffers(): | ||
| if "expert_bias" in name: | ||
| yield f"vp_stages.{vp_stage}.{strip_param_name_prefix(name)}", buffer | ||
| for name, param in model_module.named_parameters(): | ||
| if param.dtype == torch.float32: | ||
| yield f"vp_stages.{vp_stage}.{strip_param_name_prefix(name)}", param |
There was a problem hiding this comment.
Parameters that do not require gradients (requires_grad=False, i.e., frozen parameters) are not managed by the distributed optimizer or DDP buffers. If a model contains any frozen parameters, they will be missed by the restore path and cause a startup assertion failure in _assert_restore_coverage. Including them in named_restore_extras ensures they are correctly backed up and restored.
| def named_restore_extras(model: Sequence[torch.nn.Module]) -> Iterator[tuple[str, torch.Tensor]]: | |
| """Tensors NOT reconstructable from the distributed optimizer's fp32 main | |
| params: expert_bias buffers (the only buffers synced today, see | |
| _named_params_and_buffers_vanilla) and fp32-dtype parameters, whose | |
| optimizer "main" is a view of the param itself (shard_fp32_groups).""" | |
| for vp_stage, model_module in enumerate(model): | |
| for name, buffer in model_module.named_buffers(): | |
| if "expert_bias" in name: | |
| yield f"vp_stages.{vp_stage}.{strip_param_name_prefix(name)}", buffer | |
| for name, param in model_module.named_parameters(): | |
| if param.dtype == torch.float32: | |
| yield f"vp_stages.{vp_stage}.{strip_param_name_prefix(name)}", param | |
| def named_restore_extras(model: Sequence[torch.nn.Module]) -> Iterator[tuple[str, torch.Tensor]]: | |
| """Tensors NOT reconstructable from the distributed optimizer's fp32 main | |
| params: expert_bias buffers (the only buffers synced today, see | |
| _named_params_and_buffers_vanilla), fp32-dtype parameters, and parameters | |
| that do not require gradients (frozen parameters).""" | |
| for vp_stage, model_module in enumerate(model): | |
| for name, buffer in model_module.named_buffers(): | |
| if "expert_bias" in name: | |
| yield f"vp_stages.{vp_stage}.{strip_param_name_prefix(name)}", buffer | |
| for name, param in model_module.named_parameters(): | |
| if param.dtype == torch.float32 or not param.requires_grad: | |
| yield f"vp_stages.{vp_stage}.{strip_param_name_prefix(name)}", param |
| for model_module in self.model: | ||
| for buffer in model_module.buffers + model_module.expert_parallel_buffers: | ||
| restorable.update(id(p) for p in buffer.params) |
There was a problem hiding this comment.
Using direct attribute access like model_module.buffers and model_module.expert_parallel_buffers can raise an AttributeError if the model chunks are not wrapped in Megatron's DDP wrapper or if the Megatron version/configuration differs (e.g., if expert parallel buffers are not initialized). Using getattr with a default empty list makes this check robust and prevents potential crashes.
| for model_module in self.model: | |
| for buffer in model_module.buffers + model_module.expert_parallel_buffers: | |
| restorable.update(id(p) for p in buffer.params) | |
| for model_module in self.model: | |
| buffers = getattr(model_module, "buffers", []) + getattr(model_module, "expert_parallel_buffers", []) | |
| for buffer in buffers: | |
| restorable.update(id(p) for p in getattr(buffer, "params", [])) |
| optimizers = getattr(self._ctx.optimizer, "chained_optimizers", [self._ctx.optimizer]) | ||
| for optimizer in optimizers: | ||
| optimizer._copy_main_params_to_model_params() | ||
| for model_chunk in self._ctx.model_chunks: | ||
| model_chunk.start_param_sync(force_sync=True) |
There was a problem hiding this comment.
To adhere to defensive programming guidelines, we should add safety guards before calling _copy_main_params_to_model_params and start_param_sync. For example, if --debug-disable-optimizer is enabled, self._ctx.optimizer will be None, leading to an AttributeError when trying to call _copy_main_params_to_model_params. Similarly, checking if model_chunk has start_param_sync prevents crashes if any model chunks are not wrapped in DDP.
| optimizers = getattr(self._ctx.optimizer, "chained_optimizers", [self._ctx.optimizer]) | |
| for optimizer in optimizers: | |
| optimizer._copy_main_params_to_model_params() | |
| for model_chunk in self._ctx.model_chunks: | |
| model_chunk.start_param_sync(force_sync=True) | |
| optimizers = getattr(self._ctx.optimizer, "chained_optimizers", [self._ctx.optimizer]) | |
| for optimizer in optimizers: | |
| if optimizer is not None and hasattr(optimizer, "_copy_main_params_to_model_params"): | |
| optimizer._copy_main_params_to_model_params() | |
| for model_chunk in self._ctx.model_chunks: | |
| if hasattr(model_chunk, "start_param_sync"): | |
| model_chunk.start_param_sync(force_sync=True) |
1a09a1a to
28afe89
Compare
d5aeeab to
161e943
Compare
28afe89 to
70c489d
Compare
6a1aa8a to
1c092aa
Compare
c91d36e to
096738b
Compare
…ptimizer fp32 mains
…licit get() membership, direct chained_optimizers
7dcb2cc to
ad929e2
Compare
Stacked on #1571.
TLDR
On GLM 5.2, 64 * GB300, before -> after optimization:
CPU memory 858.2 GB -> 729.2 GB (-129 GB)
weight update time 76.3 s -> 73.8 s (-2.5 s)
wake_up time (include rematerialize) 4.6 s -> 4.8 s (+0.2 s)
Motivation
In colocate, the weights_backuper keeps a pinned CPU copy of the bf16 weights (2 bytes/param/rank) to (a) restore params after the TMS rollout pause and (b) feed update_weights while the actor is paused. On GLM-5.2 744B that copy is 26-37.5 GB/rank — 104-150 GB per node sitting inside a rollout-peak budget that measured 858/898 GiB.
Both jobs can be done without it:
Guarantees
Measured