[Cherry-Pick][RL] change glm rope_emb calculation #7316#7318
Conversation
|
Thanks for your contribution! |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release/2.6 #7318 +/- ##
==============================================
Coverage ? 73.84%
==============================================
Files ? 376
Lines ? 52960
Branches ? 8268
==============================================
Hits ? 39110
Misses ? 11115
Partials ? 2735
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
There was a problem hiding this comment.
🤖 AI Code Review |
2026-04-11 14:34 CST
📋 Review 摘要
PR 概述:Cherry Pick PR,调整 GLM 模型 RoPE embedding 计算,新增环境变量 FD_ENABLE_RL 用于对齐训练精度
变更范围:custom_ops/gpu_ops/append_attn/、fastdeploy/envs.py、fastdeploy/model_executor/layers/rotary_embedding.py
影响面 Tag:[RL] [OP]
📝 PR 规范检查
PR 描述中的 Motivation 和 Modifications 章节未填写,建议补充:
描述模板(可直接复制):
## Motivation
为了使 GLM 模型的推理结果与训练结果对齐,需要调整 RoPE embedding 的计算精度。
## Modifications
1. 新增环境变量 `FD_ENABLE_RL`,用于控制是否启用 RL 对齐模式
2. 在 Python 层(GlmRotaryEmbedding)中,根据 `FD_ENABLE_RL` 选择不同的计算方式
3. 在 CUDA kernel 中,将 `EnforceFmulRN` 硬编码为 `false` 以避免使用 IEEE-754 compliant rounding问题
| 级别 | 文件 | 概述 |
|---|---|---|
| 🔴 Bug | custom_ops/gpu_ops/append_attn/decoder_write_cache_with_rope_kernel.cu:152 |
硬编码 EnforceFmulRN=false 影响范围过大 |
| 🔴 Bug | fastdeploy/model_executor/layers/rotary_embedding.py:91 |
Python 和 CUDA 层逻辑不一致 |
| 🔴 Bug | fastdeploy/envs.py:272 |
环境变量在 CUDA 层未被使用 |
总体评价
PR 的目的是对齐 GLM 模型的 RoPE embedding 计算精度,但实现存在问题:CUDA kernel 中硬编码 EnforceFmulRN=false 会影响所有使用这些 kernel 的模型(不仅是 GLM),且 Python 层使用环境变量控制而 CUDA 层直接硬编码,逻辑不一致。建议明确修改范围或使用更细粒度的控制机制。
| auto* kernelFn = append_decode_cache_T_neox_partial_rope_kernel< | ||
| T, | ||
| PackSize, | ||
| false>; // GLM use EnforceFmulRN=false |
There was a problem hiding this comment.
🔴 Bug 硬编码 EnforceFmulRN=false 会影响所有使用此 kernel 的模型,不仅仅是 GLM。
append_decode_cache_rope() 函数被多种模型调用(如 Qwen、Ernie 等),但注释仅说明 "GLM use EnforceFmulRN=false"。如果此修改仅针对 GLM,则会影响其他模型的精度。
建议:
- 如果只针对 GLM,需要在调用方判断模型类型或使用单独的 kernel
- 或者将
EnforceFmulRN参数化,通过getEnvEnableRL()读取环境变量来控制
| T, | ||
| PackSize, | ||
| QKV_TYPE, | ||
| false> // GLM use EnforceFmulRN=false |
There was a problem hiding this comment.
🔴 Bug 硬编码 EnforceFmulRN=false 会影响所有使用此 kernel 的模型。
同样的问题,此函数不仅用于 GLM 模型,硬编码会影响其他模型。
| bsz, max_seq_len = position_ids.shape[:2] | ||
| inv_freq = self.base ** (-paddle.arange(0, self.rotary_dim, 2, dtype="float32") / self.rotary_dim) | ||
| freqs = paddle.einsum("ij,k->ijk", position_ids.cast("float32"), inv_freq) | ||
| if envs.FD_ENABLE_RL == 1: |
There was a problem hiding this comment.
🔴 Bug Python 层使用环境变量 FD_ENABLE_RL 控制,但 CUDA 层直接硬编码为 false,逻辑不一致。
Python 代码中根据 FD_ENABLE_RL 选择不同的计算方式,但在 CUDA kernel 中,EnforceFmulRN 被硬编码为 false,没有读取环境变量 FD_ENABLE_RL。
在 custom_ops/gpu_ops/helper.cu 中定义了 getEnvEnableRL() 函数来读取 FD_ENABLE_RL,但这个函数没有被使用。
建议:如果要使用环境变量控制,CUDA kernel 也应该根据 getEnvEnableRL() 的返回值来设置 EnforceFmulRN。
| int(os.getenv("FD_SAVE_OUTPUT_CACHE_FOR_PREEMPTED_REQUEST", "1")) | ||
| ), | ||
| # Whether to align RoPE and moe gate precision with training | ||
| "FD_ENABLE_RL": lambda: int(os.getenv("FD_ENABLE_RL", "0")), |
There was a problem hiding this comment.
🔴 Bug 环境变量 FD_ENABLE_RL 在 CUDA 层未被实际使用。
在 custom_ops/gpu_ops/helper.cu:160-166 中定义了 getEnvEnableRL() 函数读取 FD_ENABLE_RL 环境变量,但在 CUDA kernel 调用时没有使用这个返回值,而是直接硬编码 EnforceFmulRN=false。
建议:要么在 CUDA kernel 中使用 getEnvEnableRL() 的返回值,要么移除这个未使用的函数。
EmmonsCurse
left a comment
There was a problem hiding this comment.
LGTM~ Skip coverage check as it mainly relies on tests with RL.
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.