Skip to content

Commit fe1c480

Browse files
geng-haoransomaagiclaude
authored
fix(libero_90): harden Libero90BaseTask._get_initial_states like its sibling (#789)
`Libero90BaseTask._get_initial_states` called `get_traj(self.traj_filepath, self.scenario.robots[0], self.handler)` with no guards, so a missing/empty trajectory raised an uncaught FileNotFoundError/ValueError and a robotless scenario raised IndexError — unlike the already-hardened `LiberoBaseTask._get_initial_states`, which degrades to None (handler defaults) in those cases. The missing/empty-traj crash is a real reachable path (e.g. a failed/absent demo download); the robotless one is latent (all real libero_90 tasks declare robots=["franka"]; the only robotless files are empty 0-byte stubs). Fix: lift the hardened pattern verbatim — return None when traj_filepath is falsy, resolve `robot_ref` defensively, wrap `get_traj` in `try/except (FileNotFoundError, KeyError, ValueError)`, and return None on an empty result, before the unchanged replicate-to-num_envs logic. None is the established base contract (`BaseTaskEnv._get_initial_states` already returns None; `set_states(None)` is a no-op fallback), so no downstream regression; the happy path is byte-identical. Test: new `test_libero_90_get_initial_states_degrades_gracefully` exercises all three guards (no-traj, get_traj raises, empty result) — red before the fix (IndexError at robots[0]). `tests/test_libero_fixes.py` 4 passed; the libero suite 22 passed, 14 skipped. Reviewed by 3 fresh-context agents. Co-authored-by: geng-haoran <ghr@somastacks.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 495bc90 commit fe1c480

2 files changed

Lines changed: 69 additions & 5 deletions

File tree

roboverse_pack/tasks/libero_90/libero_90_base.py

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,29 @@ def reset(self, states=None, env_ids=None):
4141
return states
4242

4343
def _get_initial_states(self) -> list[dict] | None:
44-
"""Give the initial states from traj file."""
45-
# Keep it simple and leave robot states to defaults; just seed object poses.
46-
# If the handler handles None gracefully, this can be set to None.
47-
initial_states, _, _ = get_traj(self.traj_filepath, self.scenario.robots[0], self.handler)
48-
# Duplicate / trim list so that its length matches num_envs
44+
"""Return per-env initial states sampled from the demo trajectory.
45+
46+
Returns None when the trajectory is missing or empty, letting the
47+
handler fall back to its own defaults. Mirrors the hardened
48+
``LiberoBaseTask._get_initial_states``: the previous version indexed
49+
``self.scenario.robots[0]`` unconditionally and called ``get_traj``
50+
with no guard, so a robotless scenario or a missing/empty traj file
51+
raised (``IndexError``/``FileNotFoundError``/``KeyError``) instead of
52+
degrading gracefully like its libero sibling.
53+
"""
54+
if not self.traj_filepath:
55+
return None
56+
# scenario.robots may legitimately be empty (perception-only tasks).
57+
robot_ref = self.scenario.robots[0] if self.scenario.robots else None
58+
try:
59+
initial_states, _, _ = get_traj(self.traj_filepath, robot_ref, self.handler)
60+
except (FileNotFoundError, KeyError, ValueError):
61+
return None
62+
63+
if not initial_states:
64+
return None
65+
66+
# Duplicate / trim list so that its length matches num_envs (n > 0 here).
4967
if len(initial_states) < self.num_envs:
5068
k = self.num_envs // len(initial_states)
5169
initial_states = initial_states * k + initial_states[: self.num_envs % len(initial_states)]

tests/test_libero_fixes.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
"""Regression test (backend-free) for Libero90BaseTask._get_initial_states hardening.
2+
3+
The method must degrade to None (handler defaults) when the trajectory is
4+
missing/empty or the scenario is robotless, matching the already-hardened
5+
LiberoBaseTask. Previously it indexed ``scenario.robots[0]`` and called get_traj
6+
with no guard, so those cases crashed.
7+
"""
8+
9+
from __future__ import annotations
10+
11+
import pytest
12+
13+
14+
@pytest.mark.general
15+
def test_libero_90_get_initial_states_degrades_gracefully(monkeypatch):
16+
"""Libero90BaseTask._get_initial_states must degrade to None (handler
17+
defaults) when the traj is missing/empty or the scenario is robotless,
18+
matching the hardened LiberoBaseTask. Previously it indexed
19+
``scenario.robots[0]`` and called get_traj with no guard → crashed.
20+
"""
21+
from types import SimpleNamespace
22+
23+
import roboverse_pack.tasks.libero_90.libero_90_base as mod
24+
from roboverse_pack.tasks.libero_90.libero_90_base import Libero90BaseTask
25+
26+
t = Libero90BaseTask.__new__(Libero90BaseTask)
27+
t.scenario = SimpleNamespace(robots=[]) # robotless
28+
t.num_envs = 1
29+
t.handler = None
30+
31+
# 1. No traj_filepath → None (new guard; previously len(None) / robots[0] crash)
32+
t.traj_filepath = None
33+
assert t._get_initial_states() is None
34+
35+
# 2. get_traj raises (missing file / bad key) → None (try/except)
36+
t.traj_filepath = "does/not/exist.pkl.gz"
37+
38+
def _raise(*a, **k):
39+
raise FileNotFoundError("missing")
40+
41+
monkeypatch.setattr(mod, "get_traj", _raise)
42+
assert t._get_initial_states() is None
43+
44+
# 3. get_traj returns empty → None (empty guard before len())
45+
monkeypatch.setattr(mod, "get_traj", lambda *a, **k: ([], None, None))
46+
assert t._get_initial_states() is None

0 commit comments

Comments
 (0)