Skip to content

Feat/sm120 deepgemm fp8 - #1362

Open
parkerpang wants to merge 8 commits into
mainfrom
feat/sm120-deepgemm-fp8
Open

Feat/sm120 deepgemm fp8#1362
parkerpang wants to merge 8 commits into
mainfrom
feat/sm120-deepgemm-fp8

Conversation

@parkerpang

Copy link
Copy Markdown
Collaborator

No description provided.

@rtp-llm-review-bot

rtp-llm-review-bot commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

PR #1362 第 5 轮评审 — BLOCKED(1)

  • 标题:Feat/sm120 deepgemm fp8(@parkerpang,open)
  • head 5eca90bd8c0b · base f1a54a5f1171 · delta 5 文件 · discover 5/5 成功

阻塞项(P0/P1,support≥3 且过全部门)

[P1] test_topk_multi_expert_invalid_ids_and_metadata 的 expert_start_loc 期望值与自身 m_indices 期望矛盾,测试必然失败

  • rtp_llm/models_py/triton_kernels/moe/test/test_ep_scatter.py:322 · 类别 testing · 票数 3/5 · 首见 r4
  • 依据:ep_scatter 以 derive_counts_from_topk=True 调用(test_ep_scatter.py:297-303),ep_kernels.py:188-207 的 _count_tokens_per_expert_kernel 按 valid=(ids>=0)&(ids<num_experts) 统计 recv_topk=[[0,1],[1,-1],[2,9],[0,2]] 得 counts=[2,2,2](-1、9 被剔除)。随后 _fwd_kernel_ep_scatter_1 以 ALIGN_M=align_m=4(ep_kernels.py:216)执行 ep_kernels.py:53 tokens_per_expert=((t+3)//4)*4 得 [4,4,4],line 54 cumsum=cumsum(t)-t 得 expert_start_loc=[0,4,8]。而 test_ep_scatter.py:313 断言 expert_start_loc.cpu().tolist()==[2,6,10];同测试 line 314-317 的 m_indices=[0,0,-1,-1,1,1,-1,-1,2,2,-1,-1] 及 line 326 expected_destinations={0:{0,1},1:{4,5},2:{8,9}} 均对应 [0,4,8],与 [2,6,10] 自相矛盾。该测试位于 TestEpScatter1Correctness,setUp 仅在无 CUDA 时 skipTest(test_ep_scatter.py:74-77),故在 CUDA 环境运行必然在 line 313 断言失败。
  • 建议:将期望值改为 [0, 4, 8],与 m_indices/expected_destinations 保持一致。
  • 事实核验:hold — verified(该测试以 derive_counts_from_topk=True 且 alig);verified(_count_tokens_per_expert_kernel 只统计 (ids);verified(_fwd_kernel_ep_scatter_1 以 ALIGN_M=4 计算 );verified(测试在线 313 断言 expert_start_loc == [2,6,10]);verified(m_indices 与 expected_destinations 均对应 [0);verified(该测试在 CUDA 环境可运行并在线 313 因 [0,4,8]≠[2,6,10)

非阻塞发现(并集报告:单票也保留,降级不丢弃)

  • [P2] DeepEP normal 路由无条件拒绝 CUDA Graph,与 executor 的 SM120 放行及既有测试相矛盾 — rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/routers/deepep_normal_router.py:53 · 票数 4/5
  • [P2] test_deepgemm_25_accepts_2d_decode_shape 未守卫 has_deep_gemm_mk_alignment,旧版 DeepGEMM 环境会失败 — rtp_llm/models_py/modules/hybrid/test/indexer_test.py:363 · 票数 3/5
  • [P2] is_deep_gemm_e8m0_used 仅凭 SM 主版本启用 UE8M0 打包,未校验 DeepGEMM 版本,与 mk-alignment 能力判定不一致 — rtp_llm/models_py/kernels/cuda/deepgemm_wrapper.py:131 · 票数 3/5
  • [P2] get_sm mock 目标从 utils.arch 改为 executor/router 模块属性,若 strategy 直接 import 则 mock 不生效 — rtp_llm/models_py/modules/factory/fused_moe/tests/test_cuda_strategies.py:308 · 票数 2/5
  • [P2] k_scale 被 .view(-1) 展平为 1-D,可能破坏旧版 DeepGEMM 对 2-D scale 形状的约定 — rtp_llm/models_py/modules/base/cuda/indexer_op.py:581 · 票数 1/5
  • [P2] _mk_alignment_lock 只序列化 CPU 侧 set/launch/restore,可能无法防护异步 GEMM 执行期读全局 mk_alignment 的竞争 — rtp_llm/models_py/kernels/cuda/deepgemm_wrapper.py:168 · 票数 1/5
  • [P3] 模块级设置 DG_JIT_CACHE_DIR 并 makedirs,os.getcwd() 回退会污染仓库目录 — rtp_llm/models_py/modules/hybrid/test/indexer_test.py:14 · 票数 3/5
  • [P3] 移除 get_sm mock 使 can_handle 测试依赖真实硬件与真实 DeepGEMM — rtp_llm/models_py/modules/factory/fused_moe/tests/test_cuda_strategies.py:536 · 票数 1/5

已确认修复(recheck)

  • [P2] direct_ue8m0 路径对 SM100 也生效,静默改变 SM100 数值行为且无 SM100 测试覆盖 — _uses_direct_ue8m0()(行910-918)将 is_deep_gemm_e8m0_used() 改为 is_sm12x()(行917),direct_ue8m0 路径仅对 SM120 生效,SM100 回退到旧 per_block_cast_to_fp8_grouped + requant_weight_ue8m0 路径;ValueError 检查(行937-943)也同步限定为 is_sm12x()。
  • [P3] m_indices 使用 -1 哨兵值且移除了 clamp,依赖 DeepGEMM 对负索引的隐式处理 — 本轮改动在 execute_masked 的 ep_scatter_v2 之后(第 254 行)重新添加了 dispose_tensor(hidden_states_fp8),修复了发现依据中描述的显式释放被移除问题。
  • [P2] configure_deep_gemm_mk_alignment 修改 DeepGEMM 全局状态,并发/多流下存在竞争 — 当前代码第 85 行新增模块级 _mk_alignment_lock,并在 configure_deep_gemm_mk_alignment(第 166-174 行)用该锁包裹 set/launch/restore,序列化了全局对齐状态的并发修改,已消除原描述的交错覆盖竞争。
  • [P3] setup_jit_cache 的 cache_dir 从字符串改为 Path 对象,下游字符串拼接可能出错 — 第241-242行将cache_dir转换为Path对象,第262行传入copy_package_with_lock,该函数内部(如第117-133行)均使用Path的'/'操作符而非字符串拼接,未发现TypeError风险。
  • [P2] quant_weight_ue8m0_packed 未校验 n/k 为 128 的倍数,非对齐维度时 scale 行数与权重行数不一致 — quant_weight_ue8m0_packed 已在本轮 diff 中整体删除,当前文件中不再存在该函数;替代路径 quant_weight_ue8m0 在第 425-427 行用 ceil_div 计算 out_s 行数,能正确处理非 128 倍数维度。
  • [P2] 移除 topk_idx.clamp_ 后越界 id 语义变更,CPU 计数与 GPU masking 可能不一致 — 非 CUDA graph 路径的 all_tokens 已改为按 expert_num_tokens_cpu 对齐求和(第 415-429 行),且非 SM120/非 CUDA graph 的 scatter 容量也用同一 CPU 计数填充(第 468-479 行),workspace 与 scatter 容量一致,不再存在截断风险
  • [P3] normalize_paged_mqa_context_lens 未加入 all — 本轮 diff 在 all 中新增了 "normalize_paged_mqa_context_lens",且当前代码第 29 行已包含该项,函数定义位于第 112 行。

本轮 KPI

{
 "reps": 5,
 "agentic_reps": 2,
 "diff_truncated": false,
 "patch_bytes": 22253,
 "coverage": 1.0,
 "shards": 1,
 "discover_ok": 5,
 "discover_fail": 0,
 "discover_attempts": 5,
 "clusters": 8,
 "synth_fail": 0,
 "confirm": {
  "candidates": 0,
  "to_block": 0,
  "confirm_fail": 0
 },
 "blocking_dedup_merged": 0,
 "fact_check": {
  "checked": 0,
  "hold": 0,
  "refuted": 0,
  "parse_fail": 0
 },
 "model": "whale/DeepSeek-V4-Pro-0813",
 "round_index": 5,
 "delta_files": 5,
 "full_files": 30,
 "new_findings": 4,
 "merged": 4,
 "recheck": {
  "fixed": 1,
  "still_open": 20,
  "cannot_substantiate": 0,
  "fail": 0
 },
 "blocking": 1,
 "blocking_on_unchanged": 0
}

本报告由 rtp-llm-agent-platform 自动生成,同一 PR 的后续轮次就地更新同一条评论。


rtp-llm-agent-platform review · 第 5 轮 · head 5eca90bd8c0b · BLOCKED(1) · 同一 PR 的结论就地更新这一条评论

@LLLLKKKK LLLLKKKK left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Code Review - PR #1362

Status: BLOCKING

Summary: P0/1 · P1/1 · P2/8 · P3/6

Reviewed: commit c495c9850019 · 2026-08-31 21:52 UTC+8

Blocking Issues

P0

  • packed UE8M0 scale 在 TP/DP/EP 切分之前生成,_split 按旧 scale 形状错切并压平 TMA 布局 @ rtp_llm/model_loader/per_block_fp8_quant_weight.py:939
    • 建议:把打包动作移回切分之后:_load_raw_tensor 只产出 float32 per-block scale(形状/语义与旧路径一致),在 _postprocess 中完成 quant_weight_ue8m0_packed 等价的直连打包(可复用 _transform_scale_ue8m0),这样既保留「避免二次量化」的精度收益,也让切分继续作用于它设计时的布局;同步调整 _postprocess:845 用 dtype 当哨兵的判定。测试缺口需一并补齐:suites_sm120.bzl:72moe_fp8pb_tp2_sm120 指向预量化 FP8 检查点,LoadQuantPerBlockFp8Weight.support() 返回 False,根本不进入新分支;覆盖新分支的 dense_fp8pb_dynamic_sm120(BF16 基座在线量化)与 fp8_deepgemm_linear_sm120_test.py 均为 tp=1。建议新增一条「BF16 基座 + --quantization FP8_PER_BLOCK + tp_size 2」的 SM120 smoke,并补一个 tp>1 加载单测,覆盖 sp_head_s_gemm_a8_blocksp_neg1 两类策略,断言每卡 scale 的 dtype == torch.int32shape == (N_local, ceil(K_local/512))stride(-2) == 1。另请注意粒度前提:切分后每卡 K 需为 512 的整数倍,否则应显式 fail-fast。

P1

  • DeepGEMM paged-MQA 的 context_lens 被无条件二维化,cuda12 仍固定 deep_gemm 2.2.0 且仓内并存两种 rank 约定 @ rtp_llm/models_py/modules/base/cuda/indexer_op.py:391
    • 建议:先确认 cuda12 栈的 deep_gemm 2.2.0 是否接受 [batch, next_n]:若不接受,需同步升级 deps/requirements_torch_gpu_cuda12.txt 或按能力选择 1-D/2-D 两种形态;若 H20 作业实际以 cuda12_9 构建,请在 PR description 写明该结论与已验证平台矩阵。更根本的是把 get_paged_mqa_logits_metadata + fp8_paged_mqa_logits 的调用(含 context_lens[batch, next_n] 归一化与 dim() == 2 断言)收敛到一个共享 wrapper(建议放在 deepgemm_wrapper.py),让 indexer_op.pydsv4/decode/indexer_decode_op.pydsv4/fp8/_indexer_score.py 统一走它并在导入时做一次显式版本/签名断言,而不是在推理热路径上以形状错误晚失败;顺带确认 indexer_decode_op.py 的 1-D 调用在 2.5.0 下是否仍有效并跑通其单测。另建议用 unsqueeze(-1) 替代 view(-1, 1),去掉对 kvlen_d 连续性的隐式依赖。

Non-blocking Suggestions

P2

  • DeepGEMM 新 wrapper 缺可用性守卫,并以 except TypeError 探测函数签名 @ rtp_llm/models_py/kernels/cuda/deepgemm_wrapper.py:118
    • 建议:两个新 wrapper 开头补 has_deep_gemm() 并复用 _missing_deep_gemm();把「DeepGEMM 是否具备 mk-alignment API」提升为 check_conditions 的显式能力检查(hasattr 探测),使不满足时降级到其它 executor 而非运行期崩溃。既然 wheel 已 pin,建议删除 except TypeError 探测(YAGNI);若确需兼容两种签名,请用 inspect.signature 配合 functools.cache 显式判定参数个数,并显式处理 AttributeError 后 fail-fast 抛出带版本要求的 RuntimeError。同时把 :94 的版本标识改成与本仓 pin 一致的可校验表述(如 deep_gemm >= 2.5.0)。
  • 非 SM120 分支的 all_tokens 上界是死代码,两分支主体逐字重复,且 CPU 计数靠跨 50 行的条件蕴含维系 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/deepgemm_hybrid_executor.py:410
    • 建议:把 else 分支精简为只确定 expert_alignment = self.EXPERT_ALIGNMENT,worst-case 上界只在 if self.enable_cuda_graph: 内计算,让非 graph 路径只走 actual_aligned,代码即可自解释;configure_deep_gemm_mk_alignment_context 提取为局部变量或小方法在两次 GEMM 前复用。num_recv_tokens_per_expert_cpu 建议在函数开头初始化为 None 并在 :476 使用处显式断言非 None(或把「CPU 计数准备」与「非 SM120 预对齐」合并到同一条件块内一次完成),把跨 50 行的隐式蕴含变成本地可检查的前置条件。这段是 MoE 前向的核心尺寸推导,留一份不生效却声称保持旧布局的计算,容易让后续修改者误判 padding 上界。
  • 删除 topk 越界告警后非法 expert id 的 token 被静默丢弃,失去 router 缺陷可观测性 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/deepgemm_hybrid_executor.py:388
    • 建议:去掉 .item() 同步是对的(CUDA Graph 必需),但请补一条不打断 graph 的可观测手段:让 ep_scatter 额外累加一个 GPU 端 dropped_assignments 计数器,在非 CUDA Graph 路径(本就有 CPU 同步、无额外开销)或按低频采样读回,打 warning 日志与 kmonitor 指标,避免这类 router/容量缺陷完全无声。
  • CUDA Graph 测试只 replay 同一份输入,且最贴近 SM120 decode 的小 batch 形状无 graph 覆盖 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/test/deepep_normal_executor_sm120_test.py:31
    • 建议:把两个 cuda_graph 用例上移到可被两个形状类共享的 mixin,让 Qwen3.5 形状类也获得覆盖。并在 graph 用例里 capture 之后用 copy_/fill_ 就地改写 payload.expert_topk_ids / expert_x(保持张量地址不变,至少覆盖均匀分布与「全部 token 集中到单个本地专家」两种 routing),graph.replay() 后与新 routing 的 eager 参考输出比对,这样才能真正锁住 GPU 侧计数推导、-1 哨兵语义与 worst-case 容量上界。
  • test_cuda_strategies.py 内仍残留 4 处指向定义处的无效 get_sm patch @ rtp_llm/models_py/modules/factory/fused_moe/tests/test_cuda_strategies.py:410
    • 建议:把这 4 处统一改为使用处 patch(router 用例 patch deepep_normal_router.get_sm,executor 用例 patch deepgemm_hybrid_executor.get_sm,两者都涉及的同时 patch)。若嫌重复,可在文件内提取一个 _patch_get_sm(major, minor) 辅助装饰器统一管理这两个 target,避免同类 patch 再次漂移,也避免后续读者误以为定义处 patch 是本仓的有效写法。
  • _postprocess 用 scale dtype 当「已打包」哨兵,该分支没有任何单测 @ rtp_llm/model_loader/per_block_fp8_quant_weight.py:845
    • 建议:把该判定收敛成一个显式意图的共享工具函数(例如 is_packed_ue8m0_scale(tensor)),供 loader 与 CudaFp8DeepGEMMLinear 共用;并补一个针对 _postprocess 的聚焦单测:分别传入 float32 block scale 与 int32 packed scale,断言前者调用 requant、后者原样透传(可对 requant_weight_ue8m0 使用处打 mock 计数)。该改动与 P0 的修复方案需一并考虑。
  • deep_gemm 跨 4 个 minor 升级作用于整个 cuda12_9 平台,新增验证只覆盖 SM120 且回滚边界未说明 @ deps/requirements_torch_gpu_cuda12_9.txt:15
    • 建议:请在 PR description 中补齐三项:1)新版对 SM90/SM100 kernel 是否有数值或调度变化(附上游 changelog 或结论);2)本次 CI 中 SM90 既有 deepgemm 相关 smoke 与性能基线的实际复跑结论,而非仅「新增 SM120 suite 通过」;3)回滚边界——SM120 路径已依赖 2.5.0 新符号,单独还原 pin 无法安全回滚,需说明 pin 与代码一并回退。同时确认 rtp-kernel metadata 对 deep-gemm 是否声明版本约束、其 fp8_group_gemm、w4a8、xqa、fused_rope_kvcache 等算子已在新版本下验证。若拿不到 SM90 证据,建议把 pin 升级拆为独立 PR 与 SM120 功能解耦,便于单独 bisect。
  • lock 未按声明的 update 目标整体重新生成,注解前缀与依赖闭包不自洽 @ deps/requirements_lock_torch_gpu_cuda12_9.txt:732
    • 建议:请在标准工作目录下用约定的 update 目标重新生成整份 lock 并提交完整 diff,使 # via 前缀全文一致,并让 deps/BUILDcompile_pip_requirements 一致性 test(当前为 manual tag)可本地跑通。由于 lock 全量带 --hash,闭包缺项会在 --require-hashes 安装阶段硬失败,重生成可把风险从「构建期偶发」前移到「本地可验证」;若确认 2.5.0 未引入新传递依赖,请在 PR description 说明重生成方式与结论。

P3

  • contiguous 路径的 silu 与量化跑满 padded workspace 并读取未初始化内存 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/deepgemm_hybrid_executor.py:531
    • 建议:让 silu 与量化只作用于有效区间:可让 ep_scatter 额外输出 GPU 端实际 padded token 数,或让两个 triton kernel 按 m_indices >= 0 做行掩码;最低成本是把 gateup_output 改为 torch.zeros 消除未初始化读。同时把「padding 行内容未定义,仅靠 output_index 过滤,下游 kernel 必须逐行无跨行归约」这一不变量写进注释,避免后续引入跨行归约时静默出错;若认为该退化在 SM120 decode 上可接受,请补上实测开销数据。
  • execute_masked 中删除 dispose_tensor 无 CUDA Graph 依据,与兄弟 executor 策略不一致 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/deepgemm_hybrid_executor.py:251
    • 建议:在 execute_masked 中恢复 dispose_tensor(hidden_states_fp8)execute_contiguous 内改为条件释放(if not self.enable_cuda_graph: dispose_tensor(...)),并把「静态输入 buffer 不可 set_(),否则 replay 会读到被置空的张量」这一真实约束写成一行注释,避免后续读者当成遗漏再加回来、或当成无用代码再次删掉。若删除动机其实是「不应释放调用方 payload 持有的张量」,请在 PR description 与注释中写明,并同步处理 masked 分支其余 dispose_tensor 以保持策略一致。
  • 向量化 fp8 store 沿用了未校验的 group_size 对齐前提 @ rtp_llm/models_py/bindings/cuda/kernels/per_token_group_quant_8bit.cu:131
    • 建议:顺手在 host 入口补一条 CHECK_EQ(group_size % (16 / input.element_size()), 0)(或等价 TORCH_CHECK),把这个原本隐式的前提变成可诊断的 fail-fast,同时也保护既有的尾元素截断问题。另外 can_cache 由运行时 group_size 决定,两条路径都会被编译并占用寄存器,如后续要做性能收敛可考虑提升为模板 constexpr。
  • 空专家用例的零值断言不具备判别力 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/test/deepep_normal_executor_test.py:197
    • 建议:在执行(CUDA Graph 场景为 graph.replay())之前把输出缓冲填成非零哨兵(例如 fill_(float("nan"))fill_(1.0)),执行并 synchronize 后再断言全零;这样只有 combine kernel 真正写入零才能通过,用例即可同时守住「不崩溃」与「确实写出零」两件事。
  • masked 策略在 CUDA Graph 组合下既无门禁也无断言 @ rtp_llm/models_py/modules/factory/fused_moe/tests/test_cuda_strategies.py:354
    • 建议:在 TestCudaFp8PerBlockNoDPMaskedStrategy(及对应 EpNormalMasked 用例)中补一个与 test_can_handle_cuda_graph 对称的断言,明确 masked 策略在 enable_cuda_graph=True 下的期望行为;若期望是拒绝,请同时在 deepgemm_masked_executor.pycheck_conditionschecker.check(not config.enable_cuda_graph),让门禁与断言成对存在,而不是依赖「auto 不会选中」这一隐式前提。
  • 同名 per_block_cast_to_fp8 存在两套不兼容签名,新旧量化路径各用其一 @ rtp_llm/model_loader/per_block_fp8_quant_weight.py:103
    • 建议:给两者取可区分的名字(例如把 loader 内部的重命名为 per_block_cast_to_fp8_grouped),或把 loader 侧实现下沉到 fp8_kernel 并统一为带关键字参数的单一入口;至少在 loader 侧定义上加一行注释说明它与 fp8_kernel 同名函数的差异,避免后续维护者跨模块误替换。

Checklist Findings (15 fail / 54 total)

General Principles Checklist

  • [6.1] Architecture — 兼容性:外部 HTTP/RPC API、持久数据、配置、环境迁移安全 → issue lock 未按声明的 update 目标整体重新生成,注解前缀与依赖闭包不自洽
    文件头(:1-6)声明由 bazel run //deps:requirements_torch_gpu_cuda12_9.update 生成。但改动后只有 deep-ep(:732)与 deep-gemm(:736)两处写成带 deps/ 前缀的 -r deps/requirements_torch_gpu_cuda12_9.txt,同文件其余 23 处仍为无前缀写法(如 :729 的 -r requirements_base.txt);对照 requirements_lock_torch_gpu_cuda13.txt 全文只用一种前缀。pip-compile 会整体重写注解而非只改两条;同时 deep-ep 版本与 hash 未变却产生注解 churn,说明该 hunk 是在不同工作目录下解析后局部粘贴,因此 2.5.0 自身的 Requires-Dist 闭包是否被重新求解无法确认。
  • [6.1] Architecture — 可观测性:日志/指标/超时可操作、非噪声 → issue 删除 topk 越界告警后非法 expert id 的 token 被静默丢弃,失去 router 缺陷可观测性
    原实现会检查 topk_idx.max()/min(),越界时打印 [DeepGemm CLAMP] topk_ids out of range ... 后 clamp;该日志文案已从全仓消失。新实现把校验完全交给内核:_fwd_kernel_ep_scatter_2expert_id >= 0 and expert_id < num_experts(ep_kernels.py:122)跳过非法 id,写入还受 dest_token_index < output_capacity(:125)掩码保护,_fwd_kernel_ep_gathersource_token_index >= 0(:437)跳过未路由项——不越界写内存,这部分是正确改进。但代价是:若 router 或 DeepEP 元数据真的产出越界 expert id(原日志文案说明历史上出现过),或写入超出静态容量,该 token 的这一路专家贡献变成 0,输出被静默污染,线上没有任何日志、指标或断言可用于定位。
  • [6.1] Architecture — 回滚路径:风险行为存在运维回滚手段 → issue deep_gemm 跨 4 个 minor 升级作用于整个 cuda12_9 平台,新增验证只覆盖 SM120 且回滚边界未说明
    pin 从 deep_gemm-2.1.1+local 抬到 2.5.0+d7d5eca.cu129,跨越 2.2/2.3/2.4/2.5。该 lock 经 deps/pip.bzlpip_gpu_cuda12_9_torch 被 cuda12_9 下所有 bazel Python 目标共享;.bazelrc:60-61/94-102 显示 cuda12_9 继承 cuda12 的 TF_CUDA_COMPUTE_CAPABILITIES="7.0..9.0",即 H20/H800(SM90)既有 FP8 GEMM 生产路径的运行时二进制被无门控替换,而本 PR 新增 suite 与 golden 全为 sm120(suites_sm120.bzl*_sm120.json)。deepgemm_hybrid_executor.py:410-418 只保证 RTP 侧布局不变,无法保证 DeepGEMM 内部 kernel 在 4 个 minor 间行为等价。另外 lock 中 deep-gemm 条目新增了 rtp-kernel 这条 re
  • [6.1] Architecture — 状态不变量:创建/更新/失败/重试/回滚路径有效 → issue execute_masked 中删除 dispose_tensor 无 CUDA Graph 依据,与兄弟 executor 策略不一致
    execute_maskedep_scatter_v2(:238-250)之后直接分配 (E, alignment, N)upgate_output(:251),中间已无 dispose_tensor(hidden_states_fp8)。contiguous 路径的删除有依据:CUDA Graph 下 payload.expert_x 是 graph runner 持有的静态输入 buffer,dispose_tensor 通过 Tensor.set_() 生效会破坏 replay。但 execute()(:149)仅在 not self.enable_cuda_graph 时进入 masked,check_conditions(:80)也只对 SM120 放开 CUDA Graph,即 masked 在图捕获下不可达,该处删除没有任何 CUDA Graph 理由;同函数其余三处 dispose_tensor(:322、:343、:344)与兄弟 `deepgemm_masked_executor.py:207-208/:3
  • [6.1] Architecture — 错误语义:fail-fast/retry/fallback/silent 行为显式 → issue masked 策略在 CUDA Graph 组合下既无门禁也无断言
    本次只为 continuous 路径新增了 SM120 放行断言(NoDP 见 :312-339,EpNormal 见 :457-499)。TestCudaFp8PerBlockNoDPMaskedStrategy(:354 起)只有 test_can_handle_single_gpu / test_can_handle_tp_equal_ep / test_priority,没有任何 cuda graph 用例;而独立的 deepgemm_masked_executor.py:45-57check_conditions 只有 get_sm()[0] >= 9,没有任何 enable_cuda_graph 门禁(注意 deepgemm_hybrid_executor.py:149 内部的 masked 分支已被 not self.enable_cuda_graph 保护,此处指独立的 masked executor / strategy)。该 strategy 需显式 `moe_strategy == "fp8_per_block_no_
  • [6.1] Quality — PR description 说明动机与设计 → issue deep_gemm 跨 4 个 minor 升级作用于整个 cuda12_9 平台,新增验证只覆盖 SM120 且回滚边界未说明
    pin 从 deep_gemm-2.1.1+local 抬到 2.5.0+d7d5eca.cu129,跨越 2.2/2.3/2.4/2.5。该 lock 经 deps/pip.bzlpip_gpu_cuda12_9_torch 被 cuda12_9 下所有 bazel Python 目标共享;.bazelrc:60-61/94-102 显示 cuda12_9 继承 cuda12 的 TF_CUDA_COMPUTE_CAPABILITIES="7.0..9.0",即 H20/H800(SM90)既有 FP8 GEMM 生产路径的运行时二进制被无门控替换,而本 PR 新增 suite 与 golden 全为 sm120(suites_sm120.bzl*_sm120.json)。deepgemm_hybrid_executor.py:410-418 只保证 RTP 侧布局不变,无法保证 DeepGEMM 内部 kernel 在 4 个 minor 间行为等价。另外 lock 中 deep-gemm 条目新增了 rtp-kernel 这条 re
  • [6.1] Quality — 逻辑变更未混入无关格式化 → issue lock 未按声明的 update 目标整体重新生成,注解前缀与依赖闭包不自洽
    文件头(:1-6)声明由 bazel run //deps:requirements_torch_gpu_cuda12_9.update 生成。但改动后只有 deep-ep(:732)与 deep-gemm(:736)两处写成带 deps/ 前缀的 -r deps/requirements_torch_gpu_cuda12_9.txt,同文件其余 23 处仍为无前缀写法(如 :729 的 -r requirements_base.txt);对照 requirements_lock_torch_gpu_cuda13.txt 全文只用一种前缀。pip-compile 会整体重写注解而非只改两条;同时 deep-ep 版本与 hash 未变却产生注解 churn,说明该 hunk 是在不同工作目录下解析后局部粘贴,因此 2.5.0 自身的 Requires-Dist 闭包是否被重新求解无法确认。
  • [6.1] Software Engineering — DRY:重复非平凡逻辑被抽取或显式复用 → issue 同名 per_block_cast_to_fp8 存在两套不兼容签名,新旧量化路径各用其一
    per_block_fp8_quant_weight.py:103 定义 per_block_cast_to_fp8(x, group_size: int),而 models_py/kernels/cuda/fp8_kernel/fp8_kernel.py:382 定义同名 per_block_cast_to_fp8(x, use_ue8m0: bool)。本 PR 后,同一个 _load_raw_tensor 中的 elif self.scale: 分支调用前者(:943),新增的 direct_ue8m0 分支经 quant_weight_ue8m0_packed 调用后者(fp8_kernel.py:458)。两者第二个位置参数一个是 int 一个是 bool,误用不会被类型检查拦下(per_block_cast_to_fp8(x, 128) 在 bool 语义下等价于 use_ue8m0=True),新增的 SM120 单测又同时导入了两个模块,容易混淆。
  • [6.1] Software Engineering — KISS/YAGNI:无投机性抽象 → issue 非 SM120 分支的 all_tokens 上界是死代码,两分支主体逐字重复,且 CPU 计数靠跨 50 行的条件蕴含维系
    :398-418if self.is_sm120 / else 两分支中 max_active_expertsall_tokens 表达式逐字相同,唯一差异是 expert_alignment 取值。else 分支只在 is_sm120=False 时进入,此时 check_conditions(:80)保证 enable_cuda_graph=False,于是 :434all_tokens = actual_aligned 必然覆盖刚算出的值——该分支的上界公式永不生效,注释(:411-412)却声称「保持 pre-upgrade 布局」。另 num_recv_tokens_per_expert_cpu 仅在 :420 if not self.enable_cuda_graph: 内赋值(:421-429),却在 :473 if not self.is_sm120 and not self.enable_cuda_graph: 的分支体(:476)中读取:当前靠「后者蕴含前者」避免 `UnboundLoca
  • [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue deep_gemm 跨 4 个 minor 升级作用于整个 cuda12_9 平台,新增验证只覆盖 SM120 且回滚边界未说明
    pin 从 deep_gemm-2.1.1+local 抬到 2.5.0+d7d5eca.cu129,跨越 2.2/2.3/2.4/2.5。该 lock 经 deps/pip.bzlpip_gpu_cuda12_9_torch 被 cuda12_9 下所有 bazel Python 目标共享;.bazelrc:60-61/94-102 显示 cuda12_9 继承 cuda12 的 TF_CUDA_COMPUTE_CAPABILITIES="7.0..9.0",即 H20/H800(SM90)既有 FP8 GEMM 生产路径的运行时二进制被无门控替换,而本 PR 新增 suite 与 golden 全为 sm120(suites_sm120.bzl*_sm120.json)。deepgemm_hybrid_executor.py:410-418 只保证 RTP 侧布局不变,无法保证 DeepGEMM 内部 kernel 在 4 个 minor 间行为等价。另外 lock 中 deep-gemm 条目新增了 rtp-kernel 这条 re
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue masked 策略在 CUDA Graph 组合下既无门禁也无断言
    本次只为 continuous 路径新增了 SM120 放行断言(NoDP 见 :312-339,EpNormal 见 :457-499)。TestCudaFp8PerBlockNoDPMaskedStrategy(:354 起)只有 test_can_handle_single_gpu / test_can_handle_tp_equal_ep / test_priority,没有任何 cuda graph 用例;而独立的 deepgemm_masked_executor.py:45-57check_conditions 只有 get_sm()[0] >= 9,没有任何 enable_cuda_graph 门禁(注意 deepgemm_hybrid_executor.py:149 内部的 masked 分支已被 not self.enable_cuda_graph 保护,此处指独立的 masked executor / strategy)。该 strategy 需显式 `moe_strategy == "fp8_per_block_no_
  • [6.1] Tests — 边界 case 覆盖(空、单元素、最大值) → issue 空专家用例的零值断言不具备判别力
    _run_deepep_normal_executorempty_local_experts 分支只有两条断言(:197-202):shape == payload.expert_x.shapecount_nonzero(...) == 0。eager 变体下所有计数为 0 → all_tokens == 0,executor 在 :435-442 直接早退返回 torch.zeros,根本没有走 kernel;CUDA Graph 变体虽然会走 ep_scatter/ep_gather,但输出缓冲是 torch.empty(deepgemm_hybrid_executor.py:566)且来自 graph 私有内存池(实践中常为全零),故「combine kernel 根本没写输出」与「正确写入了零」两种情况无法区分。这两条用例真正证明的偏向「capture/replay 与 eager 执行不崩溃」。

RTP-LLM Checklist

  • [I] 代码质量 — 同一功能用统一工具函数 → issue 同名 per_block_cast_to_fp8 存在两套不兼容签名,新旧量化路径各用其一
    per_block_fp8_quant_weight.py:103 定义 per_block_cast_to_fp8(x, group_size: int),而 models_py/kernels/cuda/fp8_kernel/fp8_kernel.py:382 定义同名 per_block_cast_to_fp8(x, use_ue8m0: bool)。本 PR 后,同一个 _load_raw_tensor 中的 elif self.scale: 分支调用前者(:943),新增的 direct_ue8m0 分支经 quant_weight_ue8m0_packed 调用后者(fp8_kernel.py:458)。两者第二个位置参数一个是 int 一个是 bool,误用不会被类型检查拦下(per_block_cast_to_fp8(x, 128) 在 bool 语义下等价于 use_ue8m0=True),新增的 SM120 单测又同时导入了两个模块,容易混淆。

Python Static-First Checklist

  • [P.G] 测试规范 — mock.patch target 是使用处而非定义处 → issue test_cuda_strategies.py 内仍残留 4 处指向定义处的无效 get_sm patch
    本 PR 已把两处 patch 改到使用处(:312 的 deepgemm_hybrid_executor.get_sm、:457/:460 的两个 target),但同文件 :410:433:503:529 仍是 patch("rtp_llm.models_py.utils.arch.get_sm")deepgemm_hybrid_executor.py:46deepep_normal_router.py 都是模块级 from rtp_llm.models_py.utils.arch import get_sm,名字在 import 时已绑定,patch 定义处对 checker.check(get_sm()[0] >= 9)(deepgemm_hybrid_executor.py:79)完全无效。因此 test_can_handle_ep_enabledtest_can_handle_tp_dp_eptest_can_handle_false_low_latency、`test_can_handle_false_
  • [P.G] 测试规范 — mock/fake/stub 不得替代本次声称覆盖的生产边界 → issue 空专家用例的零值断言不具备判别力
    _run_deepep_normal_executorempty_local_experts 分支只有两条断言(:197-202):shape == payload.expert_x.shapecount_nonzero(...) == 0。eager 变体下所有计数为 0 → all_tokens == 0,executor 在 :435-442 直接早退返回 torch.zeros,根本没有走 kernel;CUDA Graph 变体虽然会走 ep_scatter/ep_gather,但输出缓冲是 torch.empty(deepgemm_hybrid_executor.py:566)且来自 graph 私有内存池(实践中常为全零),故「combine kernel 根本没写输出」与「正确写入了零」两种情况无法区分。这两条用例真正证明的偏向「capture/replay 与 eager 执行不崩溃」。

Strengths

  • NormalExecutor.cc:120-126 用新增的 CacheConfig::effectiveKernelSeqSizePerBlock()(CacheConfig.h:68-73)为 kernel_seq_size_per_block == 0(复用物理块大小的哨兵)补上兜底,把哨兵语义收敛到一个带注释的访问器;KVCacheResourceTest.cc 对 0 与非 0 两种取值都补了断言。这正是 SM120 smoke 用 --enable_cuda_graph 1 --seq_size_per_block 64 而不显式配 kernel 块大小时的必经路径。
  • contiguous 路径彻底移除 expert_num_tokens.cpu().tolist()topk_idx.max()/min().item() 等 device→host 同步点,计数下沉到 _count_tokens_per_expert_kernel(ep_kernels.py:15-35),workspace 完全由 shape 推导——这是可 capture/replay 的正确形态,也顺带消掉了 eager 路径的隐藏同步。
  • 容量上界推导经独立核算成立:all_tokens = align_up(routed + min(routed, E) * (A-1), A)sum(align_up(c_i, A)) 是严格上界,assert m_indices.shape[0] % align_m == 0(ep_kernels.py:178)在 sm120+graph、sm120 eager、非 sm120 三条路径下均满足。
  • 非 SM120 的兼容处理很克制:scatter_alignment = 1 + CPU 端预对齐计数(deepgemm_hybrid_executor.py:473-484),逐字节保留升级前 SM9x/SM100x 的 workspace 布局,新逻辑严格限制在 self.is_sm120 分支内。
  • m_indices/output_indextorch.empty(+ clamp_(0, E-1))改为 -1 哨兵,与内核侧守卫闭合:expert_id >= 0 and < num_experts(ep_kernels.py:122)、source_token_index >= 0 and < total_input_tokens(:437),padding 行被 DeepGEMM 真正跳过而非按 expert 0 计算废数据;ep_gather 累加器零初始化且无条件 store(:426、:450),padding 垃圾值不会跨行传播。
  • 新增 output_capacity / m_indices_size 两个显式边界参数,把原先「调用方保证 workspace 足够大」的隐式契约变成内核侧可强制的掩码,dest_is_valid(:125)同时保护 output_index、output_tensor、output_tensor_scale 三处写入。
  • test_ep_scatter.py 的两条新用例质量很高:test_metadata_counts_cannot_write_past_workspace(:201)与 test_topk_multi_expert_invalid_ids_and_metadata(:253)都用 guard row + sentinel 证明越界零写入,并对故意不一致的元数据计数、-1 与越界 expert id 逐个断言 expert_start_locm_indicesoutput_index 与搬运数据,判别力充分。
  • per_block_cast_to_fp8 新增 .clamp(fp8_min, fp8_max)(fp8_kernel.py:396),修掉 x / sf 边界处因浮点舍入溢出为 fp8 Inf/NaN 的隐患,并统一改用 fp8_dtype 常量。
  • quant_weight_ue8m0_packed 以 1024 行(128 的整数倍)分块量化,压住在线加载期的浮点临时张量,分块边界与 128×128 量化块严格对齐;fp8_deepgemm_linear_sm120_test.py:80-89 用 1152 行跨块用例证明与整矩阵一次量化逐位相等,:46-78 不经 GEMM 直接比较重构误差,证明直连量化优于 dequant/requant,并同时钉住 dtype == int32shape == (768, 2)stride(-2) == 1
  • per_token_group_quant_8bit.cu 的寄存器缓存边界正确:can_cache 为真时每 lane 在两个循环恒定命中同一 i = lane_id,写入(:83)与读取(:116)受同一 i < num_vec_elems 条件保护,fp32(32 vecs)自动回退重载;fp8 打包 store 经 std::is_same_v<DST_DTYPE, __nv_fp8_e4m3>(:122)真实实例化,非死代码。
  • 测试基类重构修掉了原先「具体 TestCase 与 mixin 同名遮蔽」的写法,并把带默认参数的入口拆成 _run_deepep_normal_executor + 显式用例,原有 eager 覆盖全部保留;get_sm 的 patch 目标从定义处改到使用处(:312、:457、:460)是正确修法,test_can_handle_cuda_graph 用 (9,0)/(10,0)/(12,0) 三档锁住新判据。
  • 依赖 pin 保持「直链 wheel + --hash=sha256」双重校验,源清单与 lock 的 wheel 文件名版本一致,local version 段带 .cu129 可与 cuda13 系同版本号不同 build 区分,避免跨 CUDA ABI 误装。

Comment thread rtp_llm/model_loader/per_block_fp8_quant_weight.py Outdated
Comment thread rtp_llm/models_py/modules/base/cuda/indexer_op.py Outdated

@LLLLKKKK LLLLKKKK left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Code Review - PR #1362 (non-blocking suggestions)

14 条 P2/P3 建议,不阻塞合并。阻塞判定与完整摘要见上一条 review。

deep_gemm.set_num_sms(original_num_sms)


def get_theoretical_mk_alignment_for_contiguous_layout(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] DeepGEMM 新 wrapper 缺可用性守卫,并以 except TypeError 探测函数签名

get_theoretical_mk_alignment_for_contiguous_layout(:118)与 configure_deep_gemm_mk_alignment(:139)直接 import deep_gemm 并调用新符号,缺少同文件 configure_deep_gemm_num_sms(:101)那样的 has_deep_gemm() 前置检查;而 check_conditions 只校验 has_deep_gemm()(deepgemm_hybrid_executor.py:78),旧版 DeepGEMM 在 SM120 上仍会选中该 executor,然后在首次 forward 抛裸 AttributeError 而非统一 RuntimeError。前者先按 (expected_m, num_groups) 两参调用,except TypeError(:130)后退化为语义完全不同的单参 (expected_m + num_groups - 1) // num_groups:DeepGEMM 函数体内部任何...

建议: 两个新 wrapper 开头补 has_deep_gemm() 并复用 _missing_deep_gemm();把「DeepGEMM 是否具备 mk-alignment API」提升为 check_conditions 的显式能力检查(hasattr 探测),使不满足时降级到其它 executor 而非运行期崩溃。既然 wheel 已 pin,建议删除 except TypeError 探测(YAGNI);若确需兼容两种签名,请用 inspect.signature 配合 functools.cache 显式判定参数个数,并显式处理 AttributeError 后 fail-fast 抛出带版本要求的 RuntimeError。同时把 :94 的版本标识改成与本仓 pin 一致的可校验表述(如 deep_gemm >= 2.5.0)。

routed_tokens + max_active_experts * (expert_alignment - 1),
expert_alignment,
)
else:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 非 SM120 分支的 all_tokens 上界是死代码,两分支主体逐字重复,且 CPU 计数靠跨 50 行的条件蕴含维系

:398-418if self.is_sm120 / else 两分支中 max_active_expertsall_tokens 表达式逐字相同,唯一差异是 expert_alignment 取值。else 分支只在 is_sm120=False 时进入,此时 check_conditions(:80)保证 enable_cuda_graph=False,于是 :434all_tokens = actual_aligned 必然覆盖刚算出的值——该分支的上界公式永不生效,注释(:411-412)却声称「保持 pre-upgrade 布局」。另 num_recv_tokens_per_expert_cpu 仅在 :420 if not self.enable_cuda_graph: 内赋值(:421-429),却在 :473 if not self.is_sm120 and not self.enable_cuda_graph: 的分支体(:476)中读取:当前靠「后者蕴含前者」避免 `UnboundL...

建议:else 分支精简为只确定 expert_alignment = self.EXPERT_ALIGNMENT,worst-case 上界只在 if self.enable_cuda_graph: 内计算,让非 graph 路径只走 actual_aligned,代码即可自解释;configure_deep_gemm_mk_alignment_context 提取为局部变量或小方法在两次 GEMM 前复用。num_recv_tokens_per_expert_cpu 建议在函数开头初始化为 None 并在 :476 使用处显式断言非 None(或把「CPU 计数准备」与「非 SM120 预对齐」合并到同一条件块内一次完成),把跨 50 行的隐式蕴含变成本地可检查的前置条件。这段是 MoE 前向的核心尺寸推导,留一份不生效却声称保持旧布局的计算,容易让后续修改者误判 padding 上界。

Checklist: [6.1] KISS/YAGNI:无投机性抽象

raise ValueError("expert_num_tokens GPU tensor is required")

if all_tokens <= 0:
num_experts_local = num_recv_tokens_per_expert.shape[0]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 删除 topk 越界告警后非法 expert id 的 token 被静默丢弃,失去 router 缺陷可观测性

原实现会检查 topk_idx.max()/min(),越界时打印 [DeepGemm CLAMP] topk_ids out of range ... 后 clamp;该日志文案已从全仓消失。新实现把校验完全交给内核:_fwd_kernel_ep_scatter_2expert_id >= 0 and expert_id < num_experts(ep_kernels.py:122)跳过非法 id,写入还受 dest_token_index < output_capacity(:125)掩码保护,_fwd_kernel_ep_gathersource_token_index >= 0(:437)跳过未路由项——不越界写内存,这部分是正确改进。但代价是:若 router 或 DeepEP 元数据真的产出越界 expert id(原日志文案说明历史上出现过),或写入超出静态容量,该 token 的这一路专家贡献变成 0,输出被静默污染,线上没有任何日志、指标或断言可用于定位。

建议: 去掉 .item() 同步是对的(CUDA Graph 必需),但请补一条不打断 graph 的可观测手段:让 ep_scatter 额外累加一个 GPU 端 dropped_assignments 计数器,在非 CUDA Graph 路径(本就有 CPU 同步、无额外开销)或按低频采样读回,打 warning 日志与 kmonitor 指标,避免这类 router/容量缺陷完全无声。

Checklist: [6.1] 可观测性:日志/指标/超时可操作、非噪声

)


class DeepGemmHybridExecutorQwen35ShapeSM120Test(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] CUDA Graph 测试只 replay 同一份输入,且最贴近 SM120 decode 的小 batch 形状无 graph 覆盖

DeepGemmHybridExecutorQwen35ShapeSM120Test 只有 pass(:31-34),仅继承基类 eager 用例;两个 CUDA Graph 用例(:22、:25)只挂在 DeepGemmHybridExecutorSM120Test 上(基类 batch 128 / 128 experts)。而 executor 的动机注释(deepgemm_hybrid_executor.py:145-148)明确写的是「避免 E * padded_M masked 布局在 SM120 小 decode batch 上的开销」,Qwen3.5 形状(MAX_GENERATE_BATCH_SIZE=1、256 experts、EP=4,deepep_normal_executor_test.py:274-285)才是目标场景,其 expert_alignment = min(EXPERT_ALIGNMENT, get_theoretical_mk_alignment_for_contiguous_layout(...))(:399-404)...

建议: 把两个 cuda_graph 用例上移到可被两个形状类共享的 mixin,让 Qwen3.5 形状类也获得覆盖。并在 graph 用例里 capture 之后用 copy_/fill_ 就地改写 payload.expert_topk_ids / expert_x(保持张量地址不变,至少覆盖均匀分布与「全部 token 集中到单个本地专家」两种 routing),graph.replay() 后与新 routing 的 eager 参考输出比对,这样才能真正锁住 GPU 侧计数推导、-1 哨兵语义与 worst-case 容量上界。

@@ -441,15 +453,25 @@ def test_can_handle_tp_dp_ep(
self.assertTrue(strategy.can_handle(config))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📍 实际位置 rtp_llm/models_py/modules/factory/fused_moe/tests/test_cuda_strategies.py:410(不在 diff 展示范围内,就近挂载)

[P2] test_cuda_strategies.py 内仍残留 4 处指向定义处的无效 get_sm patch

本 PR 已把两处 patch 改到使用处(:312 的 deepgemm_hybrid_executor.get_sm、:457/:460 的两个 target),但同文件 :410:433:503:529 仍是 patch("rtp_llm.models_py.utils.arch.get_sm")deepgemm_hybrid_executor.py:46deepep_normal_router.py 都是模块级 from rtp_llm.models_py.utils.arch import get_sm,名字在 import 时已绑定,patch 定义处对 checker.check(get_sm()[0] >= 9)(deepgemm_hybrid_executor.py:79)完全无效。因此 test_can_handle_ep_enabledtest_can_handle_tp_dp_eptest_can_handle_false_low_latency、`test_can_handle_fal...

建议: 把这 4 处统一改为使用处 patch(router 用例 patch deepep_normal_router.get_sm,executor 用例 patch deepgemm_hybrid_executor.get_sm,两者都涉及的同时 patch)。若嫌重复,可在文件内提取一个 _patch_get_sm(major, minor) 辅助装饰器统一管理这两个 target,避免同类 patch 再次漂移,也避免后续读者误以为定义处 patch 是本仓的有效写法。

Checklist: [P.G] mock.patch target 是使用处而非定义处

)
dispose_tensor(hidden_states_fp8)

upgate_output = torch.empty(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] execute_masked 中删除 dispose_tensor 无 CUDA Graph 依据,与兄弟 executor 策略不一致

execute_maskedep_scatter_v2(:238-250)之后直接分配 (E, alignment, N)upgate_output(:251),中间已无 dispose_tensor(hidden_states_fp8)。contiguous 路径的删除有依据:CUDA Graph 下 payload.expert_x 是 graph runner 持有的静态输入 buffer,dispose_tensor 通过 Tensor.set_() 生效会破坏 replay。但 execute()(:149)仅在 not self.enable_cuda_graph 时进入 masked,check_conditions(:80)也只对 SM120 放开 CUDA Graph,即 masked 在图捕获下不可达,该处删除没有任何 CUDA Graph 理由;同函数其余三处 dispose_tensor(:322、:343、:344)与兄弟 `deepgemm_masked_executor.py:207-208...

建议:execute_masked 中恢复 dispose_tensor(hidden_states_fp8)execute_contiguous 内改为条件释放(if not self.enable_cuda_graph: dispose_tensor(...)),并把「静态输入 buffer 不可 set_(),否则 replay 会读到被置空的张量」这一真实约束写成一行注释,避免后续读者当成遗漏再加回来、或当成无用代码再次删掉。若删除动机其实是「不应释放调用方 payload 持有的张量」,请在 PR description 与注释中写明,并同步处理 masked 分支其余 dispose_tensor 以保持策略一致。

Checklist: [6.1] 状态不变量:创建/更新/失败/重试/回滚路径有效

float q_val = fminf(fmaxf(val / y_s, min_8bit), max_8bit);
out_vec[j] = DST_DTYPE(q_val);
}
out_vec.store(group_output + i * vec_size);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] 向量化 fp8 store 沿用了未校验的 group_size 对齐前提

新增分支用 out_vec.store(group_output + i * vec_size)(:131)打包写出,vec_t<__nv_fp8_e4m3, 8> 内部为 uint2,store 要求目标地址 8 字节对齐;目标偏移为 global_group_id * group_size + i * vec_size,只有 group_size % vec_size == 0(bf16/fp16 为 8,fp32 为 4)时恒成立。host 侧 per_token_group_quant_8bit 仅校验 input.numel() % group_size == 0(:156)与 output_s.dim() == 2(:157),没有 group_sizevec_size 关系的检查,而 group_size 是 Python 侧传入的运行时参数。需要说明的是,同一循环内保留的 input_vec.cast_load(group_input + i * vec_size)(:82、非本 PR 新增)已要求更强的 16...

建议: 顺手在 host 入口补一条 CHECK_EQ(group_size % (16 / input.element_size()), 0)(或等价 TORCH_CHECK),把这个原本隐式的前提变成可诊断的 fail-fast,同时也保护既有的尾元素截断问题。另外 can_cache 由运行时 group_size 决定,两条路径都会被编译并占用寄存器,如后续要做性能收敛可考虑提升为模板 constexpr。

combine_payload = self._execute(
executor, payload, enable_cuda_graph=enable_cuda_graph
)
self.assertEqual(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] 空专家用例的零值断言不具备判别力

_run_deepep_normal_executorempty_local_experts 分支只有两条断言(:197-202):shape == payload.expert_x.shapecount_nonzero(...) == 0。eager 变体下所有计数为 0 → all_tokens == 0,executor 在 :435-442 直接早退返回 torch.zeros,根本没有走 kernel;CUDA Graph 变体虽然会走 ep_scatter/ep_gather,但输出缓冲是 torch.empty(deepgemm_hybrid_executor.py:566)且来自 graph 私有内存池(实践中常为全零),故「combine kernel 根本没写输出」与「正确写入了零」两种情况无法区分。这两条用例真正证明的偏向「capture/replay 与 eager 执行不崩溃」。

建议: 在执行(CUDA Graph 场景为 graph.replay())之前把输出缓冲填成非零哨兵(例如 fill_(float("nan"))fill_(1.0)),执行并 synchronize 后再断言全零;这样只有 combine kernel 真正写入零才能通过,用例即可同时守住「不崩溃」与「确实写出零」两件事。

Checklist: [6.1] 边界 case 覆盖(空、单元素、最大值);[P.G] mock/fake/stub 不得替代本次声称覆盖的生产边界

@@ -838,7 +838,11 @@ def _postprocess(
)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📍 实际位置 rtp_llm/model_loader/per_block_fp8_quant_weight.py:103(不在 diff 展示范围内,就近挂载)

[P3] 同名 per_block_cast_to_fp8 存在两套不兼容签名,新旧量化路径各用其一

per_block_fp8_quant_weight.py:103 定义 per_block_cast_to_fp8(x, group_size: int),而 models_py/kernels/cuda/fp8_kernel/fp8_kernel.py:382 定义同名 per_block_cast_to_fp8(x, use_ue8m0: bool)。本 PR 后,同一个 _load_raw_tensor 中的 elif self.scale: 分支调用前者(:943),新增的 direct_ue8m0 分支经 quant_weight_ue8m0_packed 调用后者(fp8_kernel.py:458)。两者第二个位置参数一个是 int 一个是 bool,误用不会被类型检查拦下(per_block_cast_to_fp8(x, 128) 在 bool 语义下等价于 use_ue8m0=True),新增的 SM120 单测又同时导入了两个模块,容易混淆。

建议: 给两者取可区分的名字(例如把 loader 内部的重命名为 per_block_cast_to_fp8_grouped),或把 loader 侧实现下沉到 fp8_kernel 并统一为带关键字参数的单一入口;至少在 loader 侧定义上加一行注释说明它与 fp8_kernel 同名函数的差异,避免后续维护者跨模块误替换。

Checklist: [6.1] DRY:重复非平凡逻辑被抽取或显式复用;[I] 同一功能用统一工具函数

@LLLLKKKK LLLLKKKK left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Code Review - PR #1362

Status: BLOCKING

Summary: P0/0 · P1/3 · P2/9 · P3/9

Reviewed: commit 08adc2f2ede4 · 2026-09-01 00:20 UTC+8

Blocking Issues

P1

  • 新增 SM120 dense FP8_PER_BLOCK golden 固化了一段与参考基准完全不同的拒答输出,疑似 direct UE8M0 数值错误被 golden 掩盖 @ rtp_llm/test/smoke/data/model/qwen3/q_r_fp8pb_sm120.json:84
    • 建议:请先用 --quantization FP8_DYNAMIC_PER_TENSOR 与 BF16 在 RTX 5000 Pro 上跑同一 prompt 做三方对照,定位 per-block direct UE8M0 路径的数值偏差(重点核对 _load_raw_tensor.Tquant_weight_ue8m0 产出的 scale 行序,与 _postprocesspack_weight_scale_ue8m0(scale, kernel_weight.shape[-2])mn 取值是否对应同一维度);确认数值修复后再重新录制该 golden。若经核查该输出确为 FP8 量化在 1.7B 小模型上的真实退化,请在 PR description 中给出与 H20/per-tensor 对照的 logits 差异证据,并把该 case 的 prompt 换成对量化噪声不敏感的短问答,避免用一段拒答文本充当正确性基线。
  • direct packed UE8M0 量化路径同时改变 SM100 的加载期数值与失败语义,新增验证却全是 SM120 @ rtp_llm/model_loader/per_block_fp8_quant_weight.py:936
    • 建议:建议二选一:(1)把 direct 路径收窄为 SM120 专用(复用 rtp_llm.models_py.utils.arch.is_sm12x()),SM100 继续走 requant_weight_ue8m0,后续单独提 PR 切换——考虑到上一条 finding 指向该路径可能存在数值问题,收窄门控同时也降低了 SM100 的暴露面;(2)若确认要同时改 SM100,请在本 PR 内给出 moe_deepep_normal_tp2_sm100 等既有 SM100 FP8_PER_BLOCK smoke 的实际复跑结论与数值差异幅度,并把三个 test_online_loader_* 上移到与架构无关的共享基类,使 SM100/SM120 目标共同继承。另建议在 direct 分支断言切分后 kernel_weight.shape[-2] 是 128 的整数倍——_transform_scale_ue8m0index_select(-2, arange(mn)//128) 反查 scale 行,非 128 对齐的切分会取到错位 scale。
  • 分发单测未 mock has_deep_gemm_mk_alignment,断言绑定宿主 DeepGEMM 构建,在仍锁 2.2.0 的 cuda12 栈上确定性失败 @ rtp_llm/models_py/modules/factory/fused_moe/tests/test_cuda_strategies.py:338
    • 建议:在两处用例上按使用处 patch(...deepgemm_hybrid_executor.has_deep_gemm_mk_alignmentreturn_value=True),让该用例只验证 SM 版本分发决策,真实版本探测交给 RTX_5000_PRO 目标下的 test_sm120。同时补两条用例:「SM=12 且 has_deep_gemm_mk_alignment() 为 False 时 can_handle 必须为 False」(覆盖新增守卫本身),以及 masked 策略在 enable_cuda_graph=True 组合下的期望分发结果,把该组合的门禁语义固化下来。

Non-blocking Suggestions

P2

  • contiguous 路径 mk-alignment 设置后无回读校验,expected_m 语义与 min(128,·) 下钳均缺说明 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/deepgemm_hybrid_executor.py:402
    • 建议:在 configure_deep_gemm_mk_alignment 内 set 之后回读 deep_gemm.get_mk_alignment_for_contiguous_layout() 并断言等于请求值,不等则 fail-fast(复用已取的 original 变量,零额外成本)。同时在 get_theoretical_mk_alignment_for_contiguous_layout 的 wrapper docstring 中写明 expected_m 在 contiguous 语义下指总 m(与 masked 的每组均值不同),并给 min(128, theoretical) 补一行说明为何需要上限;若无必要则直接用 theoretical 值同时驱动 all_tokensep_scatter(align_m=) 与 GEMM 三者。
  • 删除 topk 越界告警后非法 expert id 与容量溢出全部静默丢弃,失去 router 缺陷可观测性 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/deepgemm_hybrid_executor.py:388
    • 建议:保留一条零同步的诊断:在 ep_scatter 内对被屏蔽的越界/溢出条目做 atomic_add 到一个小计数器,forward 之外按采样频率或 debug flag 读回并 logger.error,既不引入 per-forward D2H,也不丢失可观测性。同时在 __init__execute_contiguous 开头补一条零成本形状断言(num_experts_local == self.num_experts_per_partition == self.E),把原 clamp 承担的防御性以不需要同步的方式补回。
  • SM120 eager 路径 workspace 容量与布局来自两个数据源,CPU 计数变量靠跨分支条件蕴含维系 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/deepgemm_hybrid_executor.py:425
    • 建议:把容量与布局收敛到同一数据源:SM120 eager 也传 derive_counts_from_topk=True 并复用形状推导上界,或显式断言 sum(align_up(cpu_counts, A)) == all_tokens。同时把 num_recv_tokens_per_expert_cpu 的求取提到分支之前(或抽成一个按 (is_sm120, enable_cuda_graph) 分派、返回 (all_tokens, scatter_counts, scatter_alignment) 的小函数),消除跨分支的隐式绑定,也让 pyright strict 可静态检查。
  • CUDA Graph 用例只 replay 同一份输入,最贴近 SM120 decode 的小 batch 形状无 graph 覆盖 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/test/deepep_normal_executor_sm120_test.py:31
    • 建议:在 _execute 的 graph 分支中,捕获完成后重新采样一批路由分布不同的输入(对捕获时的张量做 in-place copy_,保持 shape 不变)再 replay(),并与该批次对应的 eager 结果比对,形成「同图、双输入」断言。同时给 DeepGemmHybridExecutorQwen35ShapeSM120Testenable_cuda_graph=True 用例,覆盖 routed_tokens 远小于 num_experts_local * expert_alignment 时的 workspace 上界分支与小 M 的 alignment 分支。
  • quant_weight_ue8m0_packed 在生产路径不可达,其 docstring 承诺的加载期显存优化未生效 @ rtp_llm/models_py/kernels/cuda/fp8_kernel/fp8_kernel.py:433
    • 建议:二者取一:(1)把 _load_raw_tensor 的 direct 分支改调 quant_weight_ue8m0_packed,并相应去掉 _postprocess 中的 pack_weight_scale_ue8m0(该函数已内含 _transform_scale_ue8m0,:465,否则会二次打包),同时确认与 TP/EP 切分顺序兼容;(2)若因切分顺序无法复用,则删除该函数及 __all__ 导出,把分块逻辑并入 quant_weight_ue8m0,并把测试断言改为覆盖 quant_weight_ue8m0 + pack_weight_scale_ue8m0 这条真实组合。
  • indexer 参考实现与测试改用 2-D context_lens,与其对标的生产 1-D 路径分叉 @ rtp_llm/models_py/modules/hybrid/test/indexer_ref.py:641
    • 建议:统一两侧形状约定:若 1-D 在 DeepGEMM 2.5.0 仍受支持,参考实现与 indexer_test.py 应回到 1-D 以继续覆盖生产真实契约;若新 wheel 只接受 2-D,则应连同 indexer_op.py 与其注释一起改,并说明 cuda12 2.2 栈上的降级方式。无论选哪种,建议把 context_lens 的整形收敛到一个由生产、DSv4 与参考实现共同调用的 helper,在其中记录 _indexer_score.py 的 2-D 断言属多 query 场景例外,并去掉 indexer_test.py 中对 schedule_metadata 的无效赋值。
  • 在线 loader 单测被 SM120 硬件门独占,且 stub 掉了用例名声称覆盖的 TP 切分边界 @ rtp_llm/models_py/modules/factory/linear/impl/cuda/test/fp8_deepgemm_linear_sm120_test.py:122
    • 建议:把三个 loader 用例抽到与架构无关的共享基类(或 mock is_deep_gemm_e8m0_used 后放入无硬件门目标),使 SM100/SM120 共同继承,直接补齐前述 SM100 覆盖缺口;..._after_tp_split 改为通过真实 quant_config / src_weight_info 构造 loader,让 CompositeWeight._postprocess 走真实切分,若确需隔离则把用例名与注释改为「仅校验 pack 尾部」并另补一条覆盖真实切分的用例。
  • deep_gemm 跨 4 个 minor 升级作用于整个 cuda12_9 平台,新增验证只覆盖 SM120 且回滚边界未说明 @ deps/requirements_torch_gpu_cuda12_9.txt:15
    • 建议:请在 PR description 中补齐三点:1)上游 2.2→2.5 是否改变 SM90/SM100 的 kernel 选择或数值行为(附 changelog 或结论摘要);2)本次 CI 中既有 cu129 smoke(H20 MLA/dense)与性能基线的实际复跑结果,而非仅「新增 SM120 suite 通过」;3)回滚手段(保留旧 wheel URL 即可还原)。若拿不到 SM90 侧证据,建议把 pin 升级拆为独立 PR 与 SM120 功能解耦,便于单独 bisect 与回滚。
  • lock 未按声明的 update 目标整体重生成,via 注解自相矛盾且新增的 rtp-kernel 耦合未验证 @ deps/requirements_lock_torch_gpu_cuda12_9.txt:733
    • 建议:请在标准工作目录下用约定的 update 目标重新生成整份 lock 并提交完整 diff,使 # via 前缀全文一致、compile_pip_requirements 一致性 test 可本地跑通(lock 全量带 --hash,闭包缺项会在 --require-hashes 安装阶段硬失败,重生成可把风险从「构建期偶发」前移到「本地可验证」)。同时确认 rtp_kernel-0.1.0+125c29e5 与 deep_gemm 2.5.0 兼容(至少跑通相关 group gemm 单测),否则同步升级该 wheel,并建议在 rtp-kernel 侧声明 deep-gemm 版本下限。

P3

  • m_indices = -1 的 padding 行使 gateup_output 未初始化即进入 silu_and_mul 与量化 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/deepgemm_hybrid_executor.py:495
    • 建议:让 padding 行语义显式化:在 SM120 / CUDA Graph 路径把 gateup_output 改为 torch.zeros(一次 memset 相对两段 GEMM 可忽略),或给 silu_and_mul 传入有效行数上界只算 [0, valid_rows)。无论哪种,请在 m_indices-1 处补一行注释说明「-1 行由 DeepGEMM 跳过,其 gateup_output 内容未定义」这一非显然的不变量。
  • dispose_tensor 被无条件删除,与捕获无关的 masked / eager 路径出现峰值显存回归 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/deepgemm_hybrid_executor.py:254
    • 建议:按 capture 需求收窄而非整体删除:execute_masked 恢复无条件 dispose_tensor(hidden_states_fp8)(该路径与 graph 互斥);execute_contiguous 改为 if not self.enable_cuda_graph: dispose_tensor(hidden_states_fp8),并在该行注释说明「捕获期不能释放调用方持有的 storage」,使图安全约束显式化而不是通过删除代码隐式表达。
  • SM120 + CUDA Graph 下 masked 路径与 masked_max_token_num 被静默旁路,缺少独立回滚开关 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/deepgemm_hybrid_executor.py:152
    • 建议:在 __init__ 中检测到 self.is_sm120 and self.enable_cuda_graph 且用户显式配置过 masked_max_token_num 时打一条 logger.info,说明该配置在此组合下不生效及原因;并考虑提供一个独立开关(config 字段或环境变量)允许强制走 masked 实现,作为 SM120 contiguous 路径出问题时的运维回滚手段。
  • 向量化 fp8 store 引入新的目标地址对齐前提,未校验 group_size 与 vec_size 的整除关系 @ rtp_llm/models_py/bindings/cuda/kernels/per_token_group_quant_8bit.cu:131
    • 建议:在 per_token_group_quant_8bit 的 host 端补一条 CHECK_EQ(group_size % (16 / input.element_size()), 0)(或 TORCH_CHECK),或在 kernel 内按 group_size % vec_size == 0 选择打包/标量写回路径,把该前提从隐含变为显式;也可在 can_cache 附近的注释中一并说明打包写回的对齐依赖。
  • 直接量化路径对非 2-D 或非 128 group_size 改为硬失败,缺少降级 @ rtp_llm/model_loader/per_block_fp8_quant_weight.py:949
    • 建议:把这两个 raise 改为退回既有 per_block_cast_to_fp8_grouped + requant_weight_ue8m0 路径并打一条 warning,使 direct 优化成为可降级的加速而非新增的启动阻塞;若确实希望硬失败,请在错误信息中给出可操作的规避方式(例如提示改用预量化 ckpt 或指定哪种 quantization 配置)。
  • effectiveKernelSeqSizePerBlock 与既有 sentinel 逻辑重复,退化边界无断言 @ rtp_llm/cpp/cache/CacheConfig.h:71
    • 建议:让 kernelBlocksPerKvBlock() 复用新函数(seq_size_per_block / effectiveKernelSeqSizePerBlock()),并考虑把 HybridPoolConfigCreator / CacheConfigCreator / NormalEngine 中的同类规范化一并收敛到该访问器;测试补一组 seq_size_per_block = 0; kernel_seq_size_per_block = 0; 断言返回 0(并注明调用方为何仍需再判一次),以及一条不整除组合触发校验失败的断言,同时把测试名调整为体现 effectiveKernelSeqSizePerBlock 语义。
  • 注释记录的 DeepGEMM 构建标识与实际 pin 无法互相对应 @ rtp_llm/models_py/kernels/cuda/deepgemm_wrapper.py:114
    • 建议:统一为可核对的标识:把注释改为引用 deps 中的实际版本(2.5.0+d7d5eca),或在 deps 的 wheel 行注明其对应的上游 commit,使「代码要求的最低版本」与「依赖实际 pin」可以双向追溯。
  • SM120 GEMM 断言阈值与仓库统一的 calc_diff 口径不一致 @ rtp_llm/models_py/modules/factory/linear/impl/cuda/test/fp8_deepgemm_linear_sm120_test.py:191
    • 建议:改用 calc_diff 并给出与基类同量纲的阈值(先记录实测值再取 2-3 倍余量);若确有理由使用 relative L2,请在断言旁注明与 calc_diff 的换算关系。同时删除 :54 未使用的 requant_weight,或改为直接断言生产函数 requant_weight_ue8m0 的返回值参与对比。
  • test_cuda_strategies.py 内仍残留 4 处指向定义处的无效 get_sm patch @ rtp_llm/models_py/modules/factory/fused_moe/tests/test_cuda_strategies.py:410
    • 建议:顺手把这 4 处也改为使用处 patch(与本 PR 已修正的两处保持一致),或抽一个测试 helper 统一 patch 所有 get_sm 使用点,避免同一文件里两种写法并存造成后续误用。

Checklist Findings (16 fail / 54 total)

General Principles Checklist

  • [6.1] Architecture — 兼容性:外部 HTTP/RPC API、持久数据、配置、环境迁移安全 → issue lock 未按声明的 update 目标整体重生成,via 注解自相矛盾且新增的 rtp-kernel 耦合未验证
    文件头声明该 lock 由 bazel run //deps:requirements_torch_gpu_cuda12_9.update 生成,但改动后全文只有 deep-ep(:732)与 deep-gemm(:736)写成带前缀的 -r deps/requirements_torch_gpu_cuda12_9.txt,其余 23 处仍为无前缀写法(抽验 :729/:784/:3631/:4062);deep-ep 的版本与 hash 完全未变却产生注解 churn,说明该 hunk 是在不同工作目录下局部粘贴,2.5.0 自身的 Requires-Dist 闭包无法确认,而 deps/BUILD:67 的一致性 test 带 manual tag,CI 不会拦下这种不自洽。同时 deep-gemm 的 requester 新增 rtp-kernel(:737),而 rtp-kernel 自身 pin(rtp_kernel-0.1.0+125c29e5,源清单 :21)未变、也未记录任何 deep-gemm 版本约束,其 group GEMM 是生产路径依赖
  • [6.1] Architecture — 可观测性:日志/指标/超时可操作、非噪声 → issue SM120 + CUDA Graph 下 masked 路径与 masked_max_token_num 被静默旁路,缺少独立回滚开关
    路由条件从 if token_num <= self.masked_max_token_num: 变为追加 and not self.enable_cuda_graph。由于 check_conditions(:83)只在 SM120 上允许 enable_cuda_graph=True,这等价于:SM120 一旦开图(线上 decode 常见配置),masked 路径永久不可达,用户配置的 masked_max_token_num 完全失效且无任何日志提示。若 contiguous 路径在 SM120 出现性能或数值回退,运维唯一手段是整体关闭 CUDA Graph,代价远大于切回 masked 实现。
  • [6.1] Architecture — 回滚路径:风险行为存在运维回滚手段 → issue SM120 + CUDA Graph 下 masked 路径与 masked_max_token_num 被静默旁路,缺少独立回滚开关
    路由条件从 if token_num <= self.masked_max_token_num: 变为追加 and not self.enable_cuda_graph。由于 check_conditions(:83)只在 SM120 上允许 enable_cuda_graph=True,这等价于:SM120 一旦开图(线上 decode 常见配置),masked 路径永久不可达,用户配置的 masked_max_token_num 完全失效且无任何日志提示。若 contiguous 路径在 SM120 出现性能或数值回退,运维唯一手段是整体关闭 CUDA Graph,代价远大于切回 masked 实现。
  • [6.1] Architecture — 状态不变量:创建/更新/失败/重试/回滚路径有效 → issue dispose_tensor 被无条件删除,与捕获无关的 masked / eager 路径出现峰值显存回归
    diff 删除了两处 dispose_tensor(hidden_states_fp8)execute_maskedep_scatter_v2 之后(现 :253-254 交界)与 execute_contiguousep_scatter 之后(现 :494 附近)。两个函数都已把 shape/device 缓存到局部变量、之后不再使用张量本体,说明提前释放本来有效;同文件对 upgate_output / down_inputdispose_tensor 仍保留,策略不一致。删除对捕获路径确有必要(dispose_tensor 通过 x.set_() 释放调用方持有的 storage),但 execute() :152 的路由条件是 token_num <= masked_max_token_num and not self.enable_cuda_graph,即 execute_masked 开图时根本不可达;contiguous 在 SM90/SM100 及 SM120 eager 下同样不涉及捕获。diff
  • [6.1] Architecture — 错误语义:fail-fast/retry/fallback/silent 行为显式 → issue 直接量化路径对非 2-D 或非 128 group_size 改为硬失败,缺少降级
    新增分支在 source_weight.dim() != 2raise ValueError(:949-954),在 self.group_size != 128 时同样 raise ValueError(:937-941)。此前这两类输入都会落到 per_block_cast_to_fp8_grouped,而后者显式支持 3-D(is_2d = x.dim() == 2unsqueeze(0),:106-108)并支持任意 group_size。由于门控只看 GPU capability、不看权重形状,w8a8_weight_list 中任何以 3-D 形态到达或使用非 128 group_size 的 dense 权重,在 SM100/SM120 上会从「可加载」变为加载期直接抛异常。
  • [6.1] Quality — PR description 说明动机与设计 → issue 注释记录的 DeepGEMM 构建标识与实际 pin 无法互相对应
    is_deep_gemm_e8m0_used 上方注释(:113-114)称 SM120 需要 "vLLM-pinned DeepGEMM build (a6b593d or newer)",同文件 :143-146 与 :160-163 的报错文案写 "requires DeepGEMM >= 2.5.0",而本次实际 pin 为 deep_gemm-2.5.0+d7d5eca.cu129。全仓搜索 a6b593d 仅出现在该注释一处,d7d5eca 仅出现在 deps 两个文件,三个标识无法互相换算,维护者无从核对当前 pin 是否满足注释所述下限。
  • [6.1] Quality — 逻辑变更未混入无关格式化 → issue lock 未按声明的 update 目标整体重生成,via 注解自相矛盾且新增的 rtp-kernel 耦合未验证
    文件头声明该 lock 由 bazel run //deps:requirements_torch_gpu_cuda12_9.update 生成,但改动后全文只有 deep-ep(:732)与 deep-gemm(:736)写成带前缀的 -r deps/requirements_torch_gpu_cuda12_9.txt,其余 23 处仍为无前缀写法(抽验 :729/:784/:3631/:4062);deep-ep 的版本与 hash 完全未变却产生注解 churn,说明该 hunk 是在不同工作目录下局部粘贴,2.5.0 自身的 Requires-Dist 闭包无法确认,而 deps/BUILD:67 的一致性 test 带 manual tag,CI 不会拦下这种不自洽。同时 deep-gemm 的 requester 新增 rtp-kernel(:737),而 rtp-kernel 自身 pin(rtp_kernel-0.1.0+125c29e5,源清单 :21)未变、也未记录任何 deep-gemm 版本约束,其 group GEMM 是生产路径依赖
  • [6.1] Software Engineering — DRY:重复非平凡逻辑被抽取或显式复用 → issue effectiveKernelSeqSizePerBlock 与既有 sentinel 逻辑重复,退化边界无断言
    kernel_seq_size_per_block == 0 视为等于 seq_size_per_block」这一语义在同一头文件内已由 kernelBlocksPerKvBlock()if (kernel_seq_size_per_block == 0) return 1;(:91-94)与 kernelBlocksPerKvBlockForGroup() 的同款判断(:78-80)表达过,新函数是又一份副本且未被前者复用。NormalExecutor.cc:125-126 的第二层三元并非冗余(它兜住 !cache_managerattn_config.kernel_tokens_per_block 为 0 的分支),但「sentinel 回退后仍可能为 0」这一不变量在测试中无任何断言:KVCacheResourceTest.cc:216-227 只覆盖 (1,0)(8,2),缺 (0,0) 退化组合与 kernel > seq 触发 RTP_LLM_CHECK 的组合。
  • [6.1] Software Engineering — KISS/YAGNI:无投机性抽象 → issue quant_weight_ue8m0_packed 在生产路径不可达,其 docstring 承诺的加载期显存优化未生效
    全仓检索 quant_weight_ue8m0_packed 仅命中定义(:433)、fp8_kernel/__init__.py 导出,以及 fp8_deepgemm_linear_sm120_test.py(:52/86/187)三处测试引用,无任何生产调用方。其 docstring 明确写「Limit floating-point temporaries during online loading … Chunking on a 128-row boundary」,而真实加载路径 _load_raw_tensor 走的是未分块的 quant_weight_ue8m0(per_block_fp8_quant_weight.py:956),后者内部 per_block_cast_to_fp8 会一次性 materialize align(m,128) x align(n,128) 的 padded 副本与同尺寸浮点中间量——该函数声称要解决的问题在生产上完全没被解决,且 test_direct_weight_runs_sm120_deepgemm 的端
  • [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue deep_gemm 跨 4 个 minor 升级作用于整个 cuda12_9 平台,新增验证只覆盖 SM120 且回滚边界未说明
    pin 由 deep_gemm-2.1.1+local 抬到 2.5.0+d7d5eca.cu129,跨越 2.2/2.3/2.4/2.5。该清单经 deps/BUILD:61-68 生成 lock,再经 deps/pip.bzlpip_gpu_cuda12_9_torch 被 cuda12_9 下全部 bazel Python 目标共享,而 .bazelrc 中 cuda12_9 继承的算力集含 SM70–SM90,即 H20/H800 既有 FP8 GEMM 生产路径的运行时二进制被无门控替换。但本 PR 新增 suite 与 golden 全为 SM120(suites_sm120.bzl 全部 gpu_type = ["RTX_5000_PRO"]),diff 内没有既有 cu129 smoke 或性能基线的复跑结论,仓内也不存在任何 deep_gemm 版本断言可佐证跨 minor 行为等价。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue test_cuda_strategies.py 内仍残留 4 处指向定义处的无效 get_sm patch
    本 PR 已把两个 test_can_handle_cuda_graph 的 mock target 正确迁移到使用处(deepgemm_hybrid_executor.get_sm / deepep_normal_router.get_sm),但同文件仍有 4 处 @patch("rtp_llm.models_py.utils.arch.get_sm")(:410、:433、:503、:529)。这些生产模块都是模块级 from rtp_llm.models_py.utils.arch import get_sm(deepep_normal_router.py:33、deepgemm_hybrid_executor.py:47),定义处 patch 不生效,用例实际依赖宿主真实 SM(H20 恰为 (9,0),与 mock 值巧合一致)才通过;一旦该目标被调度到其它机型,这些断言会以误导性的方式失败。
  • [6.1] Tests — 边界 case 覆盖(空、单元素、最大值) → issue effectiveKernelSeqSizePerBlock 与既有 sentinel 逻辑重复,退化边界无断言
    kernel_seq_size_per_block == 0 视为等于 seq_size_per_block」这一语义在同一头文件内已由 kernelBlocksPerKvBlock()if (kernel_seq_size_per_block == 0) return 1;(:91-94)与 kernelBlocksPerKvBlockForGroup() 的同款判断(:78-80)表达过,新函数是又一份副本且未被前者复用。NormalExecutor.cc:125-126 的第二层三元并非冗余(它兜住 !cache_managerattn_config.kernel_tokens_per_block 为 0 的分支),但「sentinel 回退后仍可能为 0」这一不变量在测试中无任何断言:KVCacheResourceTest.cc:216-227 只覆盖 (1,0)(8,2),缺 (0,0) 退化组合与 kernel > seq 触发 RTP_LLM_CHECK 的组合。

RTP-LLM Checklist

  • [I] 代码质量 — 删除或重命名内部 file、registry entry、model name、metric enum、op binding、plugin symbol 时,必须全仓搜索消费者,并提供替代实现、迁移说明或 smoke 覆盖;只有暴露到 HTTP/RPC/config/persisted format 时才按外部兼容性处理 → issue quant_weight_ue8m0_packed 在生产路径不可达,其 docstring 承诺的加载期显存优化未生效
    全仓检索 quant_weight_ue8m0_packed 仅命中定义(:433)、fp8_kernel/__init__.py 导出,以及 fp8_deepgemm_linear_sm120_test.py(:52/86/187)三处测试引用,无任何生产调用方。其 docstring 明确写「Limit floating-point temporaries during online loading … Chunking on a 128-row boundary」,而真实加载路径 _load_raw_tensor 走的是未分块的 quant_weight_ue8m0(per_block_fp8_quant_weight.py:956),后者内部 per_block_cast_to_fp8 会一次性 materialize align(m,128) x align(n,128) 的 padded 副本与同尺寸浮点中间量——该函数声称要解决的问题在生产上完全没被解决,且 test_direct_weight_runs_sm120_deepgemm 的端
  • [I] 代码质量 — 同一功能用统一工具函数 → issue SM120 GEMM 断言阈值与仓库统一的 calc_diff 口径不一致
    test_direct_weight_runs_sm120_deepgemm 手写 (actual - expected).norm() / expected.norm()assertLess(relative_l2, 0.06)(:191-194),而该文件继承的 fp8_linear_test.py 全篇使用 rtp_llm/test/utils/numeric_util.pycalc_diff(阈值 0.0011,见 :520/:580/:994)。按 calc_diff = 1 - 2Σxy/Σ(x²+y²) ≈ relative_l2²/2(numeric_util.py:118-122),0.0011 约对应 relative_l2 0.047,故新阈值比同目录既有口径松约 1.6 倍;两套量纲不同的指标并列出现会让后续调阈值的人难以判断松紧关系。同文件 :54 的 requant_weight 赋值后从未被使用。

Python Static-First Checklist

  • [P.G] 测试规范 — mock.patch target 是使用处而非定义处 → issue test_cuda_strategies.py 内仍残留 4 处指向定义处的无效 get_sm patch
    本 PR 已把两个 test_can_handle_cuda_graph 的 mock target 正确迁移到使用处(deepgemm_hybrid_executor.get_sm / deepep_normal_router.get_sm),但同文件仍有 4 处 @patch("rtp_llm.models_py.utils.arch.get_sm")(:410、:433、:503、:529)。这些生产模块都是模块级 from rtp_llm.models_py.utils.arch import get_sm(deepep_normal_router.py:33、deepgemm_hybrid_executor.py:47),定义处 patch 不生效,用例实际依赖宿主真实 SM(H20 恰为 (9,0),与 mock 值巧合一致)才通过;一旦该目标被调度到其它机型,这些断言会以误导性的方式失败。
  • [P.G] 测试规范 — mock/fake/stub 不得替代本次声称覆盖的生产边界 → issue 在线 loader 单测被 SM120 硬件门独占,且 stub 掉了用例名声称覆盖的 TP 切分边界
    _三个 test_online_loader_* 验证的 direct 分支由 is_deep_gemm_e8m0_used() 驱动,对 SM100 与 SM120 均为真,但它们只存在于 tags = ["RTX_5000_PRO"] 的新目标,fp8_deepgemm_linear_sm100_test / ..._sm100_arm_test 的 srcs 不含该文件(test/BUILD:30-61),SM100 永不执行。三个用例都用 object.__new__ 跳过 __init__CompositeWeight.__init__test_online_loader_packs_ue8m0_scale_after_tp_split 进一步 patch("...CompositeWeight._postprocess", return_value=...)(:143-149)直接返回测试自己预切好的张量——用例名声称覆盖的 TP split 生产边界(真正做 N 维切分并同步切 scale 行的 `super().postp

Strengths

  • m_indices / output_indextorch.empty / empty_like 改为 torch.full(-1) / full_like(-1)(deepgemm_hybrid_executor.py:462-465),与 _fwd_kernel_ep_gather 既有的 source_token_index >= 0 && < total_input_tokens 守卫(ep_kernels.py:437)第一次形成闭环:未初始化的垃圾下标不再可能偶然通过守卫;填充是被捕获的 GPU 节点,replay 会重新执行。
  • 越界防护并未随 host clamp 一起消失,而是下沉为 kernel 内判定:_count_tokens_per_expert_kernel(expert_ids >= 0) & (expert_ids < num_experts)(:34)、_fwd_kernel_ep_scatter_2dest_is_valid = dest_token_index < output_capacity(:125)、_fwd_kernel_ep_scatter_1cur_expert_start + offs < m_indices_size(:70),三层自洽且消除了捕获期 D2H 同步。
  • ep_scatter 新增两个带默认值的关键字参数并把 assert m_indices.shape[0] % BLOCK_E == 0 放宽为 % align_m,对既有调用方(dsv4/moe/strategies/grouped_fp4.py 传入已按 128 对齐的计数)是幂等运算,无跨模块契约破坏。
  • test_ep_scatter.py 新增用例质量高:test_metadata_counts_cannot_write_past_workspace 用「元数据故意写成 [5,5,5] 而实际仅 3 个 token」+ guard row sentinel 验证不越界;test_topk_multi_expert_invalid_ids_and_metadata 精确断言 expert_start_loc / m_indices / output_index 与搬运内容,把 derive_counts_from_topk 的契约真正锁住。
  • has_deep_gemm_mk_alignment()hasattr 探测三个符号并接入 check_conditions(deepgemm_hybrid_executor.py:81-82),DeepGEMM 版本不足时 executor 优雅落选而非运行期抛错;configure_deep_gemm_mk_alignment 正确识别该配置是进程级全局状态,用 threading.Lock 覆盖 set/launch/restore 全程。
  • CacheConfig.h:71effectiveKernelSeqSizePerBlock() + NormalExecutor.cc:122-126 修正了一处真实缺陷:cache_manager 存在且 kernel_seq_size_per_block 为默认 0 时,旧代码会把 0 传给 CudaGraphRunner,而后者要求该值 > 0。
  • per_block_cast_to_fp8 在 cast 前补 .clamp(fp8_min, fp8_max)(fp8_kernel.py:396),避免越界值转 FP8 产生 Inf;per_block_cast_to_fp8_grouped 重命名消除了同名不同签名的歧义。
  • per_token_group_quant_8bit_kernelcan_cache = (num_vec_elems <= 16) 与两个循环 i = lane_id; i += 16 的迭代条件严格一致,未进入第一循环的 lane 也不会进入第二循环,不存在读未初始化 cached_vec 的路径。
  • deepep_normal_executor_test.py 的 graph 分支不止「能捕获即通过」:warmup → 捕获 → graph.replay()calc_diff 数值比对;SM120 MoE smoke 亦带 --enable_cuda_graph 1 --decode_capture_config '1,2' --tp_size 2,对 contiguous+graph 提供了端到端覆盖。

"index": 0,
"message": {
"role": "assistant",
"content": "I'm sorry, but I can't assist with that request. I'm an AI assistant designed to help with tasks like"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] 新增 SM120 dense FP8_PER_BLOCK golden 固化了一段与参考基准完全不同的拒答输出,疑似 direct UE8M0 数值错误被 golden 掩盖

三份 golden 指向同一 Qwen3-1.7B 快照、同一 $prompt:m2 + $prompt:s6、同一 top_k=1 贪心、enable_thinking=false。H20 的 dense_fp8pb_dynamicq_r_h20.json:83,同为 --quantization FP8_PER_BLOCK)输出「我理解您希望获得关于新高考的三年规划…」;本 PR 新增的 SM120 per-tensor golden(q_r_fp8pt_sm120.json:84max_tokens=6)输出「我理解您希望获得关于」,与之逐 token 一致。唯独本 PR 新增的 SM120 per-block golden 输出 "I'm sorry, but I can't assist with that request. …"。贪心解码下同权重同 prompt 的前 24 token 必须是同一前缀,语言与语义整体翻转是量化路径数值失效的典型征兆;该 case 已挂入 smoke_sm120_dense(smoke/B...

建议: 请先用 --quantization FP8_DYNAMIC_PER_TENSOR 与 BF16 在 RTX 5000 Pro 上跑同一 prompt 做三方对照,定位 per-block direct UE8M0 路径的数值偏差(重点核对 _load_raw_tensor.Tquant_weight_ue8m0 产出的 scale 行序,与 _postprocesspack_weight_scale_ue8m0(scale, kernel_weight.shape[-2])mn 取值是否对应同一维度);确认数值修复后再重新录制该 golden。若经核查该输出确为 FP8 量化在 1.7B 小模型上的真实退化,请在 PR description 中给出与 H20/per-tensor 对照的 logits 差异证据,并把该 case 的 prompt 换成对量化噪声不敏感的短问答,避免用一段拒答文本充当正确性基线。

is_deep_gemm_e8m0_used,
)

direct_ue8m0 = self._uses_direct_ue8m0()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] direct packed UE8M0 量化路径同时改变 SM100 的加载期数值与失败语义,新增验证却全是 SM120

_uses_direct_ue8m0()(:910-919)的门控是 is_deep_gemm_e8m0_used(),而后者返回 get_device_capability()[0] in (10, 12)(deepgemm_wrapper.py:115),SM100 同样命中(报错文案 :939 亦写 "SM100/SM120")。因此 SM100 上 w8a8_weight_list 中全部 dense 权重(attn_qkv/attn_o/ffn/mla_*)从「per_block_cast_to_fp8_grouped 浮点 scale → _postprocessrequant_weight_ue8m0 二次舍入」切换为「.Tquant_weight_ue8m0 一次量化 + 仅打包 scale」,数值必然变化;source_weight.contiguous().to(device)(:957)还把未分片权重整体搬上 GPU,改变加载期显存曲线。SM100 既有 smoke(`suites_sm100.bzl:41...

建议: 建议二选一:(1)把 direct 路径收窄为 SM120 专用(复用 rtp_llm.models_py.utils.arch.is_sm12x()),SM100 继续走 requant_weight_ue8m0,后续单独提 PR 切换——考虑到上一条 finding 指向该路径可能存在数值问题,收窄门控同时也降低了 SM100 的暴露面;(2)若确认要同时改 SM100,请在本 PR 内给出 moe_deepep_normal_tp2_sm100 等既有 SM100 FP8_PER_BLOCK smoke 的实际复跑结论与数值差异幅度,并把三个 test_online_loader_* 上移到与架构无关的共享基类,使 SM100/SM120 目标共同继承。另建议在 direct 分支断言切分后 kernel_weight.shape[-2] 是 128 的整数倍——_transform_scale_ue8m0index_select(-2, arange(mn)//128) 反查 scale 行,非 128 对齐的切分会取到错位 scale。

self.assertFalse(strategy.can_handle(config))
mock_get_sm.return_value = (10, 0)
self.assertFalse(strategy.can_handle(config))
mock_get_sm.return_value = (12, 0)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] 分发单测未 mock has_deep_gemm_mk_alignment,断言绑定宿主 DeepGEMM 构建,在仍锁 2.2.0 的 cuda12 栈上确定性失败

check_conditionsif get_sm()[0] == 12: 分支执行 checker.check(has_deep_gemm_mk_alignment())(deepgemm_hybrid_executor.py:81-82),该符号为模块级 import(:17)。两处 test_can_handle_cuda_graph(:338-339、:499-500)把 get_sm mock 为 (12,0)assertTrue(can_handle(config)),却未 patch has_deep_gemm_mk_alignment;由于 has_deep_gemm 已被 mock 为 True,其内部 if not has_deep_gemm(): return False 短路失效,会真实 import deep_gemmhasattr 探测三个 mk-alignment 符号。该目标 exec_properties = {'gpu':'H20'}(tests/BUILD:17),而 `d...

建议: 在两处用例上按使用处 patch(...deepgemm_hybrid_executor.has_deep_gemm_mk_alignmentreturn_value=True),让该用例只验证 SM 版本分发决策,真实版本探测交给 RTX_5000_PRO 目标下的 test_sm120。同时补两条用例:「SM=12 且 has_deep_gemm_mk_alignment() 为 False 时 can_handle 必须为 False」(覆盖新增守卫本身),以及 masked 策略在 enable_cuda_graph=True 组合下的期望分发结果,把该组合的门禁语义固化下来。

@LLLLKKKK LLLLKKKK left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Code Review - PR #1362 (non-blocking suggestions)

18 条 P2/P3 建议,不阻塞合并。阻塞判定与完整摘要见上一条 review。

)
)
expert_alignment = self.EXPERT_ALIGNMENT
if self.is_sm120:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] contiguous 路径 mk-alignment 设置后无回读校验,expected_m 语义与 min(128,·) 下钳均缺说明

该路径的正确性前提是 DeepGEMM 的 BLOCK_M 与 workspace 的 per-expert 填充粒度一致,只有一致时 m_indices[tile_start] 才能代表整个 tile,否则 tile 跨专家边界并用错误权重计算且不报错。两处疑点::405 传入 routed_tokens(总路由 token 数)作为 expected_m,而同文件 masked 路径 :203-205 的同名参数是每专家均值(ceil_div(max_token_num, num_experts)),代码中无注释澄清 contiguous 版按总量解释;:403min(self.EXPERT_ALIGNMENT, theoretical) 会在 theoretical > 128 时无理由下钳,而 configure_deep_gemm_mk_alignment(deepgemm_wrapper.py:168-174)已取过 get_mk_alignment_for_contiguous_layout() 保存原值,却没有在 se...

建议:configure_deep_gemm_mk_alignment 内 set 之后回读 deep_gemm.get_mk_alignment_for_contiguous_layout() 并断言等于请求值,不等则 fail-fast(复用已取的 original 变量,零额外成本)。同时在 get_theoretical_mk_alignment_for_contiguous_layout 的 wrapper docstring 中写明 expected_m 在 contiguous 语义下指总 m(与 masked 的每组均值不同),并给 min(128, theoretical) 补一行说明为何需要上限;若无必要则直接用 theoretical 值同时驱动 all_tokensep_scatter(align_m=) 与 GEMM 三者。

]
all_tokens: int = sum(num_recv_tokens_per_expert)
num_recv_tokens_per_expert = payload.expert_tokens_meta.expert_num_tokens
if num_recv_tokens_per_expert is None:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 删除 topk 越界告警后非法 expert id 与容量溢出全部静默丢弃,失去 router 缺陷可观测性

diff 删除了整段 topk_max/topk_min 检查(含 logger.error("[DeepGemm CLAMP] topk_ids out of range …"))与 m_indices.clamp_(0, E-1)。删除动机(.item() 会同步、必须为 capture 让路)成立,但没有任何替代诊断:非法 expert id 由 ep_kernels.py:34/122 屏蔽,容量溢出由 :125 的 dest_is_valid 屏蔽,ep_gather 再由 :437 跳过——三层都是静默跳过,线上只表现为该 token 少一路专家贡献,无日志无指标。原告警文案本身说明这类越界曾真实发生过。此外 num_experts_local(:391,取自 meta 长度)与权重分组数 self.E(:120)之间也不再有任何断言;删除后文件内 logger(:11)已无使用点。

建议: 保留一条零同步的诊断:在 ep_scatter 内对被屏蔽的越界/溢出条目做 atomic_add 到一个小计数器,forward 之外按采样频率或 debug flag 读回并 logger.error,既不引入 per-forward D2H,也不丢失可观测性。同时在 __init__execute_contiguous 开头补一条零成本形状断言(num_experts_local == self.num_experts_per_partition == self.E),把原 clamp 承担的防御性以不需要同步的方式补回。

)
elif isinstance(num_recv_tokens_per_expert_cpu, torch.Tensor):
num_recv_tokens_per_expert_cpu = num_recv_tokens_per_expert_cpu.tolist()
actual_aligned = sum(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] SM120 eager 路径 workspace 容量与布局来自两个数据源,CPU 计数变量靠跨分支条件蕴含维系

SM120 且未开图时:all_tokensexpert_num_tokens_cpu 求和得到(:425-429),而 ep_scatter 的布局由 GPU 张量在 kernel 内按 align_m=expert_alignment 对齐算出(:485-492,derive_counts_from_topk=False)。两者一旦不一致(上游 meta 的 CPU 副本过期或含 padding 计数),dest_token_index < output_capacity 会静默丢弃溢出 token——旧实现中容量与布局同源,不存在该风险。此外 num_recv_tokens_per_expert_cpu 只在 if self.enable_cuda_graph: 的 else 分支绑定(:416-424),却在 40 余行后的 if not self.is_sm120 and not self.enable_cuda_graph:(:468-472)里使用,靠条件蕴含维系,后续任一条件改动都会变成 UnboundLocalError。

建议: 把容量与布局收敛到同一数据源:SM120 eager 也传 derive_counts_from_topk=True 并复用形状推导上界,或显式断言 sum(align_up(cpu_counts, A)) == all_tokens。同时把 num_recv_tokens_per_expert_cpu 的求取提到分支之前(或抽成一个按 (is_sm120, enable_cuda_graph) 分派、返回 (all_tokens, scatter_counts, scatter_alignment) 的小函数),消除跨分支的隐式绑定,也让 pyright strict 可静态检查。

)


class DeepGemmHybridExecutorQwen35ShapeSM120Test(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] CUDA Graph 用例只 replay 同一份输入,最贴近 SM120 decode 的小 batch 形状无 graph 覆盖

_execute 的 graph 分支是「warmup 一次 → 捕获一次 → graph.replay() 一次」(deepep_normal_executor_test.py:253-265),三次调用共用同一个未修改的 payload。本 PR 放开 SM120 graph 的正确性前提正是 ep_scatter(..., derive_counts_from_topk=True)(executor :493)让 m_indices / expert_start_loc / output_index 在回放时按新 topk 重算;用同一份数据回放时,「GPU 重算」与「捕获时烧死常量」结果完全一致,用例无法区分。另 DeepGemmHybridExecutorQwen35ShapeSM120Testpass(:31-34),而其 MAX_GENERATE_BATCH_SIZE = 1 / NUM_EXPERTS = 256 正是 executor :150-151 注释声称的动机场景,对 contiguous+graph 零覆盖。

建议:_execute 的 graph 分支中,捕获完成后重新采样一批路由分布不同的输入(对捕获时的张量做 in-place copy_,保持 shape 不变)再 replay(),并与该批次对应的 eager 结果比对,形成「同图、双输入」断言。同时给 DeepGemmHybridExecutorQwen35ShapeSM120Testenable_cuda_graph=True 用例,覆盖 routed_tokens 远小于 num_experts_local * expert_alignment 时的 workspace 上界分支与小 M 的 alignment 分支。

Comment thread rtp_llm/models_py/kernels/cuda/fp8_kernel/fp8_kernel.py Outdated
from rtp_llm.models_py.kernels.cuda.fp8_kernel import quant_weight_ue8m0

source_weight = kernel.get(self.kernel.name)
if source_weight.dim() != 2:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] 直接量化路径对非 2-D 或非 128 group_size 改为硬失败,缺少降级

新增分支在 source_weight.dim() != 2raise ValueError(:949-954),在 self.group_size != 128 时同样 raise ValueError(:937-941)。此前这两类输入都会落到 per_block_cast_to_fp8_grouped,而后者显式支持 3-D(is_2d = x.dim() == 2unsqueeze(0),:106-108)并支持任意 group_size。由于门控只看 GPU capability、不看权重形状,w8a8_weight_list 中任何以 3-D 形态到达或使用非 128 group_size 的 dense 权重,在 SM100/SM120 上会从「可加载」变为加载期直接抛异常。

建议: 把这两个 raise 改为退回既有 per_block_cast_to_fp8_grouped + requant_weight_ue8m0 路径并打一条 warning,使 direct 优化成为可降级的加速而非新增的启动阻塞;若确实希望硬失败,请在错误信息中给出可操作的规避方式(例如提示改用预量化 ckpt 或指定哪种 quantization 配置)。

Checklist: [6.1] 错误语义:fail-fast/retry/fallback/silent 行为显式

// A zero kernel block size is the public sentinel for using the physical
// KV-cache block size. Keep that compatibility at consumers which require
// an explicit size (for example CUDA Graph initialization).
size_t effectiveKernelSeqSizePerBlock() const {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] effectiveKernelSeqSizePerBlock 与既有 sentinel 逻辑重复,退化边界无断言

kernel_seq_size_per_block == 0 视为等于 seq_size_per_block」这一语义在同一头文件内已由 kernelBlocksPerKvBlock()if (kernel_seq_size_per_block == 0) return 1;(:91-94)与 kernelBlocksPerKvBlockForGroup() 的同款判断(:78-80)表达过,新函数是又一份副本且未被前者复用。NormalExecutor.cc:125-126 的第二层三元并非冗余(它兜住 !cache_managerattn_config.kernel_tokens_per_block 为 0 的分支),但「sentinel 回退后仍可能为 0」这一不变量在测试中无任何断言:KVCacheResourceTest.cc:216-227 只覆盖 (1,0)(8,2),缺 (0,0) 退化组合与 kernel > seq 触发 RTP_LLM_CHECK 的组合。

建议:kernelBlocksPerKvBlock() 复用新函数(seq_size_per_block / effectiveKernelSeqSizePerBlock()),并考虑把 HybridPoolConfigCreator / CacheConfigCreator / NormalEngine 中的同类规范化一并收敛到该访问器;测试补一组 seq_size_per_block = 0; kernel_seq_size_per_block = 0; 断言返回 0(并注明调用方为何仍需再判一次),以及一条不整除组合触发校验失败的断言,同时把测试名调整为体现 effectiveKernelSeqSizePerBlock 语义。

Checklist: [6.1] DRY:重复非平凡逻辑被抽取或显式复用;[6.1] 边界 case 覆盖(空、单元素、最大值)

Comment thread rtp_llm/models_py/kernels/cuda/deepgemm_wrapper.py Outdated
@@ -441,15 +453,25 @@ def test_can_handle_tp_dp_ep(
self.assertTrue(strategy.can_handle(config))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📍 实际位置 rtp_llm/models_py/modules/factory/fused_moe/tests/test_cuda_strategies.py:410(不在 diff 展示范围内,就近挂载)

[P3] test_cuda_strategies.py 内仍残留 4 处指向定义处的无效 get_sm patch

本 PR 已把两个 test_can_handle_cuda_graph 的 mock target 正确迁移到使用处(deepgemm_hybrid_executor.get_sm / deepep_normal_router.get_sm),但同文件仍有 4 处 @patch("rtp_llm.models_py.utils.arch.get_sm")(:410、:433、:503、:529)。这些生产模块都是模块级 from rtp_llm.models_py.utils.arch import get_sm(deepep_normal_router.py:33、deepgemm_hybrid_executor.py:47),定义处 patch 不生效,用例实际依赖宿主真实 SM(H20 恰为 (9,0),与 mock 值巧合一致)才通过;一旦该目标被调度到其它机型,这些断言会以误导性的方式失败。

建议: 顺手把这 4 处也改为使用处 patch(与本 PR 已修正的两处保持一致),或抽一个测试 helper 统一 patch 所有 get_sm 使用点,避免同一文件里两种写法并存造成后续误用。

Checklist: [6.1] 新逻辑有聚焦单测 + 相关集成/smoke 测试;[P.G] mock.patch target 是使用处而非定义处

@parkerpang
parkerpang force-pushed the feat/sm120-deepgemm-fp8 branch from 08adc2f to 6d56a28 Compare September 1, 2026 02:05

@LLLLKKKK LLLLKKKK left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Code Review - PR #1362

Status: BLOCKING

Summary: P0/0 · P1/3 · P2/5 · P3/5

Reviewed: commit 6d56a2872ba5 · 2026-09-01 10:54 UTC+8

Blocking Issues

P1

  • 新增 SM120 dense FP8_PER_BLOCK golden 与 BF16 基线及 per-tensor 基线均分歧,疑似缺陷被 golden 固化 @ rtp_llm/test/smoke/data/model/qwen3/q_r_fp8pb_sm120.json:84
    • 建议:冻结 golden 前,先在 SM120 上以同一 prompt 跑 BF16 或关闭 direct UE8M0 走 legacy requant 取参考输出,逐 token 对比确认是否应为「我理解您希望获得关于」。若不一致请定位 direct 路径(per_block_fp8_quant_weight.py:946-980 的取向与 pack_weight_scale_ue8m0mn),修复后再以 --config=rewrite_smoke 重录,并在 PR 描述中记录该 golden 的参考来源。同时补一条非方阵(如 [256,512])经 direct 与 legacy 两路加载后 GEMM 输出在容差内一致的等价测试——现有 GEMM 数值用例(fp8_deepgemm_linear_sm120_test.py:173-182)为 768×768 方阵,两个 loader 用例只与自身实现比对。
  • EP scatter expert 起点仍通过无同步的全局内存回读获取,本 PR 测试却声明该路径已修复 @ rtp_llm/models_py/triton_kernels/moe/ep_kernels.py:59
    • 建议:直接从本 program 已有的 cumsum 寄存器张量中按 cur_expert 选取起点(lane select 或归约),不经全局内存回读;若必须依赖全局结果,则拆成两个有 kernel-launch 同步边界的内核。同时补 cur_expert_start + offs >= 0 下界保护;修好后让 poison 回归测试真正断言寄存器本地语义,并补 ALIGN_M > 1 + 256 experts + poison 初值的组合用例(现有 poison 用例仅 ALIGN_M=1test_aligned_expert_layout 仅 3 experts)。
  • 升级到 DeepGEMM 2.5.0 后 context_lens 契约在测试与生产 decode 路径上分叉且无可执行覆盖 @ rtp_llm/models_py/modules/base/cuda/indexer_op.py:392
    • 建议:给出 cuda12_9 + deep-gemm 2.5.0 下一维 decode 路径的实测证据(可执行测试或运行日志),或统一改为二维并同步 fmha_params 消费方;同时补一个只依赖 DeepGEMM 与张量构造、不依赖完整 Indexer 初始化的轻量用例,分别锁定一维 legacy 与二维 DSv4 两种契约。

Non-blocking Suggestions

P2

  • Indexer 契约测试被硬编码禁用且调用签名已过期,形成无法直接复活的死测试 @ rtp_llm/models_py/modules/hybrid/test/indexer_test.py:26
    • 建议:删除 :26 强制赋值,让跳过条件由真实 CUDA/DeepGEMM 能力决定;按当前 Indexer.forward 签名修正调用参数,并把该测试注册到满足条件的 GPU target,确认 prefill 与 decode 用例真正执行。
  • deep-gemm 跨 4 个 minor 升级作用于整个 cuda12_9 平台,新增验证只覆盖 SM120 @ deps/requirements_torch_gpu_cuda12_9.txt:15
    • 建议:补跑并在 PR 描述中记录既有 SM90/SM100 的 DeepGEMM smoke、数值与性能基线;同时说明「代码 + 依赖 pin」需成组回退,或将依赖升级与 SM120 功能拆成可独立回滚的两次变更。
  • 新暴露的自定义 JIT 缓存目录未创建即被加锁 @ rtp_llm/test/utils/jit_sys_path_setup.py:242
    • 建议:在进入复制循环前执行 cache_dir.mkdir(parents=True, exist_ok=True),并补一个传入临时不存在目录的用例。
  • 删除 topk 越界诊断后非法 expert id 与容量溢出全部静默丢弃,失去 router 缺陷可观测性 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/deepgemm_hybrid_executor.py:388
    • 建议:保留 kernel 级 mask(正确且 graph-safe)的前提下,在非 graph/debug 路径补一次廉价校验或 GPU 侧计数 + 低频上报;至少在 :388 附近显式注释「越界 topk id 与超容量写入由 kernel mask 静默丢弃」这一不变量,便于后续 router 缺陷定位。
  • SM120 eager 连续布局的工作区容量与 scatter 布局来自两个数据源 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/deepgemm_hybrid_executor.py:434
    • 建议:让 SM120 eager 与 CUDA Graph 路径共用同一数据源(统一走 derive_counts_from_topk=True,或由 GPU counts 派生容量),并在容量掩码命中时通过一次性 warn 或 metric 暴露丢弃事件。

P3

  • m_indices=-1 的 padding 行未初始化即进入 silu_and_mul 与量化 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/deepgemm_hybrid_executor.py:505
    • 建议:将 gateup_output 改为 torch.zeros,或让 silu_and_mul 仅作用于 m_indices >= 0 的行;如刻意保留 empty 以省一次清零,请在 :505 附近注释说明 padding 行垃圾值为何不影响输出。
  • SM120 + CUDA Graph 下 masked 路径与 masked_max_token_num 被静默旁路,缺少独立回滚开关 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/deepgemm_hybrid_executor.py:152
    • 建议:在首次分派时打印一条 info 级日志说明 masked_max_token_num 在 SM120+CUDA Graph 下被旁路;或增加一个可独立关闭 SM120 contiguous 路径的配置项,使其能在不放弃 CUDA Graph 的前提下回退。
  • masked 路径的 dispose_tensor 被无关删除,峰值显存回升 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/deepgemm_hybrid_executor.py:254
    • 建议:恢复该 dispose_tensor 调用,或在 PR 描述/注释中说明为何 masked 路径不再需要提前释放。
  • SM 架构判定重复实现,未复用 arch.py 封装 @ rtp_llm/models_py/kernels/cuda/deepgemm_wrapper.py:115
    • 建议:统一改用 arch.is_blackwell()/is_sm10x()/is_sm12x(),或在 wrapper 内注释说明为何刻意保留无守卫的裸调用。
  • 锁文件未按声明的生成流程整体重建,via 注解自相矛盾 @ deps/requirements_lock_torch_gpu_cuda12_9.txt:733
    • 建议:按文件头约定的 update 目标完整重建 lock,确认结果稳定、依赖闭包与来源注解一致,避免手工编辑与自动生成混杂。

Checklist Findings (13 fail / 54 total)

General Principles Checklist

  • [6.1] Architecture — 兼容性:外部 HTTP/RPC API、持久数据、配置、环境迁移安全 → issue 锁文件未按声明的生成流程整体重建,via 注解自相矛盾
    文件头声明整份 lock 由 //deps:requirements_torch_gpu_cuda12_9.update 统一生成,但本次只有 deep-ep(:732)与 deep-gemm(:735-737)的 # via 改成带 deps/ 前缀并新增 rtp-kernel 关联,flashinfer-python(:800)、rtp-kernel(:3631)等条目仍是无前缀形式;deep-ep 的 URL 与哈希未变却出现注解变化。注解不影响安装,但当前结果无法证明新增的 rtp-kernel → deep-gemm 依赖闭包与整份 lock 来自同一次完整解析。
  • [6.1] Architecture — 可观测性:日志/指标/超时可操作、非噪声 → issue SM120 + CUDA Graph 下 masked 路径与 masked_max_token_num 被静默旁路,缺少独立回滚开关
    execute 的分派条件由 token_num <= self.masked_max_token_num 改为 token_num <= self.masked_max_token_num and not self.enable_cuda_graph:152)。SM120 开启 CUDA Graph 后,无论 masked_max_token_num 配置为何,小 batch decode 一律强制走新的 contiguous 路径,既有配置项在该组合下完全失效且无日志提示。若 contiguous 路径在线上暴露问题,运维只能整体关闭 CUDA Graph(连带损失 decode 性能),没有仅回退 MoE 执行路径的手段。
  • [6.1] Architecture — 回滚路径:风险行为存在运维回滚手段 → issue SM120 + CUDA Graph 下 masked 路径与 masked_max_token_num 被静默旁路,缺少独立回滚开关
    execute 的分派条件由 token_num <= self.masked_max_token_num 改为 token_num <= self.masked_max_token_num and not self.enable_cuda_graph:152)。SM120 开启 CUDA Graph 后,无论 masked_max_token_num 配置为何,小 batch decode 一律强制走新的 contiguous 路径,既有配置项在该组合下完全失效且无日志提示。若 contiguous 路径在线上暴露问题,运维只能整体关闭 CUDA Graph(连带损失 decode 性能),没有仅回退 MoE 执行路径的手段。
  • [6.1] Architecture — 状态不变量:创建/更新/失败/重试/回滚路径有效 → issue masked 路径的 dispose_tensor 被无关删除,峰值显存回升
    execute_masked 中原本紧跟 ep_scatter_v2dispose_tensor(hidden_states_fp8) 被删除,而 masked 路径在 SM120 + CUDA Graph 下根本不会被选中(:152),该删除与本 PR 的 graph 目标无关。同一函数仍保留 :325:346-347 的 dispose 调用,说明该释放模式仍在使用;移除后 masked/eager 路径在分配 upgate_output:254)之前会多持有一份 fp8 激活,峰值显存高于改动前。
  • [6.1] Architecture — 错误语义:fail-fast/retry/fallback/silent 行为显式 → issue SM120 eager 连续布局的工作区容量与 scatter 布局来自两个数据源
    SM120 且未开 CUDA Graph 时,容量 all_tokens 由 CPU 侧 expert_num_tokens_cpu 求和得到(:425-438),而 scatter 布局由 GPU 侧 num_recv_tokens_per_expert 在 kernel 内按 ALIGN_M=expert_alignment 对齐得到(:475-476,490,502-503derive_counts_from_topk=False)。非 SM120 分支(:477-489)用 CPU 预对齐计数 + align_m=1,两者同源;SM120 eager 是唯一两源分支。ep_kernels.py:182-185 的注释明确承认「DeepEP metadata 可能含填充/冗余计数」。一旦两源不一致,本 PR 新增的 dest_is_valid/m_indices_size 掩码会把溢出写静默丢弃,token 不参与计算且无日志或计数器。
  • [6.1] Quality — 逻辑变更未混入无关格式化 → issue 锁文件未按声明的生成流程整体重建,via 注解自相矛盾
    文件头声明整份 lock 由 //deps:requirements_torch_gpu_cuda12_9.update 统一生成,但本次只有 deep-ep(:732)与 deep-gemm(:735-737)的 # via 改成带 deps/ 前缀并新增 rtp-kernel 关联,flashinfer-python(:800)、rtp-kernel(:3631)等条目仍是无前缀形式;deep-ep 的 URL 与哈希未变却出现注解变化。注解不影响安装,但当前结果无法证明新增的 rtp-kernel → deep-gemm 依赖闭包与整份 lock 来自同一次完整解析。
  • [6.1] Software Engineering — DRY:重复非平凡逻辑被抽取或显式复用 → issue SM 架构判定重复实现,未复用 arch.py 封装
    is_deep_gemm_e8m0_used():115)与 maybe_pack_ue8m0_scale 相关判定直接使用 torch.cuda.get_device_capability()[0] in (10, 12),而 arch.py:67-71is_blackwell() 就是同一谓词且带 is_cuda() 守卫、device 归一化与 functools.cache;同 PR 的 deepgemm_hybrid_executor.py:80,110per_block_fp8_quant_weight.py:911,917 统一走 get_sm()/is_sm12x()。两套写法并存增加理解成本,也让「哪些 arch 使用 packed UE8M0」缺少单一事实来源;非 CUDA 环境下裸调用还可能抛异常而非返回 False。
  • [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue deep-gemm 跨 4 个 minor 升级作用于整个 cuda12_9 平台,新增验证只覆盖 SM120
    deep-gemm 由 2.1.1+local 直接升至 2.5.0+d7d5eca.cu129(lock :733-734 版本与 SHA256 已同步)。该锁文件服务于整个 pip_gpu_cuda12_9_torch 仓库,而 cuda12_9 配置覆盖 SM70–SM100,并非仅 SM120;本 PR 新增验证(suites_sm120.bzl*_sm120_test.pyq_r_*_sm120.json)全部面向 RTX_5000_PRO。既有 SM90/SM100 任务会无条件切换 DeepGEMM 二进制(含 masked grouped GEMM、fp8_paged_mqa_logits 等路径),却缺少数值、启动与性能回归证据。代码侧 is_deep_gemm_e8m0_used()deepgemm_wrapper.py:115)同时把 SM100 纳入 packed UE8M0,回滚需同时回退代码与依赖 pin。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue Indexer 契约测试被硬编码禁用且调用签名已过期,形成无法直接复活的死测试
    :24 取到真实 check_cuda_version() 结果后,:26 无条件覆盖为 CUDA_VERSION_OK = False,导致 :33 跳过 Indexer 导入、整个 IndexerTest 在所有环境跳过;本 PR 在该文件唯一的改动(:332 的二维 metadata)因此完全没有执行路径。即使删除 :26:369-374 仍以 params=fmha_params 调用,而 indexer.pyforward 签名已改为 (hidden_states, q_lora, kv_cache, fmha_params, attention_inputs, use_fast_path, cp_params=None),会直接 TypeError。该禁用与签名漂移为存量问题,但本 PR 把新契约写进了这份死测试。
  • [6.1] Tests — 被删除测试有等价替代覆盖 → issue Indexer 契约测试被硬编码禁用且调用签名已过期,形成无法直接复活的死测试
    :24 取到真实 check_cuda_version() 结果后,:26 无条件覆盖为 CUDA_VERSION_OK = False,导致 :33 跳过 Indexer 导入、整个 IndexerTest 在所有环境跳过;本 PR 在该文件唯一的改动(:332 的二维 metadata)因此完全没有执行路径。即使删除 :26:369-374 仍以 params=fmha_params 调用,而 indexer.pyforward 签名已改为 (hidden_states, q_lora, kv_cache, fmha_params, attention_inputs, use_fast_path, cp_params=None),会直接 TypeError。该禁用与签名漂移为存量问题,但本 PR 把新契约写进了这份死测试。
  • [6.1] Tests — 边界 case 覆盖(空、单元素、最大值) → issue 新暴露的自定义 JIT 缓存目录未创建即被加锁
    本 PR 将 setup_jit_cache() 改为 setup_jit_cache(cache_dir=None, packages=None),新增的 cache_dir:242 仅做 expanduser().resolve(),未创建根目录。copy_package_with_lock:85 于该目录下拼出锁文件路径、:96 直接 FileLock(...) 创建该文件,而目标目录直到 :109mkdir(parents=True)。传入尚不存在的缓存目录会抛 FileNotFoundError。当前唯一调用方未传该参数(默认 ~/.cache 通常已存在)掩盖了这条失败路径,但新暴露的参数把它变成了可触发的公开行为。

RTP-LLM Checklist

  • [I] 代码质量 — 同一功能用统一工具函数 → issue SM 架构判定重复实现,未复用 arch.py 封装
    is_deep_gemm_e8m0_used():115)与 maybe_pack_ue8m0_scale 相关判定直接使用 torch.cuda.get_device_capability()[0] in (10, 12),而 arch.py:67-71is_blackwell() 就是同一谓词且带 is_cuda() 守卫、device 归一化与 functools.cache;同 PR 的 deepgemm_hybrid_executor.py:80,110per_block_fp8_quant_weight.py:911,917 统一走 get_sm()/is_sm12x()。两套写法并存增加理解成本,也让「哪些 arch 使用 packed UE8M0」缺少单一事实来源;非 CUDA环境下裸调用还可能抛异常而非返回 False。

Python Static-First Checklist

  • [P.G] 测试规范 — mock/fake/stub 不得替代本次声称覆盖的生产边界 → issue Indexer 契约测试被硬编码禁用且调用签名已过期,形成无法直接复活的死测试
    :24 取到真实 check_cuda_version() 结果后,:26 无条件覆盖为 CUDA_VERSION_OK = False,导致 :33 跳过 Indexer 导入、整个 IndexerTest 在所有环境跳过;本 PR 在该文件唯一的改动(:332 的二维 metadata)因此完全没有执行路径。即使删除 :26:369-374 仍以 params=fmha_params 调用,而 indexer.pyforward 签名已改为 (hidden_states, q_lora, kv_cache, fmha_params, attention_inputs, use_fast_path, cp_params=None),会直接 TypeError。该禁用与签名漂移为存量问题,但本 PR 把新契约写进了这份死测试。

Strengths

  • CacheConfig::effectiveKernelSeqSizePerBlock()(CacheConfig.h:71)将 0-sentinel 收敛为显式回退,与 kernelBlocksPerKvBlockForGroupgroup_kernel==0 → 1 语义一致,NormalExecutor.cc:123 消费端与 KVCacheResourceTest.cc:220,225 双向断言齐备。
  • direct UE8M0 路径在源权重上一次量化即得目标表示,规避 legacy dequant→requant 的二次舍入;_load_raw_tensor.T 再量化并跳过后置转置,最终 [N,K] 与 pack_weight_scale_ue8m0(scale, mn=N) 同 legacy requant_weight_ue8m0_transform_scale_ue8m0(out_s, mn=out_w.shape[-2]) 完全对齐。
  • configure_deep_gemm_mk_alignment 正确识别 alignment 为进程级全局状态,以锁串行化 set/launch/restore,回读校验失败时先还原再抛错,finally 中恢复原值。
  • MoE 连续布局路径系统性去除 .item()/.cpu(),改用 shape 派生工作区上界 + derive_counts_from_topk 在 GPU 重算 counts;容量式 routed_tokens + max_active_experts*(A-1)Σ align_up(count_e, A) 的正确上界。
  • _fwd_kernel_ep_scatter_1/2 新增 m_indices_sizeoutput_capacity 写入上界与非法 expert id 屏蔽,把原本可能的越界写收敛为受控写入。
  • has_deep_gemm_mk_alignment 显式探测三个 API 是否齐备,避免旧版 DeepGEMM 出现半可用状态;pack_weight_scale_ue8m0 在边界处校验 dtype/维度并给出可诊断错误。
  • FP8 量化内核在 bf16+group_size=128 常见形态下复用寄存器向量,省掉一次激活 HBM 重读,大 group 保留重载 fallback,判定条件与循环迭代空间严格一致。

"index": 0,
"message": {
"role": "assistant",
"content": "I'm sorry, but I can't assist with that request. I'm an AI assistant designed to help with tasks like"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] 新增 SM120 dense FP8_PER_BLOCK golden 与 BF16 基线及 per-tensor 基线均分歧,疑似缺陷被 golden 固化

suites_sm120.bzl:50-63 中两个新 suite 仅 --quantization 不同(FP8_PER_BLOCK vs FP8_DYNAMIC_PER_TENSOR),其余 task json、--act_type BF16LOAD_PYTHON_MODEL=1 完全一致。三份 golden 同用模型快照 Qwen3-1.7B/0060bc56、同 prompt($prompt:m2+$prompt:s6prompt_tokens=497)、同贪心配置(top_k=1):BF16 基线 q_r_h20.json:83 输出以「我理解您希望获得关于新高考的三年规划」开头,同批新增 per-tensor golden q_r_fp8pt_sm120.json:84 为「我理解您希望获得关于」,与既有 q_r_h20_per_tensor_w13.json:84 逐字符一致。唯独本行 per-block 为英文拒答 `I'm sorry, but I can't assist with that request....

建议: 冻结 golden 前,先在 SM120 上以同一 prompt 跑 BF16 或关闭 direct UE8M0 走 legacy requant 取参考输出,逐 token 对比确认是否应为「我理解您希望获得关于」。若不一致请定位 direct 路径(per_block_fp8_quant_weight.py:946-980 的取向与 pack_weight_scale_ue8m0mn),修复后再以 --config=rewrite_smoke 重录,并在 PR 描述中记录该 golden 的参考来源。同时补一条非方阵(如 [256,512])经 direct 与 legacy 两路加载后 GEMM 输出在容差内一致的等价测试——现有 GEMM 数值用例(fp8_deepgemm_linear_sm120_test.py:173-182)为 768×768 方阵,两个 loader 用例只与自身实现比对。

Comment thread rtp_llm/models_py/triton_kernels/moe/ep_kernels.py Outdated
Comment thread rtp_llm/models_py/modules/base/cuda/indexer_op.py Outdated

@LLLLKKKK LLLLKKKK left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Code Review - PR #1362 (non-blocking suggestions)

10 条 P2/P3 建议,不阻塞合并。阻塞判定与完整摘要见上一条 review。

@@ -329,7 +329,7 @@ def _run_indexer_forward_test(
attn_inputs, config.attn_config.tokens_per_block

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📍 实际位置 rtp_llm/models_py/modules/hybrid/test/indexer_test.py:26(不在 diff 展示范围内,就近挂载)

[P2] Indexer 契约测试被硬编码禁用且调用签名已过期,形成无法直接复活的死测试

:24 取到真实 check_cuda_version() 结果后,:26 无条件覆盖为 CUDA_VERSION_OK = False,导致 :33 跳过 Indexer 导入、整个 IndexerTest 在所有环境跳过;本 PR 在该文件唯一的改动(:332 的二维 metadata)因此完全没有执行路径。即使删除 :26:369-374 仍以 params=fmha_params 调用,而 indexer.pyforward 签名已改为 (hidden_states, q_lora, kv_cache, fmha_params, attention_inputs, use_fast_path, cp_params=None),会直接 TypeError。该禁用与签名漂移为存量问题,但本 PR 把新契约写进了这份死测试。

建议: 删除 :26 强制赋值,让跳过条件由真实 CUDA/DeepGEMM 能力决定;按当前 Indexer.forward 签名修正调用参数,并把该测试注册到满足条件的 GPU target,确认 prefill 与 decode 用例真正执行。

Checklist: [6.1] 新逻辑有聚焦单测 + 相关集成/smoke 测试;[6.1] 被删除测试有等价替代覆盖;[P.G] mock/fake/stub 不得替代本次声称覆盖的生产边界

https://rtp-opensource.oss-cn-hangzhou.aliyuncs.com/rtp_llm/cu129/torchvision-0.23.0%2Bcu129-cp310-cp310-manylinux_2_28_x86_64.whl
https://rtp-opensource.oss-cn-hangzhou.aliyuncs.com/rtp_llm/cu129/deep_ep-1.2.1.10%2Bd7d7b48-cp310-cp310-linux_x86_64.whl
https://rtp-opensource.oss-cn-hangzhou.aliyuncs.com/rtp_llm/cu129/deep_gemm-2.1.1%2Blocal-cp310-cp310-linux_x86_64.whl
https://rtp-opensource.oss-cn-hangzhou.aliyuncs.com/rtp_llm/cu129/deep_gemm-2.5.0%2Bd7d5eca.cu129-cp310-cp310-linux_x86_64.whl

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] deep-gemm 跨 4 个 minor 升级作用于整个 cuda12_9 平台,新增验证只覆盖 SM120

deep-gemm 由 2.1.1+local 直接升至 2.5.0+d7d5eca.cu129(lock :733-734 版本与 SHA256 已同步)。该锁文件服务于整个 pip_gpu_cuda12_9_torch 仓库,而 cuda12_9 配置覆盖 SM70–SM100,并非仅 SM120;本 PR 新增验证(suites_sm120.bzl*_sm120_test.pyq_r_*_sm120.json)全部面向 RTX_5000_PRO。既有 SM90/SM100 任务会无条件切换 DeepGEMM 二进制(含 masked grouped GEMM、fp8_paged_mqa_logits 等路径),却缺少数值、启动与性能回归证据。代码侧 is_deep_gemm_e8m0_used()deepgemm_wrapper.py:115)同时把 SM100 纳入 packed UE8M0,回滚需同时回退代码与依赖 pin。

建议: 补跑并在 PR 描述中记录既有 SM90/SM100 的 DeepGEMM smoke、数值与性能基线;同时说明「代码 + 依赖 pin」需成组回退,或将依赖升级与 SM120 功能拆成可独立回滚的两次变更。

Checklist: [6.1] 分布式/跨平台变更有对应覆盖

packages = ["flashinfer", "torch", "deep_gemm", "tvm_ffi"]
if cache_dir is None:
cache_dir = Path.home() / ".cache"
cache_dir = Path(cache_dir).expanduser().resolve()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 新暴露的自定义 JIT 缓存目录未创建即被加锁

本 PR 将 setup_jit_cache() 改为 setup_jit_cache(cache_dir=None, packages=None),新增的 cache_dir:242 仅做 expanduser().resolve(),未创建根目录。copy_package_with_lock:85 于该目录下拼出锁文件路径、:96 直接 FileLock(...) 创建该文件,而目标目录直到 :109mkdir(parents=True)。传入尚不存在的缓存目录会抛 FileNotFoundError。当前唯一调用方未传该参数(默认 ~/.cache 通常已存在)掩盖了这条失败路径,但新暴露的参数把它变成了可触发的公开行为。

建议: 在进入复制循环前执行 cache_dir.mkdir(parents=True, exist_ok=True),并补一个传入临时不存在目录的用例。

Checklist: [6.1] 边界 case 覆盖(空、单元素、最大值)


num_experts_local = len(num_recv_tokens_per_expert)
num_recv_tokens_per_expert = payload.expert_tokens_meta.expert_num_tokens
if num_recv_tokens_per_expert is None:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 删除 topk 越界诊断后非法 expert id 与容量溢出全部静默丢弃,失去 router 缺陷可观测性

execute_contiguous 通过 topk_idx.max().item()/min().item() 检测越界并打印 [DeepGemm CLAMP] topk_ids out of range 后 clamp,本 PR 将该段整体删除(现址为 :391-398num_experts_local/routed_tokens 计算)。移除 .item() 对 CUDA Graph capture 与热路径是必要且正确的,但越界 expert id 此后完全依赖 ep_kernels.py:34,122expert_id >= 0 and expert_id < num_experts mask 静默丢弃(output_index 保持 -1),ep_kernels.py:125dest_is_valid < output_capacity 溢出屏蔽同样无任何信号。router 产生非法 id 时表现为静默精度下降而非可定位故障。

建议: 保留 kernel 级 mask(正确且 graph-safe)的前提下,在非 graph/debug 路径补一次廉价校验或 GPU 侧计数 + 低频上报;至少在 :388 附近显式注释「越界 topk id 与超容量写入由 kernel mask 静默丢弃」这一不变量,便于后续 router 缺陷定位。

)
elif isinstance(num_recv_tokens_per_expert_cpu, torch.Tensor):
num_recv_tokens_per_expert_cpu = num_recv_tokens_per_expert_cpu.tolist()
actual_aligned = sum(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] SM120 eager 连续布局的工作区容量与 scatter 布局来自两个数据源

SM120 且未开 CUDA Graph 时,容量 all_tokens 由 CPU 侧 expert_num_tokens_cpu 求和得到(:425-438),而 scatter 布局由 GPU 侧 num_recv_tokens_per_expert 在 kernel 内按 ALIGN_M=expert_alignment 对齐得到(:475-476,490,502-503derive_counts_from_topk=False)。非 SM120 分支(:477-489)用 CPU 预对齐计数 + align_m=1,两者同源;SM120 eager 是唯一两源分支。ep_kernels.py:182-185 的注释明确承认「DeepEP metadata 可能含填充/冗余计数」。一旦两源不一致,本 PR 新增的 dest_is_valid/m_indices_size 掩码会把溢出写静默丢弃,token 不参与计算且无日志或计数器。

建议: 让 SM120 eager 与 CUDA Graph 路径共用同一数据源(统一走 derive_counts_from_topk=True,或由 GPU counts 派生容量),并在容量掩码命中时通过一次性 warn 或 metric 暴露丢弃事件。

Checklist: [6.1] 错误语义:fail-fast/retry/fallback/silent 行为显式

)
m_indices.clamp_(min=0, max=self.num_experts_per_partition - 1)
dispose_tensor(hidden_states_fp8)
gateup_output = torch.empty(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] m_indices=-1 的 padding 行未初始化即进入 silu_and_mul 与量化

gateup_outputtorch.empty((all_tokens, N)) 分配(:505),m_grouped_fp8_gemm_nt_contiguousm_indices 跳过 -1 的 padding 行,这些行保持未初始化;:536silu_and_mul(down_input, gateup_output):544sgl_per_token_group_quant_fp8 对全部 all_tokens 行无条件计算。由于量化逐行且 ep_gather 仅按有效 output_index 取值,垃圾行不污染最终输出;但未初始化位模式可能含 NaN/Inf,会产生异常 amax、浮点异常噪声与不必要的非确定性,CUDA Graph 模式下 padding 占比随 expert_alignment 上升。

建议:gateup_output 改为 torch.zeros,或让 silu_and_mul 仅作用于 m_indices >= 0 的行;如刻意保留 empty 以省一次清零,请在 :505 附近注释说明 padding 行垃圾值为何不影响输出。

# all routing metadata work on GPU, so it is safe to capture/replay.
# It avoids the E * padded_M masked layout that dominates small decode
# batches on SM120.
if token_num <= self.masked_max_token_num and not self.enable_cuda_graph:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] SM120 + CUDA Graph 下 masked 路径与 masked_max_token_num 被静默旁路,缺少独立回滚开关

execute 的分派条件由 token_num <= self.masked_max_token_num 改为 token_num <= self.masked_max_token_num and not self.enable_cuda_graph:152)。SM120 开启 CUDA Graph 后,无论 masked_max_token_num 配置为何,小 batch decode 一律强制走新的 contiguous 路径,既有配置项在该组合下完全失效且无日志提示。若 contiguous 路径在线上暴露问题,运维只能整体关闭 CUDA Graph(连带损失 decode 性能),没有仅回退 MoE 执行路径的手段。

建议: 在首次分派时打印一条 info 级日志说明 masked_max_token_num 在 SM120+CUDA Graph 下被旁路;或增加一个可独立关闭 SM120 contiguous 路径的配置项,使其能在不放弃 CUDA Graph 的前提下回退。

Checklist: [6.1] 可观测性:日志/指标/超时可操作、非噪声;[6.1] 回滚路径:风险行为存在运维回滚手段

)
dispose_tensor(hidden_states_fp8)

upgate_output = torch.empty(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] masked 路径的 dispose_tensor 被无关删除,峰值显存回升

execute_masked 中原本紧跟 ep_scatter_v2dispose_tensor(hidden_states_fp8) 被删除,而 masked 路径在 SM120 + CUDA Graph 下根本不会被选中(:152),该删除与本 PR 的 graph 目标无关。同一函数仍保留 :325:346-347 的 dispose 调用,说明该释放模式仍在使用;移除后 masked/eager 路径在分配 upgate_output:254)之前会多持有一份 fp8 激活,峰值显存高于改动前。

建议: 恢复该 dispose_tensor 调用,或在 PR 描述/注释中说明为何 masked 路径不再需要提前释放。

Checklist: [6.1] 状态不变量:创建/更新/失败/重试/回滚路径有效

return torch.cuda.get_device_capability()[0] in [10, 12]
# Blackwell SM100 and SM120 DeepGEMM kernels consume packed UE8M0 scales.
# SM120 support is pinned in deps to DeepGEMM 2.5.0+d7d5eca.cu129.
return torch.cuda.get_device_capability()[0] in (10, 12)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] SM 架构判定重复实现,未复用 arch.py 封装

is_deep_gemm_e8m0_used():115)与 maybe_pack_ue8m0_scale 相关判定直接使用 torch.cuda.get_device_capability()[0] in (10, 12),而 arch.py:67-71is_blackwell() 就是同一谓词且带 is_cuda() 守卫、device 归一化与 functools.cache;同 PR 的 deepgemm_hybrid_executor.py:80,110per_block_fp8_quant_weight.py:911,917 统一走 get_sm()/is_sm12x()。两套写法并存增加理解成本,也让「哪些 arch 使用 packed UE8M0」缺少单一事实来源;非 CUDA 环境下裸调用还可能抛异常而非返回 False。

建议: 统一改用 arch.is_blackwell()/is_sm10x()/is_sm12x(),或在 wrapper 内注释说明为何刻意保留无守卫的裸调用。

Checklist: [6.1] DRY:重复非平凡逻辑被抽取或显式复用;[I] 同一功能用统一工具函数

--hash=sha256:430ea2635d204d7e2d182860c496d787d83e46ac81ee10043eaa86eb8eadf88d
# via -r requirements_torch_gpu_cuda12_9.txt
# via -r deps/requirements_torch_gpu_cuda12_9.txt
deep-gemm @ https://rtp-opensource.oss-cn-hangzhou.aliyuncs.com/rtp_llm/cu129/deep_gemm-2.5.0%2Bd7d5eca.cu129-cp310-cp310-linux_x86_64.whl \

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] 锁文件未按声明的生成流程整体重建,via 注解自相矛盾

文件头声明整份 lock 由 //deps:requirements_torch_gpu_cuda12_9.update 统一生成,但本次只有 deep-ep(:732)与 deep-gemm(:735-737)的 # via 改成带 deps/ 前缀并新增 rtp-kernel 关联,flashinfer-python(:800)、rtp-kernel(:3631)等条目仍是无前缀形式;deep-ep 的 URL 与哈希未变却出现注解变化。注解不影响安装,但当前结果无法证明新增的 rtp-kernel → deep-gemm 依赖闭包与整份 lock 来自同一次完整解析。

建议: 按文件头约定的 update 目标完整重建 lock,确认结果稳定、依赖闭包与来源注解一致,避免手工编辑与自动生成混杂。

Checklist: [6.1] 兼容性:外部 HTTP/RPC API、持久数据、配置、环境迁移安全;[6.1] 逻辑变更未混入无关格式化

@parkerpang
parkerpang force-pushed the feat/sm120-deepgemm-fp8 branch from 6d56a28 to 884232d Compare September 1, 2026 03:24

@LLLLKKKK LLLLKKKK left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Code Review - PR #1362

Status: BLOCKING

Summary: P0/0 · P1/2 · P2/3 · P3/0

Reviewed: commit 884232d1d5e1 · 2026-09-01 12:07 UTC+8

Blocking Issues

P1

  • Indexer 解码调用了错误模块中的上下文长度适配函数 @ rtp_llm/models_py/modules/base/cuda/indexer_op.py:390
    • 建议:从 deepgemm_wrapper 显式导入并调用该函数,并增加实际执行 _get_topk_paged() 的生产链路测试。
  • DeepEP normal 路由与 CUDA Graph 契约冲突 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/deepgemm_hybrid_executor.py:83
    • 建议:在 router 或策略门控中拒绝 DeepEP normal 与 CUDA Graph 的组合,或改用固定容量、图兼容的路由,并增加完整 MoE 层在不同路由分布下的捕获重放测试。

Non-blocking Suggestions

P2

  • 新增 SM120 dense FP8_PER_BLOCK golden 与 BF16 基线及 per-tensor 基线均分歧,疑似缺陷被 golden 固化 @ rtp_llm/test/smoke/data/model/qwen3/q_r_fp8pb_sm120.json:84
    • 建议:对同一输入比较 BF16、旧 FP8 和 SM120 直接 UE8M0 路径的首步 logits,修复根因后重新生成 golden。
  • Indexer 端到端测试被无条件跳过 @ rtp_llm/models_py/modules/hybrid/test/indexer_test.py:29
    • 建议:移除硬编码禁用并恢复 GPU 集成测试,至少覆盖受支持 DeepGEMM 版本的 prefill、decode 和完整 logits 调用链。
  • 删除 topk 越界诊断后非法 expert id 与容量溢出全部静默丢弃,失去 router 缺陷可观测性 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/deepgemm_hybrid_executor.py:388
    • 建议:保留设备侧安全 mask,同时暴露非法 ID 和容量溢出计数;至少在 eager 或校验模式下 fail-fast。

Checklist Findings (8 fail / 130 total)

General Principles Checklist

  • [6.1] Architecture — 依赖方向:无循环依赖/跨层惊喜 → issue Indexer 解码调用了错误模块中的上下文长度适配函数
    deep_gemm 是第三方包,而 normalize_paged_mqa_context_lens 仅定义在 RTP 的 deepgemm_wrapper.py。所有进入 _get_topk_paged() 的 decode 请求都会在 metadata kernel 前调用不存在的属性并触发 AttributeError;新增测试只直接调用 wrapper。
  • [6.1] Architecture — 可观测性:日志/指标/超时可操作、非噪声 → issue 删除 topk 越界诊断后非法 expert id 与容量溢出全部静默丢弃,失去 router 缺陷可观测性
    原有 top-k 范围诊断已删除。当前 scatter 忽略范围外 expert ID,并用容量 mask 丢弃溢出写入;output_index 保持 -1,gather 随后静默缺失对应贡献。上游路由或容量契约异常时只表现为结果偏差。
  • [6.1] Architecture — 状态不变量:创建/更新/失败/重试/回滚路径有效 → issue DeepEP normal 路由与 CUDA Graph 契约冲突
    此处允许 SM120 CUDA Graph,策略又将 executor 与 DeepepNormalRouterFp8PerBlock 配对。完整 MoE forward 会捕获 router 的 dispatch/combine,而框架明确标记 DeepEP high-throughput all-to-all 不兼容 CUDA Graph,dispatch 输出形状也随路由变化。新增测试仅捕获固定 payload 的 executor,绕过 router。
  • [6.1] Architecture — 错误语义:fail-fast/retry/fallback/silent 行为显式 → issue 删除 topk 越界诊断后非法 expert id 与容量溢出全部静默丢弃,失去 router 缺陷可观测性
    原有 top-k 范围诊断已删除。当前 scatter 忽略范围外 expert ID,并用容量 mask 丢弃溢出写入;output_index 保持 -1,gather 随后静默缺失对应贡献。上游路由或容量契约异常时只表现为结果偏差。
  • [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue DeepEP normal 路由与 CUDA Graph 契约冲突
    此处允许 SM120 CUDA Graph,策略又将 executor 与 DeepepNormalRouterFp8PerBlock 配对。完整 MoE forward 会捕获 router 的 dispatch/combine,而框架明确标记 DeepEP high-throughput all-to-all 不兼容 CUDA Graph,dispatch 输出形状也随路由变化。新增测试仅捕获固定 payload 的 executor,绕过 router。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue Indexer 端到端测试被无条件跳过
    CUDA 版本检测后又将 CUDA_VERSION_OK 无条件设为 False,使整个 IndexerTest 永久跳过。新增 helper 测试只验证独立形状转换,无法覆盖生产 decode 调用、prefill scale 或 metadata/logits 完整链路。

RTP-LLM Checklist

  • [I] 代码质量 — 同一功能用统一工具函数 → issue Indexer 解码调用了错误模块中的上下文长度适配函数
    deep_gemm 是第三方包,而 normalize_paged_mqa_context_lens 仅定义在 RTP 的 deepgemm_wrapper.py。所有进入 _get_topk_paged() 的 decode 请求都会在 metadata kernel 前调用不存在的属性并触发 AttributeError;新增测试只直接调用 wrapper。

Python Static-First Checklist

  • [P.G] 测试规范 — mock/fake/stub 不得替代本次声称覆盖的生产边界 → issue Indexer 端到端测试被无条件跳过
    CUDA 版本检测后又将 CUDA_VERSION_OK 无条件设为 False,使整个 IndexerTest 永久跳过。新增 helper 测试只验证独立形状转换,无法覆盖生产 decode 调用、prefill scale 或 metadata/logits 完整链路。

Strengths

  • EP scatter 消除了跨 warp 无同步回读,并增加容量边界测试。
  • SM120 权重量化在切分后打包 scale,覆盖非方阵及 TP 布局。
  • KV-cache 零值哨兵回退行为明确且有单测。
  • DeepGEMM 依赖声明、锁定版本和哈希保持同步。

Comment thread rtp_llm/models_py/modules/base/cuda/indexer_op.py Outdated
checker.check(not config.enable_cuda_graph)
if get_sm()[0] == 12:
checker.check(has_deep_gemm_mk_alignment())
checker.check(not config.enable_cuda_graph or get_sm()[0] == 12)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] DeepEP normal 路由与 CUDA Graph 契约冲突

此处允许 SM120 CUDA Graph,策略又将 executor 与 DeepepNormalRouterFp8PerBlock 配对。完整 MoE forward 会捕获 router 的 dispatch/combine,而框架明确标记 DeepEP high-throughput all-to-all 不兼容 CUDA Graph,dispatch 输出形状也随路由变化。新增测试仅捕获固定 payload 的 executor,绕过 router。

建议: 在 router 或策略门控中拒绝 DeepEP normal 与 CUDA Graph 的组合,或改用固定容量、图兼容的路由,并增加完整 MoE 层在不同路由分布下的捕获重放测试。

Checklist: [6.1] 状态不变量:创建/更新/失败/重试/回滚路径有效;[6.1] 分布式/跨平台变更有对应覆盖

@LLLLKKKK LLLLKKKK left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Code Review - PR #1362 (non-blocking suggestions)

3 条 P2/P3 建议,不阻塞合并。阻塞判定与完整摘要见上一条 review。

"index": 0,
"message": {
"role": "assistant",
"content": "I'm sorry, but I can't assist with that request. I'm an AI assistant designed to help with tasks like"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 新增 SM120 dense FP8_PER_BLOCK golden 与 BF16 基线及 per-tensor 基线均分歧,疑似缺陷被 golden 固化

相同模型、prompt 和 top_k=1 下,FP8_PER_BLOCK 从首 token 起生成英文拒答,而 SM120 per-tensor、BF16 及既有 FP8 基线均以中文回答。生成长度差异不能解释首 token 分歧,当前 golden 只会将明显的数值或布局异常视为正确结果。

建议: 对同一输入比较 BF16、旧 FP8 和 SM120 直接 UE8M0 路径的首步 logits,修复根因后重新生成 golden。



def check_cuda_version() -> bool:
"""Check if CUDA version is >= 12.9"""

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📍 实际位置 rtp_llm/models_py/modules/hybrid/test/indexer_test.py:29(不在 diff 展示范围内,就近挂载)

[P2] Indexer 端到端测试被无条件跳过

CUDA 版本检测后又将 CUDA_VERSION_OK 无条件设为 False,使整个 IndexerTest 永久跳过。新增 helper 测试只验证独立形状转换,无法覆盖生产 decode 调用、prefill scale 或 metadata/logits 完整链路。

建议: 移除硬编码禁用并恢复 GPU 集成测试,至少覆盖受支持 DeepGEMM 版本的 prefill、decode 和完整 logits 调用链。

Checklist: [6.1] 新逻辑有聚焦单测 + 相关集成/smoke 测试;[P.G] mock/fake/stub 不得替代本次声称覆盖的生产边界

all_tokens: int = sum(num_recv_tokens_per_expert)

num_experts_local = len(num_recv_tokens_per_expert)
num_recv_tokens_per_expert = payload.expert_tokens_meta.expert_num_tokens

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 删除 topk 越界诊断后非法 expert id 与容量溢出全部静默丢弃,失去 router 缺陷可观测性

原有 top-k 范围诊断已删除。当前 scatter 忽略范围外 expert ID,并用容量 mask 丢弃溢出写入;output_index 保持 -1,gather 随后静默缺失对应贡献。上游路由或容量契约异常时只表现为结果偏差。

建议: 保留设备侧安全 mask,同时暴露非法 ID 和容量溢出计数;至少在 eager 或校验模式下 fail-fast。

Checklist: [6.1] 可观测性:日志/指标/超时可操作、非噪声;[6.1] 错误语义:fail-fast/retry/fallback/silent 行为显式

parkerpang and others added 8 commits September 1, 2026 18:40
…_8bit

The kernel read the input twice (absmax pass + quantize pass). For the
common case (bf16 + group_size=128 -> 1 vec/lane) cache the loaded
vector in registers and reuse it in the quantize pass, eliminating the
second HBM read; larger groups fall back to reload. Also emit the 8 fp8
outputs as one packed vec_t<__nv_fp8_e4m3,8> store (if constexpr; int8
path unchanged). Bit-identical output (math unchanged).
quant kernel 304.8->130.9 us/step (-57%); PER_BLOCK bs64 3.91->3.43ms.
Benefits every per_token_group_quant_8bit user (PER_BLOCK / DeepGEMM).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@parkerpang
parkerpang force-pushed the feat/sm120-deepgemm-fp8 branch from 884232d to 5eca90b Compare September 1, 2026 10:41

@LLLLKKKK LLLLKKKK left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Code Review - PR #1362

Status: BLOCKING

Summary: P0/0 · P1/3 · P2/7 · P3/0

Reviewed: commit 5eca90bd8c0b · 2026-09-01 19:23 UTC+8

Blocking Issues

P1

  • 旧版 DeepGEMM 的 ragged-MQA scale 被无条件切换为一维 @ rtp_llm/models_py/modules/base/cuda/indexer_op.py:477
    • 建议:增加共享的 ragged scale 适配函数:旧版保留 [N,1],2.5 使用 [N],并覆盖普通与 CP prefill。
  • eager 连续路径不再提前释放输入激活,增加峰值显存 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/deepgemm_hybrid_executor.py:508
    • 建议:在非 Graph 路径恢复提前释放,仅在 CUDA Graph 路径保留固定输入存储,并补充峰值显存回归测试。
  • 默认设置 DeepGEMM 缓存目录会绕过远端 JIT 缓存管理 @ rtp_llm/test/utils/jit_sys_path_setup.py:255
    • 建议:仅规范化调用方显式设置的 DG_JIT_CACHE_DIR;未设置时由 JIT cache manager 分配目录,并增加两个环境变量组合测试。

Non-blocking Suggestions

P2

  • 对齐配置初始化异常时可能遗留进程全局状态 @ rtp_llm/models_py/kernels/cuda/deepgemm_wrapper.py:194
    • 建议:读取原值后立即进入 try/finally,将设置、校验和 yield 全部纳入恢复范围,并避免恢复异常覆盖原始异常。
  • 删除 topk 越界诊断后非法 expert id 与容量溢出全部静默丢弃,失去 router 缺陷可观测性 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/deepgemm_hybrid_executor.py:494
    • 建议:保留 Graph-safe 掩码,同时累计非法 ID 和容量溢出计数,并在 eager、调试或低频诊断路径告警或失败。
  • Indexer 端到端测试被无条件跳过 @ rtp_llm/models_py/modules/hybrid/test/indexer_test.py:36
    • 建议:移除硬编码禁用,将 DeepGEMM 2.2/2.5 契约测试接入对应 CUDA CI,并恢复 prefill/decode 端到端用例。
  • CUDA Graph 测试未验证 replay 时路由数据变化 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/test/deepep_normal_executor_test.py:256
    • 建议:capture 后原地修改输入、top-k ID 和权重,再与新输入的 eager 结果比较,并覆盖集中路由、全 -1 和小 M 场景。
  • FP8 向量缓存与打包写入缺少有效数值回归测试 @ rtp_llm/models_py/bindings/cuda/kernels/per_token_group_quant_8bit.cu:77
    • 建议:增加参数化数值断言,与 Torch/Triton reference 比较量化值和 scale,覆盖缓存边界、回退和布局组合。
  • TP 切分测试绕过真实切分链路 @ rtp_llm/models_py/modules/factory/linear/impl/cuda/test/fp8_deepgemm_linear_sm120_test.py:133
    • 建议:使用真实 AtomicWeightLoadConfig 和 TP=2 加载链路,校验各 rank 的形状、dtype、stride 与 GEMM 输出。
  • 新增 SM120 dense FP8_PER_BLOCK golden 与 BF16 基线及 per-tensor 基线均分歧,疑似缺陷被 golden 固化 @ rtp_llm/test/smoke/data/model/qwen3/q_r_fp8pb_sm120.json:84
    • 建议:在目标 GPU 比较 BF16、per-tensor 和 per-block 的首步 logits;确认数值路径后再录制 golden,或提供量化退化符合预期的误差证据。

Checklist Findings (8 fail / 141 total)

General Principles Checklist

  • [6.1] Architecture — 兼容性:外部 HTTP/RPC API、持久数据、配置、环境迁移安全 → issue 默认设置 DeepGEMM 缓存目录会绕过远端 JIT 缓存管理
    GPU 测试包装器在启动子进程前无条件设置 DG_JIT_CACHE_DIRresolve_scope() 会跳过已有该变量的组件。因此配置 REMOTE_JIT_DIR 时 DeepGEMM 不再参与远端恢复和发布,远端执行器需要重复 JIT 编译。
  • [6.1] Architecture — 可观测性:日志/指标/超时可操作、非噪声 → issue 删除 topk 越界诊断后非法 expert id 与容量溢出全部静默丢弃,失去 router 缺陷可观测性
    ep_scatter 对越界 expert ID 或超容量写入仅保留 output_index=-1,gather 随后静默忽略该贡献。路由或元数据异常会表现为无告警的数值偏差。
  • [6.1] Architecture — 状态不变量:创建/更新/失败/重试/回滚路径有效 → issue 对齐配置初始化异常时可能遗留进程全局状态
    全局 alignment 在进入 try/finally 前被设置;若随后读取配置抛异常,恢复逻辑不会执行,后续 DeepGEMM 调用可能继承临时对齐值。
  • [6.1] Architecture — 错误语义:fail-fast/retry/fallback/silent 行为显式 → issue 删除 topk 越界诊断后非法 expert id 与容量溢出全部静默丢弃,失去 router 缺陷可观测性
    ep_scatter 对越界 expert ID 或超容量写入仅保留 output_index=-1,gather 随后静默忽略该贡献。路由或元数据异常会表现为无告警的数值偏差。
  • [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue 新增 SM120 dense FP8_PER_BLOCK golden 与 BF16 基线及 per-tensor 基线均分歧,疑似缺陷被 golden 固化
    相同模型、prompt 和 top_k=1 下,该 golden 首 token 即转为英文拒答,而 H20 per-block、H20 per-tensor 和 SM120 per-tensor 均从中文“我理解”开始。确定性首 token 完全分叉可能掩盖数值异常。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue 新增 SM120 dense FP8_PER_BLOCK golden 与 BF16 基线及 per-tensor 基线均分歧,疑似缺陷被 golden 固化
    相同模型、prompt 和 top_k=1 下,该 golden 首 token 即转为英文拒答,而 H20 per-block、H20 per-tensor 和 SM120 per-tensor 均从中文“我理解”开始。确定性首 token 完全分叉可能掩盖数值异常。
  • [6.1] Tests — 边界 case 覆盖(空、单元素、最大值) → issue FP8 向量缓存与打包写入缺少有效数值回归测试
    直接覆盖该 C++ 内核的测试仅使用 FP16、group size 8、行主序并打印结果,没有数值断言;未覆盖 BF16 group 128 缓存、大 group 回退、packed store、int8 和 UE8M0 分支。

Python Static-First Checklist

  • [P.G] 测试规范 — mock/fake/stub 不得替代本次声称覆盖的生产边界 → issue TP 切分测试绕过真实切分链路
    测试手工切片 weight/scale,并 mock CompositeWeight._postprocess,没有执行 WeightModule.load()_splitAtomicWeight TP strategy;真实切分轴或 scale 分块错误时该单测仍可能通过。

Strengths

  • CUDA Graph 路径使用静态容量和 GPU 侧路由计数,并排除不安全的 DeepEP normal 路由。
  • DeepGEMM 依赖、能力探测及 SM120 BUILD/smoke 配置保持同步。
  • KV-cache 零值回退、FP8 权重切分后打包和 EP scatter 边界保护均有针对性实现。

# Compute logits
weights = weights.squeeze(-1)
kv_fp8 = (k_fp8, k_scale.view(torch.float32))
kv_fp8 = (k_fp8, k_scale.view(torch.float32).view(-1))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] 旧版 DeepGEMM 的 ragged-MQA scale 被无条件切换为一维

CUDA 12.6 仍锁定 DeepGEMM 2.2;paged 路径按能力适配版本,但普通和 CP ragged 分支均将原 [N,1] scale 无条件展平为 [N]。旧版 prefill 可能触发 shape 校验失败或错误解释 scale。

建议: 增加共享的 ragged scale 适配函数:旧版保留 [N,1],2.5 使用 [N],并覆盖普通与 CP prefill。

)
m_indices.clamp_(min=0, max=self.num_experts_per_partition - 1)
dispose_tensor(hidden_states_fp8)
gateup_output = torch.empty(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] eager 连续路径不再提前释放输入激活,增加峰值显存

本次删除了 ep_scatter 后的 dispose_tensor(hidden_states_fp8)。非 CUDA Graph 连续路径因此让整块 FP8 输入与 scatter、gate/up 和 down 工作区同时存活,较大 token batch 会提高峰值显存并可能导致 OOM。

建议: 在非 Graph 路径恢复提前释放,仅在 CUDA Graph 路径保留固定输入存储,并补充峰值显存回归测试。

.resolve()
)
deep_gemm_cache_dir.mkdir(parents=True, exist_ok=True)
os.environ["DG_JIT_CACHE_DIR"] = str(deep_gemm_cache_dir)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] 默认设置 DeepGEMM 缓存目录会绕过远端 JIT 缓存管理

GPU 测试包装器在启动子进程前无条件设置 DG_JIT_CACHE_DIRresolve_scope() 会跳过已有该变量的组件。因此配置 REMOTE_JIT_DIR 时 DeepGEMM 不再参与远端恢复和发布,远端执行器需要重复 JIT 编译。

建议: 仅规范化调用方显式设置的 DG_JIT_CACHE_DIR;未设置时由 JIT cache manager 分配目录,并增加两个环境变量组合测试。

Checklist: [6.1] 兼容性:外部 HTTP/RPC API、持久数据、配置、环境迁移安全

@LLLLKKKK LLLLKKKK left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Code Review - PR #1362 (non-blocking suggestions)

7 条 P2/P3 建议,不阻塞合并。阻塞判定与完整摘要见上一条 review。

# concurrent forwards cannot launch with each other's scheduler setting.
with _mk_alignment_lock:
original = deep_gemm.get_mk_alignment_for_contiguous_layout()
deep_gemm.set_mk_alignment_for_contiguous_layout(alignment)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 对齐配置初始化异常时可能遗留进程全局状态

全局 alignment 在进入 try/finally 前被设置;若随后读取配置抛异常,恢复逻辑不会执行,后续 DeepGEMM 调用可能继承临时对齐值。

建议: 读取原值后立即进入 try/finally,将设置、校验和 yield 全部纳入恢复范围,并避免恢复异常覆盖原始异常。

Checklist: [6.1] 状态不变量:创建/更新/失败/重试/回滚路径有效

).cuda(non_blocking=True)
scatter_alignment = 1
expert_start_loc = torch.empty_like(scatter_num_tokens_per_expert)
ep_scatter(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 删除 topk 越界诊断后非法 expert id 与容量溢出全部静默丢弃,失去 router 缺陷可观测性

ep_scatter 对越界 expert ID 或超容量写入仅保留 output_index=-1,gather 随后静默忽略该贡献。路由或元数据异常会表现为无告警的数值偏差。

建议: 保留 Graph-safe 掩码,同时累计非法 ID 和容量溢出计数,并在 eager、调试或低频诊断路径告警或失败。

Checklist: [6.1] 可观测性:日志/指标/超时可操作、非噪声;[6.1] 错误语义:fail-fast/retry/fallback/silent 行为显式



def check_cuda_version() -> bool:
"""Check if CUDA version is >= 12.9"""

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📍 实际位置 rtp_llm/models_py/modules/hybrid/test/indexer_test.py:36(不在 diff 展示范围内,就近挂载)

[P2] Indexer 端到端测试被无条件跳过

CUDA 版本探测结果随后被强制设为 False,完整 IndexerTest 永久跳过;其 BUILD 目标还带有 open_skip。新增 helper 测试无法覆盖 paged decode、ragged prefill 和 CP prefill 的生产调用链。

建议: 移除硬编码禁用,将 DeepGEMM 2.2/2.5 契约测试接入对应 CUDA CI,并恢复 prefill/decode 端到端用例。

if not enable_cuda_graph:
return executor.execute(payload, "silu", None, None, False, None)

# Warm every JIT path before capture, then validate the exact executor

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] CUDA Graph 测试未验证 replay 时路由数据变化

warmup、capture 和 replay 使用同一输入与路由分布,无法发现 expert counts、索引或 padding 被错误固化;Qwen3.5 小 M 测试类也没有执行 SM120 Graph 用例。

建议: capture 后原地修改输入、top-k ID 和权重,再与新输入的 eager 结果比较,并覆盖集中路由、全 -1 和小 M 场景。

// single loaded vector can be cached in registers and reused in the quantize
// pass, eliminating a second HBM read of the activations. For larger groups
// we fall back to re-loading.
const bool can_cache = (num_vec_elems <= 16);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] FP8 向量缓存与打包写入缺少有效数值回归测试

直接覆盖该 C++ 内核的测试仅使用 FP16、group size 8、行主序并打印结果,没有数值断言;未覆盖 BF16 group 128 缓存、大 group 回退、packed store、int8 和 UE8M0 分支。

建议: 增加参数化数值断言,与 Torch/Triton reference 比较量化值和 scale,覆盖缓存边界、回退和布局组合。

Checklist: [6.1] 边界 case 覆盖(空、单元素、最大值)

maybe_rewrite_weight_by_key=lambda _, tensor: tensor
)

with patch(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] TP 切分测试绕过真实切分链路

测试手工切片 weight/scale,并 mock CompositeWeight._postprocess,没有执行 WeightModule.load()_splitAtomicWeight TP strategy;真实切分轴或 scale 分块错误时该单测仍可能通过。

建议: 使用真实 AtomicWeightLoadConfig 和 TP=2 加载链路,校验各 rank 的形状、dtype、stride 与 GEMM 输出。

Checklist: [P.G] mock/fake/stub 不得替代本次声称覆盖的生产边界

"index": 0,
"message": {
"role": "assistant",
"content": "I'm sorry, but I can't assist with that request. I'm an AI assistant designed to help with tasks like"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 新增 SM120 dense FP8_PER_BLOCK golden 与 BF16 基线及 per-tensor 基线均分歧,疑似缺陷被 golden 固化

相同模型、prompt 和 top_k=1 下,该 golden 首 token 即转为英文拒答,而 H20 per-block、H20 per-tensor 和 SM120 per-tensor 均从中文“我理解”开始。确定性首 token 完全分叉可能掩盖数值异常。

建议: 在目标 GPU 比较 BF16、per-tensor 和 per-block 的首步 logits;确认数值路径后再录制 golden,或提供量化退化符合预期的误差证据。

Checklist: [6.1] 分布式/跨平台变更有对应覆盖;[6.1] 新逻辑有聚焦单测 + 相关集成/smoke 测试

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants