[RL][BugFix][Optimization] Support chunked part files loading and fix model path format in IPC snapshot strategy#6852
Open
wikilsh wants to merge 9 commits intoPaddlePaddle:developfrom
Open
[RL][BugFix][Optimization] Support chunked part files loading and fix model path format in IPC snapshot strategy#6852wikilsh wants to merge 9 commits intoPaddlePaddle:developfrom
wikilsh wants to merge 9 commits intoPaddlePaddle:developfrom
Conversation
## Motivation
When using IPC snapshot for elastic recovery in RL training, loading a single large pdparams file causes a significant memory spike. This PR refactors `_update_ipc_snapshot` to support loading chunked part files to avoid the memory spike.
## Modifications
Refactored `_update_ipc_snapshot` in `fastdeploy/rl/dynamic_weight_manager.py` with a three-level loading priority:
1. **Chunked part files** (`model_state.tpR{id}.part{N}.pdparams`): Load multiple smaller shards sequentially, freeing memory between each chunk via `gc.collect()` to avoid memory spike.
2. **Single full file** (`model_state.tpR{id}.pdparams`): Legacy single-file loading path (preserved for backward compatibility).
3. **Shared fallback directory** (`/shared_ipc_meta/...`): Oldest legacy fallback path (preserved for backward compatibility).
Also fixed the rank ID in the file name pattern from hardcoded `tp0` to dynamic `paddle.distributed.get_rank()`.
## Checklist
- [ ] Add at least a tag in the PR title.
- [ ] Format your code, run `pre-commit` before commit.
- [ ] Add unit tests. Please write the reason in this PR if no unit tests.
- [ ] Provide accuracy results.
- [ ] If the current PR is submitting to the `release` branch, make sure the PR has been submitted to the `develop` branch, then cherry-pick it to the `release` branch with the `[Cherry-Pick]` PR tag.
Co-Authored-By: lishuaihui <lishuaihui@baidu.com>
## Motivation
When using IPC snapshot for elastic recovery in RL training, loading a single large pdparams file causes a significant memory spike. This PR refactors `_update_ipc_snapshot` to support loading chunked part files to avoid the memory spike.
## Modifications
Refactored `_update_ipc_snapshot` in `fastdeploy/rl/dynamic_weight_manager.py` with a three-level loading priority:
1. **Chunked part files** (`model_state.tpR{id}.part{N}.pdparams`): Load multiple smaller shards sequentially, freeing memory between each chunk via `gc.collect()` to avoid memory spike.
2. **Single full file** (`model_state.tpR{id}.pdparams`): Legacy single-file loading path (preserved for backward compatibility).
3. **Shared fallback directory** (`/shared_ipc_meta/...`): Oldest legacy fallback path (preserved for backward compatibility).
Also fixed the rank ID in the file name pattern from hardcoded `tp0` to dynamic `paddle.distributed.get_rank()`.
## Checklist
- [ ] Add at least a tag in the PR title.
- [ ] Format your code, run `pre-commit` before commit.
- [ ] Add unit tests. Please write the reason in this PR if no unit tests.
- [ ] Provide accuracy results.
- [ ] If the current PR is submitting to the `release` branch, make sure the PR has been submitted to the `develop` branch, then cherry-pick it to the `release` branch with the `[Cherry-Pick]` PR tag.
Co-Authored-By: lishuaihui <lishuaihui@baidu.com>
…in IPC snapshot
## Motivation
The previous snapshot file naming `model_state.tp{rank}{id}` concatenated
rank and id without a separator, causing ambiguity (e.g., rank=1, id=234
and rank=12, id=34 both produce `tp1234`). Additionally, after the naming
format is updated, existing checkpoints saved in the old format would fail
to load during elastic recovery, causing unnecessary failures.
## Modifications
- Add dot separator between rank and id in snapshot file name:
`model_state.tp{rank}{id}` → `model_state.tp{rank}.{id}`
- Add Priority 3 legacy fallback to load old-format files
(`model_state.tp0{id}.pdparams`) for backward compatibility during
rolling upgrades
- Update docstring and error message to reflect the new 4-level priority
Co-Authored-By: lishuaihui <lishuaihui@baidu.com>
Cover all 4 loading priority branches (chunked part files, single full pdparams, legacy format, shared directory fallback) with mock-based tests to verify correct behavior without filesystem or GPU dependencies. Co-Authored-By: lishuaihui <lishuaihui@baidu.com>
|
Thanks for your contribution! |
Co-Authored-By: lishuaihui <lishuaihui@baidu.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #6852 +/- ##
==========================================
Coverage ? 73.42%
==========================================
Files ? 399
Lines ? 55668
Branches ? 8768
==========================================
Hits ? 40876
Misses ? 11868
Partials ? 2924
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This was referenced Mar 18, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
该 PR 聚焦于 RL 训练的 IPC snapshot 弹性恢复路径:通过支持按 chunk 分片加载来降低大权重文件一次性加载导致的内存峰值,并修正 snapshot 文件命名格式以避免歧义与多进程加载错误。
Changes:
- 重构
DynamicWeightManager._update_ipc_snapshot:按“分片文件 → 单文件 → 旧格式 → shared 兜底”的优先级加载,并在分片之间gc.collect()。 - 修复 snapshot 文件名格式:
tp{rank}{id}→tp{rank}.{id},并新增旧格式兼容分支与更清晰的FileNotFoundError。 - 新增单测
tests/rl/test_update_ipc_snapshot.py覆盖 4 条分支与错误路径,并验证分片排序行为。
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| fastdeploy/rl/dynamic_weight_manager.py | 实现 chunked part files 优先加载、文件名格式修复、兼容旧格式与兜底路径,并在找不到快照时抛出明确异常 |
| tests/rl/test_update_ipc_snapshot.py | 为 _update_ipc_snapshot 新增单测,覆盖 4 级优先级与异常分支,验证分片顺序与 gc.collect() 行为 |
You can also share your feedback on Copilot code review. Take the survey.
tests/rl/test_update_ipc_snapshot.py
Outdated
Comment on lines
+57
to
+58
| # Without this, fastdeploy.__getattr__ silently returns None for "rl", | ||
| # causing _importer to fail with AttributeError when the test runs first |
Comment on lines
+226
to
+229
| for idx, part_path in enumerate(part_files, start=1): | ||
| logger.info(f"Loading snapshot part {idx}/{len(part_files)} from {part_path}") | ||
| ipc_state_dict = paddle.load(part_path, safetensors=True) | ||
| self._update_model_from_state(ipc_state_dict, f"snapshot-part{idx}") |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Parse part index from filename (e.g. .part0.) instead of using enumerate index, so that logs and src_type stay consistent with the actual file naming convention. Co-Authored-By: wikilsh <wiki_hui@qq.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
In RL training elastic recovery via IPC snapshot, two issues existed in
_update_ipc_snapshot:.pdparamsfile in one shot causes a significant memory spike, risking OOM during recovery.model_state.tp{rank}{id}without a separator between rank and id, causing naming ambiguity (e.g., rank=1, id=234 and rank=12, id=34 both producetp1234). Additionally, the rank was previously hardcoded astp0, causing all ranks to load rank-0's shard in multi-process scenarios.Modifications
Refactored
_update_ipc_snapshotinfastdeploy/rl/dynamic_weight_manager.pywith a four-level loading priority:model_state.tp{rank}.{id}.part{N}.pdparams): Load multiple smaller shards sequentially in ascending numeric order, releasing memory between each chunk viagc.collect()to avoid memory spike.model_state.tp{rank}.{id}.pdparams): Standard single-file loading with corrected naming format (dot separator added).model_state.tp0{id}.pdparams): Backward-compatible fallback for checkpoints saved in the old format, ensuring smooth rolling upgrades./shared_ipc_meta/...): Oldest legacy fallback path preserved for compatibility.Key fixes:
tp{rank}{id}→tp{rank}.{id}to eliminate naming ambiguitytp0withpaddle.distributed.get_rank()so each rank loads its own shard correctlyFileNotFoundErrorwith clear message when no snapshot is found in any candidate pathUsage or Command
During RL elastic recovery, the trainer will automatically select the appropriate loading strategy based on available snapshot files. No additional configuration is needed.
To generate chunked part files on the trainer side, split the full state dict and save as:
model_state.tp{rank}.{id}.part0.pdparamsmodel_state.tp{rank}.{id}.part1.pdparams...
Accuracy Tests
This PR only modifies the weight loading path and chunking strategy in
_update_ipc_snapshot. It does not affect model forward computation or kernel logic. No accuracy regression is expected.Checklist
pre-commitbefore commit.developbranch.