[Sensor] Fix ray caster path matching regressions - #7516
Conversation
There was a problem hiding this comment.
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.
Greptile SummaryThis PR fixes whole-path regular-expression handling for ray-caster cameras and legacy Newton tracked targets.
Confidence Score: 5/5The 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
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 |
There was a problem hiding this comment.
should this also be guarded with len(reset_env_ids) > 0? would this impact perf on the environments?
|
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 |
# 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`
# 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)
|
run-ci |
# 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`
Description
This PR fixes the two remaining ray-caster path-matching failures:
/World/Origin_.*/CameraSensorallowed.*to cross path separators under whole-path regular-expression matching.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
/World/Origin_[^/]+/CameraSensor.--enable_camerasargument from the tutorial smoke case.Fixes #6572
Related: #7236, resolved by #7523.
Type of change
Release backport
developScreenshots
Not applicable.
Validation
uv run python -m pytest source/isaaclab_newton/test/sensors/test_newton_raycast_sensor.py -vv(18 passed, including the [Newton] Refresh FK before ray-cast sensor reads #7523 FK regressions in eager and CUDA-graph modes)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=upstream-develop uv run isaaclab -fThe 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
CONTRIBUTORS.md