SolverMuJoCo: SolimpLimit parsing and update support#1047
Merged
adenzler-nvidia merged 37 commits intoNov 20, 2025
Merged
Conversation
Signed-off-by: Alain Denzler <adenzler@nvidia.com>
Signed-off-by: Alain Denzler <adenzler@nvidia.com>
Signed-off-by: Alain Denzler <adenzler@nvidia.com>
Signed-off-by: Alain Denzler <adenzler@nvidia.com>
Signed-off-by: Alain Denzler <adenzler@nvidia.com>
Signed-off-by: Alain Denzler <adenzler@nvidia.com>
Contributor
📝 WalkthroughWalkthroughAdds a per-joint MuJoCo Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Import as ImportUtils
participant Builder as ModelBuilder
participant Solver as SolverMuJoCo
participant Kernel as update_joint_dof_properties_kernel
rect rgb(250,250,255)
Import->>Builder: parse_custom_attributes(stage/dict, default=attr.default)
Note right of Builder `#DCEEFF`: missing attrs -> filled with attr.default
Builder->>Solver: register_custom_attributes(..., solimplimit)
end
rect rgb(250,255,250)
Builder->>Solver: build/expand model (populate template_dof_to_mjc_joint, joint_mjc_dof_start)
Solver->>Kernel: call update_joint_dof_properties_kernel(..., solimplimit, dof_to_mjc_joint, ...)
Note right of Kernel `#E8F8E8`: map template DOFs -> MuJoCo joints, emit jnt_solimp/jnt_solref and per‑DOF arrays
Kernel-->>Solver: jnt_solimp, jnt_solref, dof_armature, dof_frictionloss
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
adenzler-nvidia
commented
Nov 7, 2025
adenzler-nvidia
commented
Nov 7, 2025
adenzler-nvidia
commented
Nov 7, 2025
adenzler-nvidia
commented
Nov 7, 2025
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
newton/tests/test_import_mjcf.py (2)
704-775: Make joint-name lookup resilient to merge naming; assert array shapes earlyThe test assumes a merged joint key
"joint1_joint2", which can change with importer heuristics. Also, asserting expected shapes early helps catch 1D/2D mismatches sooner than during solver init.- joint_names = model.joint_key - joint1_idx = joint_names.index("joint1_joint2") + joint_names = model.joint_key + # Find the merged joint that contains both tokens + joint1_idx = next( + i for i, k in enumerate(joint_names) if "joint1" in k and "joint2" in k + ) @@ - solimplimit = model.mujoco.solimplimit.numpy() + solimplimit = model.mujoco.solimplimit.numpy() + # Shape guard: (joint_dof_count, 5) + self.assertEqual(solimplimit.ndim, 2) + self.assertEqual(solimplimit.shape[1], 5)Optional: If importer ever returns per‑DOF lists for multi‑DOF joints, also assert
solimplimit.shape[0] == model.joint_dof_count. This will localize regressions quickly.
776-805: Solver verification is valuable; add explicit shape check for jnt_solimpAdd a quick sanity check to avoid silent misindexing.
- jnt_solimp = solver.mjw_model.jnt_solimp.numpy() + jnt_solimp = solver.mjw_model.jnt_solimp.numpy() + # Expect (num_worlds, mjc_dof_count, 5) when separate_worlds=False + self.assertEqual(jnt_solimp.ndim, 3) + self.assertEqual(jnt_solimp.shape[2], 5)
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
newton/_src/sim/builder.py(1 hunks)newton/_src/solvers/mujoco/solver_mujoco.py(8 hunks)newton/_src/utils/import_utils.py(1 hunks)newton/tests/test_import_mjcf.py(2 hunks)newton/tests/test_import_usd.py(2 hunks)newton/tests/test_mujoco_solver.py(1 hunks)
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
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: 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.
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: 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.
📚 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/sim/builder.pynewton/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/sim/builder.pynewton/_src/solvers/mujoco/solver_mujoco.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/sim/builder.pynewton/_src/solvers/mujoco/solver_mujoco.py
📚 Learning: 2025-08-12T05:17:34.423Z
Learnt from: shi-eric
Repo: newton-physics/newton PR: 521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The Newton migration guide (docs/migration.rst) is specifically for documenting how to migrate existing warp.sim functionality to Newton equivalents. New Newton-only features that didn't exist in warp.sim do not need migration documentation.
Applied to files:
newton/_src/sim/builder.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/sim/builder.pynewton/_src/solvers/mujoco/solver_mujoco.pynewton/tests/test_import_usd.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/sim/builder.pynewton/_src/solvers/mujoco/solver_mujoco.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_import_mjcf.pynewton/tests/test_mujoco_solver.pynewton/_src/solvers/mujoco/solver_mujoco.pynewton/tests/test_import_usd.py
📚 Learning: 2025-07-14T03:57:29.670Z
Learnt from: Kenny-Vilella
Repo: newton-physics/newton PR: 398
File: newton/examples/example_mujoco.py:352-352
Timestamp: 2025-07-14T03:57:29.670Z
Learning: The use_mujoco option in newton/examples/example_mujoco.py is currently unsupported and causes crashes. The code automatically disables this option with a warning message when users attempt to enable it. This is intentionally kept as a placeholder for future implementation.
Applied to files:
newton/tests/test_import_mjcf.pynewton/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.pynewton/_src/solvers/mujoco/solver_mujoco.pynewton/tests/test_import_usd.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/tests/test_mujoco_solver.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/_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
🧬 Code graph analysis (4)
newton/tests/test_import_mjcf.py (1)
newton/_src/solvers/mujoco/solver_mujoco.py (1)
register_custom_attributes(1284-1311)
newton/tests/test_mujoco_solver.py (3)
newton/_src/sim/state.py (1)
joint_dof_count(108-112)newton/_src/solvers/mujoco/solver_mujoco.py (2)
register_custom_attributes(1284-1311)notify_model_changed(1512-1522)newton/_src/solvers/flags.py (1)
SolverNotifyFlags(22-56)
newton/_src/solvers/mujoco/solver_mujoco.py (3)
newton/_src/sim/joints.py (1)
JointType(20-44)newton/_src/sim/builder.py (4)
add_custom_attribute(587-632)ModelBuilder(71-5082)CustomAttribute(271-361)joint_count(844-848)newton/_src/sim/model.py (2)
ModelAttributeFrequency(49-70)ModelAttributeAssignment(31-46)
newton/tests/test_import_usd.py (2)
newton/_src/solvers/mujoco/solver_mujoco.py (1)
register_custom_attributes(1284-1311)newton/_src/sim/builder.py (2)
ModelBuilder(71-5082)joint_count(844-848)
🪛 GitHub Actions: GPU Unit Tests on AWS EC2
newton/_src/solvers/mujoco/solver_mujoco.py
[error] 985-985: WarpCodegenTypeError: Error while parsing function "update_joint_dof_properties_kernel" has_solimplimit = solimplimit is not None ;None type unsupported
🪛 GitHub Actions: Pull Request
newton/tests/test_import_mjcf.py
[error] 777-777: SolverMuJoCo initialization failed during solimplimit parsing; related to 2D/1D array mismatch in solimplimit handling.
newton/tests/test_mujoco_solver.py
[error] 604-604: AssertionError: MuJoCo DOF 0 in world 0 armature should match Newton value (expected 0.01, got 0.0).
[error] 741-741: AttributeError: 'Model' object has no attribute 'mujoco' when accessing solver_mujoco from test_jnt_solimp_conversion_and_updates.
newton/_src/solvers/mujoco/solver_mujoco.py
[error] 2621-2621: Error launching kernel 'update_joint_dof_properties_kernel', argument 'solimplimit' expects an array with 2 dimension(s) but the passed array has 1 dimension(s).
[error] 985-985: WarpCodegenTypeError: None type unsupported. In update_joint_dof_properties_kernel, has_solimplimit = solimplimit is not None; None type unsupported.
🪛 Ruff (0.14.3)
newton/_src/sim/builder.py
756-759: Avoid specifying long messages outside the exception class
(TRY003)
762-764: Avoid specifying long messages outside the exception class
(TRY003)
newton/tests/test_import_usd.py
662-662: 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). (1)
- GitHub Check: Run GPU Benchmarks (Pull Request)
🔇 Additional comments (2)
newton/_src/utils/import_utils.py (1)
118-122: Sensible defaulting for missing custom attributesGood change. Applying
attr.defaultwhen the source is missing keeps downstream arrays dense and avoids per‑callers having to backfill. No functional concerns.If any defaults are mutable structs, ensure callers don’t mutate them in-place after finalize. If that’s possible in your pipeline, consider copying on write at assignment time.
newton/tests/test_import_mjcf.py (1)
26-27: Importing SolverMuJoCo directly in tests is fineNo issues. Keeps the test self-contained.
Signed-off-by: Alain Denzler <adenzler@nvidia.com>
Signed-off-by: Alain Denzler <adenzler@nvidia.com>
Signed-off-by: Alain Denzler <adenzler@nvidia.com>
Signed-off-by: Alain Denzler <adenzler@nvidia.com>
Signed-off-by: Alain Denzler <adenzler@nvidia.com>
Signed-off-by: Alain Denzler <adenzler@nvidia.com>
Signed-off-by: Alain Denzler <adenzler@nvidia.com>
Signed-off-by: Alain Denzler <adenzler@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
newton/_src/sim/builder.py (2)
746-753: Good fix: accept single Warp vec/mat for JOINT_DOF and wrapDirect
wp.types.type_is_vector/matrixwith list-wrapping is correct and unblocks single‑DOF vector inputs (e.g., solimplimit). Also please update the method docstring above (“For DOF and COORD attributes, values must always be provided as lists…”) to reflect this relaxed single‑value rule for single‑DOF joints to avoid confusion.
754-757: Shorten the TypeError message (Ruff TRY003)Trim the message to satisfy TRY003 and keep it actionable.
- else: - raise TypeError( - f"JOINT_DOF attribute '{attr_key}' must be a list with length equal to joint DOF count ({dof_count}), " - f"or a single Warp vector/matrix value for single-DOF joints" - ) + else: + raise TypeError( + f"JOINT_DOF '{attr_key}': expected list(len={dof_count}) or single Warp vec/mat for single-DOF." + )
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
newton/_src/sim/builder.py(1 hunks)newton/_src/solvers/mujoco/kernels.py(1 hunks)newton/_src/solvers/mujoco/solver_mujoco.py(15 hunks)newton/tests/test_mujoco_solver.py(5 hunks)
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
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.
📚 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.pynewton/tests/test_mujoco_solver.pynewton/_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/solver_mujoco.pynewton/tests/test_mujoco_solver.pynewton/_src/solvers/mujoco/kernels.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/_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/solver_mujoco.pynewton/tests/test_mujoco_solver.pynewton/_src/solvers/mujoco/kernels.pynewton/_src/sim/builder.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/solver_mujoco.pynewton/_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/solver_mujoco.pynewton/tests/test_mujoco_solver.pynewton/_src/solvers/mujoco/kernels.pynewton/_src/sim/builder.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.pynewton/tests/test_mujoco_solver.pynewton/_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/solver_mujoco.pynewton/_src/solvers/mujoco/kernels.pynewton/_src/sim/builder.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/solver_mujoco.pynewton/tests/test_mujoco_solver.pynewton/_src/solvers/mujoco/kernels.py
📚 Learning: 2025-09-25T16:14:22.033Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_objectives.py:0-0
Timestamp: 2025-09-25T16:14:22.033Z
Learning: In NVIDIA Warp's Newton physics library, wp.transform supports direct numerical indexing (e.g., body_tf[0], body_tf[1], body_tf[2] for position and body_tf[3], body_tf[4], body_tf[5], body_tf[6] for quaternion components) to access its elements. This is the standard API used throughout the codebase.
Applied to files:
newton/_src/solvers/mujoco/solver_mujoco.py
📚 Learning: 2025-08-12T05:17:34.423Z
Learnt from: shi-eric
Repo: newton-physics/newton PR: 521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The Newton migration guide (docs/migration.rst) is specifically for documenting how to migrate existing warp.sim functionality to Newton equivalents. New Newton-only features that didn't exist in warp.sim do not need migration documentation.
Applied to files:
newton/_src/sim/builder.py
🧬 Code graph analysis (2)
newton/_src/solvers/mujoco/solver_mujoco.py (3)
newton/_src/solvers/mujoco/kernels.py (1)
update_joint_dof_properties_kernel(915-992)newton/_src/sim/builder.py (4)
add_custom_attribute(587-632)ModelBuilder(71-5079)CustomAttribute(271-361)joint_count(841-845)newton/_src/sim/model.py (2)
ModelAttributeFrequency(49-70)ModelAttributeAssignment(31-46)
newton/tests/test_mujoco_solver.py (5)
newton/_src/sim/state.py (1)
joint_dof_count(108-112)newton/_src/sim/builder.py (2)
joint_count(841-845)ModelBuilder(71-5079)newton/_src/sim/joints.py (1)
JointType(20-44)newton/_src/solvers/mujoco/solver_mujoco.py (2)
register_custom_attributes(152-179)notify_model_changed(380-390)newton/_src/solvers/flags.py (1)
SolverNotifyFlags(22-56)
🪛 Ruff (0.14.3)
newton/_src/sim/builder.py
754-757: Avoid specifying long messages outside the exception class
(TRY003)
761-761: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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). (1)
- GitHub Check: Run GPU Benchmarks (Pull Request)
🔇 Additional comments (8)
newton/_src/sim/builder.py (1)
759-765: LGTM: count validation and per‑DOF assignmentLength check and per‑DOF application via
enumerate(value_sanitized)are correct.newton/tests/test_mujoco_solver.py (5)
540-584: Per-DOF seeding logic looks good.Patterns are clear and world/dof indexing is correct. No issues.
626-626: Precision relaxation to 3 decimals is fine.Reduces flaky failures on GPU/CPU backends; acceptable for armature checks.
649-680: Update-pattern generation is consistent with initial seeding.World/joint/DOF offsets align; no functional concerns.
753-756: Verify custom attribute propagation through replicate.You register attributes on template_builder before replicate(); confirm replicate carries custom attribute specs into builder so model.mujoco.solimplimit exists post-finalize. If not, call register_custom_attributes(builder) before builder.finalize().
Also applies to: 804-809
1051-1053: Correct notify flag.Updating jnt_solref/jnt_solimp via JOINT_DOF_PROPERTIES is the right path.
newton/_src/solvers/mujoco/solver_mujoco.py (1)
169-179: Custom attribute registration LGTM.Per-DOF vec5 with MuJoCo defaults and USD key mjc:solimplimit is correct.
newton/_src/solvers/mujoco/kernels.py (1)
915-931: Fix Warp guards for optional solimplimit (None/array truthiness is illegal).
- Warp kernels can’t take None, and
if solimplimit:is invalid (arrays aren’t booleans).- Add an explicit has_solimplimit flag and guard writes with it.
Apply this diff:
@wp.kernel def update_joint_dof_properties_kernel( joint_qd_start: wp.array(dtype=wp.int32), joint_dof_dim: wp.array2d(dtype=wp.int32), joint_mjc_dof_start: wp.array(dtype=wp.int32), template_dof_to_mjc_joint: wp.array(dtype=wp.int32), joint_armature: wp.array(dtype=float), joint_friction: wp.array(dtype=float), joint_limit_ke: wp.array(dtype=float), joint_limit_kd: wp.array(dtype=float), - solimplimit: wp.array(dtype=wp.types.vector(length=5, dtype=wp.float32)), + solimplimit: wp.array(dtype=wp.types.vector(length=5, dtype=wp.float32)), + has_solimplimit: int, joints_per_world: int, @@ - # Update solimplimit (per joint) - if solimplimit: + # Update solimplimit (per joint) + if has_solimplimit != 0: jnt_solimp[worldid, mjc_joint_index] = solimplimit[newton_dof_index] @@ - # Update solimplimit (per joint) - if solimplimit: + # Update solimplimit (per joint) + if has_solimplimit != 0: jnt_solimp[worldid, mjc_joint_index] = solimplimit[newton_dof_index]Also applies to: 970-972, 991-993
⛔ Skipped due to learnings
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.Learnt from: eric-heiden Repo: newton-physics/newton PR: 818 File: newton/_src/viewer/viewer_usd.py:249-259 Timestamp: 2025-09-25T16:48:54.358Z Learning: In Warp kernels and functions, use `if array: ...` to check for arrays being not None, rather than explicit `if array is not None:` checks. This is the correct and idiomatic way in the Warp ecosystem.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.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.Learnt from: gdaviet Repo: newton-physics/newton PR: 750 File: newton/_src/solvers/implicit_mpm/solve_rheology.py:969-986 Timestamp: 2025-09-18T07:05:56.836Z Learning: In newton/_src/solvers/implicit_mpm/solve_rheology.py, transposed_strain_mat parameter cannot be None - the type signature was corrected to reflect this guarantee, eliminating the need for None checks when accessing transposed_strain_mat.offsets.
This was referenced Nov 7, 2025
Closed
Closed
Signed-off-by: Alain Denzler <adenzler@nvidia.com>
…re we get the mapping correct. Signed-off-by: Alain Denzler <adenzler@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
newton/_src/utils/import_utils.py (1)
80-103: LGTM: Vector/matrix padding logic is correct.The padding implementation correctly handles vectors and matrices, using
warp_dtype._length_to determine the expected length and padding with defaults as needed.The static analysis hint suggests extracting the error message to avoid specifying long messages in the exception class. This is a minor style improvement:
- raise ValueError(f"Unable to parse vector/matrix value: {value} as {warp_dtype}.") + raise ValueError(f"Unable to parse vector/matrix value: {value} as {warp_dtype}.")However, this is purely stylistic and the current implementation is acceptable.
newton/_src/sim/builder.py (2)
770-824: JOINT_DOF dict/list/single‑value handling looks correct; consider optional scalar shortcutThe JOINT_DOF branch correctly:
- Derives per‑joint DOF ranges (
dof_start/dof_end).- Validates dict keys as in‑range ints and maps them to global DOF indices.
- For non‑dict input, treats a single non‑sequence Warp vec/mat as a length‑1 list and then enforces
len == dof_count, so only single‑DOF joints can use that shortcut.- Applies values via
_process_custom_attributesper DOF.This matches the intended behavior and keeps multi‑DOF validation strict. If you ever want a looser UX, you could optionally also accept a bare scalar for single‑DOF joints (wrap it into a one‑element list alongside the Warp vec/mat case), but that’s not required for correctness.
826-874: JOINT_COORD dict/list handling is sound; Ruff TRY003 warnings are purely stylisticThe JOINT_COORD logic mirrors JOINT_DOF correctly:
- Computes local
coord_start/coord_endandcoord_count.- Dict format validates integer indices and bounds, then forwards per‑coord values via
_process_custom_attributes, leaving unspecified coordinates to pick up defaults at finalization.- List/tuple format enforces exact length equality with
coord_countbefore applying values.No functional issues here. Ruff’s TRY003 hints on the various
TypeError/ValueErrormessages in this block are style-only; if they’re noisy in CI, you can shorten the messages slightly (similar to the concise JOINT_DOF length error) or centralize them in small helper functions, but there’s no need to change behavior.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
newton/_src/sim/builder.py(4 hunks)newton/_src/utils/import_mjcf.py(3 hunks)newton/_src/utils/import_utils.py(5 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
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: 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.
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: 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.
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-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/sim/builder.pynewton/_src/utils/import_utils.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/sim/builder.pynewton/_src/utils/import_utils.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/sim/builder.py
📚 Learning: 2025-08-12T05:17:34.423Z
Learnt from: shi-eric
Repo: newton-physics/newton PR: 521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The Newton migration guide (docs/migration.rst) is specifically for documenting how to migrate existing warp.sim functionality to Newton equivalents. New Newton-only features that didn't exist in warp.sim do not need migration documentation.
Applied to files:
newton/_src/utils/import_utils.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/utils/import_utils.py
🧬 Code graph analysis (2)
newton/_src/sim/builder.py (1)
newton/_src/sim/model.py (1)
ModelAttributeFrequency(49-70)
newton/_src/utils/import_utils.py (1)
newton/_src/usd/utils.py (3)
get_vector(200-200)get_vector(204-204)get_vector(207-226)
🪛 Ruff (0.14.5)
newton/_src/sim/builder.py
785-787: Avoid specifying long messages outside the exception class
(TRY003)
789-791: Avoid specifying long messages outside the exception class
(TRY003)
808-811: Avoid specifying long messages outside the exception class
(TRY003)
815-815: Avoid specifying long messages outside the exception class
(TRY003)
841-843: Avoid specifying long messages outside the exception class
(TRY003)
845-847: Avoid specifying long messages outside the exception class
(TRY003)
857-860: Avoid specifying long messages outside the exception class
(TRY003)
863-865: Avoid specifying long messages outside the exception class
(TRY003)
newton/_src/utils/import_utils.py
90-90: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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 (6)
newton/_src/utils/import_mjcf.py (2)
572-572: LGTM: Type change enables per-DOF attribute mapping.The change from
list[Any]todict[int, Any]correctly enables sparse storage of DOF attributes, where each attribute value is keyed by its DOF index rather than stored sequentially in a list.
587-652: LGTM: Per-DOF index tracking is correctly implemented.The
current_dof_indexcounter correctly tracks DOF positions within each body. The logic properly:
- Initializes to 0 for each body
- Stores attributes keyed by DOF index
- Increments only for joints that add DOFs (hinge/slide)
- Avoids incrementing for free/fixed joints that break early
newton/_src/utils/import_utils.py (3)
26-44: LGTM: Default parameter addition is well-documented.The addition of the
defaultparameter and corresponding docstring updates clearly explain the new padding behavior for incomplete vectors/matrices/quaternions.
63-73: LGTM: Quaternion padding logic is correct.The quaternion padding implementation correctly handles both scalar and iterable defaults, extracting the appropriate components to pad up to 4 values.
106-149: LGTM: Transformer correctly propagates defaults.The transformer updates correctly:
- Accept the
defaultparameter from the custom attribute- Pass it through to
parse_warp_value_from_string- Only include explicitly specified attributes in output, as documented
This enables the default-padding behavior while preserving the ability to distinguish between specified and unspecified attributes.
newton/_src/sim/builder.py (1)
738-747: Docstring for joint custom attributes matches new semanticsThe expanded docstring clearly reflects the new dict/list support and the single Warp vec/mat case for single‑DOF
JOINT_DOFattributes; no changes needed here.
Member
Author
|
@eric-heiden would be great to get another look on this - this is ready to go except some flaky tests I'm looking into. I don't think it's being caused by this PR. |
…dof joints Signed-off-by: Alain Denzler <adenzler@nvidia.com>
vreutskyy
reviewed
Nov 20, 2025
Signed-off-by: Alain Denzler <adenzler@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
newton/_src/solvers/mujoco/kernels.py(3 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
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, 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: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.
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-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
📚 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/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: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: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-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-09-25T16:14:22.033Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_objectives.py:0-0
Timestamp: 2025-09-25T16:14:22.033Z
Learning: In NVIDIA Warp's Newton physics library, wp.transform supports direct numerical indexing (e.g., body_tf[0], body_tf[1], body_tf[2] for position and body_tf[3], body_tf[4], body_tf[5], body_tf[6] for quaternion components) to access its elements. This is the standard API used throughout the codebase.
Applied to files:
newton/_src/solvers/mujoco/kernels.py
📚 Learning: 2025-09-25T16:14:22.033Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_objectives.py:0-0
Timestamp: 2025-09-25T16:14:22.033Z
Learning: wp.transform in NVIDIA Warp supports indexing using numerical indices (e.g., body_tf[0], body_tf[3], etc.) to access translation and rotation components. The indexing approach used in the Newton codebase is valid and compiles correctly.
Applied to files:
newton/_src/solvers/mujoco/kernels.py
⏰ 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 (4)
newton/_src/solvers/mujoco/kernels.py (4)
906-922: LGTM! Kernel signature properly reflects per-joint mapping requirements.The renamed kernel and expanded parameters correctly support the new per-joint template→MuJoCo DOF mapping approach, with appropriate inputs for solimplimit and solref handling.
1033-1035: LGTM! Template DOF lookup correctly fixed for multi-world joints.The code now correctly uses
joint_in_worldto derivetemplate_dof_startbefore indexingdof_to_mjc_joint, addressing the past review comment about multi-world indexing. This ensures the template→MuJoCo joint mapping works correctly across multiple worlds.
1019-1026: LGTM! Body transform update logic is correct.The code correctly:
- Uses
joint_in_worldto get the Newton child body ID (per-world index)- Maps to MuJoCo body ID using
body_mapping- Computes the body transform from joint anchors
- Converts quaternion format from Newton (xyzw) to MuJoCo (wxyz)
965-966: Fix conditional array check in Warp kernel.The conditional
if solimplimit:at lines 965 and 986 won't work correctly in Warp kernels. Warp's type system doesn't support checking array parameters as booleans to determine if they were "passed" or are "None". This pattern would either fail at kernel compilation or always evaluate as true.Apply this diff to add a boolean flag parameter:
def update_joint_dof_properties_kernel( joint_qd_start: wp.array(dtype=wp.int32), joint_dof_dim: wp.array2d(dtype=wp.int32), joint_mjc_dof_start: wp.array(dtype=wp.int32), dof_to_mjc_joint: wp.array(dtype=wp.int32), joint_armature: wp.array(dtype=float), joint_friction: wp.array(dtype=float), joint_limit_ke: wp.array(dtype=float), joint_limit_kd: wp.array(dtype=float), solimplimit: wp.array(dtype=vec5), + use_solimplimit: bool, joints_per_world: int, # outputs dof_armature: wp.array2d(dtype=float), dof_frictionloss: wp.array2d(dtype=float), jnt_solimp: wp.array2d(dtype=vec5), jnt_solref: wp.array2d(dtype=wp.vec2), ):Then update the conditional checks:
- if solimplimit: + if use_solimplimit: jnt_solimp[worldid, mjc_joint_index] = solimplimit[newton_dof_index]And similarly at line 986.
Also applies to: 986-987
⛔ Skipped due to learnings
Learnt from: eric-heiden Repo: newton-physics/newton PR: 818 File: newton/_src/viewer/viewer_usd.py:249-259 Timestamp: 2025-09-25T16:48:54.358Z Learning: In Warp kernels and functions, use `if array: ...` to check for arrays being not None, rather than explicit `if array is not None:` checks. This is the correct and idiomatic way in the Warp ecosystem.Learnt from: adenzler-nvidia Repo: newton-physics/newton PR: 1047 File: newton/_src/solvers/mujoco/solver_mujoco.py:1723-1749 Timestamp: 2025-11-10T12:33:57.428Z Learning: In Warp, it is valid to pass None as a kernel input parameter for optional arrays. Inside the kernel, use `if array:` to check whether the array was provided before accessing it. This pattern eliminates the need for separate presence flags and dummy arrays.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.
vreutskyy
approved these changes
Nov 20, 2025
This was referenced Nov 21, 2025
This was referenced Dec 1, 2025
This was referenced Dec 11, 2025
This was referenced Dec 18, 2025
eric-heiden
pushed a commit
to eric-heiden/newton
that referenced
this pull request
Jan 28, 2026
…1047) Signed-off-by: Alain Denzler <adenzler@nvidia.com>
This was referenced Jan 28, 2026
mmacklin
pushed a commit
to mmacklin/newton
that referenced
this pull request
Apr 7, 2026
…1047) Signed-off-by: Alain Denzler <adenzler@nvidia.com>
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
3 participants
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.
This follows the custom attribute setup that we merged recently and test-drives parsing as well as setting up the right values.
I made the following decisions that can be discussed:
Before your PR is "Ready for review"
newton/tests/test_examples.py)pre-commit run -aSummary by CodeRabbit
New Features
Bug Fixes
Tests
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.