Conversation
convert_warp_coords_to_mj_kernel used per-world joint index for joint_type, joint_child, and joint_dof_dim lookups instead of the global index. This caused wrong body CoM references for worldid > 0. The sibling convert_mj_coords_to_warp_kernel already uses the global index. Extract joint_id = joints_per_world * worldid + jntid once and reuse it for all per-joint lookups. Rename the shadowed type local to jtype for consistency with other kernels in the file. Add regression test that exercises the CoM lookup path via per-world body_com overrides and non-zero initial angular velocity. The test uses get_test_devices() and auto-skips on CPU-only runners since the buggy code path only executes on CUDA (SolverMuJoCo with use_mujoco_cpu=False).
Reduce the test model to a single free-floating body per world (the articulated 2-link structure was not required to trigger the indexing bug). Build each world directly via add_builder with its target base CoM
| ): | ||
| worldid, jntid = wp.tid() | ||
|
|
||
| joint_id = joints_per_world * worldid + jntid |
There was a problem hiding this comment.
📝 Review comment (will ship upstream) — v1
Kernel indexing looks right: joint_id for the Newton-side arrays that span all replicated worlds (joint_type/joint_child/joint_dof_dim), per-world jntid retained for the MuJoCo-side mj_q_start/mj_qd_start. LGTM.
There was a problem hiding this comment.
❌ Dropped from review
Reason: user dropped.
|
|
||
| Regression test for a bug in convert_warp_coords_to_mj_kernel where | ||
| joint_type and joint_child were indexed with the per-world joint index | ||
| instead of the global index, causing incorrect CoM references for |
There was a problem hiding this comment.
📝 Review comment (will ship upstream) — v1
Thanks for addressing all three test-scope questions from the earlier review — explicit device coverage, the comment now correctly identifies angular velocity (not multi-joint) as the critical factor, and begin_world()/end_world() per template is a cleaner setup than the earlier post-finalize body_com mutation.
There was a problem hiding this comment.
❌ Dropped from review
Reason: user dropped.
|
Synced to upstream newton-physics#2332 as review newton-physics#2332 (review). |
2 participants
Scratch pad for iterating on re-review of upstream newton-physics#2332. Do not merge.