Skip to content

Commit 582c348

Browse files
authored
fix(tasks): resolve ALOHA handover finger joints by sorted name, not last-4 (#775)
_observation sliced robot_joint_pos[:, -4:] as the gripper fingers, but joint_pos is ordered by sorted joint name and ALOHA interleaves the two arms: the four *_finger joints land at columns 2,3,10,11, while [-4:] grabs the right-arm shoulder/waist/wrist joints. The finger-vs-box observation term was therefore built from arm joints on every backend. Resolve the finger columns by name (endswith 'finger') from the handler's sorted joint order, cached. The cache index is declared as a CLASS attribute because RLTaskEnv.__init__ probes _observation during construction, before an instance attribute set after super().__init__() would exist. Adds a backend-free red/green regression test (fake handler + hand-built state: the finger obs term must track the finger columns), a class-level-default guard, and a source guard against the [-4:] slice. Verified red on pre-fix, green after.
1 parent 4f8ec85 commit 582c348

2 files changed

Lines changed: 137 additions & 2 deletions

File tree

roboverse_pack/tasks/mujoco_playground/handover.py

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,12 @@ class HandOver(RLTaskEnv):
7171
)
7272
max_episode_steps = 250 # 5 seconds
7373

74+
# Gripper-finger columns in the sorted-joint-name order of joint_pos, resolved
75+
# lazily in _observation. Declared at class level (not in __init__) because
76+
# RLTaskEnv.__init__ probes _observation before __init__ finishes setting
77+
# instance attributes, so this must already exist on first observation.
78+
_finger_joint_idx: list[int] | None = None
79+
7480
def __init__(self, scenario, device=None):
7581
self.robot_name = self.scenario.robots[0].name
7682
self._last_action = None
@@ -308,8 +314,15 @@ def _observation(self, env_states) -> torch.Tensor:
308314
if right_gripper_mat.dim() == 3:
309315
right_gripper_mat = right_gripper_mat.view(self.num_envs, -1)
310316

311-
# Finger joint positions (assuming finger joints are the last 4)
312-
finger_joints = robot_joint_pos[:, -4:] # [num_envs, 4]
317+
# Finger joint positions. joint_pos is ordered by *sorted joint name*,
318+
# which interleaves the two arms — ALOHA's four gripper fingers land at
319+
# non-contiguous columns (left at 2-3, right at 10-11), NOT the last 4
320+
# (which are right-arm shoulder/waist/wrist). Resolve them by name from
321+
# the handler's sorted joint order so the obs matches joint_pos. Cached.
322+
if self._finger_joint_idx is None:
323+
joint_names = self.handler.get_joint_names(self.robot_name, sort=True)
324+
self._finger_joint_idx = [i for i, n in enumerate(joint_names) if n.endswith("finger")]
325+
finger_joints = robot_joint_pos[:, self._finger_joint_idx] # [num_envs, 4]
313326
box_width = 0.04 # Box width for finger positioning
314327
finger_box_diff = finger_joints - box_width
315328

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
"""Regression: ALOHA handover obs reads finger joints by sorted-name order.
2+
3+
``RobotState.joint_pos`` is ordered by sorted joint name. For the dual-arm ALOHA
4+
that interleaves the two arms, so the four ``*_finger`` joints land at columns
5+
2, 3, 10, 11 — NOT the last four columns (which are right-arm shoulder/waist/
6+
wrist_angle/wrist_rotate).
7+
8+
The ``_observation`` method previously sliced ``robot_joint_pos[:, -4:]`` for the
9+
finger-vs-box term, feeding arm joints into the policy observation. This pins the
10+
corrected name-based resolution. Backend-free: a fake handler supplies the sorted
11+
joint names and ``_observation`` is called on a hand-built state.
12+
"""
13+
14+
from __future__ import annotations
15+
16+
from types import SimpleNamespace
17+
18+
import torch
19+
20+
from metasim.types import ObjectState, RobotState
21+
22+
# Alphabetically sorted ALOHA joint names (matches roboverse_pack/robots/aloha.py).
23+
_ALOHA_SORTED = [
24+
"left/elbow",
25+
"left/forearm_roll",
26+
"left/left_finger",
27+
"left/right_finger",
28+
"left/shoulder",
29+
"left/waist",
30+
"left/wrist_angle",
31+
"left/wrist_rotate",
32+
"right/elbow",
33+
"right/forearm_roll",
34+
"right/left_finger",
35+
"right/right_finger",
36+
"right/shoulder",
37+
"right/waist",
38+
"right/wrist_angle",
39+
"right/wrist_rotate",
40+
]
41+
_FINGER_COLS = [2, 3, 10, 11] # where "*_finger" lands after sorting
42+
43+
44+
def _identity_object():
45+
rs = torch.zeros(1, 13)
46+
rs[0, 3] = 1.0 # quaternion wxyz identity (matrix_from_quat needs a valid quat)
47+
return ObjectState(root_state=rs)
48+
49+
50+
def _make_task_and_states(finger_val: float, arm_val: float):
51+
from roboverse_pack.tasks.mujoco_playground.handover import HandOver
52+
53+
task = HandOver.__new__(HandOver) # bypass __init__/backend
54+
task.robot_name = "aloha"
55+
task.num_envs = 1
56+
task.device = "cpu"
57+
# Deliberately do NOT set task._finger_joint_idx — it must come from the
58+
# class-level default so this also guards against it being (re)introduced as
59+
# a post-super().__init__() instance attribute (which RLTaskEnv.__init__'s
60+
# obs-probe would hit before it exists, crashing real construction).
61+
task.handler = SimpleNamespace(get_joint_names=lambda name, sort=True: _ALOHA_SORTED)
62+
63+
jp = torch.full((1, 16), arm_val)
64+
for c in _FINGER_COLS:
65+
jp[0, c] = finger_val
66+
states = SimpleNamespace(
67+
objects={"box": _identity_object(), "target": _identity_object()},
68+
extras={
69+
"left_gripper_pos": torch.zeros(1, 3),
70+
"left_gripper_mat": torch.zeros(1, 9),
71+
"right_gripper_pos": torch.zeros(1, 3),
72+
"right_gripper_mat": torch.zeros(1, 9),
73+
},
74+
robots={"aloha": RobotState(root_state=torch.zeros(1, 13), joint_pos=jp, joint_vel=torch.zeros(1, 16))},
75+
)
76+
return task, states
77+
78+
79+
def test_handover_obs_finger_term_uses_finger_columns():
80+
"""The finger-vs-box obs term (obs cols 32:36) must come from the finger joints.
81+
82+
Finger columns are set to 0.5 and every arm column to 9.0; the term is
83+
``finger_joints - 0.04``. Correct -> ~0.46. The pre-fix ``[:, -4:]`` read arm
84+
columns 12-15 (=9.0) -> ~8.96, so this asserts the gap only the fix produces.
85+
"""
86+
from roboverse_pack.tasks.mujoco_playground.handover import HandOver
87+
88+
task, states = _make_task_and_states(finger_val=0.5, arm_val=9.0)
89+
obs = HandOver._observation(task, states)
90+
91+
assert task._finger_joint_idx == _FINGER_COLS, (
92+
f"finger columns should resolve to {_FINGER_COLS} from sorted ALOHA joint names, got {task._finger_joint_idx}"
93+
)
94+
finger_box_diff = obs[0, 32:36] # robot_joint_pos(16) + robot_joint_vel(16) -> finger term at 32:36
95+
assert torch.allclose(finger_box_diff, torch.full((4,), 0.46), atol=1e-4), (
96+
f"finger-vs-box obs term should be finger(0.5)-0.04=0.46, got {finger_box_diff.tolist()}; "
97+
f"a value near 8.96 means the obs is reading arm joints (the URDF last-4 bug)."
98+
)
99+
100+
101+
def test_finger_idx_is_class_level_default():
102+
"""`_finger_joint_idx` must be a class attribute so it exists before __init__ finishes.
103+
104+
RLTaskEnv.__init__ probes `_observation` during construction; if the cache were
105+
only initialised after super().__init__(), the probe would AttributeError.
106+
"""
107+
from roboverse_pack.tasks.mujoco_playground.handover import HandOver
108+
109+
assert HandOver.__dict__.get("_finger_joint_idx", "MISSING") is None
110+
111+
112+
def test_handover_does_not_slice_last_four_joints():
113+
"""Source guard: the URDF-order ``[:, -4:]`` finger slice must not return."""
114+
import pathlib
115+
116+
src = (
117+
pathlib.Path(__file__).resolve().parents[1] / "roboverse_pack" / "tasks" / "mujoco_playground" / "handover.py"
118+
).read_text()
119+
assert "robot_joint_pos[:, -4:]" not in src, (
120+
"handover.py slices the last 4 joint columns as fingers; ALOHA fingers are at "
121+
"sorted columns 2,3,10,11 — resolve by name instead."
122+
)

0 commit comments

Comments
 (0)