Skip to content

feat(mori-ep): Add MoRI Expert Parallelism support for ROCm - #977

Merged
LLLLKKKK merged 6 commits into
mainfrom
develop/mori_355x_support_0507_after_rebase
May 14, 2026
Merged

feat(mori-ep): Add MoRI Expert Parallelism support for ROCm#977
LLLLKKKK merged 6 commits into
mainfrom
develop/mori_355x_support_0507_after_rebase

Conversation

@jacobwin-ai

@jacobwin-ai jacobwin-ai commented May 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

This PR introduces MoRI (Model-parallel Routing with Intranode) Expert Parallelism support for ROCm/MI300 platforms, enabling
efficient MoE model inference with EP (Expert Parallelism) on AMD GPUs.

Commit 1: feat: add MoriEP expert parallelism support for ROCm

Core MoRI EP infrastructure and integration:

  • MoriEPWrapper (moriep_wrapper.py) — Singleton wrapper around mori EpDispatchCombineOp (JIT-compiled HIP kernels for intra-node
    dispatch/combine), with unit tests.
  • MoriEpIntranodeRouter — EP router implementing prepare() (dispatch) and finalize() (combine) via MoRI buffer wrapper:
    • Supports single-shot and chunked dispatch for large token counts.
    • Global-to-local expert ID remapping for EP partitioning.
  • MoriEpFp4Strategy — FP4 quantization strategy pairing MoriEpIntranodeRouter with RocmExpertsFp4PerGroup.
  • Config flow — USE_MORI_EP env var / CLI flag → deep_ep_config → MoeConfig.use_mori_ep → C++ pybind binding.
  • ROCm MoE executors — Enhanced rocm_moe.py with EP-aware effective_expert_mask, FP4 kernel support, and skip_allreduce parameter
    for unified allreduce optimization.
  • GenericMoeLayer — Restores use_unified_allreduce: when moe_style == 2 (shared expert) and TP > 1, skip individual allreduce in
    fused_moe and DenseMLP, then do a single allreduce after combining routed experts, shared expert, and gate scaling. Fixes garbled
    output on Qwen3.5-397B-A17B with EP=8 TP=8.

Commit 2: feat: add fake balance expert and MoriEP tests

Testing and load-balancing utilities:

  • FakeBalanceExpert — ROCm C++ op + Python module for post-topk expert rebalancing (enabled via FAKE_BALANCE_EXPERT=1).
  • MoriEpIntranodeRouter unit tests — Comprehensive test coverage for dispatch/combine and chunked dispatch paths.
  • BlockPoolConfigHelper — Adjusted MoE cache allocation for EP workloads.

@jacobwin-ai
jacobwin-ai requested a review from LLLLKKKK as a code owner May 8, 2026 06:25
@LLLLKKKK

LLLLKKKK commented May 8, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #977

Status: BLOCKING

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

Blocking Issues

P1

  • ROCm MoE 策略探测会中断后续策略选择 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/strategy/ep.py:95
    • 建议：让 get_attributes() 对缺依赖返回可被 can_handle() 跳过的 ImportError，或先用 check_conditions 限定 EP 场景再探测依赖。
  • MoriEP wrapper 未接入 Bazel 运行文件依赖 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:4
    • 建议：为 moriep_wrapper.py 增加 py_library，并把使用它的 modules/server 目标显式依赖该 target。

Non-blocking Suggestions

P2

  • MoriEP 路由测试没有接入 Bazel @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/test/BUILD:66
    • 建议：接入可跳过缺 mori 环境的 py_test，至少覆盖 MoriEP wrapper 初始化、router prepare/finalize 和 FP4/BF16 路径。
  • USE_MORI_EP 功能开关绕过标准配置链路 @ rtp_llm/server/backend_manager.py:112
    • 建议：把 MoriEP 选择接入标准 server args / MoeConfig，并在策略和初始化处使用同一配置字段。

Checklist Violations (20 fail / 93 total)

General Principles Checklist

  • [6.1] Software Engineering — DIP：高层策略不依赖非必要具体细节 → issue USE_MORI_EP 功能开关绕过标准配置链路
    策略选择和后端初始化直接读取 USE_MORI_EP，绕过 EngineConfig/MoeConfig。
  • [6.1] Software Engineering — DRY：重复非平凡逻辑被抽取或显式复用 → issue USE_MORI_EP 功能开关绕过标准配置链路
    USE_MORI_EP 在 backend_manager.py 和 strategy/ep.py 重复解析。
  • [6.1] Architecture — 依赖方向：无循环依赖/跨层惊喜 → issue MoriEP wrapper 未接入 Bazel 运行文件依赖
    router/server 引入 moriep_wrapper，但 Bazel 目标依赖未补齐，运行文件依赖不完整。
  • [6.1] Architecture — 错误语义：fail-fast/retry/fallback/silent 行为显式 → issue ROCm MoE 策略探测会中断后续策略选择
    RocmEpNormalStrategy 缺依赖时抛 ValueError，can_handle 不会把它当不可用策略跳过。
  • [6.1] Architecture — 兼容性：公开 API/持久数据/配置/环境迁移安全 → issue USE_MORI_EP 功能开关绕过标准配置链路
    新增 USE_MORI_EP 未进入标准配置对象，无法通过现有参数链路验证和记录。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue MoriEP 路由测试没有接入 Bazel
    新增 MoriEP 测试文件未接入可运行 Bazel target。
  • [6.1] Tests — 边界 case 覆盖（空、单元素、最大值） → checklist-only
    新增测试覆盖部分 token 数，但未接入 CI；具体缺口已由测试未接入问题覆盖。
  • [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue MoriEP 路由测试没有接入 Bazel
    ROCm 多卡 MoriEP 变更的测试 target 被注释，不会稳定运行。

RTP-LLM Checklist

  • [A] 兼容性与配置 — 环境变量/命令行参数/proto 重命名有向后兼容 fallback → issue USE_MORI_EP 功能开关绕过标准配置链路
    新增 USE_MORI_EP 功能开关未接入命令行/配置对象。
  • [A] 兼容性与配置 — 可选依赖 lazy import，pybind 新字段有 C++ 默认值 → issue ROCm MoE 策略探测会中断后续策略选择
    策略缺依赖时抛 ValueError，未被 can_handle 的 ImportError 跳过逻辑处理。
  • [A] 兼容性与配置 — BUILD 重构后 testdata 相对路径仍有效 → issue MoriEP wrapper 未接入 Bazel 运行文件依赖
    新增 wrapper 被 import，但对应 Bazel py_library/deps 未接入。
  • [B] 正确性与逻辑 — 逻辑错误、off-by-one、null/zero 检查 → issue ROCm MoE 策略探测会中断后续策略选择
    RocmEpNormalStrategy 缺依赖时中断整个策略选择，是控制流错误。
  • [B] 正确性与逻辑 — 边界 case（空输入、单元素、最大值） → checklist-only
    边界测试文件未接入 CI；主问题已由测试 target 缺失覆盖。
  • [F] 跨平台 — ROCm 路径错误处理非静默 → issue ROCm MoE 策略探测会中断后续策略选择
    ROCm 策略探测缺依赖时抛未被捕获的 ValueError，错误语义不符合策略跳过预期。
  • [H] 测试与 CI — 测试覆盖充分：大重构等价覆盖，新功能端到端测试 → issue MoriEP 路由测试没有接入 Bazel
    MoriEP 多卡测试 target 被注释，新增功能没有可运行 CI 覆盖。
  • [I] 代码质量 — 同一功能用统一工具函数 → issue USE_MORI_EP 功能开关绕过标准配置链路
    USE_MORI_EP 解析逻辑重复出现在 backend_manager.py 与 strategy/ep.py。

Python Static-First Checklist

  • [P.A] 静态结构与类型纪律 — 字符串分发用 Enum/Literal → issue USE_MORI_EP 功能开关绕过标准配置链路
    USE_MORI_EP 以字符串集合解析，未进入枚举/配置字段。
  • [P.B] 错误处理 — 异常链用 raise X from Y → checklist-only
    部分 RuntimeError 未用 from e 保留异常链；当前已有日志记录原异常，影响主要是可诊断性，不单独升级。
  • [P.G] 测试规范 — 数据驱动测试用 pytest.mark.parametrize → checklist-only
    新增 unittest/subTest 循环可读性一般，但更关键问题是测试未接入 Bazel，已单独提 issue。
  • [P.H] 类型标注 — Any 必须附注释说明原因 → checklist-only
    moriep_wrapper 对外部 mori op 使用 Any，原因可从外部未类型化依赖推断；建议补注释但不构成具体缺陷。

Strengths

  • MoriEP 路由把 global expert id 映射为本地 id，并通过 expert_ids_are_local 避免继续套用 global expert_mask，方向是清晰的。
  • DeepEP import 改为 lazy fallback，降低了未安装 deep_ep 环境的顶层导入风险。

@LLLLKKKK

LLLLKKKK commented May 8, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #977

Status: BLOCKING

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

Blocking Issues

P1

  • MoriEP wrapper 未接入 Bazel 运行依赖 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:6
    • 建议：为 moriep_wrapper.py 增加 Bazel py_library，并把 modules/server 相关 target 显式依赖该 target。

Non-blocking Suggestions

P2

  • MoriEP 路由在 forward 热路径打印 info 日志 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:75
    • 建议：改为 debug 级别或限频日志，只在初始化、异常或状态变化时记录。
  • MoriEP 测试没有有效接入 CI @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/test/BUILD:66
    • 建议：接入可在缺 mori 环境下 skip 的 py_test，并覆盖 wrapper 初始化、prepare/finalize、BF16/FP4 和 chunked 路径。

Checklist Violations (12 fail / 56 total)

General Principles Checklist

  • [6.1] Architecture — 依赖方向：无循环依赖/跨层惊喜 → issue MoriEP wrapper 未接入 Bazel 运行依赖
    router import moriep_wrapper，但 Bazel 依赖未声明该 distributed target，运行文件依赖不完整。
  • [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue MoriEP 路由在 forward 热路径打印 info 日志
    MoriEpIntranodeRouter.prepare/finalize 在每次 forward 打 info 日志，属于高频噪声。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue MoriEP 测试没有有效接入 CI
    新增 MoriEP 测试文件未有效接入 BUILD，ROCm 多卡路径不会被 CI 稳定执行。
  • [6.1] Tests — 边界 case 覆盖（空、单元素、最大值） → checklist-only
    chunked/max token 边界需要 CI 覆盖；当前主要缺陷已由测试未接入问题覆盖，不单独升级。
  • [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue MoriEP 测试没有有效接入 CI
    MoriEP 是 ROCm 多卡分布式路径，但新增测试 target 注释掉，无法形成稳定跨平台 gate。
  • [6.1] Quality — 无 per-forward 调试日志 / 噪声热路径输出 → issue MoriEP 路由在 forward 热路径打印 info 日志
    prepare/finalize 每次 forward 均 logging.info。

Python Static-First Checklist

  • [P.A] 静态结构与类型纪律 — 禁止 getattr/setattr literal 访问 → checklist-only
    _ep.py 使用 getattr(self, "config", None) 兼容策略状态；这是局部实现细节，不构成独立运行缺陷。
  • [P.A] 静态结构与类型纪律 — 禁止 hasattr 做控制流分支 → checklist-only
    reset_op 和测试清理中使用 hasattr 判断可选外部库接口；属于外部 mori 兼容处理，不单独升级。
  • [P.B] 错误处理 — 异常链用 raise X from Y → checklist-only
    部分 RuntimeError/ValueError 未使用 from e；当前日志包含原异常，主要影响诊断链路，不单独升级。
  • [P.F] 语言陷阱 — 禁止模块级 import 副作用 → checklist-only
    moriep_test.py 顶层 import mori 会让缺依赖时 import 失败；主要影响并入“测试没有有效接入 CI”的测试问题。
  • [P.G] 测试规范 — 数据驱动测试用 pytest.mark.parametrize → checklist-only
    新增测试使用 unittest/subTest 循环；更关键问题是测试未接入 CI，已单独提 issue。
  • [P.H] 类型标注 — Any 必须附注释说明原因 → checklist-only
    moriep_wrapper 对外部 mori op 使用 Any，原因可由未类型化外部依赖推断；建议补注释但不是具体缺陷。

Strengths

  • use_mori_ep 已补到 C++ MoeConfig、pybind 和 Python config adapter，配置传播路径比上一轮更完整。
  • MoriEP router 显式缓存 global dispatch_ids，并用 expert_ids_are_local 避免本地 expert id 再套全局 mask，方向清晰。

@LLLLKKKK

LLLLKKKK commented May 8, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #977

Status: LGTM

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

lgtm ready to ci

Non-blocking Suggestions

P2

  • MoriEP 路由在 forward 热路径打印 info 日志 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:75
    • 建议：改为 debug 级别或限频日志，只在初始化、异常或状态变化时记录。
  • MoriEP 新增测试没有有效接入 CI @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/test/BUILD:66
    • 建议：接入可在缺 mori/缺 GPU 环境下 skip 的 py_test，覆盖 wrapper 初始化、prepare/finalize、BF16/FP4 和 chunked 路径。
  • ROCm FakeBalanceExpert 新绑定未接入现有 Python 调用路径 @ rtp_llm/models_py/bindings/rocm/RegisterBaseBindings.hpp:104
    • 建议：对齐 CUDA 的 FakeBalanceExpertOp 类接口，或更新 ROCm select_topk/FakeBalanceExpert wrapper 调用新函数，并补一条 ROCm 单测。

Checklist Violations (12 fail / 56 total)

General Principles Checklist

  • [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue MoriEP 路由在 forward 热路径打印 info 日志
    prepare/finalize 每次 forward 都 logging.info，属于热路径噪声。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue MoriEP 新增测试没有有效接入 CI
    新增 MoriEP 测试文件未有效接入 BUILD/CI。
  • [6.1] Tests — 边界 case 覆盖（空、单元素、最大值） → checklist-only
    新增测试包含 chunk/max token 意图，但未接入 CI；主风险已由测试未接入问题覆盖。
  • [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue MoriEP 新增测试没有有效接入 CI
    MoriEP 是 ROCm 多卡分布式路径，新增测试 target 仍未运行。
  • [6.1] Quality — 无 per-forward 调试日志 / 噪声热路径输出 → issue MoriEP 路由在 forward 热路径打印 info 日志
    MoriEpIntranodeRouter.prepare/finalize 每次 forward 均 logging.info。

RTP-LLM Checklist

  • [I] 代码质量 — 同一功能用统一工具函数 → issue ROCm FakeBalanceExpert 新绑定未接入现有 Python 调用路径
    ROCm FakeBalanceExpert 新增了底层 binding，但没有复用/对齐现有 FakeBalanceExpert Python 模块接口。

Python Static-First Checklist

  • [P.A] 静态结构与类型纪律 — 禁止 getattr/setattr literal 访问 → checklist-only
    _ep.py 使用 getattr(self, "config", None) 兼容 strategy priority/can_handle 状态；局部实现细节，不单独升级。
  • [P.A] 静态结构与类型纪律 — 禁止 hasattr 做控制流分支 → checklist-only
    reset_op 和测试清理对外部 mori API 使用 hasattr；属于可选依赖兼容处理，不构成独立缺陷。
  • [P.B] 错误处理 — 异常链用 raise X from Y → checklist-only
    部分 RuntimeError/ValueError 未使用 raise ... from e；当前日志包含原异常，主要影响诊断链路。
  • [P.F] 语言陷阱 — 禁止模块级 import 副作用 → checklist-only
    moriep_test.py 顶层 import mori 会在缺依赖时 import 失败；主要问题已由测试未接入 CI 覆盖。
  • [P.G] 测试规范 — 数据驱动测试用 pytest.mark.parametrize → checklist-only
    新增测试使用 unittest/subTest 循环；更关键问题是测试未接入 CI，已单独提 issue。
  • [P.H] 类型标注 — Any 必须附注释说明原因 → checklist-only
    moriep_wrapper 和测试中对外部 mori op 使用 Any，原因可推断为外部未类型化依赖；建议补注释但不构成运行缺陷。

Strengths

  • use_mori_ep 已进入 Python/C++ config、pybind 和 server args，配置传播比早期版本更完整。
  • MoriEP router 明确区分 global dispatch_ids 和本地 expert id，并通过 expert_ids_are_local 避免错误套用 global expert_mask。

LLLLKKKK
LLLLKKKK requested changes May 8, 2026
View reviewed changes
Comment thread rtp_llm/cpp/cache/KVCacheManager.cc Resolved
Comment thread rtp_llm/models_py/model_desc/generic_moe.py Resolved
Comment thread rtp_llm/server/backend_manager.py Resolved
@jacobwin-ai
jacobwin-ai force-pushed the develop/mori_355x_support_0507_after_rebase branch from 1069362 to 3214d3a Compare May 9, 2026 02:04
@LLLLKKKK

LLLLKKKK commented May 9, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #977

Status: BLOCKING

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

Blocking Issues

P1

  • MoriEP dtype 配置未转换为 torch.dtype @ rtp_llm/models_py/distributed/moriep_wrapper.py:88
    • 建议：在 from_config_adapter 中先用 to_torch_dtype(model_config.data_type) 转成 torch.dtype，并复用该值计算 element_size。
  • 新增 MoriEP 测试没有进入 Bazel 目标 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/test/BUILD:66
    • 建议：注册可执行的 py_test target，并用 tags/skipTest 处理缺 mori 或 GPU 不满足的环境，而不是提交未接入的测试文件。

Non-blocking Suggestions

P2

  • MoriEP router 在热路径同步并打印 INFO 日志 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:75
    • 建议：将 per-forward 日志降为 debug 或限频；避免 .tolist() 热路径回读，必要时用 GPU/异步统计或仅在测试调试路径启用。

Checklist Violations (5 fail / 56 total)

General Principles Checklist

  • [6.1] Architecture — 状态不变量：创建/更新/失败/重试/回滚路径有效 → issue MoriEP dtype 配置未转换为 torch.dtype
    MoriEPWrapperConfig 生产路径保留字符串 dtype，传入 MORI op 前没有保持 API 需要的 torch.dtype 不变量。
  • [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue MoriEP router 在热路径同步并打印 INFO 日志
    MoriEP router prepare/finalize 每次 forward 都打 INFO，属于热路径噪声。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue 新增 MoriEP 测试没有进入 Bazel 目标
    新增测试文件没有注册为 Bazel py_test，无法作为 CI gate 覆盖新 MoriEP 路径。
  • [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue 新增 MoriEP 测试没有进入 Bazel 目标
    MoriEP 是 ROCm 分布式路径，但新增测试未进入 Bazel/CI，跨 rank 行为没有 gate。
  • [6.1] Quality — 无 per-forward 调试日志 / 噪声热路径输出 → issue MoriEP router 在热路径同步并打印 INFO 日志
    MoriEP router prepare/finalize 每次 forward 都 logging.info。

Strengths

  • DeepEP import 改为 lazy import，降低缺少可选依赖时非 DeepEP 路径的启动风险。
  • MoriEP router 明确区分本地 expert id，executor 侧也增加了 expert_ids_are_local 保护，接口意图比较清晰。

@LLLLKKKK

LLLLKKKK commented May 9, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #977

Status: LGTM

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

lgtm ready to ci

Non-blocking Suggestions

P2

  • MoriEP 可选依赖在模块导入时硬失败 @ rtp_llm/models_py/distributed/moriep_wrapper.py:14
    • 建议：将 mori 改为 lazy import，并把依赖 mori.ops 的默认值移到工厂方法或运行期初始化中。
  • MoriEP router 在每次 forward 打 info 日志 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:75
    • 建议：删除热路径 info，或改为 debug/采样/LOG_FIRST_N 类限频日志。
  • MoriEP 路由测试没有注册到 Bazel @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/test/BUILD:67
    • 建议：启用 py_test，并在测试内部用 mori/GPU 条件 skip；或添加可在 MI308X CI 上运行的等价 target。

Checklist Violations (9 fail / 67 total)

General Principles Checklist

  • [6.1] Architecture — 依赖方向：无循环依赖/跨层惊喜 → issue MoriEP 可选依赖在模块导入时硬失败
    MoriEP wrapper 顶层硬依赖 mori，破坏 optional dependency 边界。
  • [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue MoriEP router 在每次 forward 打 info 日志
    MoriEP prepare/finalize 在 forward 热路径无条件 info 日志。
  • [6.1] Architecture — 兼容性：公开 API/持久数据/配置/环境迁移安全 → issue MoriEP 可选依赖在模块导入时硬失败
    未安装 mori 的环境导入 moriep_wrapper 会直接 ImportError。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue MoriEP 路由测试没有注册到 Bazel
    新增 MoriEP router 测试文件存在，但 Bazel target 被注释。
  • [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue MoriEP 路由测试没有注册到 Bazel
    ROCm 多卡 MoriEP 变更未启用 Bazel/CI 覆盖。
  • [6.1] Quality — 无 per-forward 调试日志 / 噪声热路径输出 → issue MoriEP router 在每次 forward 打 info 日志
    MoriEP router 的 prepare/finalize 每次 forward 打 info。

RTP-LLM Checklist

  • [A] 兼容性与配置 — 可选依赖 lazy import，pybind 新字段有 C++ 默认值 → issue MoriEP 可选依赖在模块导入时硬失败
    MoriEP 的 mori 依赖不是 lazy import；C++ 字段已有默认值。
  • [H] 测试与 CI — 测试覆盖充分：大重构等价覆盖，新功能端到端测试 → issue MoriEP 路由测试没有注册到 Bazel
    MoriEP router 测试 target 被注释，新增路径缺少 CI gate。

Python Static-First Checklist

  • [P.F] 语言陷阱 — 禁止模块级 import 副作用 → issue MoriEP 可选依赖在模块导入时硬失败
    moriep_wrapper 模块级 import mori 会在未安装 mori 时直接失败。

Strengths

  • use_mori_ep 已从 C++ MoeConfig、pybind 到 Python config adapter 和策略注册链路做了贯通。
  • DeepEP wrapper 的导入路径开始做 optional dependency 处理，方向上有助于降低非 DeepEP 环境的启动耦合。

@jacobwin-ai
jacobwin-ai force-pushed the develop/mori_355x_support_0507_after_rebase branch 2 times, most recently from 33f4344 to 1a842ad Compare May 9, 2026 03:27
@LLLLKKKK

LLLLKKKK commented May 9, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #977

Status: BLOCKING

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

Blocking Issues

P1

  • MoriEP 生产配置把字符串 dtype 传给 MORI @ rtp_llm/models_py/distributed/moriep_wrapper.py:66
    • 建议：在 from_config_adapter 中先执行 torch_dtype = to_torch_dtype(model_config.data_type)，data_type 和 element_size 都复用该 torch.dtype。
  • MoriEP fallback 与 wrapper 初始化条件不一致 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/strategy/ep.py:73
    • 建议：只在 use_mori_ep 明确开启时选择 MoriEP，或在 fallback 选择 MoriEP 前同步设置配置并保证 backend 初始化 wrapper。
  • MoriEP FP4 executor 新增后没有接入策略选择 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/strategy/ep.py:47
    • 建议：在 ROCm EP strategy 中按 quant_method 选择 RocmExpertsFp4PerGroup，并补 MoriEP FP4 strategy/路由测试。

Non-blocking Suggestions

P2

  • MoriEP router 在 forward 热路径打印 INFO 并同步回读 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:76
    • 建议：将热路径日志改为 debug/限频；避免 chunked 路径中对设备 tensor 做 .tolist()，或仅在必要诊断路径启用。
  • MoriEP router 测试没有注册到 Bazel @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/test/BUILD:66
    • 建议：启用可在缺 mori/缺 GPU 环境下 skip 的 Bazel py_test，覆盖 BF16、FP4、prepare/finalize 和 chunked 路径。

Checklist Violations (14 fail / 56 total)

General Principles Checklist

  • [6.1] Software Engineering — DIP：高层策略不依赖非必要具体细节 → issue MoriEP fallback 与 wrapper 初始化条件不一致
    strategy 自行 fallback 到 MoriEP，但 backend 初始化仍只看 use_mori_ep，策略选择和初始化策略没有统一抽象。
  • [6.1] Architecture — 状态不变量：创建/更新/失败/重试/回滚路径有效 → issue MoriEP 生产配置把字符串 dtype 传给 MORI
    MoriEPWrapperConfig 生产路径保留 model_config.data_type 原值，破坏 MORI op 期望 torch.dtype 的配置不变量。
  • [6.1] Architecture — 错误语义：fail-fast/retry/fallback/silent 行为显式 → issue MoriEP fallback 与 wrapper 初始化条件不一致
    deep_ep 缺失时 fallback 到 mori，但初始化链路未同步，fallback 不是可运行的显式降级路径。
  • [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue MoriEP router 在 forward 热路径打印 INFO 并同步回读
    MoriEP prepare/finalize 每次 forward 都 logging.info，会在高 QPS MoE 路径产生噪声日志。
  • [6.1] Architecture — 兼容性：公开 API/持久数据/配置/环境迁移安全 → issue MoriEP FP4 executor 新增后没有接入策略选择
    新增 FP4 executor 没有进入 strategy，FP4 MoriEP 配置无法选到匹配 executor。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue MoriEP router 测试没有注册到 Bazel
    新增 MoriEP router 测试文件存在，但 Bazel py_test 仍被注释，不能作为 CI gate。
  • [6.1] Tests — 边界 case 覆盖（空、单元素、最大值） → issue MoriEP router 测试没有注册到 Bazel
    新增 router 测试未注册到 Bazel，空 token、chunked、FP4 等边界无法在 CI 中形成有效覆盖。
  • [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue MoriEP router 测试没有注册到 Bazel
    MoriEP 是 ROCm 多卡分布式路径，但新增测试没有有效进入 Bazel/CI。
  • [6.1] Quality — 无 per-forward 调试日志 / 噪声热路径输出 → issue MoriEP router 在 forward 热路径打印 INFO 并同步回读
    MoriEpIntranodeRouter.prepare/finalize 每次 forward 均输出 INFO。

RTP-LLM Checklist

  • [I] 代码质量 — 同一功能用统一工具函数 → issue MoriEP fallback 与 wrapper 初始化条件不一致
    MoriEP 是否启用分别由 strategy fallback 和 backend use_mori_ep 初始化条件决定，缺少统一选择/初始化入口。

Python Static-First Checklist

  • [P.A] 静态结构与类型纪律 — 字符串分发用 Enum/Literal → issue MoriEP FP4 executor 新增后没有接入策略选择
    FP4 quant_method 已在 executor check_conditions 中判断，但 strategy 没有基于该分发选择 FP4 executor。
  • [P.F] 语言陷阱 — 禁止模块级 import 副作用 → checklist-only
    moriep_wrapper 顶层 import mori 对可选依赖不友好；当前主要运行缺陷已由 fallback/初始化不一致覆盖，暂不单独升级。
  • [P.G] 测试规范 — 数据驱动测试用 pytest.mark.parametrize → checklist-only
    新增测试使用 unittest/subTest 循环；更关键的问题是测试未注册到 Bazel，已单独提 issue。
  • [P.H] 类型标注 — Any 必须附注释说明原因 → checklist-only
    MoriEP wrapper 对外部 mori op 使用 Any，原因可推断为外部未类型化依赖；建议补注释，但不构成独立运行缺陷。

Strengths

  • use_mori_ep 已贯通到 C++ MoeConfig、pybind、Python config adapter 和 server args，配置链路比早期版本更完整。
  • MoriEP router 明确区分 global dispatch_ids 与本地 expert id，并用 expert_ids_are_local 避免本地 id 再套全局 expert_mask。

@jacobwin-ai
jacobwin-ai force-pushed the develop/mori_355x_support_0507_after_rebase branch from 1a842ad to 942d2a7 Compare May 9, 2026 07:07
@LLLLKKKK

LLLLKKKK commented May 9, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #977

Status: BLOCKING

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

Blocking Issues

P1

  • MoriEP fallback 与 wrapper 初始化条件不一致 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/strategy/ep.py:95
    • 建议：只在 use_mori_ep 显式开启时选择 MoriEP，或让 strategy 选择结果同步驱动 backend 初始化。
  • MoriEP 生产配置把字符串 dtype 传给 MORI @ rtp_llm/models_py/distributed/moriep_wrapper.py:74
    • 建议：在 from_config_adapter 中先转换 torch_dtype = to_torch_dtype(model_config.data_type)，data_type 和 element_size 都复用该值。
  • MoriEP FP4 executor 新增后没有接入策略选择 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/strategy/ep.py:34
    • 建议：在 ROCm EP strategy 中按 quant_method 选择 RocmExpertsFp4PerGroup，并补 FP4 MoriEP 路由/执行测试。

Non-blocking Suggestions

P2

  • MoriEP router 在 forward 热路径打印 INFO 并同步回读 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:91
    • 建议：将热路径日志降为 debug/限频，并避免 .tolist() 同步回读，必要时用异步统计或只在诊断模式启用。
  • MoriEP router 测试没有注册到 Bazel @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/test/BUILD:66
    • 建议：启用可在缺 mori/缺 GPU 时 skip 的 Bazel py_test，覆盖 BF16、FP4、prepare/finalize 和 chunked 路径。

Checklist Violations (18 fail / 56 total)

General Principles Checklist

  • [6.1] Software Engineering — OCP：本地扩展点优先于修改中心逻辑 → issue MoriEP FP4 executor 新增后没有接入策略选择
    新增 RocmExpertsFp4PerGroup 后未接入 strategy 分发，FP4 MoriEP 仍走不到本地扩展点。
  • [6.1] Software Engineering — DIP：高层策略不依赖非必要具体细节 → issue MoriEP fallback 与 wrapper 初始化条件不一致
    strategy 自行 fallback 到 MoriEP，但 backend 初始化仍只看 use_mori_ep，高层选择与具体初始化条件分裂。
  • [6.1] Architecture — 状态不变量：创建/更新/失败/重试/回滚路径有效 → issue MoriEP 生产配置把字符串 dtype 传给 MORI
    生产配置把字符串 dtype 放进 MoriEPWrapperConfig，破坏 MORI op 期望的 dtype 不变量。
  • [6.1] Architecture — 错误语义：fail-fast/retry/fallback/silent 行为显式 → issue MoriEP fallback 与 wrapper 初始化条件不一致
    deep_ep 缺失时 fallback 到 MoriEP，但初始化条件未同步，fallback 不是可运行的显式降级路径。
  • [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue MoriEP router 在 forward 热路径打印 INFO 并同步回读
    MoriEP router 在 prepare/finalize 每次 forward 打 INFO，属于热路径噪声。
  • [6.1] Architecture — 兼容性：公开 API/持久数据/配置/环境迁移安全 → issue MoriEP FP4 executor 新增后没有接入策略选择
    新增 FP4 executor 没有进入 strategy，FP4 MoriEP 配置无法迁移到匹配执行路径。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue MoriEP router 测试没有注册到 Bazel
    新增 MoriEP router 测试文件存在，但 Bazel py_test 被注释，无法作为 CI gate。
  • [6.1] Tests — 边界 case 覆盖（空、单元素、最大值） → issue MoriEP router 测试没有注册到 Bazel
    chunked/FP4 等边界意图在未注册测试中，实际 CI 不会覆盖。
  • [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue MoriEP router 测试没有注册到 Bazel
    MoriEP 是 ROCm 多卡分布式路径，但新增测试未进入 Bazel/CI。
  • [6.1] Quality — 无 per-forward 调试日志 / 噪声热路径输出 → issue MoriEP router 在 forward 热路径打印 INFO 并同步回读
    MoriEP router 的 prepare/finalize 每次 forward 均输出 INFO。

RTP-LLM Checklist

  • [I] 代码质量 — 同一功能用统一工具函数 → issue MoriEP fallback 与 wrapper 初始化条件不一致
    MoriEP 是否启用分别由 strategy fallback 与 backend use_mori_ep 初始化条件决定，缺少统一选择/初始化入口。

Python Static-First Checklist

  • [P.A] 静态结构与类型纪律 — 禁止 getattr/setattr literal 访问 → checklist-only
    ep.py 使用 getattr(self, "_config", None) 保存 can_handle 状态；这是局部兼容写法，主要运行风险已由 fallback/init 问题覆盖。
  • [P.A] 静态结构与类型纪律 — 禁止 hasattr 做控制流分支 → checklist-only
    reset_op 和测试清理对外部 mori API 使用 hasattr；属于可选外部依赖兼容处理，不单独升级。
  • [P.A] 静态结构与类型纪律 — 字符串分发用 Enum/Literal → issue MoriEP FP4 executor 新增后没有接入策略选择
    FP4 quant_method 已在 executor check_conditions 中判断，但 strategy 没有基于该分发选择 FP4 executor。
  • [P.B] 错误处理 — 异常链用 raise X from Y → checklist-only
    raise ValueError(...) 和 raise RuntimeError(...) 未使用 from e 保留链路；当前日志包含原异常，主要影响诊断性。
  • [P.F] 语言陷阱 — 禁止模块级 import 副作用 → checklist-only
    moriep_wrapper.py 模块级 import mori 对可选依赖不友好；当前主要运行缺陷由 fallback/初始化不一致覆盖。
  • [P.G] 测试规范 — 数据驱动测试用 pytest.mark.parametrize → issue MoriEP router 测试没有注册到 Bazel
    新增测试使用 unittest/subTest 循环，且更关键的是测试未注册到 Bazel，无法形成独立 CI case。
  • [P.H] 类型标注 — Any 必须附注释说明原因 → checklist-only
    MoriEP wrapper 对外部 mori op 使用 Any，原因可推断为外部未类型化依赖；建议补注释但不是独立运行缺陷。

Strengths

  • use_mori_ep 已贯通到 C++ MoeConfig、pybind、Python config adapter 和 server args，配置链路比早期版本完整。
  • MoriEP router 明确区分 global dispatch_ids 与本地 expert id，并用 expert_ids_are_local 避免本地 id 再套全局 expert_mask。

@LLLLKKKK

LLLLKKKK commented May 9, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #977

Status: BLOCKING

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

Blocking Issues

P1

  • USE_MORI_EP 单独开启会保留低延迟默认值导致 MoriEP 策略不可选 @ rtp_llm/config/server_config_setup.py:99
    • 建议：当 use_mori_ep 显式为 True 时同步关闭 use_deepep_low_latency/use_deepep_moe，或在 MoriEP strategy 中明确处理该默认值冲突。

Non-blocking Suggestions

P2

  • MoriEP router 在 forward 热路径输出 INFO 日志 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:91
    • 建议：移除热路径 INFO，或改为 debug/限频日志，并只在诊断开关打开时记录。
  • MoriEP 多卡测试未进入 Bazel gate @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/test/BUILD:66
    • 建议：启用可在缺 mori/缺 GPU 时 skip 的 Bazel py_test，覆盖 BF16/FP4、prepare/finalize、chunked 和 USE_MORI_EP 配置链路。

Checklist Violations (14 fail / 67 total)

General Principles Checklist

  • [6.1] Architecture — 状态不变量：创建/更新/失败/重试/回滚路径有效 → issue USE_MORI_EP 单独开启会保留低延迟默认值导致 MoriEP 策略不可选
    USE_MORI_EP=true 只设置 use_mori_ep，不重置默认 use_deepep_low_latency=true，后续 strategy 条件会拒绝 MoriEP normal router。
  • [6.1] Architecture — 错误语义：fail-fast/retry/fallback/silent 行为显式 → issue USE_MORI_EP 单独开启会保留低延迟默认值导致 MoriEP 策略不可选
    用户显式启用 MoriEP 后会在 strategy 选择阶段失败，而不是在配置阶段 fail-fast 提示低延迟默认值冲突。
  • [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue MoriEP router 在 forward 热路径输出 INFO 日志
    MoriEP router 的 prepare/finalize 每次 forward 都输出 INFO，属于热路径噪声。
  • [6.1] Architecture — 兼容性：公开 API/持久数据/配置/环境迁移安全 → issue USE_MORI_EP 单独开启会保留低延迟默认值导致 MoriEP 策略不可选
    新增 USE_MORI_EP 参数后，单独启用该参数仍继承 DeepEP low-latency 默认值，配置迁移不安全。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue MoriEP 多卡测试未进入 Bazel gate
    新增 MoriEP 分布式实现和测试文件，但关键多卡测试未注册为 Bazel py_test。
  • [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue MoriEP 多卡测试未进入 Bazel gate
    MoriEP 是 ROCm 多卡分布式路径，当前新增测试不进入 CI gate。
  • [6.1] Quality — 无 per-forward 调试日志 / 噪声热路径输出 → issue MoriEP router 在 forward 热路径输出 INFO 日志
    MoriEP router 在 prepare/finalize 的 per-forward 路径使用 logging.info。

RTP-LLM Checklist

  • [A] 兼容性与配置 — 默认值变更已评估对现有用户影响 → issue USE_MORI_EP 单独开启会保留低延迟默认值导致 MoriEP 策略不可选
    新增 use_mori_ep 后未处理 MoeConfig.use_deepep_low_latency=true 的既有默认值，显式 MoriEP 配置会被低延迟条件拦截。
  • [A] 兼容性与配置 — 可选依赖 lazy import，pybind 新字段有 C++ 默认值 → checklist-only
    moriep_wrapper.py 顶层 import mori；不过 backend/strategy 仅在 use_mori_ep 路径导入，pybind 字段已有 C++ 默认值，暂不单独升级。
  • [H] 测试与 CI — 测试覆盖充分：大重构等价覆盖，新功能端到端测试 → issue MoriEP 多卡测试未进入 Bazel gate
    MoriEP router/wrapper 新功能缺少实际 Bazel gate 覆盖。

Python Static-First Checklist

  • [P.A] 静态结构与类型纪律 — 禁止 getattr/setattr literal 访问 → checklist-only
    _ep.py 使用 getattr(self, "config", None) 缓存 can_handle 状态；当前主要运行风险已由配置默认值问题覆盖，暂不单独升级。
  • [P.B] 错误处理 — 异常链用 raise X from Y → checklist-only
    mori/deep_ep import 失败后重新 raise ValueError/ImportError 未使用 from e；日志已包含原异常，主要影响诊断链路。
  • [P.F] 语言陷阱 — 禁止模块级 import 副作用 → checklist-only
    moriep_wrapper.py 模块级 import mori 对可选依赖不够友好；当前入口只在 use_mori_ep 路径导入，未作为独立缺陷升级。
  • [P.G] 测试规范 — 数据驱动测试用 pytest.mark.parametrize → issue MoriEP 多卡测试未进入 Bazel gate
    新增多卡测试仍是脚本/unittest/subTest 形态且未注册 Bazel，无法形成独立 CI case。

Strengths

  • use_mori_ep 已贯通到 C++ MoeConfig、pybind、Python config adapter 和 server args，配置链路比早期版本完整。
  • MoriEP router 明确区分 global dispatch_ids 与本地 expert id，并通过 expert_ids_are_local 避免本地 id 再套全局 expert_mask。

@LLLLKKKK

LLLLKKKK commented May 9, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #977

Status: BLOCKING

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

Blocking Issues

P1

  • use_all_gather 规则变更会打红既有 Bazel 单测 @ rtp_llm/config/server_config_setup.py:66
    • 建议：同步更新 auto_configure_deepep_test 和注释契约，或保留 ep_size == tp_size 的旧分支直到测试/调用方迁移。

Non-blocking Suggestions

P2

  • MoriEP router 在 forward 热路径输出 INFO 并同步回读 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:92
    • 建议：将 per-forward 日志降为 debug/限频，并避免在热路径 .tolist()，必要统计走诊断开关或异步 CPU 侧路径。
  • MoriEP 多卡测试未注册且脚本接口不匹配 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/test/BUILD:68
    • 建议：注册可在缺 mori/缺 GPU 时 skip 的 py_test，并先修正测试脚本与 MoriEPWrapper._create 的接口一致性。
  • ROCm FakeBalanceExpert 绑定未接入 Python 调用路径 @ rtp_llm/models_py/bindings/rocm/RegisterBaseBindings.hpp:109
    • 建议：补 ROCm FakeBalanceExpert Python wrapper，对齐 CUDA 的类接口，或移除未接入的底层 binding 并补对应单测。

Checklist Violations (17 fail / 83 total)

General Principles Checklist

  • [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue MoriEP router 在 forward 热路径输出 INFO 并同步回读
    MoriEP prepare/finalize 每次 forward 输出 INFO。
  • [6.1] Architecture — 兼容性：公开 API/持久数据/配置/环境迁移安全 → issue use_all_gather 规则变更会打红既有 Bazel 单测
    auto_configure_deepep 的 use_all_gather 条件改变，但既有测试契约未同步。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue MoriEP 多卡测试未注册且脚本接口不匹配
    新增 MoriEP 多卡测试仍未注册为 Bazel py_test。
  • [6.1] Tests — 边界 case 覆盖（空、单元素、最大值） → checklist-only
    MoriEP 边界覆盖意图在新增测试中，但测试未进入 Bazel；主问题已由测试未注册 issue 覆盖。
  • [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue MoriEP 多卡测试未注册且脚本接口不匹配
    MoriEP 是 ROCm 多卡分布式路径，但新增测试未作为 CI gate 运行。
  • [6.1] Quality — 无 per-forward 调试日志 / 噪声热路径输出 → issue MoriEP router 在 forward 热路径输出 INFO 并同步回读
    MoriEP router prepare/finalize 在 forward 路径 logging.info。

RTP-LLM Checklist

  • [A] 兼容性与配置 — 默认值变更已评估对现有用户影响 → issue use_all_gather 规则变更会打红既有 Bazel 单测
    use_all_gather 对 ep_size==tp_size 的行为改变未同步既有测试契约。
  • [D] 性能 — 设备同步 .item()/.cpu()/.tolist() 在关键路径 → issue MoriEP router 在 forward 热路径输出 INFO 并同步回读
    chunked prepare 中 dispatch_recv_token_num.tolist() 会同步回读。
  • [F] 跨平台 — CUDA/ROCm 两侧 binding 对称清理 → issue ROCm FakeBalanceExpert 绑定未接入 Python 调用路径
    ROCm 新增 fake_balance_expert binding，但 Python ROCm FakeBalanceExpert 仍是 NotImplementedOp。
  • [H] 测试与 CI — 测试覆盖充分：大重构等价覆盖，新功能端到端测试 → issue MoriEP 多卡测试未注册且脚本接口不匹配
    MoriEP 多卡测试未注册为 Bazel target。
  • [I] 代码质量 — 同一功能用统一工具函数 → issue ROCm FakeBalanceExpert 绑定未接入 Python 调用路径
    ROCm FakeBalanceExpert 只注册底层函数，未复用/对齐现有 CUDA Python wrapper 接口。

Python Static-First Checklist

  • [P.A] 静态结构与类型纪律 — 禁止 getattr/setattr literal 访问 → checklist-only
    _ep.py 使用 getattr(self, "config", None) 保存 can_handle 状态；局部实现细节，不单独升级。
  • [P.A] 静态结构与类型纪律 — 禁止 hasattr 做控制流分支 → checklist-only
    reset_op 和测试清理对外部 mori API 使用 hasattr；属于可选外部依赖兼容处理。
  • [P.B] 错误处理 — 异常链用 raise X from Y → checklist-only
    部分 RuntimeError/ValueError 未使用 from e；当前日志保留原异常文本，主要影响诊断链路。
  • [P.F] 语言陷阱 — 禁止模块级 import 副作用 → checklist-only
    moriep_wrapper 顶层 import mori；当前生产入口只在 use_mori_ep 路径导入，主要风险由测试/可选依赖问题覆盖。
  • [P.G] 测试规范 — 数据驱动测试用 pytest.mark.parametrize → checklist-only
    新增测试使用 unittest/subTest；更关键问题是测试未注册 Bazel，已单独提 issue。
  • [P.H] 类型标注 — Any 必须附注释说明原因 → checklist-only
    MoriEP wrapper/test 对外部 mori op 使用 Any；原因可推断为外部未类型化依赖，建议补注释但不构成独立运行缺陷。

Strengths

  • use_mori_ep 已贯通到 C++ MoeConfig、pybind、Python config adapter 和 server args，配置链路比早期版本完整。
  • MoriEP router 明确区分 global dispatch_ids 与本地 expert id，并通过 expert_ids_are_local 避免本地 id 再套全局 expert_mask。

@jacobwin-ai
jacobwin-ai force-pushed the develop/mori_355x_support_0507_after_rebase branch from 7ecc663 to 9cee69f Compare May 9, 2026 09:19
@LLLLKKKK

LLLLKKKK commented May 9, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #977

Status: LGTM

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

lgtm ready to ci

Non-blocking Suggestions

P2

  • ROCm fake_balance_expert 绑定未接到现有 Python 调用链 @ rtp_llm/models_py/bindings/rocm/RegisterBaseBindings.hpp:104
    • 建议：补 ROCm FakeBalanceExpert Python wrapper 并在 base/init.py 导出，或保持与 CUDA FakeBalanceExpertOp 相同 API。
  • MoriEP router 在 forward 热路径打印 info 日志 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:91
    • 建议：改为 debug 级别并默认关闭，或使用采样/首次日志，避免 per-forward info 输出。
  • MoriEP 测试未接入 CI 且手写测试调用签名已失效 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/test/BUILD:66
    • 建议：把可运行的 MoriEP 测试接入 ROCm CI；依赖 mori 时用 skip/tag/exec_properties 控制，并同步修正 _create 调用。

Checklist Violations (5 fail / 56 total)

General Principles Checklist

  • [6.1] Architecture — 分层边界：新概念在正确层级，不泄漏内部 → issue ROCm fake_balance_expert 绑定未接到现有 Python 调用链
    ROCm fake_balance_expert 只注册底层 binding，现有 Python FakeBalanceExpert 入口仍指向 NotImplemented。
  • [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue MoriEP router 在 forward 热路径打印 info 日志
    MoriEP router 在 prepare/finalize 热路径使用 logging.info，默认会逐 forward 输出。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue MoriEP 测试未接入 CI 且手写测试调用签名已失效
    新增 MoriEP 测试文件未接入 BUILD；router 测试 py_test 被注释，无法形成 CI 覆盖。
  • [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue MoriEP 测试未接入 CI 且手写测试调用签名已失效
    ROCm/MoriEP 分布式测试新增但未进入 BUILD，覆盖无法在 CI 生效。
  • [6.1] Quality — 无 per-forward 调试日志 / 噪声热路径输出 → issue MoriEP router 在 forward 热路径打印 info 日志
    MoriEpIntranodeRouter.prepare/finalize 中新增 per-forward logging.info。

Strengths

  • USE_ALL_GATHER 从 ep_size==tp_size 改为 ep_size==1 的规则补了对应回归用例。
  • DeepEP import 改为可选路径，避免未安装 deep_ep 时在非 DeepEP 场景提前崩溃。

@jacobwin-ai
jacobwin-ai force-pushed the develop/mori_355x_support_0507_after_rebase branch 2 times, most recently from 025e986 to eb41f58 Compare May 11, 2026 06:26
LLLLKKKK
LLLLKKKK previously approved these changes May 11, 2026
View reviewed changes
Comment thread rtp_llm/cpp/cache/BlockPoolConfigHelper.h Resolved
@LLLLKKKK
LLLLKKKK enabled auto-merge (rebase) May 11, 2026 07:04
@wht21

wht21 commented May 11, 2026

Copy link
Copy Markdown
Collaborator

internal source has been updated, please review the changes!

@jacobwin-ai
jacobwin-ai force-pushed the develop/mori_355x_support_0507_after_rebase branch from 5ec2e71 to f7b9023 Compare May 11, 2026 08:01
@LLLLKKKK

LLLLKKKK commented May 11, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #977

Status: BLOCKING

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

Blocking Issues

P1

  • 共享专家统一 allreduce 会跳过 DeepEP 的 TP all_gather @ rtp_llm/models_py/model_desc/generic_moe.py:155
    • 建议：不要用 skip_allreduce 跳过 DeepEP 的 TP all_gather；将“TP token gather”和“TP 输出求和”拆成不同开关，或先恢复完整 routed expert 输出再统一规约。

Non-blocking Suggestions

P2

  • MoriEP router 在 forward 热路径输出 INFO 并同步回读 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:75
    • 建议：将 per-forward 日志改为 debug/限频，并避免在热路径 .tolist()；必要统计放到诊断开关或异步 CPU 路径。
  • MoriEP 多卡测试未注册且脚本调用签名不匹配 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/test/BUILD:66
    • 建议：注册可在缺 mori/GPU 时 skip 的 Bazel py_test，并同步修正初始化调用或 _create 签名。
  • ROCm fake_balance_expert binding 未接入 Python 调用链 @ rtp_llm/models_py/bindings/rocm/RegisterBaseBindings.hpp:104
    • 建议：补齐 ROCm FakeBalanceExpert Python wrapper 并在 base/init.py 平台分支导出，或移除未接入的底层 binding。

Checklist Violations (23 fail / 88 total)

General Principles Checklist

  • [6.1] Architecture — 分层边界：新概念在正确层级，不泄漏内部 → issue ROCm fake_balance_expert binding 未接入 Python 调用链
    ROCm fake_balance_expert 只注册底层 binding，Python 平台入口仍是 NotImplemented，底层能力没有接入既有模块边界。
  • [6.1] Architecture — 状态不变量：创建/更新/失败/重试/回滚路径有效 → issue 共享专家统一 allreduce 会跳过 DeepEP 的 TP all_gather
    GenericMoeLayer 在 EP+TP+shared expert 下跳过 router finalize 的 TP gather，但 EP 分支没有恢复 routed expert 的完整 token 输出。
  • [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue MoriEP router 在 forward 热路径输出 INFO 并同步回读
    MoriEP router 的 prepare/finalize 在每次 forward 输出 INFO，属于高频热路径噪声。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue MoriEP 多卡测试未注册且脚本调用签名不匹配
    MoriEP 多卡测试 target 被注释，且脚本调用 wrapper 签名不匹配，无法形成可运行 gate。
  • [6.1] Tests — 边界 case 覆盖（空、单元素、最大值） → checklist-only
    MoriEP 测试意图覆盖部分 token 边界，但未进入 Bazel gate；主缺口已由测试未注册问题覆盖。
  • [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue MoriEP 多卡测试未注册且脚本调用签名不匹配
    MoriEP 是 ROCm 多卡分布式路径，但新增测试未作为 Bazel py_test 运行。
  • [6.1] Quality — 无 per-forward 调试日志 / 噪声热路径输出 → issue MoriEP router 在 forward 热路径输出 INFO 并同步回读
    MoriEP router prepare/finalize 使用 logging.info，并位于每次 forward 路径。

RTP-LLM Checklist

  • [A] 兼容性与配置 — 可选依赖 lazy import，pybind 新字段有 C++ 默认值 → checklist-only
    moriep_wrapper 顶层 import mori 对可选依赖不够友好；当前生产入口仅在 use_mori_ep 路径导入，主要影响由测试/可选依赖问题覆盖。
  • [D] 性能 — Python 循环替代 fused kernel 导致性能回退 → checklist-only
    MoriEP chunked path 逐 chunk dispatch/combine 是容量兜底；当前更具体风险是同步回读和日志噪声，已单独提 issue。
  • [D] 性能 — 设备同步 .item()/.cpu()/.tolist() 在关键路径 → issue MoriEP router 在 forward 热路径输出 INFO 并同步回读
    MoriEP chunked prepare 对 dispatch_recv_token_num.tolist() 做同步回读。
  • [D] 性能 — 硬编码容量值 → checklist-only
    MoriEP config 中 block_num/warp_num_per_block/gpu_per_node 有固定值；当前缺少直接缺陷证据，作为调优风险记录。
  • [E] 分布式 — 跨 rank 数据一致性 → issue 共享专家统一 allreduce 会跳过 DeepEP 的 TP all_gather
    EP+TP+shared expert 下 routed expert 输出可能未完成 TP gather 就与 shared expert full-token 输出合并。
  • [E] 分布式 — collective op 匹配 → issue 共享专家统一 allreduce 会跳过 DeepEP 的 TP all_gather
    skip_allreduce 命名用于跳过 routed expert finalize，但 CUDA DeepEP normal 的被跳过 collective 是 TP all_gather 而非输出 all_reduce。
  • [F] 跨平台 — CUDA/ROCm 两侧 binding 对称清理 → issue ROCm fake_balance_expert binding 未接入 Python 调用链
    ROCm 新增 fake_balance_expert 底层 binding，但 Python FakeBalanceExpert 平台分支仍是 NotImplemented。
  • [H] 测试与 CI — 测试覆盖充分：大重构等价覆盖，新功能端到端测试 → issue MoriEP 多卡测试未注册且脚本调用签名不匹配
    MoriEP 多卡测试新增但未注册，且脚本签名错误导致手动运行也不可用。
  • [I] 代码质量 — 同一功能用统一工具函数 → issue ROCm fake_balance_expert binding 未接入 Python 调用链
    ROCm fake_balance_expert 没有复用既有 Python FakeBalanceExpert 接口，导致底层 binding 与调用链分离。

Python Static-First Checklist

  • [P.A] 静态结构与类型纪律 — 禁止 getattr/setattr literal 访问 → checklist-only
    _ep.py 使用 getattr(self, "config", None) 保存 can_handle 状态；局部实现细节，当前主要运行风险已由 collective issue 覆盖。
  • [P.A] 静态结构与类型纪律 — 禁止 hasattr 做控制流分支 → checklist-only
    reset_op 和测试清理对外部 mori API 使用 hasattr；属于可选外部依赖兼容处理，不单独升级。
  • [P.A] 静态结构与类型纪律 — 公开 API 禁止 **kwargs 透传 → checklist-only
    MoriEPWrapper.dispatch/combine 透传 *args/**kwargs 到外部 op；这是外部库薄包装接口，暂不作为独立缺陷。
  • [P.B] 错误处理 — 异常链用 raise X from Y → checklist-only
    backend_manager 和 strategy 中部分重新抛 RuntimeError/ValueError 未使用 from e；日志包含原异常，主要影响诊断链路，不单独升级。
  • [P.F] 语言陷阱 — 禁止模块级 import 副作用 → checklist-only
    moriep_wrapper 和 moriep_test 顶层 import mori；生产入口受 use_mori_ep 限制，测试入口缺陷已由测试 issue 覆盖。
  • [P.G] 测试规范 — 数据驱动测试用 pytest.mark.parametrize → checklist-only
    新增测试使用 unittest/subTest 和手写多进程脚本；更关键问题是未注册 Bazel 且签名错误，已单独提 issue。
  • [P.H] 类型标注 — Any 必须附注释说明原因 → checklist-only
    moriep_wrapper 对外部 mori op 使用 Any，原因可推断为外部未类型化依赖；建议补注释但不构成具体运行缺陷。

Strengths

  • use_mori_ep 已贯通到 C++ MoeConfig、pybind、Python config adapter 和 server args，配置链路比早期版本完整。
  • MoriEP router 明确区分 global dispatch_ids 与本地 expert id，并通过 expert_ids_are_local 避免本地 id 再套全局 expert_mask。

@LLLLKKKK

LLLLKKKK commented May 11, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #977

Status: BLOCKING

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

Blocking Issues

P1

  • MoriEP 新测试没有纳入 Bazel/CI @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/test/BUILD:66
    • 建议：启用可被 Bazel 调度的 py_test；缺少 mori 时在测试内部 skip，而不是注释 target，并为 distributed/test/moriep_test.py 增加 BUILD 入口。

Non-blocking Suggestions

P2

  • ROCm fake_balance_expert 只注册底层函数，Python 入口仍不可用 @ rtp_llm/models_py/bindings/rocm/RegisterBaseBindings.hpp:107
    • 建议：补齐 ROCm 侧 FakeBalanceExpert nn.Module/compute_ops 兼容入口，或暴露与 CUDA FakeBalanceExpertOp 等价的类并更新 base/init.py 分发。
  • MoriEP router 在 forward 热路径打印 info 日志 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:76
    • 建议：改为 debug 级别，或只在初始化/首轮打印；必要的热路径观测使用采样指标而不是逐 forward info 日志。

Checklist Violations (4 fail / 56 total)

General Principles Checklist

  • [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue MoriEP router 在 forward 热路径打印 info 日志
    MoriEP router 在 prepare/finalize 每次 forward 打 info 日志，属于热路径噪声。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue MoriEP 新测试没有纳入 Bazel/CI
    新增 MoriEP wrapper/router 测试文件没有可执行 Bazel target，ROCm router target 还被注释。
  • [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue MoriEP 新测试没有纳入 Bazel/CI
    MoriEP 是分布式/ROCm 路径，但新增测试未进入 Bazel/CI。
  • [6.1] Quality — 无 per-forward 调试日志 / 噪声热路径输出 → issue MoriEP router 在 forward 热路径打印 info 日志
    MoriEP router prepare/finalize 每次 forward 打 logging.info。

Strengths

  • MoriEP 配置从 server args、Python config、C++ pybind 到策略选择的传播链路基本补齐。
  • shared expert 的 skip_allreduce 语义在 DenseMLP 和 FusedMoe router 接口中显式化，便于后续验证 TP/EP collective 行为。

@LLLLKKKK

LLLLKKKK commented May 11, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #977

Status: BLOCKING

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

Blocking Issues

P1

  • MoriEP router 多卡测试未申请多卡资源 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/test/BUILD:74
    • 建议：为该 Bazel target 显式设置 gpu_count，或拆成 2 卡/4 卡目标并让 CI 实际调度多卡资源。

Non-blocking Suggestions

P2

  • moriep_test 的无 mori skip 在导入阶段失效 @ rtp_llm/models_py/distributed/test/moriep_test.py:36
    • 建议：把 wrapper 加载延迟到 _require_mori() 之后，或让 moriep_wrapper 自身对可选 mori 依赖做 lazy import。
  • MoriEP router 测试未覆盖真实 finalize 路径 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/test/moriep_intranode_router_test.py:227
    • 建议：优先实例化真实 MoriEpIntranodeRouter 跑 prepare/finalize；若保留内联逻辑，也要对齐 _dispatch_ids、chunk 和裁剪语义。

Checklist Violations (10 fail / 60 total)

General Principles Checklist

  • [6.1] Software Engineering — DRY：重复非平凡逻辑被抽取或显式复用 → issue MoriEP router 测试未覆盖真实 finalize 路径
    router 测试复制了 finalize/prepare 关键逻辑，但复制版本没有对齐生产 _dispatch_ids 和 chunk 语义。
  • [6.1] Architecture — 错误语义：fail-fast/retry/fallback/silent 行为显式 → issue moriep_test 的无 mori skip 在导入阶段失效
    moriep_test.py 试图 skip 缺 mori 环境，但顶层加载 wrapper 会在 skip 前失败，错误语义不符合测试意图。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue MoriEP router 多卡测试未申请多卡资源
    MoriEP router 多卡测试已注册，但 target 未声明 gpu_count，可能被调度为单卡后全部 skip。
  • [6.1] Tests — 边界 case 覆盖（空、单元素、最大值） → issue MoriEP router 测试未覆盖真实 finalize 路径
    router 测试没有覆盖真实生产 finalize 的 chunk/cached dispatch id 路径，边界覆盖的等价性不足。
  • [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue MoriEP router 多卡测试未申请多卡资源
    MoriEP 是 ROCm 多卡路径，但 router 测试 target 未申请多卡资源，CI 覆盖可能实际为空。

RTP-LLM Checklist

  • [H] 测试与 CI — 测试覆盖充分：大重构等价覆盖，新功能端到端测试 → issue MoriEP router 多卡测试未申请多卡资源
    MoriEP router 测试未申请多卡资源，且内联 finalize 未对齐生产路径，当前 CI gate 覆盖不充分。
  • [I] 代码质量 — 同一功能用统一工具函数 → issue MoriEP router 测试未覆盖真实 finalize 路径
    router 测试复制生产 finalize 语义但未对齐，导致测试与生产实现分叉。

Python Static-First Checklist

  • [P.F] 语言陷阱 — 禁止模块级 import 副作用 → issue moriep_test 的无 mori skip 在导入阶段失效
    moriep_test.py 模块级加载 moriep_wrapper.py 会触发 wrapper 顶层 import mori，让 skip 逻辑失效。
  • [P.G] 测试规范 — 数据驱动测试用 pytest.mark.parametrize → checklist-only
    新增测试使用 unittest/subTest；项目内已有 unittest 风格，当前更关键缺陷是资源声明和真实路径覆盖，暂不单独升级。
  • [P.H] 类型标注 — Any 必须附注释说明原因 → checklist-only
    router 测试中 Any 主要用于外部 mori 和 stub 模块，原因可从测试上下文推断；建议补注释，但不是独立运行缺陷。

Strengths

  • 本轮已把 MoriEP wrapper/router 测试注册为 Bazel py_test，比上一轮只提交脚本更接近可执行 CI gate。
  • router 测试增加了子进程退出码和超时检查，能比裸脚本更快暴露多进程失败。

@LLLLKKKK

LLLLKKKK commented May 11, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #977

Status: BLOCKING

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

Blocking Issues

P1

  • MoriEP router 测试调用不存在的 _create 参数 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/test/moriep_intranode_router_test.py:346
    • 建议：删除该关键字参数，或把 shmem_group_name 纳入 wrapper API，并补通对应测试。

Non-blocking Suggestions

P2

  • moriep_test 的缺依赖 skip 在导入阶段失效 @ rtp_llm/models_py/distributed/test/moriep_test.py:36
    • 建议：把 wrapper 加载延迟到 _require_mori() 之后，或让 moriep_wrapper 对 mori 做 lazy import。
  • MoriEP router 在 forward 热路径输出 INFO 并同步回读 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:91
    • 建议：将热路径日志降为 debug/限频，并避免 .tolist() 同步回读，必要统计放到诊断开关下。
  • ROCm fake_balance_expert 未接入现有 Python 调用链 @ rtp_llm/models_py/bindings/rocm/RegisterBaseBindings.hpp:109
    • 建议：补齐 ROCm FakeBalanceExpert Python wrapper 并接入平台分发，或移除未使用 binding。

Checklist Violations (12 fail / 76 total)

General Principles Checklist

  • [6.1] Software Engineering — DRY：重复非平凡逻辑被抽取或显式复用 → issue MoriEP router 测试调用不存在的 _create 参数
    router 测试复制 wrapper 初始化路径并直接调用私有 _create，已经和生产 API 签名分叉。
  • [6.1] Architecture — 错误语义：fail-fast/retry/fallback/silent 行为显式 → issue moriep_test 的缺依赖 skip 在导入阶段失效
    缺少 mori 时测试意图 skip，但顶层加载 wrapper 会先触发 ImportError，错误语义与 skip 设计不一致。
  • [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue MoriEP router 在 forward 热路径输出 INFO 并同步回读
    MoriEP router 的 prepare/finalize 位于 per-forward 路径，但使用无条件 INFO 日志。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue MoriEP router 测试调用不存在的 _create 参数
    新增多卡测试已注册，但测试自身调用不存在的 _create 参数，无法作为有效 CI gate。
  • [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue MoriEP router 测试调用不存在的 _create 参数
    ROCm/MoriEP 分布式路径的新增测试在 wrapper 初始化处会因签名不匹配失败。
  • [6.1] Quality — 无 per-forward 调试日志 / 噪声热路径输出 → issue MoriEP router 在 forward 热路径输出 INFO 并同步回读
    MoriEP router prepare/finalize 每次 forward 都 logging.info。

RTP-LLM Checklist

  • [D] 性能 — 设备同步 .item()/.cpu()/.tolist() 在关键路径 → issue MoriEP router 在 forward 热路径输出 INFO 并同步回读
    chunked prepare 中 dispatch_recv_token_num.tolist() 会在 forward 路径同步回读。
  • [F] 跨平台 — CUDA/ROCm 两侧 binding 对称清理 → issue ROCm fake_balance_expert 未接入现有 Python 调用链
    ROCm 只新增底层 fake_balance_expert binding，Python 平台入口仍是 NotImplemented。
  • [H] 测试与 CI — 测试覆盖充分：大重构等价覆盖，新功能端到端测试 → issue MoriEP router 测试调用不存在的 _create 参数
    MoriEP 多卡测试 target 已加入，但脚本调用 wrapper 签名错误，覆盖不可用。
  • [I] 代码质量 — 同一功能用统一工具函数 → issue ROCm fake_balance_expert 未接入现有 Python 调用链
    ROCm fake balance 新 binding 没有复用现有 Python FakeBalanceExpert 入口，底层能力与调用链分离。

Python Static-First Checklist

  • [P.F] 语言陷阱 — 禁止模块级 import 副作用 → issue moriep_test 的缺依赖 skip 在导入阶段失效
    moriep_wrapper.py 顶层 import mori，测试模块级加载时会绕过 skip 保护。
  • [P.H] 类型标注 — Any 必须附注释说明原因 → checklist-only
    MoriEP wrapper/test 对外部 mori op 使用 Any，原因可从外部未类型化依赖推断；建议补注释但不构成独立运行缺陷。

Strengths

  • MoriEP 配置已经贯通到 server args、Python config、C++ config 和 pybind，配置链路比早期版本完整。
  • router 显式区分 global dispatch ids 与本地 expert ids，并用 expert_ids_are_local 避免本地 id 再套全局 expert mask。

@jacobwin-ai
jacobwin-ai force-pushed the develop/mori_355x_support_0507_after_rebase branch from 0fba1a4 to 15d78e9 Compare May 11, 2026 11:58
@LLLLKKKK

LLLLKKKK commented May 13, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #977

Status: BLOCKING

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

Blocking Issues

P1

  • CUDA DeepEP finalize 引用了未定义的 skip_allreduce @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/routers/deepep_low_latency_router.py:321
    • 建议：恢复完整参数链路：在 FusedMoe.forward、router finalize 签名和调用处统一传递 skip_allreduce，或删除未使用的跳过逻辑。

Non-blocking Suggestions

P2

  • ROCm fake_balance_expert 绑定未暴露 Python 期望的类接口 @ rtp_llm/models_py/bindings/rocm/RegisterBaseBindings.hpp:105
    • 建议：按 CUDA 侧 API 注册 FakeBalanceExpertOp 类并提供 forward()，或同步修改 Python wrapper 按平台调用函数接口。
  • MoriEP router 在每次 forward 打 info 日志 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:76
    • 建议：删除热路径 info 日志，或降为 debug 并受显式调试开关控制。
  • MoriEP chunk 路径把通信计数同步回 CPU @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:159
    • 建议：避免 forward 中 CPU 回读；让 MORI 返回 CPU 计数，或把 chunk 计数累加保持在设备侧/延后同步。

Checklist Violations (11 fail / 97 total)

General Principles Checklist

  • [6.1] Software Engineering — LSP：子类/重写保持基类契约 → issue CUDA DeepEP finalize 引用了未定义的 skip_allreduce
    CUDA router finalize 引用未在签名中传入的 skip_allreduce。
  • [6.1] Architecture — 错误语义：fail-fast/retry/fallback/silent 行为显式 → issue CUDA DeepEP finalize 引用了未定义的 skip_allreduce
    未定义变量会在运行期 NameError，而非配置阶段显式失败。
  • [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue MoriEP router 在每次 forward 打 info 日志
    prepare/finalize 每次 forward 使用 logging.info，属于热路径噪声。
  • [6.1] Architecture — 兼容性：公开 API/持久数据/配置/环境迁移安全 → issue ROCm fake_balance_expert 绑定未暴露 Python 期望的类接口
    ROCm 暴露函数接口，Python 现有入口期望 FakeBalanceExpertOp 类。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue CUDA DeepEP finalize 引用了未定义的 skip_allreduce
    新增/改动 skip_allreduce 路径未见覆盖 CUDA DeepEP finalize。
  • [6.1] Quality — 无 per-forward 调试日志 / 噪声热路径输出 → issue MoriEP router 在每次 forward 打 info 日志
    MoriEP prepare/finalize 在 forward 路径逐次输出 info。

RTP-LLM Checklist

  • [B] 正确性与逻辑 — 逻辑错误、off-by-one、null/zero 检查 → issue CUDA DeepEP finalize 引用了未定义的 skip_allreduce
    skip_allreduce 未定义即被使用，属于直接运行时逻辑错误。
  • [D] 性能 — 设备同步 .item()/.cpu()/.tolist() 在关键路径 → issue MoriEP chunk 路径把通信计数同步回 CPU
    _prepare_chunked 对 dispatch_recv_token_num 调用 .tolist()。
  • [F] 跨平台 — CUDA/ROCm 两侧 binding 对称清理 → issue ROCm fake_balance_expert 绑定未暴露 Python 期望的类接口
    CUDA/Python 期望类接口，ROCm 新增为底层函数接口。
  • [H] 测试与 CI — 测试覆盖充分：大重构等价覆盖，新功能端到端测试 → issue CUDA DeepEP finalize 引用了未定义的 skip_allreduce
    MoriEP 有测试，但 CUDA DeepEP skip_allreduce 回归路径缺覆盖。
  • [I] 代码质量 — 同一功能用统一工具函数 → issue ROCm fake_balance_expert 绑定未暴露 Python 期望的类接口
    ROCm fake_balance_expert 未复用现有 FakeBalanceExpertOp Python 入口。

Strengths

  • 新增 use_mori_ep 配置从 Python、C++、pybind 和 server args 到 MoE strategy 做了完整贯通。
  • MoriEP 增加了独立 wrapper 测试和多 GPU router 集成测试，覆盖了新 backend 的基本 dispatch/combine 路径。

@LLLLKKKK

LLLLKKKK commented May 13, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #977

Status: BLOCKING

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

Blocking Issues

P1

  • PD 分离 smoke golden 被改成 pd_sep=false @ rtp_llm/test/smoke/data/model/qwen3_next/q_r_next_fp8_tp2_pd_sep.json:128
    • 建议：不要把 pd_sep golden 改成 false；先修复测试启动参数或后端路径，确认 PD 分离实际启用后再更新 golden。

Non-blocking Suggestions

P2

  • ROCm fake_balance_expert 绑定与 Python 调用接口不一致 @ rtp_llm/models_py/bindings/rocm/RegisterBaseBindings.hpp:105
    • 建议：按 CUDA 侧暴露 FakeBalanceExpertOp 类和 forward()，或同步修改 Python wrapper 做平台分发。
  • MoriEP router 在 forward 热路径输出 info 日志 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:76
    • 建议：删除热路径 info 日志，或降级为 debug 并受显式调试开关控制。
  • MoriEP chunk 路径同步读取 dispatch 计数 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:159
    • 建议：让 MORI 返回 CPU 计数，或将 chunk 计数累加保持在设备侧/延后同步，避免 forward 中隐式同步。

Checklist Violations (13 fail / 88 total)

General Principles Checklist

  • [6.1] Software Engineering — LSP：子类/重写保持基类契约 → issue ROCm fake_balance_expert 绑定与 Python 调用接口不一致
    ROCm fake_balance 新绑定没有保持 Python 侧既有 FakeBalanceExpertOp.forward 调用契约。
  • [6.1] Architecture — 错误语义：fail-fast/retry/fallback/silent 行为显式 → issue ROCm fake_balance_expert 绑定与 Python 调用接口不一致
    ROCm fake_balance 接口不一致会在启用配置后运行期 AttributeError，而不是注册阶段显式暴露。
  • [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue MoriEP router 在 forward 热路径输出 info 日志
    MoriEP router prepare/finalize 在 forward 路径逐次输出 info 日志。
  • [6.1] Architecture — 兼容性：公开 API/持久数据/配置/环境迁移安全 → issue ROCm fake_balance_expert 绑定与 Python 调用接口不一致
    新增 ROCm binding 未兼容 Python 现有 FakeBalanceExpertOp API。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue PD 分离 smoke golden 被改成 pd_sep=false
    PD sep golden 被改为 pd_sep=false，削弱了对应 smoke 对 PD 分离启用状态的覆盖。
  • [6.1] Quality — 逻辑变更未混入无关格式化 → checklist-only
    moe_group_args.py 和 ConfigModules.h 有少量格式化混入；不单独升级 issue，但建议后续与逻辑改动分离。
  • [6.1] Quality — 无 per-forward 调试日志 / 噪声热路径输出 → issue MoriEP router 在 forward 热路径输出 info 日志
    MoriEP router 在 prepare/finalize 热路径使用 logging.info。

RTP-LLM Checklist

  • [D] 性能 — 设备同步 .item()/.cpu()/.tolist() 在关键路径 → issue MoriEP chunk 路径同步读取 dispatch 计数
    _prepare_chunked 对 dispatch_recv_token_num 调用 .tolist()。
  • [F] 跨平台 — CUDA/ROCm 两侧 binding 对称清理 → issue ROCm fake_balance_expert 绑定与 Python 调用接口不一致
    CUDA 暴露 FakeBalanceExpertOp 类；ROCm 新增为 fake_balance_expert 函数，Python 调用不对称。
  • [H] 测试与 CI — 测试覆盖充分：大重构等价覆盖，新功能端到端测试 → issue PD 分离 smoke golden 被改成 pd_sep=false
    PD sep golden 期望值被改成 false，无法覆盖该 smoke 是否真正启用 PD 分离。
  • [H] 测试与 CI — Smoke 测试名称/配置/参数一致性 → issue PD 分离 smoke golden 被改成 pd_sep=false
    文件名包含 pd_sep，但 golden 中 aux_info.pd_sep 改为 false，名称与期望行为不一致。
  • [I] 代码质量 — 同一功能用统一工具函数 → issue ROCm fake_balance_expert 绑定与 Python 调用接口不一致
    fake_balance 在 CUDA/Python 侧使用 FakeBalanceExpertOp 类，ROCm 侧新增为不同函数入口。

Python Static-First Checklist

  • [P.B] 错误处理 — 异常链用 raise X from Y → checklist-only
    部分 except ImportError as e 后重新 raise 未用 from e；当前上下文日志保留 detail，不单独升级为 issue。

Strengths

  • use_mori_ep 配置从 server args、Python/C++ config、pybind 到 ROCm strategy 做了完整贯通。
  • MoriEP wrapper 和 router 增加了多 GPU 测试，覆盖了 dispatch/combine 的基本路径。

@LLLLKKKK

LLLLKKKK commented May 13, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #977

Status: BLOCKING

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

Blocking Issues

P1

  • PD 分离 smoke golden 被改成 pd_sep=false @ rtp_llm/test/smoke/data/model/qwen3_next/q_r_next_fp8_tp2_pd_sep.json:128
    • 建议：恢复 pd_sep=true，确认启动参数和后端路径后再更新 golden。

Non-blocking Suggestions

P2

  • ROCm fake_balance_expert 绑定与 Python 调用接口不一致 @ rtp_llm/models_py/bindings/rocm/RegisterBaseBindings.hpp:109
    • 建议：对齐 CUDA 的 FakeBalanceExpertOp 类契约，或同步改 Python wrapper 并补测试。
  • MoriEP router 在 forward 热路径输出 info 日志 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:91
    • 建议：删除 info 日志，或改为 debug/限频并受诊断开关控制。
  • MoriEP chunk 路径同步读取 dispatch 计数 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:192
    • 建议：让计数保持设备侧或由 MORI 返回 CPU 计数，避免 forward 隐式同步。

Checklist Violations (12 fail / 76 total)

General Principles Checklist

  • [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue MoriEP router 在 forward 热路径输出 info 日志
    forward 路径逐次 logging.info。
  • [6.1] Architecture — 兼容性：公开 API/持久数据/配置/环境迁移安全 → issue ROCm fake_balance_expert 绑定与 Python 调用接口不一致
    ROCm binding 与 Python 契约不一致。
  • [6.1] Quality — 逻辑变更未混入无关格式化 → checklist-only
    轻微规范缺口，无独立运行缺陷。
  • [6.1] Quality — 无 per-forward 调试日志 / 噪声热路径输出 → issue MoriEP router 在 forward 热路径输出 info 日志
    forward 路径逐次 logging.info。
  • [6.1] Software Engineering — LSP：子类/重写保持基类契约 → issue ROCm fake_balance_expert 绑定与 Python 调用接口不一致
    ROCm binding 与 Python 契约不一致。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue PD 分离 smoke golden 被改成 pd_sep=false
    pd_sep golden 为 false，覆盖失效。

RTP-LLM Checklist

  • [D] 性能 — 设备同步 .item()/.cpu()/.tolist() 在关键路径 → issue MoriEP chunk 路径同步读取 dispatch 计数
    forward 中对 tensor 调用 .tolist()。
  • [F] 跨平台 — CUDA/ROCm 两侧 binding 对称清理 → issue ROCm fake_balance_expert 绑定与 Python 调用接口不一致
    ROCm binding 与 Python 契约不一致。
  • [H] 测试与 CI — 测试覆盖充分：大重构等价覆盖，新功能端到端测试 → issue PD 分离 smoke golden 被改成 pd_sep=false
    pd_sep golden 为 false，覆盖失效。
  • [H] 测试与 CI — Smoke 测试名称/配置/参数一致性 → issue PD 分离 smoke golden 被改成 pd_sep=false
    pd_sep golden 为 false，覆盖失效。
  • [I] 代码质量 — 同一功能用统一工具函数 → issue ROCm fake_balance_expert 绑定与 Python 调用接口不一致
    ROCm binding 与 Python 契约不一致。

Python Static-First Checklist

  • [P.B] 错误处理 — 异常链用 raise X from Y → checklist-only
    轻微规范缺口，无独立运行缺陷。

Strengths

  • use_mori_ep 配置链路已贯通。
  • 新增了 MoriEP/ROCm 基础测试。

@jacobwin-ai
jacobwin-ai force-pushed the develop/mori_355x_support_0507_after_rebase branch from 467b898 to 47e1823 Compare May 13, 2026 13:44
@LLLLKKKK

LLLLKKKK commented May 13, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #977

Status: LGTM

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

lgtm ready to ci

Non-blocking Suggestions

P2

  • ROCm fake_balance_expert 绑定与 Python 调用接口不一致 @ rtp_llm/models_py/bindings/rocm/RegisterBaseBindings.hpp:109
    • 建议：对齐 CUDA 注册 FakeBalanceExpertOp 类和 forward()，或同步修改 Python wrapper 做平台分发并补 ROCm 测试。
  • MoriEP router 在 forward 热路径输出 info 日志 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:91
    • 建议：删除热路径 info 日志，或改为 debug/限频并受显式诊断开关控制。
  • MoriEP chunk 路径同步读取 dispatch 计数 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:192
    • 建议：让计数保持设备侧批量累加，或由 MORI 返回 CPU 计数/延后到非热路径同步。

Checklist Violations (11 fail / 76 total)

General Principles Checklist

  • [6.1] Software Engineering — LSP：子类/重写保持基类契约 → issue ROCm fake_balance_expert 绑定与 Python 调用接口不一致
    ROCm 新增 fake_balance_expert 函数入口未保持 Python 侧 FakeBalanceExpertOp.forward 契约。
  • [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue MoriEP router 在 forward 热路径输出 info 日志
    MoriEP router 在 prepare/finalize forward 热路径逐次 logging.info，默认日志过噪。
  • [6.1] Architecture — 兼容性：公开 API/持久数据/配置/环境迁移安全 → issue ROCm fake_balance_expert 绑定与 Python 调用接口不一致
    ROCm fake_balance_expert binding 与 Python 既有 FakeBalanceExpertOp 契约不一致，配置启用后迁移不安全。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue ROCm fake_balance_expert 绑定与 Python 调用接口不一致
    ROCm fake_balance_expert 新 binding 未看到覆盖 Python 现有入口契约的测试。
  • [6.1] Quality — 无 per-forward 调试日志 / 噪声热路径输出 → issue MoriEP router 在 forward 热路径输出 info 日志
    MoriEpIntranodeRouter.prepare/finalize 在 forward 路径使用 info 日志。

RTP-LLM Checklist

  • [D] 性能 — 设备同步 .item()/.cpu()/.tolist() 在关键路径 → issue MoriEP chunk 路径同步读取 dispatch 计数
    _prepare_chunked 对 dispatch_recv_token_num 调用 .tolist()，会把 GPU 计数同步回 CPU。
  • [F] 跨平台 — CUDA/ROCm 两侧 binding 对称清理 → issue ROCm fake_balance_expert 绑定与 Python 调用接口不一致
    CUDA 暴露 FakeBalanceExpertOp 类；ROCm 新增为 fake_balance_expert 函数，Python 调用契约不对称。
  • [H] 测试与 CI — 测试覆盖充分：大重构等价覆盖，新功能端到端测试 → issue ROCm fake_balance_expert 绑定与 Python 调用接口不一致
    ROCm fake_balance_expert 新 binding 缺少覆盖现有 Python FakeBalanceExpert 调用链的测试。
  • [I] 代码质量 — 同一功能用统一工具函数 → issue ROCm fake_balance_expert 绑定与 Python 调用接口不一致
    fake_balance 在 CUDA/Python 侧是 FakeBalanceExpertOp 类接口，ROCm 侧新增为不同函数入口。

Python Static-First Checklist

  • [P.B] 错误处理 — 异常链用 raise X from Y → checklist-only
    部分 except ImportError as e 后重新 raise 未用 from e；当前日志包含 detail，主要影响诊断链完整性，不构成独立运行缺陷。
  • [P.H] 类型标注 — Any 必须附注释说明原因 → checklist-only
    MoriEP wrapper/router 对未类型化外部 MORI op 使用 Any，原因可推断但注释不完整；建议后续用 Protocol 收敛，不作为独立缺陷。

Strengths

  • use_mori_ep 已贯通 server args、Python/C++ config、pybind 和 ROCm strategy。
  • MoriEP router 明确区分 global dispatch ids 与本地 expert ids，避免本地 id 再套全局 expert mask。

@wht21

wht21 commented May 13, 2026

Copy link
Copy Markdown
Collaborator

internal source has been updated, please review the changes!

2 similar comments
@wht21

wht21 commented May 13, 2026

Copy link
Copy Markdown
Collaborator

internal source has been updated, please review the changes!

@wht21

wht21 commented May 14, 2026

Copy link
Copy Markdown
Collaborator

internal source has been updated, please review the changes!

@jacobwin-ai
jacobwin-ai force-pushed the develop/mori_355x_support_0507_after_rebase branch from 47e1823 to c589394 Compare May 14, 2026 01:17
@LLLLKKKK

LLLLKKKK commented May 14, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #977

Status: LGTM

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

lgtm ready to ci

Non-blocking Suggestions

P2

  • ROCm fake_balance_expert 绑定与 Python 调用接口不一致 @ rtp_llm/models_py/bindings/rocm/RegisterBaseBindings.hpp:109
    • 建议：对齐 CUDA 注册 FakeBalanceExpertOp 类和 forward()，或同步修改 Python wrapper 做平台分发并补 ROCm 调用链测试。
  • MoriEP router 在 forward 热路径输出 info 日志 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:91
    • 建议：删除热路径 info 日志，或降为 debug/限频并受显式诊断开关控制。
  • MoriEP chunk 路径同步读取 dispatch 计数 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:192
    • 建议：让计数保持设备侧批量累加，或由 MORI 直接返回 CPU 计数/延后到非热路径同步。

Checklist Violations (11 fail / 76 total)

General Principles Checklist

  • [6.1] Software Engineering — LSP：子类/重写保持基类契约 → issue ROCm fake_balance_expert 绑定与 Python 调用接口不一致
    ROCm fake_balance 新 binding 没保持 Python 侧 FakeBalanceExpertOp.forward 调用契约。
  • [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue MoriEP router 在 forward 热路径输出 info 日志
    MoriEP router 在 prepare/finalize forward 热路径逐次 logging.info。
  • [6.1] Architecture — 兼容性：公开 API/持久数据/配置/环境迁移安全 → issue ROCm fake_balance_expert 绑定与 Python 调用接口不一致
    ROCm fake_balance binding 与 Python 既有 FakeBalanceExpertOp 契约不一致。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue ROCm fake_balance_expert 绑定与 Python 调用接口不一致
    新增 ROCm fake_balance binding 未看到覆盖 Python 现有 FakeBalanceExpert 调用链的测试。
  • [6.1] Quality — 无 per-forward 调试日志 / 噪声热路径输出 → issue MoriEP router 在 forward 热路径输出 info 日志
    MoriEpIntranodeRouter.prepare/finalize 在 forward 路径使用 info 日志。

RTP-LLM Checklist

  • [D] 性能 — 设备同步 .item()/.cpu()/.tolist() 在关键路径 → issue MoriEP chunk 路径同步读取 dispatch 计数
    _prepare_chunked 对 dispatch_recv_token_num 调用 .tolist()，会把 GPU 计数同步回 CPU。
  • [F] 跨平台 — CUDA/ROCm 两侧 binding 对称清理 → issue ROCm fake_balance_expert 绑定与 Python 调用接口不一致
    CUDA 暴露 FakeBalanceExpertOp 类；ROCm 新增为 fake_balance_expert 函数，Python 调用契约不对称。
  • [H] 测试与 CI — 测试覆盖充分：大重构等价覆盖，新功能端到端测试 → issue ROCm fake_balance_expert 绑定与 Python 调用接口不一致
    ROCm fake_balance 新 binding 缺少覆盖现有 Python FakeBalanceExpert 调用链的测试。
  • [I] 代码质量 — 同一功能用统一工具函数 → issue ROCm fake_balance_expert 绑定与 Python 调用接口不一致
    fake_balance 在 CUDA/Python 侧是 FakeBalanceExpertOp 类接口，ROCm 侧新增为不同函数入口。

Python Static-First Checklist

  • [P.B] 错误处理 — 异常链用 raise X from Y → checklist-only
    部分 except ImportError as e 后重新 raise 未用 from e；当前日志包含 detail，主要影响诊断链完整性，不构成独立运行缺陷。
  • [P.H] 类型标注 — Any 必须附注释说明原因 → checklist-only
    MoriEP wrapper/router 对未类型化外部 MORI op 使用 Any，原因可推断但注释不完整；建议后续用 Protocol 收敛，不作为独立缺陷。

Strengths

  • use_mori_ep 已贯通 server args、Python/C++ config、pybind 和 ROCm strategy。
  • MoriEP router 明确区分 global dispatch ids 与本地 expert ids，避免本地 id 再套全局 expert mask。

@wht21

wht21 commented May 14, 2026

Copy link
Copy Markdown
Collaborator

internal source has been updated, please review the changes!

@jacobwin-ai
jacobwin-ai force-pushed the develop/mori_355x_support_0507_after_rebase branch from c589394 to 307caeb Compare May 14, 2026 01:49
@LLLLKKKK

LLLLKKKK commented May 14, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #977

Status: LGTM

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

lgtm ready to ci

Non-blocking Suggestions

P2

  • ROCm fake_balance_expert 绑定未接入 Python 入口 @ rtp_llm/models_py/bindings/rocm/RegisterBaseBindings.hpp:109
    • 建议：按 CUDA 侧暴露 FakeBalanceExpertOp 类和 forward()，或同步更新 ROCm Python wrapper 并补调用链测试。
  • MoriEP router 在 forward 热路径输出 INFO 日志 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:91
    • 建议：删除热路径 info 日志，或改为 debug/限频并受显式诊断开关控制。
  • MoriEP chunk 路径同步读取 dispatch 计数 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:192
    • 建议：让 MORI 直接返回 CPU 计数，或把计数累加保持在设备侧/延后到非热路径同步。

Checklist Violations (10 fail / 76 total)

General Principles Checklist

  • [6.1] Software Engineering — LSP：子类/重写保持基类契约 → issue ROCm fake_balance_expert 绑定未接入 Python 入口
    ROCm 新增 fake_balance_expert 函数入口未保持 Python 侧 FakeBalanceExpertOp.forward 调用契约。
  • [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue MoriEP router 在 forward 热路径输出 INFO 日志
    prepare/finalize 每次 forward 都 logging.info，默认级别过噪。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue ROCm fake_balance_expert 绑定未接入 Python 入口
    ROCm fake_balance 新 binding 未看到覆盖 Python 现有 FakeBalanceExpert 调用链的测试。
  • [6.1] Quality — 无 per-forward 调试日志 / 噪声热路径输出 → issue MoriEP router 在 forward 热路径输出 INFO 日志
    MoriEpIntranodeRouter.prepare/finalize 在 forward 路径使用 info 日志。

RTP-LLM Checklist

  • [D] 性能 — 设备同步 .item()/.cpu()/.tolist() 在关键路径 → issue MoriEP chunk 路径同步读取 dispatch 计数
    _prepare_chunked 对 dispatch_recv_token_num 调用 .tolist()，会把 GPU 计数同步回 CPU。
  • [F] 跨平台 — CUDA/ROCm 两侧 binding 对称清理 → issue ROCm fake_balance_expert 绑定未接入 Python 入口
    CUDA 暴露 FakeBalanceExpertOp 类；ROCm 新增为 fake_balance_expert 函数，Python 调用契约不对称。
  • [H] 测试与 CI — 测试覆盖充分：大重构等价覆盖，新功能端到端测试 → issue ROCm fake_balance_expert 绑定未接入 Python 入口
    ROCm fake_balance 新 binding 缺少覆盖现有 Python FakeBalanceExpert 调用链的测试。
  • [I] 代码质量 — 同一功能用统一工具函数 → issue ROCm fake_balance_expert 绑定未接入 Python 入口
    fake_balance 在 CUDA/Python 侧是 FakeBalanceExpertOp 类接口，ROCm 侧新增为不同函数入口。

Python Static-First Checklist

  • [P.B] 错误处理 — 异常链用 raise X from Y → checklist-only
    部分 except ImportError 后重新 raise 未用 from e；当前日志保留 detail，主要影响诊断链完整性，不构成独立运行缺陷。
  • [P.H] 类型标注 — Any 必须附注释说明原因 → checklist-only
    MoriEP wrapper/router 对未类型化外部 MORI op 使用 Any，原因可推断但注释不完整；建议后续用 Protocol 收敛。

Strengths

  • use_mori_ep 已贯通 server args、Python/C++ config、pybind 和 ROCm strategy，配置链路比早期版本完整。
  • MoriEP router 明确区分 global dispatch ids 与本地 expert ids，并用 expert_ids_are_local 避免本地 id 再套全局 expert mask。

@wht21

wht21 commented May 14, 2026

Copy link
Copy Markdown
Collaborator

internal source has been updated, please review the changes!

@jacobwin-ai
jacobwin-ai force-pushed the develop/mori_355x_support_0507_after_rebase branch from 307caeb to ebb9a47 Compare May 14, 2026 02:33
@LLLLKKKK

LLLLKKKK commented May 14, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #977

Status: LGTM

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

lgtm ready to ci

Non-blocking Suggestions

P2

  • ROCm FakeBalanceExpert 绑定未接到 Python API @ rtp_llm/models_py/bindings/rocm/RegisterBaseBindings.hpp:104
    • 建议：为 ROCm 增加与 CUDA FakeBalanceExpert 等价的 Python wrapper，并从 ROCm base 分支导出它，补充对应单测。
  • MoriEP 路由在 forward 热路径打印 info 日志 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:90
    • 建议：删除热路径 info 日志，或降为 debug 并加采样/一次性日志。
  • MoriEP chunked 路径回读 CUDA tensor 触发同步 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:179
    • 建议：让 MORI 返回 CPU 计数，或用异步拷贝/延迟同步；避免在 forward 热路径直接 .tolist()。

Checklist Violations (5 fail / 72 total)

General Principles Checklist

  • [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue MoriEP 路由在 forward 热路径打印 info 日志
    MoriEP router 的 prepare/finalize 每次 forward 都 info 日志，属于噪声热路径输出。
  • [6.1] Quality — 无 per-forward 调试日志 / 噪声热路径输出 → issue MoriEP 路由在 forward 热路径打印 info 日志
    prepare()/finalize() 内有 per-forward logging.info。

RTP-LLM Checklist

  • [D] 性能 — 设备同步 .item()/.cpu()/.tolist() 在关键路径 → issue MoriEP chunked 路径回读 CUDA tensor 触发同步
    chunked dispatch 后立即 dispatch_recv_token_num.tolist()。
  • [F] 跨平台 — CUDA/ROCm 两侧 binding 对称清理 → issue ROCm FakeBalanceExpert 绑定未接到 Python API
    ROCm 只新增 raw pybind 函数，Python FakeBalanceExpert 导出仍未切到 ROCm 实现。
  • [I] 代码质量 — 同一功能用统一工具函数 → issue ROCm FakeBalanceExpert 绑定未接到 Python API
    FakeBalanceExpert 在 ROCm 只暴露 raw binding，未复用 CUDA 侧 Python API 形态。

Strengths

  • MoriEP wrapper、ROCm router 和多 GPU 测试路径覆盖了主要 dispatch/combine 流程。
  • 配置层新增 use_mori_ep 并处理了与 use_all_gather、low latency 的基本互斥关系。

@wht21

wht21 commented May 14, 2026

Copy link
Copy Markdown
Collaborator

internal source has been updated, please review the changes!

@LLLLKKKK

LLLLKKKK commented May 14, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #977

Status: LGTM

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

lgtm ready to ci

Non-blocking Suggestions

P2

  • MoriEP chunked 路径延迟 combine 可能复用错误 dispatch 状态 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:149
    • 建议：将 dispatch、expert execute、combine 绑定为单 chunk 生命周期，或封装/验证 MORI op 的 per-chunk state，补充超过 max_tokens 的测试。
  • MoriEP forward 热路径留下 INFO 日志和 GPU 回读 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:91
    • 建议：删除 per-forward INFO 或改为 debug/采样日志；chunk 统计避免 .tolist() 同步，或移到非热路径收集。
  • ROCm fake balance binding 与现有 Python 调用不兼容 @ rtp_llm/models_py/bindings/rocm/RegisterBaseBindings.hpp:104
    • 建议：ROCm 侧对齐 CUDA 的 FakeBalanceExpertOp class API，或同步修改 Python 包装按平台调用新函数并补测试。

Checklist Violations (5 fail / 56 total)

General Principles Checklist

  • [6.1] Architecture — 状态不变量：创建/更新/失败/重试/回滚路径有效 → issue MoriEP chunked 路径延迟 combine 可能复用错误 dispatch 状态
    chunked 路径多次 dispatch 后延迟 combine，dispatch/combine 状态生命周期未按 chunk 闭环。
  • [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue MoriEP forward 热路径留下 INFO 日志和 GPU 回读
    prepare 和 finalize 每次 MoE forward 打 INFO，热路径日志频率过高。
  • [6.1] Architecture — 兼容性：公开 API/持久数据/配置/环境迁移安全 → issue ROCm fake balance binding 与现有 Python 调用不兼容
    ROCm 暴露函数式 fake_balance_expert，现有 Python 包装调用 FakeBalanceExpertOp class API。
  • [6.1] Tests — 边界 case 覆盖（空、单元素、最大值） → issue MoriEP chunked 路径延迟 combine 可能复用错误 dispatch 状态
    新增 router 有 num_tokens > max_inp_tokens 的 chunked 分支，但测试只覆盖小 token 正常路径。
  • [6.1] Quality — 无 per-forward 调试日志 / 噪声热路径输出 → issue MoriEP forward 热路径留下 INFO 日志和 GPU 回读
    MoriEP router 的 prepare/finalize 在每次 forward 输出 INFO。

Strengths

  • 新增 use_mori_ep 从参数、Python config 到 C++ pybind 的透传链路较完整。
  • MoriEP wrapper 使用 lazy import，避免未安装 mori 时影响普通路径导入。
  • 新增多 GPU MoriEP wrapper/router 测试，覆盖基础 dispatch/combine 行为。

@jacobwin-ai
feat: add MoriEP expert parallelism support for ROCm
fd2ea43 
Add MoRI-based expert parallelism (EP) as an alternative to DeepEP for
ROCm GPUs, enabling efficient intranode and internode dispatch/combine
for MoE models.

Core components:
- MoriEPWrapper: singleton wrapper around mori EpDispatchCombineOp
- MoriEpIntranodeRouter: EP router with dispatch/combine flow,
  chunked dispatch for large token counts, and global-to-local ID
  remapping for fused expert kernels
- MoriEpFp4Strategy: strategy pairing MoRI router with FP4 executor
- RocmEpNormalStrategy: MoRI router as first-class option alongside
  DeepEP, with mutual exclusion validation

Config flow:
- --use_mori_ep CLI flag (env: USE_MORI_EP) → deep_ep_config →
  auto_configure_deepep() → moe_config.use_mori_ep
- C++ MoeConfig.use_mori_ep field with pybind binding
- MoEConfigAdapter exposes use_mori_ep and use_deepep_moe

Bazel: moriep_wrapper py_library target with modules dependency.
@jacobwin-ai
feat: add fake balance expert and MoriEP tests
2233147 
- FakeBalanceExpert ROCm C++ op for expert load balancing testing
- MoriEpIntranodeRouter unit tests
- BlockPoolConfigHelper adjustment for MoE cache allocation
@jacobwin-ai
fix: address P1 issues in MoriEP PR
7253fd3 
- Remove TP-only unified allreduce optimization that caused numerical
  differences vs main branch (12 smoke test failures)
- Remove dead skip_allreduce code from FusedMoe.forward() and the
  entire router finalize() chain (base class, all implementations,
  associated test)
- Fix golden JSON files with Unicode curly quotes (rebuild with json.dump)
- Update stale golden values from CI actual output
- Refactor moriep_intranode_router_test to use production
  MoriEpIntranodeRouter instead of duplicated inline helpers
- Fix use_all_gather early return swallowing explicit USE_MORI_EP config
@jacobwin-ai
fix: route PD smoke prompt_batch through / to preserve pd_sep
d4dff4a 
BatchGenerateCall (used by /batch_infer) bypasses PrefillRpcServer,
so PD separation is not applied. Add explicit endpoint "/" to the
PD golden so that prompt_batch queries go through the PD-aware
GenerateStreamCall path, matching main's behavior.
@jacobwin-ai
fix: update golden apostrophe to match model output (U+2019)
597eabf 
The model generates Unicode right single quote (') in PD streaming
path, not ASCII apostrophe ('). Update golden to match actual output.
@jacobwin-ai
jacobwin-ai force-pushed the develop/mori_355x_support_0507_after_rebase branch from 4fb691f to 597eabf Compare May 14, 2026 04:57
@LLLLKKKK

LLLLKKKK commented May 14, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #977

Status: BLOCKING

Summary: P0/0 · P1/1 · P2/1 · P3/0

Blocking Issues

P1

  • MoriEP 使用 WORLD group 会错配 EP rank @ rtp_llm/models_py/distributed/moriep_wrapper.py:71
    • 建议：按 EP group 初始化 mori，传 ep_size 和 EP 组内唯一 rank；若暂不支持 TP×EP/DP，显式校验 ep_size == world_size 并补对应测试。

Non-blocking Suggestions

P2

  • MoriEP router 在每次 forward 打印 info 日志 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:76
    • 建议：删除 per-forward info 日志，或降为 debug 并加采样/首 N 次限制。

Checklist Violations (10 fail / 109 total)

General Principles Checklist

  • [6.1] Architecture — 状态不变量：创建/更新/失败/重试/回滚路径有效 → issue MoriEP 使用 WORLD group 会错配 EP rank
    MoriEP wrapper 创建时用 WORLD group 和 world_size，却把 rank 设置为 ep_rank；ep_size != world_size 时 rank 唯一性和通信规模不变量不成立。
  • [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue MoriEP router 在每次 forward 打印 info 日志
    MoriEP router 在 prepare/finalize 每次 forward 打 info 日志，属于热路径噪声。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue MoriEP 使用 WORLD group 会错配 EP rank
    新增测试覆盖 world_size=2/4 且 ep_size==world_size，但未覆盖 TP×EP/DP 中 ep_size != world_size 的初始化和 dispatch。
  • [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue MoriEP 使用 WORLD group 会错配 EP rank
    ROCm 多卡测试未覆盖 ep_size 小于 world_size 的分布式拓扑，而配置代码支持该拓扑。
  • [6.1] Quality — 无 per-forward 调试日志 / 噪声热路径输出 → issue MoriEP router 在每次 forward 打印 info 日志
    MoriEpIntranodeRouter.prepare/finalize 每次 forward 都 logging.info。

RTP-LLM Checklist

  • [B] 正确性与逻辑 — 逻辑错误、off-by-one、null/zero 检查 → issue MoriEP 使用 WORLD group 会错配 EP rank
    MoriEP 配置混用 ep_rank 与 world_size/WORLD，ep_size != world_size 时 rank/规模逻辑错误。
  • [D] 性能 — 通信 / buffer 开销评估 → issue MoriEP 使用 WORLD group 会错配 EP rank
    通信规模使用 world_size 而非 EP group，会在 TP×EP/DP 下放大/错配通信。
  • [E] 分布式 — 跨 rank 数据一致性 → issue MoriEP 使用 WORLD group 会错配 EP rank
    ep_rank 在 ep_size < world_size 时会重复，但 MoriEP 配置传 world_size，跨 rank 身份不一致。
  • [E] 分布式 — collective op 匹配 → issue MoriEP 使用 WORLD group 会错配 EP rank
    mori shmem 注册 WORLD group，但配置按 ep_rank/专家分片构造，collective 参与者与 EP 语义不匹配。
  • [H] 测试与 CI — 测试覆盖充分：大重构等价覆盖，新功能端到端测试 → issue MoriEP 使用 WORLD group 会错配 EP rank
    MoriEP 新测试没有覆盖 ep_size != world_size 的 TP×EP/DP 拓扑，无法发现 WORLD group 错配。

Strengths

  • 新增 MoriEP 配置从命令行、Python 配置到 C++ pybind 的链路基本完整。
  • ROCm MoriEP 路由补了多卡集成测试和 mori 缺失时的跳过逻辑。

@jacobwin-ai
fix: use ep_size instead of world_size in MoriEP wrapper initialization
9525d7a 
MoriEP was passing parallelism_config.world_size to mori config, which
would mismatch ep_rank when TP×EP/DP co-existence is introduced. Fix by
using ep_size and adding explicit assertions that ep_size == world_size
(the only currently supported configuration) in both init paths. Add
corresponding validation test.
@LLLLKKKK

LLLLKKKK commented May 14, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #977

Status: LGTM

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

lgtm ready to ci

Non-blocking Suggestions

P2

  • MoriEP 拓扑校验依赖 assert，优化运行会绕过保护 @ rtp_llm/models_py/distributed/moriep_wrapper.py:80
    • 建议：改为显式 if ...: raise ValueError/RuntimeError，并保留 mismatch 单测覆盖。
  • MoriEP router 在 forward 热路径输出 INFO 日志 @ rtp_llm/models_py/modules/factory/fused_moe/impl/rocm/routers/mori_ep_intranode_router.py:91
    • 建议：删除热路径 INFO，或改成 debug/采样/首 N 次日志并受显式诊断开关控制。
  • ROCm FakeBalanceExpert binding 未接入 Python 入口 @ rtp_llm/models_py/bindings/rocm/RegisterBaseBindings.hpp:104
    • 建议：为 ROCm 补与 CUDA FakeBalanceExpert 等价的 Python wrapper，或对齐注册 FakeBalanceExpertOp.forward() 并补调用链测试。

Checklist Violations (14 fail / 56 total)

General Principles Checklist

  • [6.1] Software Engineering — LSP：子类/重写保持基类契约 → issue ROCm FakeBalanceExpert binding 未接入 Python 入口
    ROCm fake_balance 只暴露函数式 binding，未保持现有 Python FakeBalanceExpert 入口契约。
  • [6.1] Architecture — 分层边界：新概念在正确层级，不泄漏内部 → issue ROCm FakeBalanceExpert binding 未接入 Python 入口
    ROCm FakeBalanceExpert 底层 binding 已新增，但 Python base 入口未同步，平台实现泄漏为 raw op。
  • [6.1] Architecture — 错误语义：fail-fast/retry/fallback/silent 行为显式 → issue MoriEP 拓扑校验依赖 assert，优化运行会绕过保护
    关键拓扑校验依赖 assert，优化运行可能被移除，不是稳定 fail-fast。
  • [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue MoriEP router 在 forward 热路径输出 INFO 日志
    MoriEP router 在 prepare/finalize 每次 forward 输出 INFO。
  • [6.1] Architecture — 兼容性：公开 API/持久数据/配置/环境迁移安全 → issue ROCm FakeBalanceExpert binding 未接入 Python 入口
    ROCm fake_balance 新 binding 未接入现有 Python API，启用已有配置仍会落到旧入口。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue ROCm FakeBalanceExpert binding 未接入 Python 入口
    MoriEP 有多卡测试，但 ROCm FakeBalanceExpert 新 binding 未看到覆盖 Python 入口的测试。
  • [6.1] Quality — 无 per-forward 调试日志 / 噪声热路径输出 → issue MoriEP router 在 forward 热路径输出 INFO 日志
    MoriEpIntranodeRouter.prepare/finalize 在 forward 路径使用 logging.info。

RTP-LLM Checklist

  • [I] 代码质量 — 同一功能用统一工具函数 → issue ROCm FakeBalanceExpert binding 未接入 Python 入口
    FakeBalanceExpert 在 CUDA 侧是 Python class API，ROCm 新增为 raw binding，入口未统一。

Python Static-First Checklist

  • [P.A] 静态结构与类型纪律 — 禁止 getattr/setattr literal 访问 → checklist-only
    ep.py 用 getattr(self, "_config", None) 缓存 strategy 状态；局部实现细节，未形成独立运行缺陷。
  • [P.A] 静态结构与类型纪律 — 禁止 hasattr 做控制流分支 → checklist-only
    reset_op 和测试清理对外部 MORI API 使用 hasattr；属于可选依赖兼容处理，不单独升级。
  • [P.A] 静态结构与类型纪律 — 字符串分发用 Enum/Literal → checklist-only
    ROCm strategy 继续用 quant_method 字符串集合，沿用现有量化配置约定；未发现本 PR 引入拼写缺陷。
  • [P.B] 错误处理 — 异常链用 raise X from Y → checklist-only
    部分 except ImportError as e 后重新 raise ValueError(...) 未使用 from e；日志保留 detail，主要影响诊断链路。
  • [P.G] 测试规范 — 数据驱动测试用 pytest.mark.parametrize → checklist-only
    新增测试使用 unittest/subTest 循环；当前可读性可接受，主要功能 gate 已通过 Bazel targets 接入。
  • [P.H] 类型标注 — Any 必须附注释说明原因 → checklist-only
    MORI 外部 op、strategy checker 和测试动态加载使用 Any，可推断为外部未类型化依赖；建议补注释但不构成独立缺陷。

Strengths

  • use_mori_ep 已贯通 server args、Python/C++ config、pybind 和 MoE strategy，配置路径比早期版本完整。
  • MoriEP wrapper 增加了 world_size mismatch 测试，上一轮 EP rank/WORLD group 错配风险已有显式保护。
  • MoriEP router 区分 global dispatch_ids 与本地 expert id，executor 侧通过 expert_ids_are_local 避免再套全局 expert mask。

@wht21

wht21 commented May 14, 2026

Copy link
Copy Markdown
Collaborator

internal source has been updated, please review the changes!

@LLLLKKKK
LLLLKKKK merged commit 04c852c into main May 14, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LLLLKKKK LLLLKKKK LLLLKKKK approved these changes
+1 more reviewer
@github-advanced-security github-advanced-security[bot] github-advanced-security[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jacobwin-ai @LLLLKKKK @wht21 @github-advanced-security