fix(sim): report joint_pos_target in sorted order on isaacgym and pybullet - #34
Open
geng-haoran wants to merge 1 commit into
Open
fix(sim): report joint_pos_target in sorted order on isaacgym and pybullet#34geng-haoran wants to merge 1 commit into
geng-haoran wants to merge 1 commit into
Conversation
…ullet 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
Review: independently reviewed against current main (verdict MERGE AS-IS); rebased, re-verified: 489 passed, 5 skipped, 577 deselected, 2 xfailed in 7.39s
🤖 Generated with Claude Code
https://claude.ai/code/session_017i6VtKoovBNed815mWFqxw