Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions docs/cli/play-train.en.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ Policy sources:
| `random` | Uniform random actions in `[-1, 1]`. |
| `trained` | Load a checkpoint and use the runner inference policy. |

The policy options (`--agent`, `--checkpoint`, `--num-envs`, `--prof*`) apply only to RL
tasks — those whose play env config is a `ManagerBasedRlEnvCfg`. Non-RL **scene-playback
demos** (e.g. `GeneLab-Rubiks-Play-v0`, `GeneLab-Wuji-Hand-Playback-v0`), whose config
subclasses the base `ManagerBasedEnvCfg`, run their own built-in playback; passing those
options prints a warning and they are ignored. `--steps` / `--vis` / `--gpu` and dotted
config overrides still apply to both.

Checkpoint replay:

```bash
Expand Down
6 changes: 6 additions & 0 deletions docs/cli/play-train.zh.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ uv run genelab play TASK_ID --agent random --steps 128
| `random` | `[-1, 1]` 均匀随机动作。 |
| `trained` | 加载 checkpoint 并使用 runner inference policy。 |

策略相关选项(`--agent`、`--checkpoint`、`--num-envs`、`--prof*`)仅适用于 RL 任务,即 play
环境配置为 `ManagerBasedRlEnvCfg` 的任务。对于配置继承自基类 `ManagerBasedEnvCfg` 的非 RL
**场景回放示例**(如 `GeneLab-Rubiks-Play-v0`、`GeneLab-Wuji-Hand-Playback-v0`),将运行其
自带的回放逻辑;传入这些选项会打印告警并被忽略。`--steps` / `--vis` / `--gpu` 以及点号配置
覆盖对两类任务都生效。

checkpoint 回放:

```bash
Expand Down
57 changes: 51 additions & 6 deletions src/genelab/cli/_dispatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from __future__ import annotations

import os
import sys
from pathlib import Path
from typing import TYPE_CHECKING, Any, Final, cast

Expand Down Expand Up @@ -65,6 +66,22 @@ def _coerce_prof_kwargs(prof_args: dict[str, str]) -> dict[str, Any]:
}


def _is_rl_play_cfg(env_cfg: Any) -> bool:
"""Whether ``env_cfg`` can back the RL play helper (``play_task`` → ``build_env`` →
``ManagerBasedRlEnv``).

Scene-playback demos (Rubiks, Wuji, downstream non-RL tasks) subclass the *non-RL*
base ``ManagerBasedEnvCfg`` and lack the RL surface (``decimation``,
``episode_length_s``, ``actions_cfg``, …), so they must run their own ``task.play()``
instead — routing them through the RL helper crashes in env construction. See P8/P9.
"""
if env_cfg is None:
return False
from genelab.envs.manager_based_rl_env import ManagerBasedRlEnvCfg

return isinstance(env_cfg, ManagerBasedRlEnvCfg)


def _dispatch_play(task: Runnable, runner_args: dict[str, str], prof_args: dict[str, str]) -> None:
task_cfg = getattr(task, "cfg", None)
agent_cfg = getattr(task_cfg, "agent", None) if task_cfg is not None else None
Expand All @@ -79,6 +96,40 @@ def _dispatch_play(task: Runnable, runner_args: dict[str, str], prof_args: dict[
# play is always single-process; either flag is accepted but mapped through the same
# resolver so the mutual-exclusion guard fires on misuse.
num_envs_per_rank = _resolve_per_rank_num_envs(runner_args, gpus=1)

# The CLI's already-overridden cfg (play_env when configured): TASKS.get returns a
# fresh task each call, so the runner re-resolving would discard the --vis / --gpu /
# --a.env.* overrides applied above.
play_env_cfg = getattr(task_cfg, "play_env", None)
if play_env_cfg is None:
play_env_cfg = getattr(task_cfg, "env", None)

# Scene-playback demos (non-RL: env cfg is a base ManagerBasedEnvCfg) can't go through
# the RL play helper — ManagerBasedRlEnv requires an RL cfg surface they don't have, so
# play_task → build_env would crash in env construction. Run their own .play() instead.
# The RL-only options below have no meaning for a fixed scene demo, so warn and ignore
# them rather than crashing. See P8/P9.
if not _is_rl_play_cfg(play_env_cfg):
ignored = [
name
for name, present in (
("--checkpoint", checkpoint_raw is not None),
("--num-envs", num_envs_per_rank is not None),
("--agent", agent_raw is not None),
("--prof*", bool(prof_args)),
)
if present
]
if ignored:
task_name = getattr(task_cfg, "name", None) or "task"
print(
f"warning: {task_name} is a non-RL scene-playback task; ignoring "
f"{', '.join(ignored)} and running its built-in playback.",
file=sys.stderr,
)
task.play()
return

if (
checkpoint_raw is None
and num_envs_per_rank is None
Expand All @@ -93,12 +144,6 @@ def _dispatch_play(task: Runnable, runner_args: dict[str, str], prof_args: dict[
task_id = getattr(task_cfg, "name", None)
if not isinstance(task_id, str):
raise SystemExit("task config is missing 'name'; cannot route through RL play helper")
# Pass the CLI's already-overridden cfg (play_env when configured): TASKS.get
# returns a fresh task each call, so the runner re-resolving would discard the
# --vis / --gpu / --a.env.* overrides applied above.
play_env_cfg = getattr(task_cfg, "play_env", None)
if play_env_cfg is None:
play_env_cfg = getattr(task_cfg, "env", None)
play_task(
task_id,
env_cfg=play_env_cfg,
Expand Down
11 changes: 9 additions & 2 deletions src/genelab/rl/_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,15 @@ def resolve_env_cfg(task_id: str, play: bool) -> Any:


def build_env(env_cfg: Any) -> "ManagerBasedRlEnv":
from genelab.envs.manager_based_rl_env import ManagerBasedRlEnv

from genelab.envs.manager_based_rl_env import ManagerBasedRlEnv, ManagerBasedRlEnvCfg

if not isinstance(env_cfg, ManagerBasedRlEnvCfg):
raise TypeError(
f"build_env requires a ManagerBasedRlEnvCfg (the RL env surface: decimation, "
f"episode_length_s, actions_cfg, …); got {type(env_cfg).__name__}, which subclasses "
f"the non-RL base ManagerBasedEnvCfg. Scene-playback demos must run their own "
f".play() — they are not routed through the RL play helper. See P8/P9."
)
return ManagerBasedRlEnv(env_cfg)


Expand Down
37 changes: 37 additions & 0 deletions tests/fake_extension.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from dataclasses import dataclass, field

from genelab.configs import ManagerBasedEnvCfg, SimulationCfg, TaskCfg
from genelab.envs.manager_based_rl_env import ManagerBasedRlEnvCfg
from genelab.registry import register_env, register_robot, register_task


Expand All @@ -17,6 +18,16 @@ class FakeEnvCfg(ManagerBasedEnvCfg):
label: str = "fake-extension"


@dataclass
class FakeRlEnvCfg(ManagerBasedRlEnvCfg):
"""RL-capable env cfg (subclasses the RL base) used to test that RL play options
route through the ``play_task`` helper — the counterpart to ``FakeEnvCfg``, which is
a non-RL scene demo."""

simulation: SimulationCfg = field(default_factory=lambda: SimulationCfg(steps=0))
label: str = "fake-extension-rl"


@dataclass
class FakeRobot:
cfg: FakeRobotCfg = field(default_factory=FakeRobotCfg)
Expand All @@ -41,6 +52,26 @@ def train(self) -> None:
raise NotImplementedError("training is not implemented for fake extension")


class FakeRlTask:
"""An RL-capable task: its env cfg is a ``ManagerBasedRlEnvCfg``, so RL play options
(``--agent`` / ``--checkpoint`` / …) must route through the ``play_task`` helper."""

def __init__(self) -> None:
self.cfg = TaskCfg(
name="External-Fake-RL-Task-v0",
env_name="fake-extension-rl-env",
robot_name="fake-extension-robot",
env=FakeRlEnvCfg(),
trainable=False,
)

def play(self) -> None:
raise AssertionError("RL-capable task must route through play_task, not task.play()")

def train(self) -> None:
raise NotImplementedError("training is not implemented for fake extension")


def register() -> None:
register_robot(
"fake-extension-robot",
Expand All @@ -64,3 +95,9 @@ def register() -> None:
"genelab play External-Fake-Task-v0 --steps 7",
],
)
register_task(
"External-Fake-RL-Task-v0",
FakeRlTask,
description="RL-capable task from a fake external package.",
cfg_type=TaskCfg,
)
4 changes: 2 additions & 2 deletions tests/test_cli_args.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,13 +146,13 @@ def _fake_play(task_id: str, **kwargs: object) -> None:
"--import",
"tests.fake_extension",
"play",
"External-Fake-Task-v0",
"External-Fake-RL-Task-v0",
"--num-envs",
"8",
]
)

assert captured["task_id"] == "External-Fake-Task-v0"
assert captured["task_id"] == "External-Fake-RL-Task-v0"
assert captured["num_envs"] == 8


Expand Down
53 changes: 50 additions & 3 deletions tests/test_cli_routing.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,49 @@ def _fake_play(task_id: str, **kwargs: object) -> None:
monkeypatch.setitem(__import__("sys").modules, "genelab.rl", fake_rl)

cli_module.main(
["--import", "tests.fake_extension", "play", "External-Fake-Task-v0", "--agent", "random"]
[
"--import",
"tests.fake_extension",
"play",
"External-Fake-RL-Task-v0",
"--agent",
"random",
]
)

assert captured["task_id"] == "External-Fake-Task-v0"
assert captured["task_id"] == "External-Fake-RL-Task-v0"
assert captured["agent"] == "random"


def test_play_non_rl_task_ignores_rl_flags_and_runs_demo(
monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
) -> None:
"""P8/P9 regression: a non-RL scene-playback task (env cfg is a base
``ManagerBasedEnvCfg``) must run its own ``task.play()`` even when RL-only flags like
``--agent`` are passed — routing it through ``play_task`` → ``build_env`` would crash
constructing ``ManagerBasedRlEnv`` from a cfg with no RL surface."""
from genelab import cli as cli_module

# Patch the real module's attribute (not sys.modules) so the dispatcher's
# `from genelab.rl import play_task` resolves to the sentinel while entry-point
# extension loading can still `from genelab.rl import RslRlModelCfg`.
import genelab.rl as genelab_rl

def _fake_play(*_args: object, **_kwargs: object) -> None:
raise AssertionError("non-RL task must not route through the RL play helper")

monkeypatch.setattr(genelab_rl, "play_task", _fake_play)

cli_module.main(
["--import", "tests.fake_extension", "play", "External-Fake-Task-v0", "--agent", "zero"]
)

out = capsys.readouterr()
assert "played External-Fake-Task-v0" in out.out
assert "non-RL scene-playback task" in out.err
assert "--agent" in out.err


def test_core_namespaces_do_not_import_example_objects() -> None:
import genelab.envs as envs
import genelab.robots as robots
Expand Down Expand Up @@ -210,7 +246,7 @@ def _fake_play(task_id: str, **kwargs: object) -> None:
"--import",
"tests.fake_extension",
"play",
"External-Fake-Task-v0",
"External-Fake-RL-Task-v0",
"--agent",
"bogus",
]
Expand Down Expand Up @@ -273,3 +309,14 @@ def test_play_unknown_override_path_exits_without_picker(
]
)
assert "env.simulation.step" in str(excinfo.value)


def test_build_env_rejects_non_rl_cfg() -> None:
"""Defense-in-depth backstop for P8/P9: handing the RL env builder a non-RL cfg fails
with a clear TypeError naming the required surface, not a cryptic AttributeError deep
inside ``ManagerBasedRlEnv.__init__``."""
from genelab.configs import ManagerBasedEnvCfg
from genelab.rl.runner import build_env

with pytest.raises(TypeError, match="ManagerBasedRlEnvCfg"):
build_env(ManagerBasedEnvCfg())
48 changes: 45 additions & 3 deletions tests/test_examples_rubiks.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import numpy as np
from numpy.typing import NDArray
import pytest

from genelab.configs import apply_overrides
from genelab.registry import TASKS, load_extension_module
Expand Down Expand Up @@ -170,14 +171,55 @@ def test_rubiks_robot_registry_writes_turnable_mjcf(tmp_path: Path) -> None:


def test_rubiks_env_cfg_inherits_device_default() -> None:
"""Regression: the Rubiks play scene uses the base ``ManagerBasedEnvCfg`` but is
built via ``ManagerBasedRlEnv``, which reads ``cfg.device`` — so the base cfg
must carry ``device`` (no longer RL-subclass-only)."""
"""The Rubiks play scene uses the base ``ManagerBasedEnvCfg``; ``device`` lives on the
base cfg (PR #153), so the demo cfg inherits it."""
from genelab_examples.rubiks.config import RubiksEnvCfg

assert RubiksEnvCfg().device == "cuda"


def test_rubiks_play_with_rl_flags_runs_scene_demo(monkeypatch: pytest.MonkeyPatch) -> None:
"""P8/P9 regression: ``genelab play GeneLab-Rubiks-Play-v0 --agent zero --steps 5`` must
run the force-driven scene demo (``RubiksPlayEnv.play``), honoring ``--steps``, and must
NOT route through the RL play helper — ``play_task`` → ``build_env`` would crash building
``ManagerBasedRlEnv`` from the non-RL ``RubiksEnvCfg``. This exercises the same dispatch
path the CLI uses, with Genesis stubbed out so it runs headless."""
from genelab.cli import main
from genelab_examples import envs

captured: dict[str, object] = {}

def _fake_play(self: envs.RubiksPlayEnv) -> None:
captured["steps"] = self.cfg.simulation.steps

monkeypatch.setattr(envs.RubiksPlayEnv, "play", _fake_play)

# Patch the real module's attribute (not sys.modules) so the dispatcher's
# `from genelab.rl import play_task` resolves to the sentinel while entry-point
# extension loading can still `from genelab.rl import RslRlModelCfg`.
import genelab.rl as genelab_rl

def _no_play_task(*_args: object, **_kwargs: object) -> None:
raise AssertionError("non-RL Rubiks demo must not route through play_task")

monkeypatch.setattr(genelab_rl, "play_task", _no_play_task)

main(
[
"--import",
"genelab_examples.tasks",
"play",
"GeneLab-Rubiks-Play-v0",
"--agent",
"zero",
"--steps",
"5",
]
)

assert captured["steps"] == 5


def test_rubiks_robot_registry_can_write_welded_mjcf(tmp_path: Path) -> None:
load_extension_module("genelab_examples.tasks")
task = TASKS.get("GeneLab-Rubiks-Play-v0")
Expand Down
Loading