Skip to content

Fix Newton external wrench frame - #6934

Merged
kellyguo11 merged 14 commits into
isaac-sim:developfrom
AntoineRichard:antoiner/fix-newton-wrench-frame
Aug 7, 2026
Merged

Fix Newton external wrench frame#6934
kellyguo11 merged 14 commits into
isaac-sim:developfrom
AntoineRichard:antoiner/fix-newton-wrench-frame

Conversation

@AntoineRichard

@AntoineRichard AntoineRichard commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Description

Newton's external-wrench writers passed the body-frame output of the wrench composer directly to Newton's world-frame State.body_f buffer. This caused forces and torques applied to rotated bodies to follow world axes instead of the requested body axes.

This change:

  • rotates composed force and torque into world coordinates inside the existing Newton packing kernels;
  • preserves the center-of-mass reference point by using pose rotation only;
  • covers both reorder-free and body-ordered articulation paths, rigid objects, and rigid-object collections;
  • adds no buffer allocation or additional kernel launch.

No new dependencies are required.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Testing

Tests ran in a dedicated virtual environment created from the rebased branch's uv.lock.

  • 379 passed: WrenchComposer plus direct and articulation-ordered Newton packing regressions on CPU/CUDA.
  • 10 passed: rigid-object external-force integration selection.
  • 18 passed: rigid-object-collection external-force integration selection.
  • 20 passed: articulation external-force integration selection.
  • 4 passed: focused articulation force-at-position parameterizations across CPU/CUDA and one/two environments.
  • ./isaaclab.sh -f passed all hooks before commit and again before push.

The corrected wrench direction made the pre-existing 100 N articulation stress load overflow Newton's constraint capacity in three of four parameterizations. A one-line 100 N to 50 N test-load reduction keeps all 100 steps and the original angular-velocity assertion while avoiding unrelated solver divergence. Direct packing tests retain exact frame and body-order assertions.

Screenshots

Not applicable.

Checklist

  • I have read and understood the contribution guidelines
  • I have run the pre-commit checks with ./isaaclab.sh -f
  • I have made corresponding documentation changes
  • My changes generate no new warnings
  • I have added tests that prove the fix is effective
  • I have added a changelog fragment under source/isaaclab_newton/changelog.d/
  • My name already exists in CONTRIBUTORS.md

@AntoineRichard
AntoineRichard requested a review from a team August 6, 2026 07:38
@github-actions github-actions Bot added bug Something isn't working isaac-lab Related to Isaac Lab team labels Aug 6, 2026
@greptile-apps

greptile-apps Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR corrects Newton external-wrench packing by rotating body-frame forces and torques into world coordinates using each link pose quaternion. The implementation covers shared, articulation-ordered, rigid-object, and rigid-object-collection paths and adds focused CPU Warp regression tests.

  • Passes link poses into the existing wrench-packing kernels without another allocation or kernel launch.
  • Preserves COM-referenced torque by using only pose rotation, not translation.
  • Handles nonidentity articulation body ordering by rotating in public order before scattering into backend order.
  • Adds a changelog fragment and tests for rotation, translation invariance, and body reordering.

Confidence Score: 5/5

The PR appears safe to merge with no concrete blocking or non-blocking defects identified.

The updated kernels consistently invert the wrench composer's world-to-body conversion, use correctly ordered link poses, preserve COM-referenced torque, and are covered by focused ordering and frame regressions.

Important Files Changed

Filename Overview
source/isaaclab_newton/isaaclab_newton/assets/kernels.py The shared packing kernel now performs the required body-to-world quaternion rotation while preserving the COM reference point.
source/isaaclab_newton/isaaclab_newton/assets/articulation/kernels.py The ordered articulation kernel rotates public-order wrenches with matching public-order poses before scattering them into backend order.
source/isaaclab_newton/isaaclab_newton/assets/articulation/articulation.py Both articulation packing paths now supply the link-pose buffer expected by their updated kernels.
source/isaaclab_newton/isaaclab_newton/assets/rigid_object/rigid_object.py Rigid-object wrench packing now receives the current link pose needed for body-to-world conversion.
source/isaaclab_newton/isaaclab_newton/assets/rigid_object_collection/rigid_object_collection.py Collection wrench packing now supplies per-object link poses to the shared conversion kernel.
source/isaaclab_newton/test/assets/test_wrench_kernels.py Regression tests cover rotation direction, translation invariance, and nonidentity articulation body ordering.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[Composed body-frame force and torque] --> B{Articulation body ordering}
    B -->|Identity or rigid asset| C[Read public-order link rotation]
    B -->|Nonidentity| D[Read public-order link rotation and body map]
    C --> E[Rotate force and torque into world frame]
    D --> E
    E --> F{Backend ordering required?}
    F -->|No| G[Write world-frame Newton wrench]
    F -->|Yes| H[Scatter to backend body index]
    H --> G
Loading

Reviews (1): Last reviewed commit: "Correct Newton torque unit notation" | Re-trigger Greptile

@isaaclab-review-bot isaaclab-review-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isaac Lab Review Bot

The Newton external-wrench fix consistently rotates composed body-frame forces and torques into world coordinates at the packing boundary, with all changed kernel launch sites updated to pass body poses.

  • Design and architecture: Performing the rotation in the Newton packing kernels preserves the wrench composer's body-frame contract and avoids additional buffers or kernel launches. The reorder-free and ordered articulation paths, rigid objects, and rigid-object collections follow the same conversion design.
  • API: No public API surface is changed. The internal kernel signatures and supplied callers were updated together, while launch dimensions, masks, spatial-vector layout, and public-to-backend body ordering remain consistent. The package changelog fragment is correctly scoped and formatted.
  • Implementation: The kernels use only the link-pose quaternion, correctly avoiding a translation-induced moment for COM-referenced wrenches. Public-order poses are indexed before the ordered path scatters into backend order, and regression tests cover rotation, nonzero translation, and nonidentity ordering. Asset-level integration remains a CI validation point because it was unavailable locally.

No blocking issues. No inline issue met the actionable-evidence threshold; the assessment above records the review feedback.

Automated review; human maintainers own approval decisions.

Document the corrected frame contract for external wrenches. The fused
packing conversion now rotates body-frame forces and torques into the
world frame before simulation.
Use N·m consistently in the wrench kernel docstrings so their
physical-unit annotations follow repository documentation conventions.
@mmichelis

Copy link
Copy Markdown
Collaborator

Could we centralize the body-to-world conversion in the existing update_wrench_with_force_and_torque helper and reuse it from both kernels? The helper currently only packs two vectors into wp.spatial_vector, while the ordered and unordered paths duplicate the quaternion rotations. I suggest renaming it to something explicit such as pack_body_wrench_to_world(force_b, torque_b, body_rot_w) and having it rotate both components before packing. This would keep the frame contract in one place and ensure both body-ordering paths use identical conversion math.

@AntoineRichard
AntoineRichard force-pushed the antoiner/fix-newton-wrench-frame branch from e0983e1 to ffed6e9 Compare August 6, 2026 12:20
inputs=[
composer.out_force_b.warp,
composer.out_torque_b.warp,
self._data.body_link_pose_w.warp,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These proxy arrays should work without .warp right?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They should, but I may as well use the right one explicitely!

@kellyguo11
kellyguo11 merged commit 00b93dc into isaac-sim:develop Aug 7, 2026
72 of 75 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants