Skip to content

Review: PR #847 manager-based API 与 mjwarp 后端合入前整改总入口 #850

Description

@TATP-233

结论先行

审阅 #847(270 files,+749,148/-558,head 25343d46)后的结论:当前状态不建议合入 main

manager-based API 与独立 mjwarp backend 在若干维度上确实做出了 mjlab 没有的、更强的保证(编译期 selector 绑定、policy ABI fingerprint、per-mutation CUDA graph 失效建模、hot-path 零分配预算、task 代码拿不到 backend handle)。这些应当保留。

但有 4 个 pre-merge blocker 与 2 个必须先决策的定位问题。核心矛盾是:证据体系的规模(~11.6k 行 gate/evidence 代码 + ~78 MB JSON)远超它实际证明的东西,而最强的那批测试在 CI 里一次都没跑过。

本 issue 是 review 总入口,只记录判定与合入前置条件;分维度的 finding、roadmap 与验收标准见 child issue。

本次审阅口径

  • 仅审阅,不改代码。
  • 每条 finding 均在 PR worktree 上独立复核到 file:line,不采信 PR body 与 artifact 的自述。
  • 明确区分「PR 自述」「已复核事实」「设计取舍」三类。

Pre-merge blockers

B1. CI 红;且 0 个 test 真正执行

test (ubuntu-slim)collection 阶段失败,exit code 2

tests/dr/test_mjwarp_recompute.py
  -> unilab.base.backend.mjwarp.armature_recompute
  -> import warp as wp
ModuleNotFoundError: No module named 'warp'

src/unilab/base/backend/mjwarp/armature_recompute.py:12mjwarp/唯一绕开既有 dependencies.py lazy boundary 的模块。后果不是「少跑一个模块」,而是 collection 中断、430 deselected, 1 error2349 个 selected test 全部没有执行

单文件修复已由 #848 精确覆盖,本 issue 不重复;但 #848 的 scope 不解释 B2。

B2. Gate 体系结构性无法发现 B1

tests/acceptance/issue_705/final_gate_plan.yaml 中每一条 mjwarp lane 都带 --extra mjwarp--with mujoco-warp --with warp-lang。而 .github/workflows/ci.yml:110 只装 --extra mujoco --extra motrix + CPU torch。

即:12 条 mandatory gate command 里没有任何一条复现 CI 的标准依赖环境tests/base/test_backend_imports.py 只有 mujoco↔motrix 两个 eager-import 隔离 case,没有 mjwarp case。

make test-all 本地 2314 passed 与 CI 失败并不冲突——本地环境有 warp。这不是执行失误,是 gate 设计缺口。

B3. Recommended 依赖循环证据

support_evidence.yaml 中该组合唯一的 mandatory_test_ids
tests/benchmark/test_mjwarp_ppo_benchmark.py::test_device_profile_meets_end_to_end_gate

该 test 不训练、不跑 physics:读取 committed phase_5_mjwarp_ppo.json,断言 artifact["gate"] == {"errors": [], "passed": True},再调 validate_artifact同一份文件重算 gate 并比对。

validate_artifactexpected_gate = {"passed": not core_errors, "errors": list(core_errors)} 是对该 artifact 自身的一致性校验——它能证明 artifact 内部自洽、未被手工篡改,不能证明任何一台机器上曾达到该性能。同时该 test 无 CUDA guard,因为它根本不需要 GPU。

B4. 全局 checkpoint 兼容性破坏(跨全部 backend)

src/unilab/training/entrypoints.py:214 _require_policy_source_metadata 在三种情况下硬 raise:无 contract_snapshot、无 entrypoints block、fingerprint 不匹配。

entrypoints: block 由本 PR 引入,任何 PR 之前训练的 run 都不可能含有它。而 play_rsl_rlscripts/train_rsl_rl.py:337)与 play_interactive.py:184action_mode == "policy")都是无条件调用,没有 mjwarp/managed 门控。

结果:全部历史 PPO checkpoint 在 mujoco / motrix 上同样无法 play / resume,代码给出的补救建议是 "Retrain the policy"。

更严重的是它与既有契约直接冲突。src/unilab/training/sim2sim.py:226-234 仍保留 CLAUDE.md 记载的软降级:

snapshot is None -> print "(old run); skipping cross-backend enforcement" -> return target_cfg

新 gate 先执行,该 fallback 在 PPO play 路径上已成为不可达死代码。

这与两项承诺冲突:

必须先决策的两个定位问题

D1. mjwarp 被发布为高于成熟默认后端

docs/.../5-support_matrix.mdmjwarp = Recommendedmujoco = Tested

同时该 Recommended 组合是全仓库唯一关闭 NaN guard 的组合(conf/ppo/task/g1_walk_flat/mjwarp.yaml:12-13 nan_guard.enabled: false;全局默认 truemujoco.yaml 继承默认),且带 no_play: true / play_render_mode: none

并且 mjwarp 的 legacy DR capability 是空集backend.py:3124-3126 返回裸 DomainRandomizationCapabilities()),owner YAML 显式关闭 G1 默认为 true 的 4 个 flag。

#705 DoD 允许「显式关闭并记录差异」,因此 DoD 按字面已满足。但净效果是:被标为 Recommended 的组合,比被标为 Tested 的组合随机化更少、无 NaN 保护、不支持 play。

这是评级口径问题,不是 bug。建议在合入前明确:Recommended 是否应要求「不低于同 task 既有 backend 的 DR 与安全默认」。

D2. 证据体系与被证明对象的比例

  • src/unilab/tools/issue705_*:~11.6k 行,16 个模块
  • scripts/*issue705*:18 个新入口
  • tests/acceptance/issue_705/artifacts/~78 MB,660,318 / 749,148 新增行
    • phase_6_mjwarp_dr_performance.json 51.8 MB(单行)
    • issue705_phase5_ppo_5593eea8_trace.json 15.3 MB / 288,375 行
  • 真实 feature 代码:~14k 行(manager/ ≈5.6k + backend/mjwarp/ ≈8k)

仓库无 .gitattributes、无 LFS。这些体积永久进入 git history。

同时,最强的那批测试在自动化中从未执行:6 个 device/graph 测试文件中 5 个带 module 级 pytest.mark.slowpyproject.toml:147 addopts = "--tb=short -m 'not slow'"make test-all 与 CI 均默认剔除;无任何 job 在 GPU runner 上跑 -m slow

需要公平记录:这些测试质量很高——真实 warp.clone 指针置换、launch_count 未推进证明 stale graph 在 replay 前被拒、跨长 rollout 的逐字节地址元组比对。问题不是测试弱,而是质量不可达

合入前置条件

  • B1 修复(Fix: 保持 mjwarp armature recompute 的可选依赖边界 #848)且 CI 绿。
  • B2:新增标准依赖 lane(无 optional extra)进入 mandatory gate;test_backend_imports.py 补 mjwarp eager-import case。
  • B3:Recommendedmandatory_test_ids 至少含一条真实产生测量的 test;或将等级降至 artifact 能支撑的档位。
  • B4:恢复 legacy checkpoint 可加载(缺 entrypoints 时降级 + warning,与 sim2sim 既有语义一致),或按 Work: 统一规划 manager-based task API 与独立 mjwarp 后端 #705 DoD 走显式 manifest 迁移方案并在 PR body 声明破坏性变更。
  • D1:Recommended 判定口径给出结论。
  • D2:artifact 保留策略给出结论(见 cleanup child issue)。
  • PR body 的 Validation 段落更正:make test-all 通过的前提是本地已装 warp,不等价于 CI 通过。

Child issues

维度 范围
P0 blockers B1–B4 落地
manager API 工程质量 结构化 / 复用 / 可维护 / 可扩展 / 接口边界 / 解耦
mjlab 一致性与迁移 概念映射缺口 + 迁移阻塞项
清理与命名 issue-id 命名移除、入口收敛、artifact 策略

应当保留的设计

避免在整改中一并回退:编译期 selector 绑定与 plan 再校验(plan.py:387);PolicyABI + fingerprint(mjlab 无对应物);keyed RNG 确定性(dr/keyed_rng.py);per-mutation graph 失效类型化(mjwarp/capability.py:82,125,mjlab 靠 docstring 约定);hot-path 分配/地址稳定预算(spec.py:539);显式校验的 term DAG(plan.py:435);以及 ManagedKernelBinding 拒绝向 task 代码暴露 backend/env/model handle(runtime.py:370)——这条正是 CLAUDE.md 冷路径与 feature-leakage 不变量的结构化落地,mjlab 的 env-passing 设计做不到。

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

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions