Skip to content

fix(envs): move device cfg field onto base ManagerBasedEnvCfg - #153

Merged
KraHsu merged 1 commit into
devfrom
fix/base-env-cfg-device
May 23, 2026
Merged

KraHsu merged 1 commit into
devfrom
fix/base-env-cfg-device

Conversation

@KraHsu

@KraHsu KraHsu commented May 23, 2026

Copy link
Copy Markdown
Owner

Problem

genelab play GeneLab-Rubiks-Play-v0 and GeneLab-Wuji-Hand-Playback-v0 both crash at env construction:

AttributeError: 'RubiksEnvCfg' object has no attribute 'device'
AttributeError: 'WujiEnvCfg' object has no attribute 'device'

Root cause

  • ManagerBasedRlEnv.__init__ reads cfg.device (src/genelab/envs/manager_based_rl_env.py:97,104).
  • device: str = "cuda" was defined only on the RL subclass ManagerBasedRlEnvCfg, not on the base ManagerBasedEnvCfg.
  • The Rubiks/Wuji play scenes use base cfgs (RubiksEnvCfg(ManagerBasedEnvCfg), WujiEnvCfg(ManagerBasedEnvCfg)) but play_task → build_env wraps them in ManagerBasedRlEnv, which requires cfg.device.

Fix

  • Add device: str = "cuda" to the base ManagerBasedEnvCfg (src/genelab/configs.py) so every env cfg carries it.
  • Drop the now-redundant duplicate on ManagerBasedRlEnvCfg.

Reference audit

mcp__serena__find_referencing_symbols on ManagerBasedRlEnvCfg/device:

src/genelab/envs/manager_based_rl_env.py:
  ManagerBasedRlEnv/_device  (line 96):  self._device = cfg.device
  ManagerBasedRlEnv/_scene   (line 103): InteractiveScene(..., device_hint=cfg.device)

A repo-wide grep for .device confirms every other access is on the runtime env.device property (managers, sensors, RL backends), never on a cfg object. Nothing relied on device being subclass-only, so the move is safe.

Tests

Headless regression tests (the AttributeError fires before scene build, so no GPU/sim needed):

  • tests/test_configs.py — base ManagerBasedEnvCfg has a device field defaulting to "cuda".
  • tests/test_examples_rubiks.py — RubiksEnvCfg().device == "cuda".
  • tests/test_examples_wuji.py — WujiEnvCfg().device == "cuda".

Full suite: uv run pytest -q → 502 passed, 0 failures (headless).

Full play verification needs a GPU (cold Genesis kernel compile ~10 min); the unit tests above are the fast gate.

Docs

Moved device from the ManagerBasedRlEnvCfg row to the ManagerBasedEnvCfg row in docs/reference/configuration.{en,zh}.md.

Rubiks/Wuji play scenes use the base ManagerBasedEnvCfg but are built via
ManagerBasedRlEnv, which reads cfg.device — crashing with AttributeError
because device was defined only on the RL subclass ManagerBasedRlEnvCfg.

Move device: str = "cuda" onto the base cfg so every env cfg carries it;
drop the now-redundant subclass duplicate. Reference audit confirmed the
only cfg.device reads are in ManagerBasedRlEnv.__init__; all other .device
access is on the runtime env.device property.

Add headless regression tests and update the config reference docs.
@KraHsu
KraHsu merged commit 97d3ade into dev May 23, 2026
3 checks passed
@KraHsu
KraHsu deleted the fix/base-env-cfg-device branch May 23, 2026 15:31
KraHsu added a commit that referenced this pull request May 27, 2026
Release dev → main: runtime-validated fixes + features (PRs #153–168)
KraHsu added a commit that referenced this pull request Jun 5, 2026
fix(envs): move device cfg field onto base ManagerBasedEnvCfg
KraHsu added a commit that referenced this pull request Jun 5, 2026
P8/P9: `genelab play GeneLab-Rubiks-Play-v0 --agent zero --steps 5` (and the
Wuji equivalent) crashed in env construction — first on `cfg.device` (fixed
superficially in #153), then on `cfg.decimation`, etc.

Root cause is architectural, not a missing field. Rubiks/Wuji are force-driven
scene-playback demos whose cfgs subclass the non-RL base ManagerBasedEnvCfg and
have their own RegisteredTask.play() → RubiksPlayEnv/WujiHandPlaybackEnv.play()
that builds a Genesis scene directly (no managers, no policy). `_dispatch_play`
already routes to task.play() when no RL flags are present, so the bare command
worked — but RL-only flags (--agent/--checkpoint/--num-envs/--prof) forced the
fall-through to play_task → build_env → ManagerBasedRlEnv, which requires the
full RL cfg surface these demos don't have. Bolting more fields onto the base
cfg would only move the crash to the next field.

Fix at the routing layer: gate RL routing on whether the play env cfg is a
ManagerBasedRlEnvCfg. Non-RL tasks always run their own .play(); RL-only options
are meaningless for a fixed scene demo, so they are warned-and-ignored rather
than crashing. --steps/--vis/--gpu and dotted overrides still apply (they are
resolved into the cfg before dispatch). Defense-in-depth: build_env now raises a
clear TypeError naming the required surface instead of a cryptic AttributeError.

Tests:
- test_examples_{rubiks,wuji}: exercise the real `genelab play … --agent zero
  --steps 5` dispatch path with Genesis stubbed (headless), asserting the scene
  demo runs, honors --steps, and is NOT routed through play_task. These guard
  the actual P8/P9 bug, which the prior #153 cfg-only tests did not.
- test_cli_routing: non-RL task + --agent runs the demo with a warning;
  build_env rejects a non-RL cfg. Added an RL-capable fake task so the existing
  agent-routing tests still validate the RL path (they previously asserted the
  buggy contract on a non-RL cfg).

Docs: clarify in cli/play-train.{en,zh} that policy options apply only to RL
tasks; scene demos run built-in playback.
KraHsu added a commit that referenced this pull request Jun 5, 2026
Release dev → main: runtime-validated fixes + features (PRs #153–168)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant