Skip to content

Refactor: manager-based API 工程质量整改(reward 双实现 / god module / API 面 / 并发断言) #854

Description

@TATP-233

Part of #850. 审阅维度 1:从工业生产级仓库标准看 manager-based API 的结构化、可复用性、可维护性、可扩展性、接口划分清晰度与解耦程度。

全部结论在 PR worktree(head 25343d46)上实测到 file:line

先说做对的部分

这些不是客套,是整改时不应回退的既有资产:

1. 通用层没有被 task / backend 污染

grep -rn -i "g1\|mjwarp\|walk_flat" src/unilab/manager/*.py3 处命中,且全部是 docstring 或错误信息文本:

manager/fingerprint.py:82      docstring 说明 mjwarp 与 mujoco 何时可共享 ABI
manager/device_runtime.py:684  docstring 说明 stream 依赖
manager/device_runtime.py:757  错误信息字符串 "mjwarp device managed runtime requires float32"

通用 manager 层没有任何 if backend == ... 分支。这在同类重构里是少见的克制,符合 CLAUDE.md「backend-specific 逻辑留在 backend / env 适配层」。

2. G1 pilot 的冷路径确实共享,不是复制粘贴

managed_fused.py:70-90managed_reference 导入 19 个符号:

from .managed_reference import (
    _ACTOR_WIDTH, _CRITIC_WIDTH, _RESET_TERM, _ROOT_RESET_SUFFIXES, _STATE_KEYS,
    G1ManagedReferenceError, _action_scale, _g1_reset_templates, _g1_selectors,
    _g1_state_requirements, _G1KernelConfig, _G1ResetSample, _G1StateViews,
    _kernel_config, _manager_buffer, _reset_suffix_for_dof, _reset_term_key,
    _validate_reference_profile,
)

selector、state requirement、kernel config、reset template、profile 校验、observation 宽度全部单一定义。「三份实现 = 三倍重复」的说法不成立,应予纠正。

3. 其他值得保留的机制

编译期 selector 绑定 + plan 再校验(plan.py:387);PolicyABI + fingerprint(plan.py:229);hot-path 分配 / 地址稳定预算(spec.py:539runtime.py:254);显式校验的 term DAG(plan.py:435);ManagedKernelBinding 拒绝向 task 代码暴露 backend / env / model handle(runtime.py:370)。

Finding 1(major,可维护性)— reward term 数学实现两遍,跨两个 tensor 框架

文件 if name == ... 分支数 框架
managed_reference.py 11 numpy
managed_fused.py 0 复用 reference
managed_device.py 11 torch

同一组 11 个 term 名(tracking_lin_veltracking_ang_velforward_progressunder_speedlin_vel_zbase_heightposeupper_body_posepenalty_close_feet_xyfeet_phasealive)在两个文件里各写一遍。

漂移风险:修改任一 reward 语义必须同步改两个文件、两种框架。生产路径(device)与 oracle 路径(reference)一旦不同步,differential test 是否覆盖到该 term 决定了错误能否被发现。

整改要求(可测)

  • reward term 数学单一定义,两个 profile 由同一份源码派生(或由代码生成保证一致)。
  • 存在测试:逐 term(非仅聚合 reward 标量)比对 reference 与 device 数值,覆盖全部 11 个 term,任一 term 单独漂移即失败。
  • 新增 reward term 时,若只在一侧实现,测试必须失败。

Finding 2(major,可扩展性)— 扩展点是「改 kernel 分支」而非「注册实现」

TermDefinitionmanager/spec.py)字段恰为 keyversionphaseroleparametersstate_requirementsoutputmutation_templatesrequired_capabilities——无 callablefind src/unilab -type d \( -name mdp -o -name terms \) → 空。

未识别 reward 名在热路径抛错(managed_reference.py 分支链末端),而非编译期。

一个 task 有 4,570 行 pilot 代码,却只对应 5TermDefinition

详细分析与 mjlab 对照见 #852,本 issue 只登记其对可扩展性的影响。

整改要求(可测)

  • 未注册 / 不支持的 term 在 compile 阶段报错,热路径不再需要该分支兜底。
  • 新增一个 reward term 不需要修改任何 if name == 链。

Finding 3(major,可维护性)— 两个 god module

文件 行数
manager/device_runtime.py 1918
manager/runtime.py 1273

device_runtime.py 单文件同时承载:runtime lifecycle、buffer 分配与地址稳定性、stream / event 编排、telemetry 与 counters、mutation 提交、reset 阶段、graph 交互、episode length 缓冲。

具体证据:record_stream 唯一调用点在 device_runtime.py:1141,位于 set_episode_length_buffer(def :1111)——buffer 生命周期管理与 runtime 主循环同处一个文件;stream / event 一次性构造在 :881-895

整改要求(可测)

  • device_runtime.py 拆分为职责单一的单元(至少分离 buffer/stream 所有权、telemetry、mutation 提交),单文件 ≤ 800 行。
  • 拆分后 manager/ 内不存在 > 1000 行文件。
  • 拆分不改变 plan fingerprint 与既有 gate 判定。

Finding 4(minor,接口划分)— 再导出面偏大

模块 __all__ 条目 文件行数
src/unilab/base/backend/__init__.py 139 471
src/unilab/manager/__init__.py 65

base/backend/__init__.py 本次 +297 行,主要是再导出。139 个公开名意味着几乎全部内部类型都成为公开 API 面,后续任何重命名都是破坏性变更。

整改要求(可测)

  • 区分「稳定公开 API」与「内部类型」,后者不进入顶层 __all__
  • 存在测试固定稳定公开 API 清单,新增公开名需显式更新该清单(防止无意扩大 API 面)。

Finding 5(minor,解耦)— reward weight 不在 manager 契约内

grep -ric weight src/unilab/manager/0。reward weight 以 _G1KernelConfig.reward_terms tuple 形式存在于 task kernel config,未进入 plan / spec,也未进入 PolicyABI fingerprint。

后果:改变 reward weight 不会使 checkpoint ABI 失效,但会改变训练语义。

整改要求(可测)

  • reward weight 进入 plan 与 fingerprint;weight 变化导致 fingerprint 变化。
  • 存在测试:仅改 weight 后,加载旧 checkpoint 的行为符合预期(失效或显式允许,二者择一并测试)。

Finding 6(minor,可测试性)— 关键并发不变量无断言

  • record_stream:生产唯一调用点 device_runtime.py:1141grep -rn "record_stream" tests/0。其失效模式是 use-after-free 竞态导致偶发错值,不是崩溃,现有 counter 与两个显存上限均检测不到。
  • stream / event 一次性构造(:881-895):代码成立但无测试固定;若某调用点开始每步新建 Stream() / Event()warm_numeric_allocationsbackend_allocationsmemory_allocatedmemory_reserved 全部不会变化。

整改要求(可测)

  • 存在测试断言 stream / event 在 warm loop 中身份不变(is 比对)且创建次数固定。
  • 存在覆盖 set_episode_length_buffer 的测试;移除 record_stream 该测试必须失败。

Roadmap

  1. Finding 1(最高优先)——消除 reward 数学双实现,补齐逐 term differential。这是当前最现实的正确性风险。
  2. Finding 3 —— 拆分 device_runtime.py,为后续所有改动降低成本。
  3. Finding 5 / 6 —— 补齐 fingerprint 覆盖与并发不变量断言。
  4. Finding 4 —— 收窄公开 API 面。
  5. Finding 2 —— 依赖 Work: manager API 与 mjlab 功能一致性缺口及迁移路径 #852 Phase A 的定位决策,不在本 issue 独立推进。

非目标

  • 不为形似 mjlab 而放弃上文「做对的部分」。
  • 不在本 issue 内调整 support 等级或 threshold。

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions