[Loader] Add values natural order check to layers grouped validation#7498
Conversation
|
Thanks for your contribution! |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
CI报告基于以下代码生成（30分钟更新一次）: 1 任务总览所有 Required 任务均已通过 ✅，PR 可合并（有 1 个 Optional 任务失败，不阻塞合并）。
2 任务状态汇总2.1 Required任务 : 10/10 通过
2.2 可选任务 — 31/32 通过
3 失败详情（仅 required）无 required 失败任务。 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #7498 +/- ##
==========================================
Coverage ? 63.33%
==========================================
Files ? 462
Lines ? 64378
Branches ? 9871
==========================================
Hits ? 40776
Misses ? 20829
Partials ? 2773
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 comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
bukejiyu
force-pushed
the
feat/weight-loading-natural-order
branch
from
b682deb to
22da62c
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
CI报告基于以下代码生成（30分钟更新一次）: 1 任务总览❌ 2 个 Required 任务失败，需优先处理后方可合并。
2 任务状态汇总2.1 Required任务 : 7/9 通过
2.2 可选任务 — 29/32 通过
3 失败详情（仅 required）Run FastDeploy Unit Tests and Coverage / run_tests_with_coverage — 测试失败（置信度: 高）Run FastDeploy Unit Tests and Coverage / run_tests_with_coverage
失败用例:
根因详情: 关键日志: 修复建议:
修复建议摘要: 关联变更: 链接: 查看日志 xpu_4cards_case_test / run_xpu_4cards_cases — 测试失败（置信度: 中）xpu_4cards_case_test / run_xpu_4cards_cases
失败用例:
根因详情: 关键日志: 修复建议:
修复建议摘要: 已知不稳定，建议 rerun 验证 关联变更: 不明确（v0 loader 路径，与 PR 修改的 v1 路径解耦） 链接: 查看日志 |
PaddlePaddle-bot
left a comment
PaddlePaddle-bot
left a comment
There was a problem hiding this comment.
🤖 Paddle-CI-Agent | pr_review |
2026-05-19 18:08:58
📋 Review 摘要
PR 概述：在 weight_map 的 layers 分组校验中新增 values 自然顺序检查，修复多 shard 场景下因文件乱序读取引发的加载 OOM 问题。
变更范围：fastdeploy/model_executor/load_weight_utils.py、model_loader/default_loader_v1.py
影响面 Tag：[Loader]
问题
| 级别 | 文件 | 概述 |
|---|---|---|
| 🟡 建议 | load_weight_utils.py:138 |
TP=1 短路条件缺少注释说明意图 |
| 🟡 建议 | load_weight_utils.py:77 |
新增函数缺少单元测试覆盖 |
| 📝 PR 规范 | — | Checklist unit tests 未勾选且未说明原因 |
📝 PR 规范检查
标题格式 [Loader] Add values natural order check to layers grouped validation 规范，Tag 合规。描述结构完整，包含所有必填 section。
Checklist 中 [ ] Add unit tests. 未勾选，但 PR 正文未说明原因（模板要求"Please write the reason in this PR if no unit tests"）。建议补充说明或直接添加测试。
总体评价
核心修复逻辑正确：通过同时要求 keys 分组和 values 自然有序来决定加载策略，TP=1 时强制走 file-by-file 规避 OOM 也合理。主要建议是为 TP=1 条件补充注释说明意图，并补充针对 values_are_naturally_ordered 的单元测试，以防后续回归。
| ) | ||
| else: | ||
| if is_layers_are_grouped: | ||
| if is_layers_are_grouped or (parallel_config is not None and parallel_config.tensor_parallel_size == 1): |
There was a problem hiding this comment.
🟡 建议 新增的 TP=1 短路条件缺少注释说明意图
当 tensor_parallel_size == 1 时，强制走 safetensors_weights_iterator(files_list)（按文件名顺序逐文件加载），绕过了 is_layers_are_grouped 的检查。这是合理的优化（单卡场景下权重加载顺序对正确性无影响，且可避免 ordered iterator 跨 shard 跳读引发的 OOM），但直接阅读时逻辑意图不明显。
建议加上注释说明：
if is_layers_are_grouped or (
# For TP=1, sequential file-by-file loading is always safe and avoids
# the OOM risk of safetensors_weights_iterator_ordered jumping between
# shard files in non-sequential order.
parallel_config is not None and parallel_config.tensor_parallel_size == 1
):|
|
||
| def values_are_naturally_ordered(values): | ||
| """Check if values are sorted in natural order.""" | ||
| return list(values) == sorted(values, key=natural_key) |
There was a problem hiding this comment.
🟡 建议 values_are_naturally_ordered 缺少单元测试覆盖
该函数是此 PR 核心修复逻辑的入口，但测试文件中未新增对应测试用例。现有 test_get_weight_iterator_ordered_and_kv_scale 的 weight_map 全部指向同一个 shard，无法覆盖「多 shard 文件中 values 无序」的触发场景。
建议补充：
- 单测
values_are_naturally_ordered（有序/无序各一个 case） - 在
test_get_weight_iterator_ordered_and_kv_scale风格下添加多 shard、values 无序的集成场景（如 layer 0 → shard-2, layer 1 → shard-1）
此逻辑是 OOM 修复的关键路径，回归测试缺失时风险较高。
4 participants
Motivation
修复
weight_map中 value（权重文件名）无序时可能引发加载 OOM 的问题。原有逻辑仅校验 keys 是否按 layer 分组，未检查 values（各权重文件名）是否按自然顺序排列，导致在某些模型下按非顺序读取多个 shard 文件，需同时持有多个 shard 数据从而 OOM。Modifications
load_weight_utils.py：新增values_are_naturally_ordered(values)辅助函数，利用natural_key校验 values 是否按自然顺序排列get_all_weights_file：将is_layers_are_grouped的计算逻辑从仅检查 keys 分组扩展为同时要求 values 自然有序，两者均满足时才启用分组加载策略Usage or Command
N/A
Accuracy Tests
N/A
Checklist
[FDConfig],[APIServer],[Engine],[Scheduler],[PD Disaggregation],[Executor],[Graph Optimization],[Speculative Decoding],[RL],[Models],[Quantization],[Loader],[OP],[KVCache],[DataProcessor],[BugFix],[Docs],[CI],[Optimization],[Feature],[Benchmark],[Others],[XPU],[HPU],[GCU],[DCU],[Iluvatar],[Metax]]pre-commitbefore commit.releasebranch, make sure the PR has been submitted to thedevelopbranch, then cherry-pick it to thereleasebranch with the[Cherry-Pick]PR tag.