Skip to content

Commit ab39ef3

Browse files
authored
fix(sim): report joint_pos_target in sorted order on isaacgym and pybullet (#34)
joint_pos/joint_vel/joint_effort_target are emitted in alphabetically-sorted joint order (isaacgym via _get_joint_ids_reindex, pybullet via joint_reindex), but both backends assembled the reported joint_pos_target by iterating their native URDF joint order instead — isaacgym from _joint_info[...]["names"] in _joint_pos_target_from_cache, pybullet from object_joint_order in _get_states. Whenever a robot's native joint order is not already alphabetical (e.g. numeric names joint_2/joint_10, or A,C,B), joint_pos_target[i] then referred to a different joint than joint_pos[i] — a silent index misalignment for downstream consumers that assume the fields share an ordering. Iterate _get_joint_names(..., sort=True) in both materializers instead. Values are name-keyed, so only the output ordering changes; the control path (isaacgym _set_dof_targets / _get_action_array_all and pybullet _apply_action, which legitimately drive the articulation in native order) is untouched. Completes the fix started in 92755f6 for sapien2/genesis, bringing isaacgym and pybullet into parity with sapien3/mujoco/mjx. Adds a general (no-GPU) AST regression guard pinning that both materializers build joint_pos_target from the sorted joint-name list and no longer reference the native-order list. Verified by inspection and the AST guard; the isaacgym and pybullet backends are not runnable in this environment (isaacgym needs a GPU and special import order), so the live end-to-end path could not be executed here.
1 parent b0880e9 commit ab39ef3

4 files changed

Lines changed: 106 additions & 2 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3636
### Fixed
3737

3838
- `RLTaskEnv.step` publishes the *terminal* observation in `info["observations"]["raw"]["obs"]` instead of the episode's first one (off-policy truncation bootstraps in clean_rl SAC/TD3 and FastTD3 read it).
39+
- IsaacGym and PyBullet reported `joint_pos_target` in native DoF order while `joint_pos` is in sorted-name order; both now use `get_joint_names(sort=True)` (completes #12).
3940
- MuJoCo: `<size memory="512M">` is reserved by default; humanoid + mesh scenes no longer die with
4041
`mj_stackAlloc: out of memory` (get_started/10_mount_camera.py).
4142
- `hf_util`: a symlinked `roboverse_data` is no longer refused as path traversal; concurrent

‎metasim/sim/isaacgym/isaacgym.py‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -834,7 +834,13 @@ def _joint_pos_target_from_cache(self, robot) -> torch.Tensor | None:
834834
cache = self._actions_cache
835835
if not cache or isinstance(cache, (torch.Tensor, np.ndarray)):
836836
return None
837-
joint_names = self._joint_info[robot.name]["names"]
837+
# Iterate joints in alphabetically-sorted order so the reported
838+
# ``joint_pos_target`` aligns with ``joint_pos`` (emitted via
839+
# ``_get_joint_ids_reindex``, i.e. sorted-name order). ``_joint_info[...]["names"]``
840+
# is native DOF order, so using it produced a target vector misaligned with
841+
# ``joint_pos`` whenever the URDF joint order was not already alphabetical.
842+
# Values are name-keyed, so only the output ordering changes.
843+
joint_names = self._get_joint_names(robot.name, sort=True)
838844
targets_per_env = []
839845
for env_idx, env_action in enumerate(cache):
840846
if env_idx >= self._num_envs:

‎metasim/sim/pybullet/pybullet.py‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,8 +449,14 @@ def _get_states(self, env_ids=None) -> TensorState:
449449
cached_action = (self._actions_cache or {}).get(robot.name)
450450
if cached_action is not None and cached_action.get("dof_pos_target") is not None:
451451
dof_pos_target = cached_action["dof_pos_target"]
452+
# Iterate joints in alphabetically-sorted order so the reported
453+
# ``joint_pos_target`` aligns with ``joint_pos``/``joint_vel`` (emitted
454+
# via ``joint_reindex``, i.e. sorted-name order). ``object_joint_order``
455+
# is native URDF order, so using it produced a target vector misaligned
456+
# with ``joint_pos`` whenever that order was not already alphabetical.
457+
# Values are name-keyed, so only the output ordering changes.
452458
joint_pos_target = torch.tensor(
453-
[dof_pos_target[name] for name in self.object_joint_order[robot.name]],
459+
[dof_pos_target[name] for name in self._get_joint_names(robot.name, sort=True)],
454460
dtype=torch.float32,
455461
).unsqueeze(0)
456462
state = RobotState(
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
"""Regression guard: ``joint_pos_target`` must be reported in the same
2+
alphabetically-sorted joint order as ``joint_pos``/``joint_vel`` on every
3+
backend that materializes it from a name-keyed action cache.
4+
5+
Motivation: ``joint_pos``/``joint_vel``/``joint_effort_target`` are emitted in
6+
sorted-name order (via ``joint_reindex`` / ``_get_joint_ids_reindex``), but
7+
several backends assembled the reported ``joint_pos_target`` by iterating their
8+
*native* URDF joint order instead. Whenever a robot's native joint order is not
9+
already alphabetical (e.g. numeric names ``joint_2``/``joint_10``, or ``A,C,B``),
10+
``joint_pos_target[i]`` then referred to a different joint than ``joint_pos[i]``
11+
— a silent index misalignment. Commit 92755f6 fixed this on sapien2/genesis;
12+
isaacgym and pybullet were the remaining offenders.
13+
14+
The faithful check needs a live backend (GPU/import order), which CI can't run,
15+
so this is a static AST guard instead: for each backend it pins that the
16+
``joint_pos_target`` materializer iterates ``_get_joint_names(..., sort=True)``
17+
and no longer references the native-order joint list. Pure-Python, no sim env,
18+
no GPU — runs under ``-k general``.
19+
"""
20+
21+
from __future__ import annotations
22+
23+
import ast
24+
from pathlib import Path
25+
26+
import pytest
27+
28+
_SIM_ROOT = Path(__file__).resolve().parents[1].joinpath("sim")
29+
30+
31+
def _find_function(tree: ast.AST, class_name: str, func_name: str) -> ast.FunctionDef:
32+
cls = next(n for n in ast.walk(tree) if isinstance(n, ast.ClassDef) and n.name == class_name)
33+
return next(n for n in ast.walk(cls) if isinstance(n, ast.FunctionDef) and n.name == func_name)
34+
35+
36+
def _calls_get_joint_names_sorted(fn: ast.FunctionDef) -> bool:
37+
"""True if ``fn`` calls ``*._get_joint_names(...)`` with ``sort=True``.
38+
39+
Accepts either the keyword form ``sort=True`` or the positional form
40+
``_get_joint_names(obj_name, True)`` — both mean sorted order.
41+
"""
42+
for node in ast.walk(fn):
43+
if not (isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute)):
44+
continue
45+
if node.func.attr != "_get_joint_names":
46+
continue
47+
for kw in node.keywords:
48+
if kw.arg == "sort" and isinstance(kw.value, ast.Constant) and kw.value.value is True:
49+
return True
50+
# positional sort is the 2nd arg after obj_name
51+
if len(node.args) >= 2 and isinstance(node.args[1], ast.Constant) and node.args[1].value is True:
52+
return True
53+
return False
54+
55+
56+
def _references_attr(fn: ast.FunctionDef, attr: str) -> bool:
57+
return any(isinstance(node, ast.Attribute) and node.attr == attr for node in ast.walk(fn))
58+
59+
60+
# (source file, class, function that materializes joint_pos_target, native-order
61+
# attribute that must NOT be used to build it).
62+
_CASES = [
63+
pytest.param(
64+
"isaacgym/isaacgym.py", "IsaacgymHandler", "_joint_pos_target_from_cache", "_joint_info", id="isaacgym"
65+
),
66+
pytest.param("pybullet/pybullet.py", "SinglePybulletHandler", "_get_states", "object_joint_order", id="pybullet"),
67+
]
68+
69+
70+
@pytest.mark.general
71+
@pytest.mark.parametrize("rel_path,class_name,func_name,native_attr", _CASES)
72+
def test_joint_pos_target_uses_sorted_joint_order(rel_path: str, class_name: str, func_name: str, native_attr: str):
73+
"""The ``joint_pos_target`` materializer must iterate sorted joint names.
74+
75+
Fails if a backend reverts to iterating its native joint order, which would
76+
re-open the silent ``joint_pos_target``/``joint_pos`` index misalignment
77+
fixed for sapien2/genesis in 92755f6 and here for isaacgym/pybullet.
78+
"""
79+
source = _SIM_ROOT.joinpath(rel_path).read_text(encoding="utf-8")
80+
fn = _find_function(ast.parse(source), class_name, func_name)
81+
82+
assert _calls_get_joint_names_sorted(fn), (
83+
f"{class_name}.{func_name} must build joint_pos_target from "
84+
f"_get_joint_names(..., sort=True) so it aligns with joint_pos "
85+
f"(sorted-name order); no such call found."
86+
)
87+
assert not _references_attr(fn, native_attr), (
88+
f"{class_name}.{func_name} still references native joint order "
89+
f"({native_attr!r}) — joint_pos_target[i] would refer to a different "
90+
f"joint than joint_pos[i] whenever the native order is not alphabetical."
91+
)

0 commit comments

Comments
 (0)