Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- 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).
- MuJoCo: `<size memory="512M">` is reserved by default; humanoid + mesh scenes no longer die with
`mj_stackAlloc: out of memory` (get_started/10_mount_camera.py).
- `hf_util`: a symlinked `roboverse_data` is no longer refused as path traversal; concurrent
Expand Down
8 changes: 7 additions & 1 deletion metasim/sim/isaacgym/isaacgym.py
Original file line number Diff line number Diff line change
Expand Up @@ -834,7 +834,13 @@ def _joint_pos_target_from_cache(self, robot) -> torch.Tensor | None:
cache = self._actions_cache
if not cache or isinstance(cache, (torch.Tensor, np.ndarray)):
return None
joint_names = self._joint_info[robot.name]["names"]
# Iterate joints in alphabetically-sorted order so the reported
# ``joint_pos_target`` aligns with ``joint_pos`` (emitted via
# ``_get_joint_ids_reindex``, i.e. sorted-name order). ``_joint_info[...]["names"]``
# is native DOF order, so using it produced a target vector misaligned with
# ``joint_pos`` whenever the URDF joint order was not already alphabetical.
# Values are name-keyed, so only the output ordering changes.
joint_names = self._get_joint_names(robot.name, sort=True)
targets_per_env = []
for env_idx, env_action in enumerate(cache):
if env_idx >= self._num_envs:
Expand Down
8 changes: 7 additions & 1 deletion metasim/sim/pybullet/pybullet.py
Original file line number Diff line number Diff line change
Expand Up @@ -449,8 +449,14 @@ def _get_states(self, env_ids=None) -> TensorState:
cached_action = (self._actions_cache or {}).get(robot.name)
if cached_action is not None and cached_action.get("dof_pos_target") is not None:
dof_pos_target = cached_action["dof_pos_target"]
# Iterate joints in alphabetically-sorted order so the reported
# ``joint_pos_target`` aligns with ``joint_pos``/``joint_vel`` (emitted
# via ``joint_reindex``, i.e. sorted-name order). ``object_joint_order``
# is native URDF order, so using it produced a target vector misaligned
# with ``joint_pos`` whenever that order was not already alphabetical.
# Values are name-keyed, so only the output ordering changes.
joint_pos_target = torch.tensor(
[dof_pos_target[name] for name in self.object_joint_order[robot.name]],
[dof_pos_target[name] for name in self._get_joint_names(robot.name, sort=True)],
dtype=torch.float32,
).unsqueeze(0)
state = RobotState(
Expand Down
91 changes: 91 additions & 0 deletions metasim/test/test_joint_target_order_general.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
"""Regression guard: ``joint_pos_target`` must be reported in the same
alphabetically-sorted joint order as ``joint_pos``/``joint_vel`` on every
backend that materializes it from a name-keyed action cache.

Motivation: ``joint_pos``/``joint_vel``/``joint_effort_target`` are emitted in
sorted-name order (via ``joint_reindex`` / ``_get_joint_ids_reindex``), but
several backends assembled the reported ``joint_pos_target`` by iterating their
*native* URDF joint order instead. 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. Commit 92755f6 fixed this on sapien2/genesis;
isaacgym and pybullet were the remaining offenders.

The faithful check needs a live backend (GPU/import order), which CI can't run,
so this is a static AST guard instead: for each backend it pins that the
``joint_pos_target`` materializer iterates ``_get_joint_names(..., sort=True)``
and no longer references the native-order joint list. Pure-Python, no sim env,
no GPU — runs under ``-k general``.
"""

from __future__ import annotations

import ast
from pathlib import Path

import pytest

_SIM_ROOT = Path(__file__).resolve().parents[1].joinpath("sim")


def _find_function(tree: ast.AST, class_name: str, func_name: str) -> ast.FunctionDef:
cls = next(n for n in ast.walk(tree) if isinstance(n, ast.ClassDef) and n.name == class_name)
return next(n for n in ast.walk(cls) if isinstance(n, ast.FunctionDef) and n.name == func_name)


def _calls_get_joint_names_sorted(fn: ast.FunctionDef) -> bool:
"""True if ``fn`` calls ``*._get_joint_names(...)`` with ``sort=True``.

Accepts either the keyword form ``sort=True`` or the positional form
``_get_joint_names(obj_name, True)`` — both mean sorted order.
"""
for node in ast.walk(fn):
if not (isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute)):
continue
if node.func.attr != "_get_joint_names":
continue
for kw in node.keywords:
if kw.arg == "sort" and isinstance(kw.value, ast.Constant) and kw.value.value is True:
return True
# positional sort is the 2nd arg after obj_name
if len(node.args) >= 2 and isinstance(node.args[1], ast.Constant) and node.args[1].value is True:
return True
return False


def _references_attr(fn: ast.FunctionDef, attr: str) -> bool:
return any(isinstance(node, ast.Attribute) and node.attr == attr for node in ast.walk(fn))


# (source file, class, function that materializes joint_pos_target, native-order
# attribute that must NOT be used to build it).
_CASES = [
pytest.param(
"isaacgym/isaacgym.py", "IsaacgymHandler", "_joint_pos_target_from_cache", "_joint_info", id="isaacgym"
),
pytest.param("pybullet/pybullet.py", "SinglePybulletHandler", "_get_states", "object_joint_order", id="pybullet"),
]


@pytest.mark.general
@pytest.mark.parametrize("rel_path,class_name,func_name,native_attr", _CASES)
def test_joint_pos_target_uses_sorted_joint_order(rel_path: str, class_name: str, func_name: str, native_attr: str):
"""The ``joint_pos_target`` materializer must iterate sorted joint names.

Fails if a backend reverts to iterating its native joint order, which would
re-open the silent ``joint_pos_target``/``joint_pos`` index misalignment
fixed for sapien2/genesis in 92755f6 and here for isaacgym/pybullet.
"""
source = _SIM_ROOT.joinpath(rel_path).read_text(encoding="utf-8")
fn = _find_function(ast.parse(source), class_name, func_name)

assert _calls_get_joint_names_sorted(fn), (
f"{class_name}.{func_name} must build joint_pos_target from "
f"_get_joint_names(..., sort=True) so it aligns with joint_pos "
f"(sorted-name order); no such call found."
)
assert not _references_attr(fn, native_attr), (
f"{class_name}.{func_name} still references native joint order "
f"({native_attr!r}) — joint_pos_target[i] would refer to a different "
f"joint than joint_pos[i] whenever the native order is not alphabetical."
)
Loading