Fix multi-world qfrc_actuator indexing for worldid > 0 by adenzler-nvidia · Pull Request #2474 · newton-physics/newton

adenzler-nvidia · 2026-04-16T12:56:45Z

Description

convert_qfrc_actuator_from_mj_kernel indexed joint_type, joint_child, and joint_dof_dim with per-world jntid instead of the global index (joints_per_world * worldid + jntid). For joint_child this caused body_com lookups to always read world 0's CoM when computing the free-joint force transform, producing wrong angular forces for worldid > 0.

Same class of bug as reported for convert_warp_coords_to_mj_kernel in #2332.

Closes #2471

Checklist

New or existing tests cover these changes
The documentation is up to date with these changes
CHANGELOG.md has been updated (if user-facing change)

Test plan

uv run --extra dev -m newton.tests -k test_mujoco_solver.TestMultiWorldQfrcActuatorCom

Without fix: test_world1_uses_own_com FAILS (world 1 angular qfrc is [0, 0, 0] instead of [0.3, 1.0, 0.0])
With fix: test PASSES

Bug fix

Steps to reproduce:

Create a 2-world model with a free-joint body and a motor actuator
Set different body_com per world
Apply actuator force and read qfrc_actuator
World 1's angular forces are identical to world 0's (wrong CoM used)

Minimal reproduction:

import newton
import warp as wp
from newton.solvers import SolverMuJoCo

mjcf = """
<mujoco>
  <option gravity="0 0 0"/>
  <worldbody>
    <body pos="0 0 1" quat="0.7071 0.7071 0 0">
      <freejoint name="root"/>
      <geom type="box" size="0.1 0.1 0.05" mass="1"/>
    </body>
  </worldbody>
  <actuator>
    <motor joint="root" gear="0 0 1 0 0 0"/>
  </actuator>
</mujoco>
"""

def make_template(com):
    t = newton.ModelBuilder()
    t.add_mjcf(mjcf, ctrl_direct=True)
    t.request_state_attributes("mujoco:qfrc_actuator")
    t.body_com[0] = com
    return t

builder = newton.ModelBuilder()
builder.request_state_attributes("mujoco:qfrc_actuator")
builder.add_world(make_template(wp.vec3(0.0, 0.0, 0.0)))
builder.add_world(make_template(wp.vec3(0.1, -0.05, 0.03)))
model = builder.finalize()
solver = SolverMuJoCo(model)

state = model.state()
ctrl = model.control()
ctrl.mujoco.ctrl = wp.array([10.0, 10.0], dtype=wp.float32)
solver.step(state, state, ctrl, None, dt=0.01)

qfrc = state.mujoco.qfrc_actuator.numpy()
dofs = model.joint_dof_count // 2
print("World 0 angular:", qfrc[3:6])      # [0, 0, 0] — correct (zero CoM)
print("World 1 angular:", qfrc[dofs+3:dofs+6])  # BUG: [0, 0, 0], should be [0.3, 1.0, 0.0]

Summary by CodeRabbit

Bug Fixes
- Fixed actuator force conversion in multi-world models, which was incorrectly using body properties from the first world for all subsequent worlds.
Tests
- Added regression test to verify correct actuator force behavior across multiple worlds.

convert_qfrc_actuator_from_mj_kernel indexed joint_type, joint_child, and joint_dof_dim with per-world jntid instead of the global index (joints_per_world * worldid + jntid). For joint_child this caused body_com lookups to always read world 0's CoM when computing the free-joint force transform, producing wrong angular forces for worldid > 0. The same class of bug was reported for convert_warp_coords_to_mj_kernel in newton-physics#2332. Closes newton-physics#2471

coderabbitai · 2026-04-16T12:57:04Z

📝 Walkthrough

Walkthrough

This PR fixes a multi-world indexing bug in the convert_qfrc_actuator_from_mj_kernel function where body center of mass lookups incorrectly used per-world joint indices instead of global indices, causing worlds with worldid > 0 to read world 0's body properties. The fix updates all relevant array lookups to use computed global joint indices and adds a regression test.

Changes

Cohort / File(s)	Summary
Multi-World Indexing Fix `newton/_src/solvers/mujoco/kernels.py`	Updated `convert_qfrc_actuator_from_mj_kernel` to compute `joint_id = joints_per_world * worldid + jntid` and use it for all joint-indexed lookups (`joint_qd_start`, `joint_type`, `joint_child`, `joint_dof_dim`) instead of per-world `jntid`, ensuring multi-world bodies use their own center of mass for actuator force transforms.
Regression Test `newton/tests/test_mujoco_solver.py`	Added `TestMultiWorldQfrcActuatorCom` test class with `test_world1_uses_own_com` that validates the fix by verifying world 0 produces zero angular qfrc (origin CoM) while world 1 produces correct non-zero angular qfrc from its distinct CoM for free-joint actuators.
Changelog Update `CHANGELOG.md`	Documented the multi-world `qfrc_actuator` conversion fix in the Unreleased section.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the main bugfix: correcting multi-world qfrc_actuator indexing for worlds with worldid > 0, which matches the core change in kernels.py.
Linked Issues check	✅ Passed	The PR directly addresses all coding objectives from issue `#2471`: fixes incorrect indexing of joint_child, joint_type, and joint_dof_dim; ensures body_com uses per-world indices; and includes regression tests validating the fix.
Out of Scope Changes check	✅ Passed	All changes are in-scope: kernels.py fixes the multi-world indexing bug, test_mujoco_solver.py adds regression tests, and CHANGELOG.md documents the fix—no unrelated modifications present.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@newton/tests/test_mujoco_solver.py`:
- Around line 8320-8325: The test is aliasing the same buffer for input and
output when calling solver.step, violating the API which expects separate
double-buffered state_in and state_out; change the call to pass distinct buffers
(e.g., create a copy or new state object like state_out = self.model.state() and
initialize it appropriately) and call self.solver.step(state, state_out, ctrl,
None, dt=0.01), then read qfrc from state_out.mujoco.qfrc_actuator.numpy() (or
swap names as needed) so state_in and state_out are not the same object.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 483d6611-0cf7-427a-aa3e-0758f50b83f3

📥 Commits

Reviewing files that changed from the base of the PR and between 779b126 and fdcc67f.

📒 Files selected for processing (3)

CHANGELOG.md
newton/_src/solvers/mujoco/kernels.py
newton/tests/test_mujoco_solver.py

codecov · 2026-04-16T13:14:25Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

jvonmuralt

Thanks!
I think there is the same bug in convert_warp_coords_to_mj_kernel, which could also be fixed here. Also the changelog entry should be at the end.

adenzler-nvidia · 2026-04-16T13:40:51Z

thanks - yes this came up because I was reviewing this: #2332 which fixes the other kernel.

for the changelog, I'm trying to get this one approved: #2472 to reduce the number of merge conflicts and relieve some of the pressure off our CI.

adenzler-nvidia requested a review from jvonmuralt April 16, 2026 13:01

coderabbitai Bot reviewed Apr 16, 2026

View reviewed changes

Comment thread newton/tests/test_mujoco_solver.py

jvonmuralt reviewed Apr 16, 2026

View reviewed changes

jvonmuralt approved these changes Apr 16, 2026

View reviewed changes

adenzler-nvidia added this pull request to the merge queue Apr 16, 2026

Merged via the queue into newton-physics:main with commit 763480a Apr 16, 2026
25 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix multi-world qfrc_actuator indexing for worldid > 0#2474

Fix multi-world qfrc_actuator indexing for worldid > 0#2474
adenzler-nvidia merged 1 commit intonewton-physics:mainfrom
adenzler-nvidia:adenzler/fix-qfrc-multiworld-indexing

adenzler-nvidia commented Apr 16, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 16, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

codecov Bot commented Apr 16, 2026

Uh oh!

jvonmuralt left a comment

Uh oh!

adenzler-nvidia commented Apr 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

adenzler-nvidia commented Apr 16, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Test plan

Bug fix

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov Bot commented Apr 16, 2026

Codecov Report

Uh oh!

jvonmuralt left a comment

Choose a reason for hiding this comment

Uh oh!

adenzler-nvidia commented Apr 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

adenzler-nvidia commented Apr 16, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 16, 2026 •

edited

Loading