[Cherry-Pick][Loader] Add values natural order check to layers grouped validation#7822
Conversation
|
Thanks for your contribution! |
bukejiyu
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
CI报告基于以下代码生成（30分钟更新一次）: 1 任务总览当前 Required 任务存在 1 个失败、0 个运行中、0 个等待中；失败原因为 GitHub Actions 账号计费锁定导致 Job 未启动，建议先解除 billing 锁定后重新触发 CI。
2 任务状态汇总2.1 Required任务 : 0/7 通过
2.2 可选任务 — 1/5 通过
3 失败详情（仅 required）Pre Commit — 基础设施（置信度: 高）Pre Commit
根因详情: 关键日志: 修复建议:
修复建议摘要: 解除 billing 锁定后 rerun 关联变更: 无，失败发生在 Job 启动前，与本 PR 修改的 loader 代码无直接关联。 链接: 查看日志 |
…addlePaddle#7498) * Add values natural order check to layers grouped validation * update
bukejiyu
force-pushed
the
cp/47a3cff6-add-values-check
branch
from
96f5722 to
e698271
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release/online/20260415 #7822 +/- ##
==========================================================
Coverage ? 72.36%
==========================================================
Files ? 387
Lines ? 54028
Branches ? 8467
==========================================================
Hits ? 39100
Misses ? 12236
Partials ? 2692
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.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
PaddlePaddle-bot
left a comment
PaddlePaddle-bot
left a comment
There was a problem hiding this comment.
🤖 Paddle-CI-Agent | pr_review |
2026-05-25 19:46:11
📋 Review 摘要
PR 概述：在 layers_grouped 校验中新增 weight_map values（文件名）自然顺序检查，并将 get_weight_iterator 参数由 LoadConfig 升级为 FDConfig 以支持更多配置访问。
变更范围：model_executor/load_weight_utils.py、model_executor/model_loader/default_loader_v1.py
影响面 Tag：[Loader]
问题
| 级别 | 文件 | 概述 |
|---|---|---|
| ❓ 疑问 | load_weight_utils.py:138 |
TP=1 短路条件语义存疑 |
| 📝 PR 规范 | — | 标题缺原始 PR 号；描述各 section 为空 |
❓ 疑问：tensor_parallel_size == 1 短路条件（load_weight_utils.py 第138行）
if is_layers_are_grouped or (parallel_config is not None and parallel_config.tensor_parallel_size == 1):
weights_iterator = safetensors_weights_iterator(files_list)当 is_layers_are_grouped=False（keys 不按 layer 连续分组，或 values 文件名不自然有序）且 TP=1 时，此短路条件会使用 safetensors_weights_iterator(files_list) 按 字典序 (sorted(weight_files_in_index)) 加载权重，而非使用 safetensors_weights_iterator_ordered(ordered_weight_map) 按 natural_key 排序的 ordered map 加载。
请作者确认：
- TP=1 时
is_layers_are_grouped=False的触发场景是什么？TP=1 时权重加载顺序对模型正确性真的无影响吗？ - 若此短路是为了兼容某类特定格式（keys 乱序但文件 natural order 本来正确），建议在代码注释中说明该场景，避免后续维护者误解。
📝 PR 规范检查
问题 1：Cherry-Pick 标题缺少原始 PR 号（格式要求：[Cherry-Pick][Tag] 描述(#原PR号)）。
标题建议（可直接复制）：
[Cherry-Pick][Loader] Add values natural order check to layers grouped validation(#原PR号)
问题 2：PR 描述各 section 均为空/仅占位符，不合规。
PR 描述建议（可直接复制，复刻 checklist §D2 模板完整结构）：
## Motivation
在使用多 safetensors 分片（`model.safetensors.index.json`）的模型中，`is_layers_are_grouped` 的判断原来只检查 weight_map 的 keys 是否按 layer 连续分组，未检查 values（分片文件名）是否按自然顺序排列。当 values 乱序时，`is_layers_are_grouped=True` 会导致使用 `safetensors_weights_iterator(files_list)` 而非 `safetensors_weights_iterator_ordered(ordered_weight_map)` 进行加载，从而可能引起权重与参数名映射错误。
## Modifications
- `load_weight_utils.py`：新增 `values_are_naturally_ordered(values)` 函数，检查 weight_map values（分片文件名列表）是否按 `natural_key` 自然顺序排列
- `load_weight_utils.py`：`get_all_weights_file` 中将 `is_layers_are_grouped` 改为 `is_keys_orders and is_values_naturally_ordered` 双重检查
- `load_weight_utils.py`：`get_weight_iterator` 参数由 `Optional[LoadConfig]` 升级为 `Optional[FDConfig]`，以同时支持 `load_config`（多线程加载）和 `parallel_config`（TP size）的访问；TP=1 时新增短路条件直接使用无序 iterator
- `default_loader_v1.py`：调用 `get_weight_iterator` 时改传 `fd_config` 而非 `fd_config.load_config`
## Usage or Command
N/A
## Accuracy Tests
N/A
## Checklist
- [x] Add at least a tag in the PR title.
- Tag list: [`[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]`]
- You can add new tags based on the PR content, but the semantics must be clear.
- [ ] 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.
- [x] 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.总体评价
变更思路正确——在原有 keys 分组检查之外补充 values 自然顺序校验，能有效减少误判。get_weight_iterator 参数升级为 FDConfig 也更合理。主要疑问是新增的 TP=1 短路条件（第138行）语义尚不明确，建议作者补充注释说明触发场景。
freeliuzc
merged commit 0d7fccd
into
PaddlePaddle:release/online/20260415
6 participants
Motivation
Modifications
Usage or Command
Accuracy Tests
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.