Fixes backend deprecation warning call sites#5418
Conversation
Greptile SummaryThis PR fixes deprecation warnings across both PhysX and Newton backends by updating call sites to use current API names for root state, joint state, wrench composer fields, and the Newton Confidence Score: 4/5Safe to merge; the API migration is consistent and the kernel logic is correct, though GPU tests timed out before running. No P0 or P1 findings. All renamed fields and method calls are applied uniformly across both backends and their tests. The Newton contact sensor refactor correctly separates the old single 3D force array into distinct total_force and force_matrix handles, with matching kernel and buffer-initialization logic. The only caveat is that targeted GPU pytest timed out during collection, so runtime correctness of the new sensor data paths was not directly validated. source/isaaclab_newton/isaaclab_newton/sensors/contact_sensor/contact_sensor.py and contact_sensor_kernels.py contain the substantive logic change and merit the most scrutiny when GPU tests are available. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[NewtonManager.add_contact_sensor\nmeasure_total=True] --> B[contact_view]
B --> C[total_force\nshape: n_envs x n_sensors]
B --> D[force_matrix\nshape: n_envs x n_sensors, n_filter_objects\nor None]
B --> E[sensing_obj_idx / sensing_obj_type]
B --> F[counterpart_indices / counterpart_type]
E --> H[flatten_metadata -> _sensor_names]
F --> I[flatten_metadata -> _filter_object_names]
C --> J[copy_from_newton_kernel\ndim = n_envs x n_sensors x max filter_objs,1]
D --> J
J --> K[net_force_total 2D\nn_envs, n_sensors]
J --> L[force_matrix_w 3D\nn_envs, n_sensors, n_filter_objects\nor None]
Reviews (1): Last reviewed commit: "Merge branch 'develop' into antoiner/bac..." | Re-trigger Greptile |
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Summary
This PR fixes deprecation warnings by updating call sites in PhysX and Newton backend tests to use current API names for root-state, joint-state, contact-sensor, and wrench-composer methods. It also updates the Newton contact sensor adapter to use the current SensorContact constructor and force/metadata fields. The changes are mechanically correct and follow the established deprecation migration patterns.
Architecture Impact
Self-contained. The changes are limited to:
- Test files updating deprecated method calls to their non-deprecated equivalents
- Newton contact sensor adapter updating to use new Newton
SensorContactAPI fields - Changelog and version bumps for both
isaaclab_newtonandisaaclab_physx
The core framework APIs remain unchanged; this PR only migrates consumers to the current API.
Implementation Verdict
Ship it — with one minor clarification suggestion.
Test Coverage
The PR modifies test files to use non-deprecated APIs, which is appropriate. The author reports running a focused deprecation scan showing 118 matches on origin/develop and 0 matches on the branch, confirming comprehensive cleanup. CI shows isaaclab_newton and isaaclab_physx tests passing. The GPU pytest timeout noted in the test plan appears to be infrastructure-related, not a code issue.
CI Status
✅ isaaclab_newton: success
✅ isaaclab_physx: success
✅ pre-commit: success
✅ license-check: success
⏳ Other checks skipped (expected for scoped changes)
Findings
🔵 Improvement: source/isaaclab_newton/isaaclab_newton/sensors/contact_sensor/contact_sensor.py:371-377 — Defensive handling of empty counterpart metadata
The flatten_metadata function handles nested structures well, but when self.contact_view.counterpart_indices and self.contact_view.counterpart_type have mismatched lengths, the zip() will silently truncate. Consider adding a length assertion:
flat_counterpart_idx = flatten_metadata(self.contact_view.counterpart_indices)
flat_counterpart_type = flatten_metadata(self.contact_view.counterpart_type)
assert len(flat_counterpart_idx) == len(flat_counterpart_type), "Counterpart metadata length mismatch"
flat_counterparts = list(zip(flat_counterpart_idx, flat_counterpart_type))This would surface data corruption issues earlier rather than producing subtly wrong filter names.
🔵 Improvement: source/isaaclab_newton/isaaclab_newton/sensors/contact_sensor/contact_sensor.py:380-381 — Clarify force_matrix shape handling
The code correctly handles force_matrix being None, but the shape extraction logic could be clearer:
force_matrix_shape = force_matrix.shape if force_matrix is not None else (total_sensor_count, 0)
self._num_filter_objects = force_matrix_shape[1] if len(force_matrix_shape) > 1 else 0If force_matrix is a 1D array (unexpected), len(force_matrix_shape) > 1 would be False, resulting in _num_filter_objects = 0. This is likely the desired behavior, but a comment explaining the expected shapes would improve maintainability.
🔵 Improvement: source/isaaclab_newton/docs/CHANGELOG.rst:4 — Changelog date format
The changelog entry uses 2026-04-28 which appears to be a future date (or placeholder). Verify this is intentional or update to the actual release date.
🔵 Improvement: source/isaaclab_physx/test/sensors/test_contact_sensor.py:750-762 — Consistent naming after API migration
Good catch updating num_bodies → num_sensors in the friction test helper. The variable rename correctly reflects the semantic meaning after the API migration. However, the loop variable body_idx → sensor_idx rename is incomplete in the comment on line 760 which still references the old name pattern implicitly. Consider updating the loop structure comment for consistency.
🔵 Improvement: source/isaaclab_newton/isaaclab_newton/sensors/contact_sensor/contact_sensor_kernels.py:18-19 — Kernel signature documentation
The kernel comment says (n_envs * n_sensors) for newton_total_force but the launch dimension comment says dim=(num_envs, num_sensors, max(num_filter_objects, 1)). These are consistent (flat vs 3D launch), but adding a note that the flat array is indexed via env * num_sensors + sensor would help readers understand the mapping immediately.
# Description Fixes partial migration of metadata access in #5418 <!-- Thank you for your interest in sending a pull request. Please make sure to check the contribution guidelines. Link: https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html 💡 Please try to keep PRs small and focused. Large PRs are harder to review and merge. --> Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change. Fixes # (issue) <!-- As a practice, it is recommended to open an issue to have discussions on the proposed pull request. This makes it easier for the community to keep track of what is being developed or added, and if a given feature is demanded by more than one party. --> ## Type of change <!-- As you go through the list, delete the ones that are not applicable. --> - Bug fix (non-breaking change which fixes an issue) - New feature (non-breaking change which adds functionality) - Breaking change (existing functionality will not work without user modification) - Documentation update ## Screenshots Please attach before and after screenshots of the change if applicable. <!-- Example: | Before | After | | ------ | ----- | | _gif/png before_ | _gif/png after_ | To upload images to a PR -- simply drag and drop an image while in edit mode and it should upload the image directly. You can then paste that source into the above before/after sections. --> ## Checklist - [ ] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [ ] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] I have updated the changelog and the corresponding version in the extension's `config/extension.toml` file - [ ] I have added my name to the `CONTRIBUTORS.md` or my name already exists there <!-- As you go through the checklist above, you can mark something as done by putting an x character in it For example, - [x] I have done this task - [ ] I have not done this task --> --------- Co-authored-by: Kelly Guo <kellyg@nvidia.com>
Summary
Test Plan