[OP][Loader]decrease code && make code clean#7877
Conversation
|
Thanks for your contribution! |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
CI报告基于以下代码生成（30分钟更新一次）: 1 任务总览当前无 required 失败、运行中或等待中的任务；Required 任务
2 任务状态汇总日志列说明：失败任务直接使用日志链接；运行中/等待中任务使用 Job 或 Workflow 链接（如可用）。 2.1 Required任务 : 9/10 通过
2.2 可选任务 — 29/33 通过
3 失败详情（仅 required）无 required 失败任务，本轮未调用深度失败分析。 |
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.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #7877 +/- ##
==========================================
Coverage ? 63.59%
==========================================
Files ? 462
Lines ? 64483
Branches ? 9881
==========================================
Hits ? 41007
Misses ? 20700
Partials ? 2776
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:
|
PaddlePaddle-bot
left a comment
PaddlePaddle-bot
left a comment
There was a problem hiding this comment.
🤖 Paddle-CI-Agent | pr_review |
2026-05-21 16:53:04
📋 Review 摘要
PR 概述：将 linear.py 中 5 处重复的 weight dtype 处理逻辑提取为公共函数 may_be_do_cast()，同时将原 else 分支的 .cast() 静默转换改为 assert 报错
变更范围：fastdeploy/model_executor/layers/linear.py、tests/model_executor/test_linear.py
影响面 Tag：[OP] [Loader]
问题
| 级别 | 文件 | 概述 |
|---|---|---|
| 🔴 Bug | fastdeploy/model_executor/layers/linear.py:54 |
assert 用于运行时 dtype 校验，在 Python -O 模式下会被跳过，导致错误静默 |
| 🔴 兼容性 | fastdeploy/model_executor/layers/linear.py:53 |
Breaking Change：原 else 分支 .cast() 允许隐式 dtype 转换，新代码改为 assert 失败，破坏已有模型加载流程 |
📝 PR 规范检查
PR 标题 decrease code && make code clean 缺少官方 Tag，PR 描述缺少所有必填 section（Motivation / Modifications / Usage or Command / Accuracy Tests / Checklist）。
标题建议（可直接复制）：
[OP] Extract may_be_do_cast() to deduplicate weight dtype handling in linear layers
PR 描述建议（可直接复制，必须复刻 checklist §D2 模板的完整结构）：
## Motivation
`linear.py` 中 `ColumnParallelLinear`、`RowParallelLinear`、`QKVParallelLinear`、`MergedColumnParallelLinear`、`gate_weight_loader` 等 5 处存在完全相同的 weight dtype 兼容处理逻辑，重复代码不利于维护。将其提取为公共函数 `may_be_do_cast()`，消除代码重复；同时将 `else` 分支由静默 `.cast()` 改为报错，以便在 dtype 不匹配时尽早暴露问题。
## Modifications
- 新增 `may_be_do_cast(loaded_weight, param)` 公共函数，封装 shape 校验、int8→float8_e4m3fn 的 `.view()` 转换及其余 dtype 不匹配时的报错逻辑
- 替换 `ColumnParallelLinear.weight_loader`、`RowParallelLinear.weight_loader`、`QKVParallelLinear.qkv_weight_loader`、`MergedColumnParallelLinear.weight_loader`、`gate_weight_loader` 中的重复 dtype 处理代码
- 更新 `tests/model_executor/test_linear.py`，将测试中 `float16`/`int8` 权重改为 `float32`，适配新的严格 dtype 校验
## Usage or Command
N/A
## Accuracy Tests
N/A
## Checklist
- [ ] 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.
- [ ] 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.总体评价
PR 成功将 5 处重复的 dtype 处理逻辑抽取为公共函数，有利于维护；但存在两个需要修复的阻塞性问题：① else 分支由 .cast() 改为 assert 属于 Breaking Change，会导致已有的隐式 dtype 转换场景在运行时直接报错；② 运行时校验使用 assert 在 Python -O 优化模式下会被静默跳过，应改为 raise ValueError。
| if loaded_weight.dtype == paddle.int8 and param.dtype == paddle.float8_e4m3fn: | ||
| loaded_weight = loaded_weight.view(param.dtype) | ||
| else: | ||
| assert ( |
There was a problem hiding this comment.
🔴 Bug assert 用于运行时 dtype 校验，在 Python -O 优化模式下会被跳过，导致错误静默
assert 语句在 Python 以 -O 参数运行时会被完全忽略，不适合用于运行时参数校验。
建议修复方式：
else:
raise ValueError(
f"loaded_weight.dtype: {loaded_weight.dtype}, param.dtype: {param.dtype}"
)| if loaded_weight.dtype != param.dtype: | ||
| if loaded_weight.dtype == paddle.int8 and param.dtype == paddle.float8_e4m3fn: | ||
| loaded_weight = loaded_weight.view(param.dtype) | ||
| else: |
There was a problem hiding this comment.
🔴 兼容性 Breaking Change：原 else 分支 .cast() 改为 assert，破坏已有隐式 dtype 转换
原代码：
else:
loaded_weight = loaded_weight.cast(param.dtype)允许隐式 dtype 转换（如 float16 → float32），兼容多种权重文件格式。新代码改为 assert / raise 后，除 int8→float8_e4m3fn 外的所有 dtype 不匹配均会在模型加载时直接报错，属 Breaking Change。
需在 PR 描述中明确说明此语义变更的意图，并确认所有已支持模型/权重格式均不依赖隐式 dtype 转换，再合入。
zhoutianzi666
changed the title
EmmonsCurse
left a comment
EmmonsCurse
left a comment
There was a problem hiding this comment.
LGTM～ Skip coverage check as it mainly relies on tests with float8.
4 participants
📋 Review 摘要
PR 概述：将
linear.py中多处重复的 weight dtype 处理逻辑提取为公共函数may_be_do_cast()，同时将原else分支的.cast()静默转换改为assert报错变更范围：
fastdeploy/model_executor/layers/linear.py影响面 Tag：
[OP][Loader]问题
fastdeploy/model_executor/layers/linear.py:50assert用于运行时 dtype 校验，在 Python-O模式下会被跳过，导致错误静默fastdeploy/model_executor/layers/linear.py:44else分支做.cast()允许隐式转换，新代码改为 assert 失败，属 Breaking Change📝 PR 规范检查
PR 标题缺少官方 Tag（当前为
add assert），PR 描述各必填 section（Motivation / Modifications / Usage or Command / Accuracy Tests）均为空。标题建议（可直接复制）：
[OP] Replace implicit dtype cast with assert in linear weight_loaderPR 描述建议（可直接复制，必须复刻 checklist §D2 模板的完整结构）：
总体评价
PR 将 5 处重复的 dtype 处理逻辑成功抽取为公共函数，有利于维护；但存在两个需要修复的问题：①
assert不应用于运行时校验（应改为raise ValueError）；② 原else分支的静默.cast()被删除属 Breaking Change，需在 PR 描述中说明意图并确认不影响已有模型加载流程。