From 3fccf57965b1dc2fff026f3756da82916b1a2203 Mon Sep 17 00:00:00 2001 From: Pepijn Date: Mon, 20 Apr 2026 10:31:09 +0200 Subject: [PATCH] fix: integrate PR #3375 review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - envs(robocasa): hoist the duplicated `_parse_camera_names` helper out of `libero.py` and `robocasa.py` into `envs/utils.py` as the public `parse_camera_names`; call sites updated. - envs(robocasa): give each factory a distinct `episode_index` (`0..n_envs-1`) and derive a per-worker seed series in `reset()` so n_envs workers don't all roll the same scene under a shared outer seed. - envs(robocasa): drop the unused `**kwargs` on `_make_env`; declare `visualization_height` / `visualization_width` on both the wrapper and the `RoboCasaEnv` config + propagate via `gym_kwargs`. - envs(robocasa): emit `info["final_info"]` on termination (matching MetaWorld) so downstream vector-env auto-reset keeps the terminal task/success flags. - docs(robocasa): add `--rename_map` (robot0_agentview_left/ eye_in_hand/agentview_right → camera1/2/3) plus CI-parity flags to all three eval snippets. - docker(robocasa): pin robocasa + robosuite git SHAs and the pip dep versions (pygame, Pillow, opencv-python, pyyaml, pynput, tqdm, termcolor, imageio, h5py, lxml, hidapi, gymnasium) for reproducible benchmark images. - ci(robocasa): update the workflow comment — there is no `lerobot[robocasa]` extra; robocasa/robosuite are installed manually because upstream's `lerobot==0.3.3` pin shadows ours. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/benchmark_tests.yml | 4 +- docker/Dockerfile.benchmark.robocasa | 16 +++++-- docs/source/robocasa.mdx | 17 ++++++-- src/lerobot/envs/configs.py | 4 ++ src/lerobot/envs/libero.py | 19 ++------- src/lerobot/envs/robocasa.py | 61 ++++++++++++++++++--------- src/lerobot/envs/utils.py | 19 +++++++++ 7 files changed, 95 insertions(+), 45 deletions(-) diff --git a/.github/workflows/benchmark_tests.yml b/.github/workflows/benchmark_tests.yml index 8bf788c20..6aa5fe06e 100644 --- a/.github/workflows/benchmark_tests.yml +++ b/.github/workflows/benchmark_tests.yml @@ -312,7 +312,9 @@ jobs: if-no-files-found: warn # ── ROBOCASA365 ────────────────────────────────────────────────────────── - # Isolated image: lerobot[robocasa] only (robocasa, robosuite, mujoco chain) + # Isolated image: robocasa + robosuite installed manually as editable + # clones (no `lerobot[robocasa]` extra — robocasa's setup.py pins + # `lerobot==0.3.3`, which would shadow this repo's lerobot). robocasa-integration-test: name: RoboCasa365 — build image + 1-episode eval runs-on: diff --git a/docker/Dockerfile.benchmark.robocasa b/docker/Dockerfile.benchmark.robocasa index 1d6d1e0fc..9de1612cb 100644 --- a/docker/Dockerfile.benchmark.robocasa +++ b/docker/Dockerfile.benchmark.robocasa @@ -28,14 +28,22 @@ FROM huggingface/lerobot-gpu:latest # `--no-deps` on robocasa is deliberate: its setup.py pins `lerobot==0.3.3` # in install_requires, which would shadow the editable lerobot baked into # this image. We install robocasa's actual runtime deps explicitly instead. -RUN git clone --depth 1 https://github.com/robocasa/robocasa.git ~/robocasa && \ - git clone --depth 1 https://github.com/ARISE-Initiative/robosuite.git ~/robosuite && \ +# Pinned SHAs for reproducible benchmark runs. Bump when you need an +# upstream fix; don't rely on `main`/`master` drift. +ARG ROBOCASA_SHA=56e355ccc64389dfc1b8a61a33b9127b975ba681 +ARG ROBOSUITE_SHA=aaa8b9b214ce8e77e82926d677b4d61d55e577ab +RUN git clone https://github.com/robocasa/robocasa.git ~/robocasa && \ + git -C ~/robocasa checkout ${ROBOCASA_SHA} && \ + git clone https://github.com/ARISE-Initiative/robosuite.git ~/robosuite && \ + git -C ~/robosuite checkout ${ROBOSUITE_SHA} && \ uv pip install --no-cache -e ~/robocasa --no-deps && \ uv pip install --no-cache -e ~/robosuite && \ uv pip install --no-cache \ "numpy==2.2.5" "numba==0.61.2" "scipy==1.15.3" "mujoco==3.3.1" \ - pygame Pillow opencv-python pyyaml pynput tqdm termcolor \ - imageio h5py lxml hidapi "tianshou==0.4.10" gymnasium + "pygame==2.6.1" "Pillow==12.2.0" "opencv-python==4.13.0.92" \ + "pyyaml==6.0.3" "pynput==1.8.1" "tqdm==4.67.3" "termcolor==3.3.0" \ + "imageio==2.37.3" "h5py==3.16.0" "lxml==6.0.4" "hidapi==0.14.0.post4" \ + "tianshou==0.4.10" "gymnasium==1.2.3" # Set up robocasa macros and download kitchen assets. We need: # - tex : base environment textures diff --git a/docs/source/robocasa.mdx b/docs/source/robocasa.mdx index a24a6038e..9f4f59c4b 100644 --- a/docs/source/robocasa.mdx +++ b/docs/source/robocasa.mdx @@ -74,6 +74,8 @@ By default the env samples objects only from the `lightwheel` registry (what `-- ## Evaluation +All eval snippets below mirror the CI command (see `.github/workflows/benchmark_tests.yml`). The `--rename_map` argument maps RoboCasa's native camera keys (`robot0_agentview_left` / `robot0_eye_in_hand` / `robot0_agentview_right`) onto the three-camera (`camera1` / `camera2` / `camera3`) input layout the released `smolvla_robocasa` policy was trained on. + ### Single-task evaluation (recommended for quick iteration) ```bash @@ -82,7 +84,10 @@ lerobot-eval \ --env.type=robocasa \ --env.task=CloseFridge \ --eval.batch_size=1 \ - --eval.n_episodes=20 + --eval.n_episodes=20 \ + --eval.use_async_envs=false \ + --policy.device=cuda \ + '--rename_map={"observation.images.robot0_agentview_left": "observation.images.camera1", "observation.images.robot0_eye_in_hand": "observation.images.camera2", "observation.images.robot0_agentview_right": "observation.images.camera3"}' ``` ### Multi-task evaluation @@ -95,7 +100,10 @@ lerobot-eval \ --env.type=robocasa \ --env.task=CloseFridge,OpenCabinet,OpenDrawer,TurnOnMicrowave,TurnOffStove \ --eval.batch_size=1 \ - --eval.n_episodes=20 + --eval.n_episodes=20 \ + --eval.use_async_envs=false \ + --policy.device=cuda \ + '--rename_map={"observation.images.robot0_agentview_left": "observation.images.camera1", "observation.images.robot0_eye_in_hand": "observation.images.camera2", "observation.images.robot0_agentview_right": "observation.images.camera3"}' ``` ### Benchmark-group evaluation @@ -108,7 +116,10 @@ lerobot-eval \ --env.type=robocasa \ --env.task=atomic_seen \ --eval.batch_size=1 \ - --eval.n_episodes=20 + --eval.n_episodes=20 \ + --eval.use_async_envs=false \ + --policy.device=cuda \ + '--rename_map={"observation.images.robot0_agentview_left": "observation.images.camera1", "observation.images.robot0_eye_in_hand": "observation.images.camera2", "observation.images.robot0_agentview_right": "observation.images.camera3"}' ``` ### Recommended evaluation episodes diff --git a/src/lerobot/envs/configs.py b/src/lerobot/envs/configs.py index 68a1538c3..9b5b3ab94 100644 --- a/src/lerobot/envs/configs.py +++ b/src/lerobot/envs/configs.py @@ -507,6 +507,8 @@ class RoboCasaEnv(EnvConfig): camera_name: str = "robot0_agentview_left,robot0_eye_in_hand,robot0_agentview_right" observation_height: int = 256 observation_width: int = 256 + visualization_height: int = 512 + visualization_width: int = 512 split: str | None = None # Object-mesh registries to sample from. Upstream default is # ("objaverse", "lightwheel"), but objaverse is ~30GB and the CI image @@ -545,6 +547,8 @@ class RoboCasaEnv(EnvConfig): "render_mode": self.render_mode, "observation_height": self.observation_height, "observation_width": self.observation_width, + "visualization_height": self.visualization_height, + "visualization_width": self.visualization_width, } if self.split is not None: kwargs["split"] = self.split diff --git a/src/lerobot/envs/libero.py b/src/lerobot/envs/libero.py index ec90d0ffd..ae32527b9 100644 --- a/src/lerobot/envs/libero.py +++ b/src/lerobot/envs/libero.py @@ -31,20 +31,7 @@ from libero.libero.envs import OffScreenRenderEnv from lerobot.types import RobotObservation -from .utils import _LazyAsyncVectorEnv - - -def _parse_camera_names(camera_name: str | Sequence[str]) -> list[str]: - """Normalize camera_name into a non-empty list of strings.""" - if isinstance(camera_name, str): - cams = [c.strip() for c in camera_name.split(",") if c.strip()] - elif isinstance(camera_name, (list | tuple)): - cams = [str(c).strip() for c in camera_name if str(c).strip()] - else: - raise TypeError(f"camera_name must be str or sequence[str], got {type(camera_name).__name__}") - if not cams: - raise ValueError("camera_name resolved to an empty list.") - return cams +from .utils import _LazyAsyncVectorEnv, parse_camera_names def _get_suite(name: str) -> benchmark.Benchmark: @@ -128,7 +115,7 @@ class LiberoEnv(gym.Env): self.visualization_width = visualization_width self.visualization_height = visualization_height self.init_states = init_states - self.camera_name = _parse_camera_names( + self.camera_name = parse_camera_names( camera_name ) # agentview_image (main) or robot0_eye_in_hand_image (wrist) @@ -437,7 +424,7 @@ def create_libero_envs( gym_kwargs = dict(gym_kwargs or {}) task_ids_filter = gym_kwargs.pop("task_ids", None) # optional: limit to specific tasks - camera_names = _parse_camera_names(camera_name) + camera_names = parse_camera_names(camera_name) suite_names = [s.strip() for s in str(task).split(",") if s.strip()] if not suite_names: raise ValueError("`task` must contain at least one LIBERO suite name.") diff --git a/src/lerobot/envs/robocasa.py b/src/lerobot/envs/robocasa.py index a39f05b9b..c3b2f2449 100644 --- a/src/lerobot/envs/robocasa.py +++ b/src/lerobot/envs/robocasa.py @@ -26,7 +26,7 @@ from gymnasium import spaces from lerobot.types import RobotObservation -from .utils import _LazyAsyncVectorEnv +from .utils import _LazyAsyncVectorEnv, parse_camera_names # Dimensions for the flat action/state vectors used by the LeRobot wrapper. # These correspond to the PandaOmron robot in RoboCasa365. @@ -69,19 +69,6 @@ _TASK_GROUP_SPLITS = { } -def _parse_camera_names(camera_name: str | Sequence[str]) -> list[str]: - """Normalize camera_name into a non-empty list of strings.""" - if isinstance(camera_name, str): - cams = [c.strip() for c in camera_name.split(",") if c.strip()] - elif isinstance(camera_name, (list | tuple)): - cams = [str(c).strip() for c in camera_name if str(c).strip()] - else: - raise TypeError(f"camera_name must be str or sequence[str], got {type(camera_name).__name__}") - if not cams: - raise ValueError("camera_name resolved to an empty list.") - return cams - - def _resolve_tasks(task: str) -> tuple[list[str], str | None]: """Resolve a `--env.task` value to (task_names, split_override). @@ -140,9 +127,12 @@ class RoboCasaEnv(gym.Env): render_mode: str = "rgb_array", observation_width: int = 256, observation_height: int = 256, + visualization_width: int = 512, + visualization_height: int = 512, split: str | None = None, episode_length: int | None = None, obj_registries: Sequence[str] = DEFAULT_OBJ_REGISTRIES, + episode_index: int = 0, ): super().__init__() self.task = task @@ -150,10 +140,16 @@ class RoboCasaEnv(gym.Env): self.render_mode = render_mode self.observation_width = observation_width self.observation_height = observation_height + self.visualization_width = visualization_width + self.visualization_height = visualization_height self.split = split self.obj_registries = tuple(obj_registries) + # Per-worker index (0..n_envs-1) used to spread the user-provided + # seed across factories so each sub-env explores a distinct layout + # even when the same seed is passed to `reset()`. + self.episode_index = int(episode_index) - self.camera_name = _parse_camera_names(camera_name) + self.camera_name = parse_camera_names(camera_name) self._max_episode_steps = episode_length if episode_length is not None else 1000 @@ -253,7 +249,12 @@ class RoboCasaEnv(gym.Env): self._ensure_env() assert self._env is not None super().reset(seed=seed) - raw_obs, info = self._env.reset(seed=seed) + # Spread the user seed across workers. With n_envs factories each + # carrying a distinct `episode_index`, the same outer seed produces + # a different layout/trajectory per worker instead of all workers + # rolling the same scene. + worker_seed = seed + self.episode_index if seed is not None else None + raw_obs, info = self._env.reset(seed=worker_seed) ep_meta = self._env.env.get_ep_meta() self.task_description = ep_meta.get("lang", self.task) @@ -280,6 +281,11 @@ class RoboCasaEnv(gym.Env): observation = self._format_raw_obs(raw_obs) if terminated: + info["final_info"] = { + "task": self.task, + "done": bool(done), + "is_success": bool(is_success), + } self.reset() return observation, reward, terminated, truncated, info @@ -298,13 +304,20 @@ def _make_env_fns( render_mode: str, observation_width: int, observation_height: int, + visualization_width: int, + visualization_height: int, split: str | None, episode_length: int | None, obj_registries: Sequence[str], ) -> list[Callable[[], RoboCasaEnv]]: - """Build n_envs factory callables for a single task.""" + """Build n_envs factory callables for a single task. - def _make_env(**kwargs) -> RoboCasaEnv: + Each factory carries a distinct ``episode_index`` (``0..n_envs-1``) so + ``RoboCasaEnv.reset()`` can derive a per-worker seed series from the + user-provided seed. + """ + + def _make_env(episode_index: int) -> RoboCasaEnv: return RoboCasaEnv( task=task, camera_name=camera_names, @@ -312,13 +325,15 @@ def _make_env_fns( render_mode=render_mode, observation_width=observation_width, observation_height=observation_height, + visualization_width=visualization_width, + visualization_height=visualization_height, split=split, episode_length=episode_length, obj_registries=obj_registries, - **kwargs, + episode_index=episode_index, ) - return [partial(_make_env) for _ in range(n_envs)] + return [partial(_make_env, i) for i in range(n_envs)] def create_robocasa_envs( @@ -353,9 +368,11 @@ def create_robocasa_envs( render_mode = gym_kwargs.pop("render_mode", "rgb_array") observation_width = gym_kwargs.pop("observation_width", 256) observation_height = gym_kwargs.pop("observation_height", 256) + visualization_width = gym_kwargs.pop("visualization_width", 512) + visualization_height = gym_kwargs.pop("visualization_height", 512) split = gym_kwargs.pop("split", None) - camera_names = _parse_camera_names(camera_name) + camera_names = parse_camera_names(camera_name) task_names, group_split = _resolve_tasks(str(task)) if group_split is not None and split is None: split = group_split @@ -377,6 +394,8 @@ def create_robocasa_envs( render_mode=render_mode, observation_width=observation_width, observation_height=observation_height, + visualization_width=visualization_width, + visualization_height=visualization_height, split=split, episode_length=episode_length, obj_registries=obj_registries, diff --git a/src/lerobot/envs/utils.py b/src/lerobot/envs/utils.py index b0d834a05..8deb0af75 100644 --- a/src/lerobot/envs/utils.py +++ b/src/lerobot/envs/utils.py @@ -34,6 +34,25 @@ from lerobot.utils.utils import get_channel_first_image_shape from .configs import EnvConfig +def parse_camera_names(camera_name: str | Sequence[str]) -> list[str]: + """Normalize ``camera_name`` into a non-empty list of strings. + + Accepts a comma-separated string (``"cam_a,cam_b"``) or a sequence of + strings (tuples/lists). Whitespace is stripped; empty entries are + dropped. Raises ``TypeError`` for unsupported input types and + ``ValueError`` when the normalized list is empty. + """ + if isinstance(camera_name, str): + cams = [c.strip() for c in camera_name.split(",") if c.strip()] + elif isinstance(camera_name, (list | tuple)): + cams = [str(c).strip() for c in camera_name if str(c).strip()] + else: + raise TypeError(f"camera_name must be str or sequence[str], got {type(camera_name).__name__}") + if not cams: + raise ValueError("camera_name resolved to an empty list.") + return cams + + def _convert_nested_dict(d): result = {} for k, v in d.items():