Skip to content

[Sensor] Fix ray caster path matching regressions - #7516

Merged
ooctipus merged 5 commits into
isaac-sim:developfrom
StafaH:fix/ray-caster-regex-paths
Sep 4, 2026
Merged

[Sensor] Fix ray caster path matching regressions#7516
ooctipus merged 5 commits into
isaac-sim:developfrom
StafaH:fix/ray-caster-regex-paths

Conversation

@StafaH

@StafaH StafaH commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Description

This PR fixes the two remaining ray-caster path-matching failures:

  1. The ray-caster camera tutorial counted four sensors for two camera prims because /World/Origin_.*/CameraSensor allowed .* to cross path separators under whole-path regular-expression matching.
  2. Legacy Newton multi-mesh ray casters could fail to associate tracked-target sites when registration used a regular-expression path but initialization resolved it to concrete prim paths.

The stale post-reset Newton sensor result reported in #7236 is now fixed at the sensor state boundary by #7523, which this branch inherits from develop. The earlier environment-loop synchronization workaround and its tests/changelog fragments have therefore been removed from this PR.

Changes

  • Restrict the tutorial environment wildcard to one path segment with /World/Origin_[^/]+/CameraSensor.
  • Associate legacy Newton tracked-target site labels by configuration order instead of path-expression strings.
  • Remove the obsolete --enable_cameras argument from the tutorial smoke case.
  • Add regression coverage for regular-expression target resolution.

Fixes #6572

Related: #7236, resolved by #7523.

Type of change

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

Release backport

  • Backport this pull request to the active release branch after it merges into develop

Screenshots

Not applicable.

Validation

The full standalone Newton multi-object demo smoke could not launch in this checkout because the newly merged installer guard rejects combining the downloaded Isaac Sim package with the active Python virtual environment; it exited before application startup.

Checklist

  • I have read and understood the contribution guidelines
  • I have run the pre-commit checks
  • I have made corresponding changes to the tutorial and smoke configuration
  • My changes generate no new warnings
  • I have added tests that prove the remaining fix is effective
  • I have added changelog fragments for the touched source packages
  • My name already exists in CONTRIBUTORS.md

@StafaH
StafaH requested a review from a team September 3, 2026 05:39
@github-actions github-actions Bot added bug Something isn't working isaac-lab Related to Isaac Lab team labels 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

Reviewed the bounded tutorial camera regex, smoke-command cleanup, and legacy Newton tracked-target association change. The patch consistently updates the affected producer/consumer paths and adds focused regression coverage for resolving a configured regex to a concrete tracked target.

  • Design and architecture: The Newton-specific fix remains confined to _LegacyNewtonRayCasterMixin. Associating registered site-label groups with tracked targets in configuration order aligns with mesh-view initialization and the existing ordered traversal in _update_mesh_transforms; the main tradeoff is reliance on that initialization order rather than expression-key identity.
  • API: No public API is added, removed, or renamed. The smoke override now matches the tutorial launcher capabilities, and the Newton package includes an appropriate user-visible changelog fragment.
  • Implementation: The tracked-target index is reset before every warp-mesh initialization, preventing stale state on reinitialization. The new regression test exercises the concrete-path-versus-regex case addressed by the fix. A residual maintenance dependency remains: tracked views must continue to be created exactly once per tracked target and in configuration order.

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.

@StafaH StafaH changed the title Fix ray caster path matching regressions [Sensor] Fix ray caster path matching regressions Sep 3, 2026
@greptile-apps

greptile-apps Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes whole-path regular-expression handling for ray-caster cameras and legacy Newton tracked targets.

  • Bounds the tutorial environment wildcard to one path segment.
  • Associates tracked-target site labels with target configurations by their stable initialization order.
  • Removes an obsolete camera-launch argument from the tutorial smoke case.
  • Adds regression coverage for an ad hoc regex target resolving to a concrete Newton path.

Confidence Score: 5/5

The PR appears safe to merge, with the changed path matching and tracked-target association consistent with the relevant initialization and launcher contracts.

The ordered label registration and consumption paths use the same stable target configuration order, reinitialization resets the cursor, the bounded regex matches the intended camera prims, and the Warp-based tutorial does not require RTX camera enablement.

Important Files Changed

Filename Overview
source/isaaclab_newton/isaaclab_newton/sensors/ray_caster/legacy_ray_caster.py Replaces brittle expression-keyed site-label lookup with ordered association and resets that association before every mesh initialization.
source/isaaclab_newton/test/sensors/test_newton_raycast_sensor.py Adds focused regression coverage proving that a regex-registered tracked target initializes and produces the expected sensor batch.
scripts/tutorials/04_sensors/run_ray_caster_camera.py Restricts the environment wildcard to a single path segment so exactly the intended camera prims are selected.
source/isaaclab/test/app/standalone_script_cases.py Stops passing an unnecessary RTX camera-enablement argument to the Warp ray-caster camera tutorial.
source/isaaclab/test/app/test_standalone_scripts.py Updates launcher capability coverage to assert that the obsolete camera argument is absent.

Reviews (1): Last reviewed commit: "Fix ray caster path matching regressions" | Re-trigger Greptile


self._reset_idx(reset_env_ids)

# update articulation kinematics before sensors and observations read the reset state

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.

should this also be guarded with len(reset_env_ids) > 0? would this impact perf on the environments?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is now removed

@StafaH StafaH changed the title [Sensor] Fix ray caster path matching regressions [Sensor] Fix ray caster path matching and post-reset poses Sep 3, 2026
@ooctipus

ooctipus commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Changes the stepping logic might be a bit concerning, I think the root issue is that raycast sensor is not following the read access lazy update that all other assets and sensors are following.

#7523 might be an alternative root fix

kellyguo11 pushed a commit that referenced this pull request Sep 4, 2026
# Description

Fixes #7236.

Newton ray-cast tasks read `NewtonManager.get_state_0().body_q` inside
their graph-capturable query pipeline. After an in-step joint or
root-state write, the reset masks are current but the derived `body_q`
remains stale until forward kinematics runs. This made the first sensor
observation after a reset use the previous pose.

This fixes the stale read at its ownership boundary instead of adding
eager simulator synchronization to the RL environment loops:

- `NewtonManager._update_sensor_tasks()` obtains state through the
guarded `get_state()` accessor before BVH refit and sensor graph
capture/replay.
- The graph-captured raycast callback keeps using raw `get_state_0()`
state, so `forward()` is never captured.
- The ray-caster's direct `get_world_poses()` accessor applies the same
lazy-FK rule.
- The renderer's now-redundant state refresh is removed because the
sensor-task scheduler owns freshness for both renderer and raycast
consumers.

This is an alternative to the in-step synchronization part of #7516. Its
regex and legacy tracked-target fixes are independent of this PR.

## Architecture and performance

The state access boundary owns FK freshness; environment stepping
remains unaware of individual sensor requirements. The guard runs only
when a sensor or renderer update is requested. Rendering does not gain
an additional refresh because its existing call moved into the shared
scheduler, and Newton's device-resident reset masks limit the actual FK
work to invalidated worlds/articulations.

## Type of change

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

## Release backport

- [x] <!-- backport-active-release --> This PR already targets the
active release branch; do not backport it again.

## Screenshots

Not applicable.

## Validation

- Added observable regressions for sensor-data reads and direct
pose-getter reads immediately after a carrier root-pose write, with no
intervening simulation step or FK-sensitive asset getter. Both tests
failed before the fix by exactly the authored displacement in eager and
CUDA-graph modes.
- `uv run --frozen --extra test python -m pytest
source/isaaclab_newton/test/sensors/test_newton_raycast_sensor.py -vv`
(16 passed)
- `uv run --frozen --extra test python -m pytest
source/isaaclab_newton/test/physics/test_newton_manager_abstraction.py
source/isaaclab/test/sim/test_newton_manager_visualization_state.py -q`
(194 passed)
- `uv run --frozen isaaclab -f` (all checks passed)

## Checklist

- [x] I have read and understood the contribution guidelines
- [x] I have run the pre-commit checks
- [x] No standalone documentation change is required; the ownership rule
is documented at the access boundaries
- [x] My changes generate no new warnings
- [x] I have added tests that prove the fix is effective
- [x] I have added a changelog fragment for the touched package
- [x] My name already exists in `CONTRIBUTORS.md`
isaaclab-bot Bot pushed a commit that referenced this pull request Sep 4, 2026
# Description

Fixes #7236.

Newton ray-cast tasks read `NewtonManager.get_state_0().body_q` inside
their graph-capturable query pipeline. After an in-step joint or
root-state write, the reset masks are current but the derived `body_q`
remains stale until forward kinematics runs. This made the first sensor
observation after a reset use the previous pose.

This fixes the stale read at its ownership boundary instead of adding
eager simulator synchronization to the RL environment loops:

- `NewtonManager._update_sensor_tasks()` obtains state through the
guarded `get_state()` accessor before BVH refit and sensor graph
capture/replay.
- The graph-captured raycast callback keeps using raw `get_state_0()`
state, so `forward()` is never captured.
- The ray-caster's direct `get_world_poses()` accessor applies the same
lazy-FK rule.
- The renderer's now-redundant state refresh is removed because the
sensor-task scheduler owns freshness for both renderer and raycast
consumers.

This is an alternative to the in-step synchronization part of #7516. Its
regex and legacy tracked-target fixes are independent of this PR.

## Architecture and performance

The state access boundary owns FK freshness; environment stepping
remains unaware of individual sensor requirements. The guard runs only
when a sensor or renderer update is requested. Rendering does not gain
an additional refresh because its existing call moved into the shared
scheduler, and Newton's device-resident reset masks limit the actual FK
work to invalidated worlds/articulations.

## Type of change

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

## Release backport

- [x] <!-- backport-active-release --> This PR already targets the
active release branch; do not backport it again.

## Screenshots

Not applicable.

## Validation

- Added observable regressions for sensor-data reads and direct
pose-getter reads immediately after a carrier root-pose write, with no
intervening simulation step or FK-sensitive asset getter. Both tests
failed before the fix by exactly the authored displacement in eager and
CUDA-graph modes.
- `uv run --frozen --extra test python -m pytest
source/isaaclab_newton/test/sensors/test_newton_raycast_sensor.py -vv`
(16 passed)
- `uv run --frozen --extra test python -m pytest
source/isaaclab_newton/test/physics/test_newton_manager_abstraction.py
source/isaaclab/test/sim/test_newton_manager_visualization_state.py -q`
(194 passed)
- `uv run --frozen isaaclab -f` (all checks passed)

## Checklist

- [x] I have read and understood the contribution guidelines
- [x] I have run the pre-commit checks
- [x] No standalone documentation change is required; the ownership rule
is documented at the access boundaries
- [x] My changes generate no new warnings
- [x] I have added tests that prove the fix is effective
- [x] I have added a changelog fragment for the touched package
- [x] My name already exists in `CONTRIBUTORS.md`

(cherry picked from commit 8365c57)
@StafaH StafaH changed the title [Sensor] Fix ray caster path matching and post-reset poses [Sensor] Fix ray caster path matching regressions Sep 4, 2026
@StafaH

StafaH commented Sep 4, 2026

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 4, 2026
@ooctipus
ooctipus merged commit 167b6b1 into isaac-sim:develop Sep 4, 2026
53 checks passed
kellyguo11 pushed a commit that referenced this pull request Sep 4, 2026
# Description

Backports #7516 to `release/3.0.0`.

The automatic backport workflow completed successfully but skipped the
backport because the source PR body did not retain the hidden `<!--
backport-active-release -->` selection marker. This branch manually
cherry-picks the canonical merged commit
`167b6b15538bc6ec23554ec1b69f45ac34ae35bd` with `-x` provenance.

The cherry-pick applied without conflicts or release-specific edits. Its
stable patch ID is identical to the source commit, and all seven source
paths are preserved exactly.

## Type of change

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

## Release backport

- This PR already targets the active release branch.

## Validation

- Stable source/backport patch IDs match:
`b30f8378be6d764298eb95b92d8f5e03ffb97b51`.
- `uv run python -m pytest
source/isaaclab_newton/test/sensors/test_newton_raycast_sensor.py -vv` —
18 passed.
- `uv run python -m pytest
source/isaaclab/test/app/test_standalone_scripts.py::test_commands_respect_script_launcher_capabilities
-vv` — 1 passed.
- `ISAACLAB_CHANGELOG_BASE_REF=release/3.0.0 uv run isaaclab -f` — all
hooks passed.
- `git diff --check upstream/release/3.0.0..HEAD` — passed.

## Checklist

- [x] I have read and understood the contribution guidelines
- [x] I have run the pre-commit checks
- [x] I have made corresponding changes to the tutorial and smoke
configuration
- [x] My changes generate no new warnings
- [x] I have added tests that prove the fix is effective
- [x] I have added changelog fragments for the touched source packages
- [x] My name already exists in `CONTRIBUTORS.md`
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.

3 participants