Skip to content

[Backport release/3.0.0] Fix articulation wrench response assertion (#7504) - #7512

Merged
kellyguo11 merged 1 commit into
isaac-sim:release/3.0.0from
kellyguo11:codex/backport-pr-7504-release-3.0.0
Sep 3, 2026
Merged

[Backport release/3.0.0] Fix articulation wrench response assertion (#7504)#7512
kellyguo11 merged 1 commit into
isaac-sim:release/3.0.0from
kellyguo11:codex/backport-pr-7504-release-3.0.0

Conversation

@kellyguo11

Copy link
Copy Markdown
Contributor

Description

Backports #7504 to release/3.0.0 by cherry-picking the actual merged commit, 541de9a7a91f9e7c24b24b923c0620475a00e051.

The automatic backport stopped during source validation because the documentation hunk from #7504 had already reached develop through #7493. GitHub therefore omitted that already-present hunk from #7504's squash commit, while the PR files API still reported the original seven-file change. The validator rejected the resulting six-file squash patch before attempting a cherry-pick.

This PR replays those six remaining files exactly. The overlapping documentation change is handled separately by #7508.

Failed automation: https://github.com/isaac-sim/IsaacLab/actions/runs/33710984910/job/100510198896

No runtime dependencies are added.

Type of change

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

Release backport

  • This PR already targets the active release branch; do not backport it again.

Validation

  • uv run --no-project python -m compileall -q on all three modified test modules
  • uv run --no-project python tools/changelog/cli.py check codex-backport-pr-7504-base
  • uvx --from pre-commit==4.6.2 pre-commit run --all-files with check-changelog-fragments skipped because that hook assumes origin/develop; the release-base check above passed
  • .github/scripts/backport.py validate-candidate ... --exact_patch
  • git diff --check upstream/release/3.0.0..HEAD
  • The simulator-backed CUDA regression remains for Linux GPU CI

Checklist

  • I have read and understood the contribution guidelines
  • I have run the available pre-commit checks
  • Documentation changes are not needed for this six-file backport
  • My changes generate no new warnings
  • The existing mirrored regression tests cover the fix
  • Each touched package includes its original .skip changelog fragment
  • My name already exists in CONTRIBUTORS.md

# Description

The `test_external_force_on_multiple_bodies_at_position` test asserted
only the world-Z component of root angular velocity. The applied offset
wrench can produce rotation on another axis, especially when body-frame
wrenches are transformed, so the articulation can rotate while the Z
component remains below the threshold.

Check the total angular-speed magnitude instead, preserving the existing
`0.1` threshold. Apply the same assertion to the OVPhysX, PhysX, and
Newton test mirrors.

This addresses the recurring CUDA failure observed in [the
`test_articulation`
job](https://github.com/isaac-sim/IsaacLab/actions/runs/33674713218/job/100398456668?pr=7458),
where the world-Z angular velocity was `0.004370182752609253`.

Related to isaac-sim#7458.

No runtime dependencies are added.

## Type of change

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

## Release backport

- [x] <!-- backport-active-release --> Backport this pull request to the
active release branch after it merges into `develop`

## Screenshots

Not applicable; this is a test-only change.

## Validation

- `uv run --no-project python -m compileall -q` on all three modified
test modules
- `uv run --no-project python tools/changelog/cli.py check
codex-changelog-base --include-worktree`
- `uvx pre-commit run --all-files`
- The simulator-backed CUDA case requires Linux GPU CI and will be
requested with `run-ci`

## Checklist

- [x] I have read and understood the [contribution
guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html)
- [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with
`./isaaclab.sh --format`
- [x] I have made corresponding changes to the documentation (not
applicable: test-only change)
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works (updated the existing mirrored regression tests)
- [x] I have added a changelog fragment under
`source/<pkg>/changelog.d/` for every touched package (do **not** edit
`CHANGELOG.rst` or bump `extension.toml` — CI handles that)
- [x] I have added my name to the `CONTRIBUTORS.md` or my name already
exists there

## Documentation CI follow-up

The docs workflow also exposed 14 duplicate object registrations for
schema fragment classes documented on both `isaaclab.sim` and
`isaaclab.sim.schemas`. Keep the top-level public API rendering but mark
its alias copies `:no-index:`, leaving the canonical schema page as the
only Sphinx cross-reference target.

Failure: [Build Latest
Docs](https://github.com/isaac-sim/IsaacLab/actions/runs/33694569196/job/100460582334?pr=7504).

A full local Sphinx build completed without the original
duplicate-object warnings. Remaining local warnings were import failures
for project packages unavailable in the macOS validation environment;
the pushed Linux docs workflow provides the authoritative warning-free
result.

(cherry picked from commit 541de9a)
@kellyguo11

Copy link
Copy Markdown
Contributor Author

run-ci

@isaaclab-bot isaaclab-bot Bot added ci:run-docker Trigger the on-demand Docker and GPU CI workflow and removed ci:run-docker Trigger the on-demand Docker and GPU CI workflow labels Sep 3, 2026
@github-actions github-actions Bot added the isaac-lab Related to Isaac Lab team label Sep 3, 2026

@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

This backport consistently replaces the frame-dependent z-axis angular-velocity assertion with a rotation-magnitude assertion across the Newton, OV, and PhysX articulation regression tests, with matching .skip changelog fragments for each package.

  • Design and architecture: The mirrored backend tests remain synchronized, and the revised assertion avoids encoding a backend- or link-frame-specific response axis while retaining the intended check that the applied wrench causes rotation.
  • API: No production code, public API, configuration, export, or documented contract changes. The empty .skip fragments appropriately record a test-only change in each touched source package without adding release notes.
  • Implementation: The new vector-norm assertion still rejects a non-rotating articulation. It is intentionally less axis-specific than the previous z-component assertion, so it would not detect rotation occurring around an unexpected axis, but that tradeoff matches the stated frame-dependent behavior being corrected.

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.

@greptile-apps

greptile-apps Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This backport corrects a frame-dependent articulation wrench regression test without changing runtime behavior.

  • Replaces world-Z angular-velocity assertions with full-vector angular-velocity norms across the Newton, OV, and PhysX test suites.
  • Adds matching changelog skip fragments for all three packages.

Confidence Score: 5/5

The PR appears safe to merge with no actionable correctness or security issues identified.

The updated assertions continue to reject absent rotational responses while avoiding an invalid assumption about which world-space axis carries rotation from a link-frame-dependent wrench.

Important Files Changed

Filename Overview
source/isaaclab_newton/test/assets/test_articulation.py Updates the Newton wrench-response test to verify rotation independently of link-frame orientation.
source/isaaclab_ov/test/assets/test_articulation.py Applies the same frame-independent wrench-response assertion to the OV test mirror.
source/isaaclab_physx/test/assets/test_articulation.py Applies the same frame-independent wrench-response assertion to the PhysX test mirror.

Reviews (1): Last reviewed commit: "Fix articulation and schema documentatio..." | Re-trigger Greptile

@kellyguo11

Copy link
Copy Markdown
Contributor Author

run-ci

@isaaclab-bot isaaclab-bot Bot added ci:run-docker Trigger the on-demand Docker and GPU CI workflow and removed ci:run-docker Trigger the on-demand Docker and GPU CI workflow labels Sep 3, 2026
@kellyguo11
kellyguo11 merged commit 6cfcb1d into isaac-sim:release/3.0.0 Sep 3, 2026
52 of 80 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant