Align Featherstone solver wrench handling with COM convention #1160
+62
−14
Conversation
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
Contributor
📝 WalkthroughWalkthroughClarifies velocity and wrench conventions and adjusts runtime handling: FREE-joint body velocities are stored as COM linear + angular velocity; external wrenches are defined at the COM for all solvers and are converted for Featherstone via a new Warp kernel; forward-kinematics calls can request velocity conversion; tests enable the Featherstone solver. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant FK as ForwardKinematics
participant State as State (body_q/body_qd/body_f)
participant Solver as SolverFeatherstone
participant Kernel as convert_body_force_com_to_origin
Note over FK,State: FK can be called with convert_velocity=True
FK->>State: read body_q (transforms)
FK->>State: compute/store body_qd (if convert_velocity: convert spatial twist -> COM v + ω)
State->>Solver: state_in (body_q, body_qd, body_f)
Solver->>Kernel: convert_body_force_com_to_origin(body_q, body_X_com, body_f)
Kernel-->>Solver: body_f (transformed to origin, negated)
Solver->>Solver: run Featherstone dynamics using transformed body_f
Solver-->>State: write updated accelerations/forces
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (11)📓 Common learnings📚 Learning: 2025-08-26T01:04:20.331ZApplied to files:
📚 Learning: 2025-09-22T21:03:39.624ZApplied to files:
📚 Learning: 2025-09-25T16:12:33.769ZApplied to files:
📚 Learning: 2025-09-25T16:14:33.003ZApplied to files:
📚 Learning: 2025-08-18T15:56:26.587ZApplied to files:
📚 Learning: 2025-11-28T11:12:40.784ZApplied to files:
📚 Learning: 2025-09-22T21:08:31.901ZApplied to files:
📚 Learning: 2025-09-22T21:08:31.901ZApplied to files:
📚 Learning: 2025-11-28T11:12:40.006ZApplied to files:
📚 Learning: 2025-09-25T16:14:22.033ZApplied to files:
🧬 Code graph analysis (1)newton/_src/sim/articulation.py (1)
⏰ 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)
🔇 Additional comments (4)
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 |
jvonmuralt
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
newton/_src/sim/articulation.py(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
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.
Learnt from: camevor
Repo: newton-physics/newton PR: 1146
File: newton/examples/basic/example_basic_joints.py:215-222
Timestamp: 2025-11-28T11:12:40.006Z
Learning: In Warp's spatial vector convention used by Newton: spatial_bottom() extracts the angular velocity component (omega) and spatial_top() extracts the linear velocity component (v) from a spatial twist (qd). Similarly for spatial wrenches: spatial_bottom() is torque and spatial_top() is force.
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: camevor
Repo: newton-physics/newton PR: 1146
File: newton/examples/basic/example_basic_joints.py:206-213
Timestamp: 2025-11-28T11:12:40.784Z
Learning: In Newton's standard solvers (XPBD, SemiImplicit, SolverMuJoCo), spatial vectors in State.body_qd use the ordering (linear, angular): wp.spatial_top(qd) returns linear velocity and wp.spatial_bottom(qd) returns angular velocity. This is the opposite of typical Modern Robotics spatial twist notation where (angular, linear) is used.
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/_src/solvers/featherstone/kernels.py:75-75
Timestamp: 2025-08-12T18:04:06.577Z
Learning: The Newton physics framework requires nightly Warp builds, which means compatibility concerns with older stable Warp versions (like missing functions such as wp.spatial_adjoint) are not relevant for this project.
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.
📚 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/articulation.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/articulation.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/articulation.py
📚 Learning: 2025-11-28T11:12:40.784Z
Learnt from: camevor
Repo: newton-physics/newton PR: 1146
File: newton/examples/basic/example_basic_joints.py:206-213
Timestamp: 2025-11-28T11:12:40.784Z
Learning: In Newton's standard solvers (XPBD, SemiImplicit, SolverMuJoCo), spatial vectors in State.body_qd use the ordering (linear, angular): wp.spatial_top(qd) returns linear velocity and wp.spatial_bottom(qd) returns angular velocity. This is the opposite of typical Modern Robotics spatial twist notation where (angular, linear) is used.
Applied to files:
newton/_src/sim/articulation.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/articulation.py
📚 Learning: 2025-11-28T11:12:40.006Z
Learnt from: camevor
Repo: newton-physics/newton PR: 1146
File: newton/examples/basic/example_basic_joints.py:215-222
Timestamp: 2025-11-28T11:12:40.006Z
Learning: In Warp's spatial vector convention used by Newton: spatial_bottom() extracts the angular velocity component (omega) and spatial_top() extracts the linear velocity component (v) from a spatial twist (qd). Similarly for spatial wrenches: spatial_bottom() is torque and spatial_top() is force.
Applied to files:
newton/_src/sim/articulation.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/sim/articulation.py
🧬 Code graph analysis (1)
newton/_src/sim/articulation.py (1)
newton/_src/sim/joints.py (1)
JointType(20-44)
⏰ 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
eric-heiden
reviewed
Dec 4, 2025
| state: State | object, | ||
| mask: wp.array(dtype=bool) | None = None, | ||
| indices: wp.array(dtype=int) | None = None, | ||
| convert_velocity: bool = False, |
Member
There was a problem hiding this comment.
Can you adapt Featherstone also to use twists in the same frame we have for the other solvers so that we don't need this flag?
For numerical stability it is probably better to change Featherstone to use the articulation frame. @dylanturpin you may be already working on this part?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Description
Newton Migration Guide
Please ensure the migration guide for warp.sim users is up-to-date with the changes made in this PR.
docs/migration.rstis up-to dateBefore your PR is "Ready for review"
newton/tests/test_examples.py)pre-commit run -aSummary by CodeRabbit
Bug Fixes
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.