SolverMuJoCo: add custom attribute for solreffriction/dof_solref by adenzler-nvidia · Pull Request #1180 · newton-physics/newton

adenzler-nvidia · 2025-12-03T14:13:10Z

Before your PR is "Ready for review"

Necessary tests have been added and new examples are tested (see newton/tests/test_examples.py)
Documentation is up-to-date
Code passes formatting and linting checks with pre-commit run -a

Summary by CodeRabbit

New Features
- Support for per-DOF solref friction parameters in MuJoCo simulations, enabling finer-grained joint friction control and runtime updates.
Tests
- Added end-to-end tests for solref friction parsing (MJCF/USD), import validation, and solver conversion/update workflows.
- Note: a duplicate test entry was introduced and should be reviewed.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Signed-off-by: Alain Denzler <adenzler@nvidia.com>

coderabbitai · 2025-12-03T14:13:23Z

📝 Walkthrough

Walkthrough

Adds end-to-end support for per-DOF MuJoCo solref (solreffriction) by registering a new custom attribute, threading per-DOF solref data through SolverMuJoCo, extending the MuJoCo kernel signature to accept/output dof_solref, and adding tests validating parsing and runtime updates.

Changes

Cohort / File(s)	Summary
Kernel Interface `newton/_src/solvers/mujoco/kernels.py`	Extended `update_joint_dof_properties_kernel` signature with `dof_solref: wp.array(dtype=wp.vec2)` input and `dof_solref_out: wp.array2d(dtype=wp.vec2)` output; populate per-DOF solref outputs alongside existing solimp handling.
Solver Integration `newton/_src/solvers/mujoco/solver_mujoco.py`	Registered new MuJoCo custom attribute `solreffriction` (vec2, default [0.02,1.0]); read via `get_custom_attribute` as `joint_dof_solref`; propagate into `joint_params["solref_friction"]` in `add_geoms`; include `"dof_solref"` in `expand_model_fields`; pass `dof_solref` into kernel and expose `self.mjw_model.dof_solref` in `update_joint_dof_properties`.
MJCF Tests `newton/tests/test_import_mjcf.py`	Renamed and expanded `test_solimp_friction_parsing` → `test_solreffriction_parsing` to validate MJCF parsing, DOF merging behavior, and explicit vs default solreffriction values.
USD Tests `newton/tests/test_import_usd.py`	Added USD parsing test(s) verifying `mujoco:solreffriction` extraction and per-DOF values (explicit and default) for revolute joints.
Solver Runtime Tests `newton/tests/test_mujoco_solver.py`	Added `test_solref_friction_conversion_and_update` to validate multi-world conversion, initial mapping, and runtime updates of per-DOF solreffriction (note: the test appears duplicated).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Areas needing attention:
- solver_mujoco.py: ensure attribute registration, field expansion, and all add_geoms paths correctly index and apply per-DOF solref values.
- kernels.py: verify array shapes/types and correct population of dof_solref_out for linear vs angular DOFs.
- Tests: remove or confirm duplicated test in test_mujoco_solver.py and validate expected merging behavior in MJCF/USD tests.

Possibly related PRs

SolverMuJoCo: expose joint limit margin as custom solver attribute #1107 — touches the same MuJoCo solver/kernel paths to add per-DOF solref/solimp propagation and attribute registration.
SolverMuJoCo: add custom attribute for jnt_actgravcomp #1174 — adds a per-joint custom attribute and propagates it into per-DOF joint parameters in SolverMuJoCo.
Add passive stiffness and damping support for joints in MuJoCo solver #1082 — modifies update_joint_dof_properties kernel and solver data-paths for per-DOF property inputs/outputs.

Suggested reviewers

vreutskyy
eric-heiden

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 76.47% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and concisely describes the main change: adding a custom attribute for solreffriction/dof_solref support to SolverMuJoCo, which is the primary focus of all modifications across the kernel, solver, and test files.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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.}

Signed-off-by: Alain Denzler <adenzler@nvidia.com>

codecov · 2025-12-03T14:32:30Z

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!

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (4)

newton/tests/test_import_mjcf.py (1)

800-873: Solreffriction MJCF parsing test looks correct; helper could be shared.

The test accurately covers custom attribute registration, merged-joint behavior, and default vs explicit solreffriction values. The local arrays_match helper is now duplicated across multiple tests (solimplimit/solimpfriction/solreffriction); consider factoring it into a small shared helper if you touch this area again, but it’s fine as-is.
newton/tests/test_mujoco_solver.py (1)
1628-1743: Solref friction conversion test matches solimpfriction path; consider aligning assignment style.

The new test_solref_friction_conversion_and_update mirrors the existing solimpfriction test nicely: multi‑world replication, per‑DOF initialization, and correct mapping via joint_qd_start and joint_mjc_dof_start into mjw_model.dof_solref. That should exercise the new dof_solref plumbing well.

The only thing I’d tweak is how you assign into the vec2‑typed attribute: elsewhere (solimpfriction / solimplimit) you wrap the numpy array in a wp.array(..., dtype=vec5, device=model.device) before calling assign. For consistency and to avoid any surprises with vec2 casting, you could do the same here, e.g.:
-        model.mujoco.solreffriction.assign(initial_values)
+        model.mujoco.solreffriction.assign(
+            wp.array(initial_values, dtype=wp.vec2, device=model.device)
+        )
...
-        model.mujoco.solreffriction.assign(updated_values)
+        model.mujoco.solreffriction.assign(
+            wp.array(updated_values, dtype=wp.vec2, device=model.device)
+        )
Also minor nit: the docstring says solref_friction (with an underscore) while the attribute is solreffriction; renaming in the docstring would avoid confusion.

Please double‑check that assign with a raw np.ndarray of shape (joint_dof_count, 2) behaves as expected for a wp.vec2 attribute in your current Warp/Newton versions; if it’s fully supported, the current code is functionally fine and the change above is just for stylistic consistency.
newton/_src/solvers/mujoco/kernels.py (1)

911-1049: Per-DOF dof_solref plumbing in joint DOF kernel is consistent with existing solimp path.

The added dof_solref input and dof_solref_out output, plus the guarded writes in both linear and angular DOF loops, follow the same indexing strategy as dof_solimp and dof_armature (using newton_dof_start / mjc_dof_start and template‑space dof_to_mjc_joint). That should make mjw_model.dof_solref track the Newton solreffriction custom attribute correctly across worlds.

If you touch this docstring again, you might note that dof_solref/dof_solimp now both have explicit per‑DOF outputs, but the current comments are still accurate enough.
newton/tests/test_import_usd.py (1)
961-961: Remove unused noqa directive.

The noqa: PLC0415 directive is unnecessary here as the rule is not enabled. Other similar tests in this file (e.g., test_solimp_friction_parsing at line 328) use the same pattern, so this is consistent, but the static analyzer flagged it.
-        from pxr import Usd  # noqa: PLC0415
+        from pxr import Usd

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0a22111 and 6461cae.

📒 Files selected for processing (5)

newton/_src/solvers/mujoco/kernels.py (4 hunks)
newton/_src/solvers/mujoco/solver_mujoco.py (8 hunks)
newton/tests/test_import_mjcf.py (1 hunks)
newton/tests/test_import_usd.py (1 hunks)
newton/tests/test_mujoco_solver.py (1 hunks)

🧰 Additional context used

🧠 Learnings (12)

📓 Common learnings

Learnt from: adenzler-nvidia
Repo: newton-physics/newton PR: 1107
File: newton/_src/solvers/mujoco/kernels.py:973-974
Timestamp: 2025-11-24T08:05:21.390Z
Learning: In Newton's MuJoCo solver integration (newton/_src/solvers/mujoco/), the mapping between Newton joints and MuJoCo joints is not 1-to-1. Instead, each Newton DOF maps to a distinct MuJoCo joint. This means that for multi-DOF joints (like D6 with 6 DOFs), there will be 6 corresponding MuJoCo joints, each with its own properties (margin, solimp, solref, etc.). The mapping is done via dof_to_mjc_joint array, ensuring each DOF's properties are written to its own MuJoCo joint without overwriting.

Learnt from: preist-nvidia
Repo: newton-physics/newton PR: 579
File: newton/examples/example_mujoco.py:350-354
Timestamp: 2025-08-19T12:27:30.629Z
Learning: In Newton examples, Example.use_mujoco is a high-level attribute that controls whether to use MuJoCo solver vs other solvers (like XPBD), while SolverMuJoCo.use_mujoco_cpu controls the backend within MuJoCo (CPU vs Warp). These are separate concepts serving different purposes - the PR rename only applies to the solver parameter, not the Example class attributes.

Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:739-752
Timestamp: 2025-09-22T21:03:39.624Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently intentionally only supports additive updates (assuming n_coords == n_dofs). Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work and should not be flagged as an issue in current reviews.

Learnt from: preist-nvidia
Repo: newton-physics/newton PR: 579
File: newton/examples/example_mujoco.py:350-354
Timestamp: 2025-08-19T12:27:30.629Z
Learning: In Newton examples, there's a distinction between solver parameters and Example class attributes. The Example class can have its own use_mujoco attribute for controlling example-level behavior (like CUDA graphs, rendering logic), while the solver uses use_mujoco_cpu for backend selection. These serve different purposes and should not be conflated during API renames.

📚 Learning: 2025-08-12T18:07:17.267Z

Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/utils.py:30-33
Timestamp: 2025-08-12T18:07:17.267Z
Learning: In newton/_src/utils/import_usd.py, the PXR/USD modules are lazily loaded using try-except blocks within the functions rather than at module import time, making it safe to import parse_usd at the module level without causing import-time failures for users without USD dependencies.

Applied to files:

newton/tests/test_import_usd.py

📚 Learning: 2025-08-12T18:07:17.267Z

Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/utils.py:30-33
Timestamp: 2025-08-12T18:07:17.267Z
Learning: In newton/_src/utils/import_usd.py, the PXR/USD modules are lazily loaded using try-except blocks within the functions (lines 109-111 and 1289-1291) rather than at module import time, making it safe to import parse_usd at the module level without causing import-time failures for users without USD dependencies.

Applied to files:

newton/tests/test_import_usd.py

📚 Learning: 2025-08-19T12:27:30.629Z

Learnt from: preist-nvidia
Repo: newton-physics/newton PR: 579
File: newton/examples/example_mujoco.py:350-354
Timestamp: 2025-08-19T12:27:30.629Z
Learning: In Newton examples, there's a distinction between solver parameters and Example class attributes. The Example class can have its own use_mujoco attribute for controlling example-level behavior (like CUDA graphs, rendering logic), while the solver uses use_mujoco_cpu for backend selection. These serve different purposes and should not be conflated during API renames.

Applied to files:

newton/tests/test_mujoco_solver.py
newton/_src/solvers/mujoco/solver_mujoco.py
newton/_src/solvers/mujoco/kernels.py

📚 Learning: 2025-08-19T12:27:30.629Z

Learnt from: preist-nvidia
Repo: newton-physics/newton PR: 579
File: newton/examples/example_mujoco.py:350-354
Timestamp: 2025-08-19T12:27:30.629Z
Learning: In Newton examples, Example.use_mujoco is a high-level attribute that controls whether to use MuJoCo solver vs other solvers (like XPBD), while SolverMuJoCo.use_mujoco_cpu controls the backend within MuJoCo (CPU vs Warp). These are separate concepts serving different purposes - the PR rename only applies to the solver parameter, not the Example class attributes.

Applied to files:

newton/tests/test_mujoco_solver.py
newton/_src/solvers/mujoco/solver_mujoco.py
newton/_src/solvers/mujoco/kernels.py

📚 Learning: 2025-11-24T08:05:21.390Z

Learnt from: adenzler-nvidia
Repo: newton-physics/newton PR: 1107
File: newton/_src/solvers/mujoco/kernels.py:973-974
Timestamp: 2025-11-24T08:05:21.390Z
Learning: In Newton's MuJoCo solver integration (newton/_src/solvers/mujoco/), the mapping between Newton joints and MuJoCo joints is not 1-to-1. Instead, each Newton DOF maps to a distinct MuJoCo joint. This means that for multi-DOF joints (like D6 with 6 DOFs), there will be 6 corresponding MuJoCo joints, each with its own properties (margin, solimp, solref, etc.). The mapping is done via dof_to_mjc_joint array, ensuring each DOF's properties are written to its own MuJoCo joint without overwriting.

Applied to files:

newton/tests/test_mujoco_solver.py
newton/_src/solvers/mujoco/solver_mujoco.py
newton/_src/solvers/mujoco/kernels.py
newton/tests/test_import_mjcf.py

📚 Learning: 2025-07-21T19:11:04.077Z

Learnt from: dylanturpin
Repo: newton-physics/newton PR: 450
File: newton/sim/ik.py:1286-1296
Timestamp: 2025-07-21T19:11:04.077Z
Learning: In Newton's IK system, the JointLimitObjective class intentionally only handles joints where DOF count equals coordinate count (1-1 mapping). This is appropriate because joint limits are typically only meaningful for simple joint types like revolute and prismatic joints, not complex joints like ball or free joints where DOF count != coordinate count.

Applied to files:

newton/_src/solvers/mujoco/solver_mujoco.py

📚 Learning: 2025-09-22T21:03:39.624Z

Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:739-752
Timestamp: 2025-09-22T21:03:39.624Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently intentionally only supports additive updates (assuming n_coords == n_dofs). Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work and should not be flagged as an issue in current reviews.

Applied to files:

newton/_src/solvers/mujoco/kernels.py

📚 Learning: 2025-09-22T21:03:18.367Z

Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:638-648
Timestamp: 2025-09-22T21:03:18.367Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently only supports additive updates, meaning it assumes n_coords == n_dofs. Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work.

Applied to files:

newton/_src/solvers/mujoco/kernels.py

📚 Learning: 2025-08-18T15:56:26.587Z

Learnt from: adenzler-nvidia
Repo: newton-physics/newton PR: 552
File: newton/_src/solvers/mujoco/solver_mujoco.py:0-0
Timestamp: 2025-08-18T15:56:26.587Z
Learning: In Newton's MuJoCo solver, when transforming joint axes from Newton's internal frame to MuJoCo's expected frame, use wp.quat_rotate(joint_rot, axis) not wp.quat_rotate_inv(joint_rot, axis). The joint_rot represents rotation from joint-local to body frame, so forward rotation is correct.

Applied to files:

newton/_src/solvers/mujoco/kernels.py

📚 Learning: 2025-09-22T21:08:31.901Z

Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/examples/ik/example_ik_franka.py:121-123
Timestamp: 2025-09-22T21:08:31.901Z
Learning: In the newton physics framework, when creating warp arrays for IK solver joint variables using wp.array(self.model.joint_q, shape=(1, coord_count)), the resulting array acts as a reference/pointer to the original model's joint coordinates, so updates from the IK solver automatically reflect in the model's joint_q buffer used for rendering, despite the general warp documentation suggesting copies are made by default.

Applied to files:

newton/_src/solvers/mujoco/kernels.py

📚 Learning: 2025-09-22T21:08:31.901Z

Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/examples/ik/example_ik_franka.py:121-123
Timestamp: 2025-09-22T21:08:31.901Z
Learning: In the newton physics framework, when creating warp arrays for IK solver joint variables using wp.array(self.model.joint_q, shape=(1, coord_count)), the resulting array acts as a reference/pointer to the original model's joint coordinates, so updates from the IK solver automatically reflect in the model's joint_q buffer used for rendering.

Applied to files:

newton/_src/solvers/mujoco/kernels.py

🧬 Code graph analysis (2)

newton/tests/test_import_usd.py (2)

newton/tests/test_import_mjcf.py (4)

test_solreffriction_parsing (800-872)

arrays_match (748-749)

arrays_match (848-849)

arrays_match (922-923)

newton/_src/solvers/mujoco/solver_mujoco.py (2)

SolverMuJoCo (77-2077)

register_custom_attributes (151-248)

newton/tests/test_mujoco_solver.py (2)

newton/_src/solvers/mujoco/solver_mujoco.py (3)

SolverMuJoCo (77-2077)

register_custom_attributes (151-248)

notify_model_changed (453-463)

newton/_src/solvers/flags.py (1)

SolverNotifyFlags (22-56)

🪛 Ruff (0.14.7)

newton/tests/test_import_usd.py

961-961: Unused noqa directive (non-enabled: PLC0415)

Remove unused noqa directive

(RUF100)

⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

GitHub Check: Run GPU Benchmarks / Run GPU Benchmarks on AWS EC2
GitHub Check: Run GPU Tests / Run GPU Unit Tests on AWS EC2

🔇 Additional comments (8)

newton/tests/test_import_mjcf.py (1)

874-875: Renamed solimpfriction test name aligns with attribute semantics.

Updating the method name to test_solimpfriction_parsing keeps it consistent with the solimpfriction attribute and the assertions inside the test.

newton/tests/test_import_usd.py (1)

958-1075: LGTM!

The test follows the established pattern from similar tests (test_solimp_friction_parsing, test_solimplimit_parsing) and properly validates:

Parsing of explicit mjc:solreffriction values

Default values when the attribute is not specified

Both values are found in the finalized model

newton/_src/solvers/mujoco/solver_mujoco.py (6)

191-201: LGTM!

The solreffriction custom attribute registration is consistent with similar attributes (solimplimit, solimpfriction) and correctly defines:

Per-DOF frequency (JOINT_DOF)

2-element float vector matching MuJoCo's solref format

Default values (0.02, 1.0) matching MuJoCo defaults

USD attribute name following the mjc: namespace convention

1022-1023: LGTM!

The attribute retrieval follows the established pattern and is consistent with the adjacent solimpfriction retrieval.

1385-1388: LGTM!

The solref_friction parameter is correctly set for linear DOFs, following the same conditional pattern as solimp_friction.

1462-1465: LGTM!

The solref_friction parameter handling for angular DOFs mirrors the linear DOF implementation, maintaining consistency.

1730-1730: LGTM!

Adding "dof_solref" to the expandable model fields is necessary for multi-world simulation support and is consistent with other per-DOF fields in the list.

1878-1918: LGTM!

The dof_solref (solreffriction) data is correctly:

Retrieved from the model's mujoco namespace

Passed as input to update_joint_dof_properties_kernel

Written to self.mjw_model.dof_solref output

This follows the established pattern for per-DOF properties like dof_solimp. Based on learnings, the kernel correctly handles the DOF-to-MuJoCo-joint mapping where each Newton DOF maps to a distinct MuJoCo joint.

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

newton/_src/solvers/mujoco/kernels.py (1)

923-1048: dof_solref propagation mirrors existing solimp path and respects DOF→joint mapping

The extension of update_joint_dof_properties_kernel to take dof_solref and fill dof_solref_out is consistent with how dof_solimp is already handled:

Per‑DOF values are indexed by newton_dof_index and written to dof_solref_out[worldid, mjc_dof_index], using joint_mjc_dof_start to get the MuJoCo DOF index.

Per‑joint quantities (like jnt_solimp, jnt_solref, jnt_stiffness, jnt_margin, jnt_range) still use dof_to_mjc_joint[template_dof_index], keeping the established “template DOFs drive joint indices” convention for all worlds. This aligns with the documented dof_to_mjc_joint mapping for multi‑DOF joints. Based on learnings, this preserves the intended 1‑DOF→1‑MuJoCo‑joint behavior.

Optional‑array guards (if dof_solref:) match the existing pattern for dof_solimp and other optional inputs.

If you want, you could tweak the docstring to explicitly mention that the kernel also updates dof_solref_out when dof_solref is provided, but that’s purely cosmetic.

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6461cae and 4032f95.

📒 Files selected for processing (5)

newton/_src/solvers/mujoco/kernels.py (4 hunks)
newton/_src/solvers/mujoco/solver_mujoco.py (8 hunks)
newton/tests/test_import_mjcf.py (1 hunks)
newton/tests/test_import_usd.py (1 hunks)
newton/tests/test_mujoco_solver.py (1 hunks)

🧰 Additional context used

🧠 Learnings (13)

📓 Common learnings

Learnt from: adenzler-nvidia
Repo: newton-physics/newton PR: 1107
File: newton/_src/solvers/mujoco/kernels.py:973-974
Timestamp: 2025-11-24T08:05:21.390Z
Learning: In Newton's MuJoCo solver integration (newton/_src/solvers/mujoco/), the mapping between Newton joints and MuJoCo joints is not 1-to-1. Instead, each Newton DOF maps to a distinct MuJoCo joint. This means that for multi-DOF joints (like D6 with 6 DOFs), there will be 6 corresponding MuJoCo joints, each with its own properties (margin, solimp, solref, etc.). The mapping is done via dof_to_mjc_joint array, ensuring each DOF's properties are written to its own MuJoCo joint without overwriting.

Learnt from: preist-nvidia
Repo: newton-physics/newton PR: 579
File: newton/examples/example_mujoco.py:350-354
Timestamp: 2025-08-19T12:27:30.629Z
Learning: In Newton examples, Example.use_mujoco is a high-level attribute that controls whether to use MuJoCo solver vs other solvers (like XPBD), while SolverMuJoCo.use_mujoco_cpu controls the backend within MuJoCo (CPU vs Warp). These are separate concepts serving different purposes - the PR rename only applies to the solver parameter, not the Example class attributes.

Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:739-752
Timestamp: 2025-09-22T21:03:39.624Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently intentionally only supports additive updates (assuming n_coords == n_dofs). Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work and should not be flagged as an issue in current reviews.

📚 Learning: 2025-11-24T08:05:21.390Z

Learnt from: adenzler-nvidia
Repo: newton-physics/newton PR: 1107
File: newton/_src/solvers/mujoco/kernels.py:973-974
Timestamp: 2025-11-24T08:05:21.390Z
Learning: In Newton's MuJoCo solver integration (newton/_src/solvers/mujoco/), the mapping between Newton joints and MuJoCo joints is not 1-to-1. Instead, each Newton DOF maps to a distinct MuJoCo joint. This means that for multi-DOF joints (like D6 with 6 DOFs), there will be 6 corresponding MuJoCo joints, each with its own properties (margin, solimp, solref, etc.). The mapping is done via dof_to_mjc_joint array, ensuring each DOF's properties are written to its own MuJoCo joint without overwriting.

Applied to files:

newton/_src/solvers/mujoco/kernels.py
newton/_src/solvers/mujoco/solver_mujoco.py
newton/tests/test_mujoco_solver.py
newton/tests/test_import_mjcf.py

📚 Learning: 2025-09-22T21:03:39.624Z

Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:739-752
Timestamp: 2025-09-22T21:03:39.624Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently intentionally only supports additive updates (assuming n_coords == n_dofs). Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work and should not be flagged as an issue in current reviews.

Applied to files:

newton/_src/solvers/mujoco/kernels.py

📚 Learning: 2025-09-22T21:03:18.367Z

Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:638-648
Timestamp: 2025-09-22T21:03:18.367Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently only supports additive updates, meaning it assumes n_coords == n_dofs. Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work.

Applied to files:

newton/_src/solvers/mujoco/kernels.py

📚 Learning: 2025-08-18T15:56:26.587Z

Learnt from: adenzler-nvidia
Repo: newton-physics/newton PR: 552
File: newton/_src/solvers/mujoco/solver_mujoco.py:0-0
Timestamp: 2025-08-18T15:56:26.587Z
Learning: In Newton's MuJoCo solver, when transforming joint axes from Newton's internal frame to MuJoCo's expected frame, use wp.quat_rotate(joint_rot, axis) not wp.quat_rotate_inv(joint_rot, axis). The joint_rot represents rotation from joint-local to body frame, so forward rotation is correct.

Applied to files:

newton/_src/solvers/mujoco/kernels.py

📚 Learning: 2025-08-19T12:27:30.629Z

Learnt from: preist-nvidia
Repo: newton-physics/newton PR: 579
File: newton/examples/example_mujoco.py:350-354
Timestamp: 2025-08-19T12:27:30.629Z
Learning: In Newton examples, there's a distinction between solver parameters and Example class attributes. The Example class can have its own use_mujoco attribute for controlling example-level behavior (like CUDA graphs, rendering logic), while the solver uses use_mujoco_cpu for backend selection. These serve different purposes and should not be conflated during API renames.

Applied to files:

newton/_src/solvers/mujoco/kernels.py
newton/_src/solvers/mujoco/solver_mujoco.py
newton/tests/test_mujoco_solver.py

📚 Learning: 2025-09-22T21:08:31.901Z

Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/examples/ik/example_ik_franka.py:121-123
Timestamp: 2025-09-22T21:08:31.901Z
Learning: In the newton physics framework, when creating warp arrays for IK solver joint variables using wp.array(self.model.joint_q, shape=(1, coord_count)), the resulting array acts as a reference/pointer to the original model's joint coordinates, so updates from the IK solver automatically reflect in the model's joint_q buffer used for rendering, despite the general warp documentation suggesting copies are made by default.

Applied to files:

newton/_src/solvers/mujoco/kernels.py

📚 Learning: 2025-09-22T21:08:31.901Z

Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/examples/ik/example_ik_franka.py:121-123
Timestamp: 2025-09-22T21:08:31.901Z
Learning: In the newton physics framework, when creating warp arrays for IK solver joint variables using wp.array(self.model.joint_q, shape=(1, coord_count)), the resulting array acts as a reference/pointer to the original model's joint coordinates, so updates from the IK solver automatically reflect in the model's joint_q buffer used for rendering.

Applied to files:

newton/_src/solvers/mujoco/kernels.py

📚 Learning: 2025-08-19T12:27:30.629Z

Learnt from: preist-nvidia
Repo: newton-physics/newton PR: 579
File: newton/examples/example_mujoco.py:350-354
Timestamp: 2025-08-19T12:27:30.629Z
Learning: In Newton examples, Example.use_mujoco is a high-level attribute that controls whether to use MuJoCo solver vs other solvers (like XPBD), while SolverMuJoCo.use_mujoco_cpu controls the backend within MuJoCo (CPU vs Warp). These are separate concepts serving different purposes - the PR rename only applies to the solver parameter, not the Example class attributes.

Applied to files:

newton/_src/solvers/mujoco/solver_mujoco.py
newton/tests/test_mujoco_solver.py

📚 Learning: 2025-07-21T19:11:04.077Z

Learnt from: dylanturpin
Repo: newton-physics/newton PR: 450
File: newton/sim/ik.py:1286-1296
Timestamp: 2025-07-21T19:11:04.077Z
Learning: In Newton's IK system, the JointLimitObjective class intentionally only handles joints where DOF count equals coordinate count (1-1 mapping). This is appropriate because joint limits are typically only meaningful for simple joint types like revolute and prismatic joints, not complex joints like ball or free joints where DOF count != coordinate count.

Applied to files:

newton/_src/solvers/mujoco/solver_mujoco.py

📚 Learning: 2025-08-12T18:07:17.267Z

Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/utils.py:30-33
Timestamp: 2025-08-12T18:07:17.267Z
Learning: In newton/_src/utils/import_usd.py, the PXR/USD modules are lazily loaded using try-except blocks within the functions rather than at module import time, making it safe to import parse_usd at the module level without causing import-time failures for users without USD dependencies.

Applied to files:

newton/tests/test_import_usd.py

📚 Learning: 2025-08-12T18:07:17.267Z

Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/utils.py:30-33
Timestamp: 2025-08-12T18:07:17.267Z
Learning: In newton/_src/utils/import_usd.py, the PXR/USD modules are lazily loaded using try-except blocks within the functions (lines 109-111 and 1289-1291) rather than at module import time, making it safe to import parse_usd at the module level without causing import-time failures for users without USD dependencies.

Applied to files:

newton/tests/test_import_usd.py

📚 Learning: 2025-10-27T18:45:18.446Z

Learnt from: Milad-Rakhsha-NV
Repo: newton-physics/newton PR: 710
File: newton/_src/utils/schema_resolver.py:164-168
Timestamp: 2025-10-27T18:45:18.446Z
Learning: In newton/_src/utils/schema_resolver.py, when using USD's GetAuthoredPropertiesInNamespace() to collect solver-specific attributes, prop.GetName() already returns the full namespaced attribute name (e.g., "physxJoint:armature", not just "armature"), so no additional namespace prefix is needed when storing the keys.

Applied to files:

newton/tests/test_import_usd.py

🧬 Code graph analysis (4)

newton/_src/solvers/mujoco/solver_mujoco.py (2)

newton/_src/sim/builder.py (3)

add_custom_attribute (623-668)

ModelBuilder (70-5751)

CustomAttribute (308-398)

newton/_src/sim/model.py (2)

ModelAttributeFrequency (49-70)

ModelAttributeAssignment (31-46)

newton/tests/test_mujoco_solver.py (2)

newton/_src/solvers/mujoco/solver_mujoco.py (2)

register_custom_attributes (151-272)

notify_model_changed (477-487)

newton/_src/solvers/flags.py (1)

SolverNotifyFlags (22-56)

newton/tests/test_import_mjcf.py (2)

newton/_src/sim/builder.py (1)

ModelBuilder (70-5751)

newton/_src/solvers/mujoco/solver_mujoco.py (1)

register_custom_attributes (151-272)

newton/tests/test_import_usd.py (1)

newton/_src/solvers/mujoco/solver_mujoco.py (2)

SolverMuJoCo (77-2108)

register_custom_attributes (151-272)

🪛 Ruff (0.14.7)

newton/tests/test_import_usd.py

961-961: Unused noqa directive (non-enabled: PLC0415)

Remove unused noqa directive

(RUF100)

⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

GitHub Check: Run GPU Tests / Run GPU Unit Tests on AWS EC2
GitHub Check: Run GPU Benchmarks / Run GPU Benchmarks on AWS EC2

🔇 Additional comments (7)

newton/tests/test_mujoco_solver.py (1)

1668-1783: Multi‑world solref_friction conversion test is consistent and thorough

The new test_solref_friction_conversion_and_update mirrors the existing solimpfriction test pattern: it correctly exercises registration, per‑DOF initialization, multi‑world mapping via joint_qd_start/joint_mjc_dof_start, runtime updates through SolverNotifyFlags.JOINT_DOF_PROPERTIES, and verifies MuJoCo dof_solref matches Newton’s mujoco.solreffriction. This is well‑aligned with the existing joint/DOF mapping design and should catch indexing regressions.

newton/tests/test_import_mjcf.py (1)

800-873: LGTM! Comprehensive test for solreffriction parsing.

The test properly validates:

MJCF parsing of the solreffriction attribute

Correct handling of merged joints with mixed explicit/default values

Per-DOF value assignment for multi-DOF joints

Explicit values for single joints

The test follows the established pattern from test_solimplimit_parsing and provides thorough coverage.

newton/_src/solvers/mujoco/solver_mujoco.py (5)

203-213: LGTM! Proper custom attribute registration.

The solreffriction attribute is correctly registered with:

JOINT_DOF frequency for per-DOF mapping

Vector length 2 for timeconst and dampratio

Appropriate default values [0.02, 1.0]

Based on learnings, the per-DOF mapping will correctly handle multi-DOF joints (like D6) where each DOF maps to its own MuJoCo joint.

1040-1040: LGTM! Proper attribute retrieval.

The solreffriction custom attribute is retrieved correctly and stored for use in joint parameter configuration.

1408-1409: LGTM! Consistent DOF parameter configuration.

The solref_friction parameter is correctly applied to both linear and angular DOFs:

Proper null check before usage

Correct indexing using per-DOF axis index

Matches MuJoCo joint parameter naming convention

The implementation is symmetric and follows the established pattern for other DOF properties.

Also applies to: 1487-1488

1755-1755: LGTM! Proper multi-world field expansion.

Adding dof_solref to the expansion list ensures that per-DOF solref values are correctly replicated across multiple MuJoCo worlds when separate_worlds=True.

1903-1903: LGTM! Complete runtime update path.

The solreffriction attribute is properly threaded through the update pipeline:

Retrieved from model custom attributes

Passed as input to the update kernel

Output to the MuJoCo Warp model's dof_solref field

This enables runtime updates of per-DOF friction reference parameters, consistent with other DOF properties like solimplimit and solimpfriction.

Also applies to: 1923-1923, 1934-1934

…ton-physics#1180) Signed-off-by: Alain Denzler <adenzler@nvidia.com>

adenzler-nvidia added 5 commits December 1, 2025 17:57

add initial implementation

545eede

Signed-off-by: Alain Denzler <adenzler@nvidia.com>

Merge branch 'main' into dev/adenzler/dof_solref

3b40a89

Merge branch 'main' into dev/adenzler/dof_solref

65788c3

align solver_mujoco

0391dd8

Signed-off-by: Alain Denzler <adenzler@nvidia.com>

update kernels

86f4de1

Signed-off-by: Alain Denzler <adenzler@nvidia.com>

improve testing

6461cae

Signed-off-by: Alain Denzler <adenzler@nvidia.com>

coderabbitai Bot reviewed Dec 3, 2025

View reviewed changes

adenzler-nvidia linked an issue Dec 3, 2025 that may be closed by this pull request

Add support for dof_solref (frictionloss) #1117

Closed

eric-heiden previously approved these changes Dec 3, 2025

View reviewed changes

eric-heiden added this pull request to the merge queue Dec 3, 2025

github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Dec 3, 2025

Merge branch 'main' into dev/adenzler/dof_solref

4032f95

adenzler-nvidia dismissed eric-heiden’s stale review via 4032f95 December 4, 2025 08:00

coderabbitai Bot reviewed Dec 4, 2025

View reviewed changes

Comment thread newton/tests/test_import_usd.py

shi-eric approved these changes Dec 4, 2025

View reviewed changes

adenzler-nvidia added this pull request to the merge queue Dec 4, 2025

Merged via the queue into newton-physics:main with commit 8f3bd12 Dec 4, 2025
24 of 28 checks passed

coderabbitai Bot mentioned this pull request Dec 8, 2025

Use mujoco attribute actuatorfrcrange #1170

Merged

4 tasks

coderabbitai Bot mentioned this pull request Dec 17, 2025

SolverMuJoCo: custom attributes for MJCF/MJC schema options #1211

Merged

3 tasks

This was referenced Jan 8, 2026

SolverMuJoCo: Custom attributes for contact pair #1234

Merged

Parse springref and ref #1288

Merged

eric-heiden pushed a commit to eric-heiden/newton that referenced this pull request Jan 28, 2026

SolverMuJoCo: add custom attribute for solreffriction/dof_solref (new…

025b353

…ton-physics#1180) Signed-off-by: Alain Denzler <adenzler@nvidia.com>

This was referenced Jan 29, 2026

Inertia and body mass randomization support #1197 #1494

Merged

Improve custom frequency handling from USD, parse MuJoCo actuators and tendons #1510

Merged

Implement qpos0 randomization #1404 #1507

Closed

This was referenced Feb 18, 2026

Fix free joint body_pos and add ref/qpos0 support for MuJoCo solver #1645

Merged

Add spatial tendon support for MuJoCo solver #1687

Merged

coderabbitai Bot mentioned this pull request Mar 6, 2026

Add gravitycomp for the body in mjspec #1957

Merged

3 tasks

coderabbitai Bot mentioned this pull request Mar 30, 2026

Document registered custom attributes and Mjc USD schemas for SolverMuJoCo #2227

Closed

3 tasks

mmacklin pushed a commit to mmacklin/newton that referenced this pull request Apr 7, 2026

SolverMuJoCo: add custom attribute for solreffriction/dof_solref (new…

5f3f292

…ton-physics#1180) Signed-off-by: Alain Denzler <adenzler@nvidia.com>

coderabbitai Bot mentioned this pull request May 27, 2026

Fix MuJoCo shape_material force-space scaling (#2009) #2964

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SolverMuJoCo: add custom attribute for solreffriction/dof_solref#1180

SolverMuJoCo: add custom attribute for solreffriction/dof_solref#1180
adenzler-nvidia merged 7 commits into
newton-physics:mainfrom
adenzler-nvidia:dev/adenzler/dof_solref

adenzler-nvidia commented Dec 3, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Dec 3, 2025 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Uh oh!

codecov Bot commented Dec 3, 2025 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

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

Uh oh!

Before your PR is "Ready for review"

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

codecov Bot commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

adenzler-nvidia commented Dec 3, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Dec 3, 2025 •

edited

Loading

codecov Bot commented Dec 3, 2025 •

edited

Loading