Updates contact sensor API to use Newton new API that allows fast initialization. by ooctipus · Pull Request #4883 · isaac-sim/IsaacLab

ooctipus · 2026-03-09T02:05:51Z

Description

This PR add contact sensor test and test utilities to newton contacts sensors

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.

Type of change

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

Screenshots

Please attach before and after screenshots of the change if applicable.

Checklist

I have read and understood the contribution guidelines
I have run the pre-commit checks 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

greptile-apps · 2026-03-09T02:10:30Z

Greptile Summary

This PR delivers a significant performance optimization for multi-world contact sensor construction — reducing Allegro finger sensor setup from ~80 s to under 1 s — by extracting pattern resolution and replicated-kernel construction into a new isaaclab_newton.cloner.contact_filter module. It also fixes RigidObjectData.body_inertia to return a (N, B, 9) strided view instead of (N, B, 3, 3), adds a visualizers parameter to build_simulation_context(), and ships a comprehensive test suite.

Key concerns to address before merging:

prune_noncolliding silently ignored for multi-world models (contact_filter.py:497–509): The parameter is accepted by build_contact_sensor and forwarded to the single-world path, but _build_replicated always uses prune_noncolliding=False without any warning. Callers expecting sparse force matrices in multi-world setups will get dense matrices silently.
Undocumented removal of the 1 cm contact margin (newton_manager.py:291): cls._builder.default_shape_cfg.contact_margin = 0.01 was removed without a changelog entry, reverting all simulations to Newton's 10 cm default. The removed comment itself acknowledged why the override existed ("default 10 cm is too large for manipulation examples"), making this a silent physics-behavior regression.
Changelog date ordering inconsistency: Both isaaclab 4.5.9 and isaaclab_newton 0.5.3 are dated 2026-02-28, which is before the preceding releases (4.5.8 / 0.5.2, both dated 2026-03-06).
Private Newton API coupling (contact_filter.py:410–421): _ReplicatedContactSensor reads five private attributes of NewtonContactSensor (_n_shape_pairs, _sp_reading, _sp_ep_offset, _sp_ep_count, _sp_sorted). This is noted as unavoidable in the comments but should at minimum be guarded with assertions or a version check.
Stride assumption in body_inertia fix (rigid_object_data.py:128–136): The stride-based (N, B, 9) reshape assumes strides[2] == 3 * strides[3] (contiguous mat33 rows); this should be asserted or documented.

Confidence Score: 3/5

This PR has two logic-level issues that could silently change runtime behavior: the undocumented removal of the contact margin and the silent prune_noncolliding drop for multi-world models.
The core optimization algorithm is well-designed and well-tested. However, the silent removal of the 1 cm contact margin is a breaking physics change for manipulation scenarios without any changelog entry, and the prune_noncolliding parameter is silently dropped for the main (multi-world) code path. These two issues could cause hard-to-diagnose regressions in existing simulations.
source/isaaclab_newton/isaaclab_newton/physics/newton_manager.py (contact margin removal) and source/isaaclab_newton/isaaclab_newton/cloner/contact_filter.py (prune_noncolliding handling for multi-world path).

Important Files Changed

Filename	Overview
source/isaaclab_newton/isaaclab_newton/cloner/contact_filter.py	New module implementing O(per_world²) replicated contact sensor; `prune_noncolliding` is silently dropped for multi-world models and private Newton API attributes are accessed without guards.
source/isaaclab_newton/isaaclab_newton/physics/newton_manager.py	Refactors add_contact_sensor to delegate to build_contact_sensor; silently removes the 1 cm contact margin override, which is an undocumented physics-behavior change for manipulation scenarios.
source/isaaclab_newton/isaaclab_newton/assets/rigid_object/rigid_object_data.py	Fixes body_inertia shape from (N,B,3,3) to (N,B,9) via a stride-based view; assumes contiguous mat33 rows without asserting or documenting that requirement.
source/isaaclab/isaaclab/sim/simulation_context.py	Adds `visualizers` parameter to build_simulation_context(); implementation is straightforward and correct.
source/isaaclab_newton/test/cloner/test_contact_filter.py	Comprehensive unit and integration tests for the new contact_filter module, including performance benchmarks and filter-correctness cross-validation against plain NewtonContactSensor.
source/isaaclab_newton/test/sensors/test_contact_sensor.py	Full contact sensor test suite covering lifecycle, collision detection, force validation, and Allegro finger isolation; no issues found.
source/isaaclab/docs/CHANGELOG.rst	Adds 4.5.9 changelog entry; date (2026-02-28) is earlier than the preceding 4.5.8 entry (2026-03-06), creating a version/date ordering inconsistency.
source/isaaclab_newton/docs/CHANGELOG.rst	Adds 0.5.3 changelog entry; same date-ordering inconsistency as the isaaclab changelog (0.5.3 dated 2026-02-28 before 0.5.2 at 2026-03-06); also does not mention the contact_margin removal.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant NewtonManager
    participant build_contact_sensor
    participant _build_replicated
    participant _ReplicatedContactSensor
    participant NewtonContactSensor

    Caller->>NewtonManager: add_contact_sensor(expr, prune_noncolliding)
    NewtonManager->>build_contact_sensor: build_contact_sensor(model, expr, prune_noncolliding)

    alt world_count == 1
        build_contact_sensor->>NewtonContactSensor: NewtonContactSensor(model, indices, prune_noncolliding)
        NewtonContactSensor-->>build_contact_sensor: sensor
        build_contact_sensor-->>NewtonManager: NewtonContactSensor
    else world_count > 1 (replicated)
        build_contact_sensor->>_build_replicated: _build_replicated(model, ..., verbose)<br/>[prune_noncolliding NOT forwarded]
        _build_replicated->>_build_replicated: resolve world-0 indices
        _build_replicated->>NewtonContactSensor: NewtonContactSensor(world-0 indices,<br/>prune_noncolliding=False [hardcoded])
        NewtonContactSensor-->>_build_replicated: template
        _build_replicated->>_build_replicated: build sp_sorted_local (localise shape pairs)
        _build_replicated->>_ReplicatedContactSensor: new(template, n_worlds, shape_world, ...)
        _ReplicatedContactSensor-->>_build_replicated: sensor
        _build_replicated-->>build_contact_sensor: _ReplicatedContactSensor
        build_contact_sensor-->>NewtonManager: _ReplicatedContactSensor
    end

    NewtonManager->>NewtonManager: store sensor_key → sensor
    NewtonManager-->>Caller: sensor_key

Comments Outside Diff (3)

source/isaaclab_newton/isaaclab_newton/cloner/contact_filter.py, line 497-509 (link)

prune_noncolliding silently ignored for multi-world models

build_contact_sensor accepts prune_noncolliding: bool = True as a parameter and passes it to NewtonContactSensor for single-world models (line 522), but for multi-world models the call to _build_replicated does not forward prune_noncolliding. Inside _build_replicated the template is always created with prune_noncolliding=False hardcoded (line 555). As a result, any caller that relies on sparse force matrices via prune_noncolliding=True will silently get a dense matrix when the model has multiple worlds.

If the technical reason is that pruning is incompatible with the replicated-kernel approach (because the full shape-pair table is required for world-local lookup), that should be documented with a log warning or docstring note, not silently dropped. Consider adding:
```
if prune_noncolliding and world_count > 1:
    logger.warning(
        "prune_noncolliding=True is not supported for replicated (multi-world) models "
        "and will be ignored. The full shape-pair table is required for correct world-local lookup."
    )
```
source/isaaclab/docs/CHANGELOG.rst, line 22 (link)

Changelog date ordering inconsistency

Version 4.5.9 is dated 2026-02-28, but the immediately following entry (4.5.8) is dated 2026-03-06. A higher version number should have a later or equal date, not an earlier one. This same issue exists in source/isaaclab_newton/docs/CHANGELOG.rst where 0.5.3 is dated 2026-02-28 and 0.5.2 is dated 2026-03-06.

Please update the date for 4.5.9 / 0.5.3 to reflect the actual release date.
source/isaaclab_newton/isaaclab_newton/assets/rigid_object/rigid_object_data.py, line 128-136 (link)

Stride-based reshape assumes contiguous mat33 rows

The new wp.array construction uses strides[3] (the innermost element stride, typically 4 bytes for float32) as the stride for the flat 9-element dimension. This correctly maps a (N, B, 3, 3) array to (N, B, 9) only if strides[2] == 3 * strides[3], i.e. the rows of each 3×3 matrix are contiguous in memory.

If the physics backend ever returns the inertia tensor with row-padding (e.g. aligned to 16 bytes → strides[2] = 4 * sizeof(float32)), the view would silently read past row boundaries and produce incorrect inertia values.

Consider adding an assertion:
```
assert _inertia_mat33.strides[2] == 3 * _inertia_mat33.strides[3], (
    "body_inertia mat33 rows are not contiguous; stride-based reshape would produce wrong values"
)
```
or a comment explaining the assumption explicitly.

_{Last reviewed commit: f249771}

greptile-apps · 2026-03-09T02:10:37Z

        logger.info(f"Finalizing model on device: {device}")
        cls._builder.up_axis = Axis.from_string(cls._up_axis)
        # Set smaller contact margin for manipulation examples (default 10cm is too large)


Silent removal of custom contact margin

The line cls._builder.default_shape_cfg.contact_margin = 0.01 was removed. The adjacent comment that remains in the diff context explicitly described why this setting existed: "Set smaller contact margin for manipulation examples (default 10cm is too large)". By removing the override, all simulations now revert to Newton's default 10 cm contact margin.

This is a physics-behavior-changing removal that is not mentioned in the CHANGELOG.rst entry for 0.5.3. Manipulation scenarios (especially the Allegro finger tests added in this same PR) may be affected. Please either:

Restore the contact_margin override (with a config option to override it), or

Add a Changed entry to the changelog explaining this intentional revert and its implications.

greptile-apps · 2026-03-09T02:10:38Z

+    )
+
+    sp_np = template._sp_sorted.numpy()
+    sw0 = int(shape_start[0])
+    sp_local = np.empty_like(sp_np)
+    for i in range(sp_np.shape[0]):
+        a, b = int(sp_np[i][0]), int(sp_np[i][1])
+        sp_local[i][0] = -1 if a == -1 else a - sw0
+        sp_local[i][1] = -1 if b == -1 else b - sw0
+    sp_sorted_local = wp.array(sp_local, dtype=wp.vec2i, device=model.device)
+
+    return _ReplicatedContactSensor(


Reliance on Newton private API

_ReplicatedContactSensor.__init__ reads five private attributes of the NewtonContactSensor template:

template._n_shape_pairs

template._sp_reading

template._sp_ep_offset

template._sp_ep_count

And in _build_replicated at line 559:

template._sp_sorted

These names are implementation details of Newton's SensorContact class. Any upstream refactor (attribute rename, restructure) will silently produce wrong results or an AttributeError at runtime. Since the PR already notes that _bisect_shape_pairs had to be duplicated because Warp kernels can't call foreign-module functions at compile time, this coupling is unavoidable to some degree — but it should at minimum be documented clearly (e.g., a version pin comment or an assertion like assert hasattr(template, "_sp_sorted"), "Newton private API changed").

AntoineRichard

It looks reasonable to me! @camevor could you take a look?

AntoineRichard · 2026-03-09T08:04:49Z

Do we also want to add the exact same tests as PhysX? I think it can be nice to validate that the behavior between PhysX and Newton is similar.

AntoineRichard · 2026-03-09T08:05:29Z

Do we want to bring these to PhysX too?

AntoineRichard · 2026-03-09T08:05:46Z

Same here, do we want to bring that to PhysX?

AntoineRichard

After talking with @camevor i don't think we should merge this in it's current state. Newton, will provide an improved sensor that supports "cloner style" logic: newton-physics/newton#1997

need merge, concern already addressed

…tialization. (isaac-sim#4883) # Description - This PR add contact sensor test and test utilities to newton contacts sensors 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. ## Type of change  - Bug fix (non-breaking change which fixes an issue) ## Screenshots Please attach before and after screenshots of the change if applicable.  ## 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` - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] I have updated the changelog and the corresponding version in the extension's `config/extension.toml` file - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there

## Summary Fix incorrect attribute name `contact_margin` → `gap` on Newton `ShapeConfig` in `NewtonManager.create_builder()`. Newton PR #1732 renamed `contact_margin` to `gap` (broad-phase AABB expansion). The IsaacLab code was never updated, creating a dead attribute that Python silently accepted. The intended 1 cm default shape gap was never applied. ## Newton PR #1732 rename mapping | Old field | New field | Semantics | |-----------|-----------|-----------| | `thickness` | `margin` | Shape surface extension (affects contact forces) | | `contact_margin` | **`gap`** | Broad-phase AABB expansion (detection range only) | | `rigid_contact_margin` | `rigid_gap` | Builder-level default gap (already 0.1) | ## Timeline | Date | Newton | IsaacLab | `contact_margin` valid? | |------|--------|----------|------------------------| | Feb 19 | pin: `51ce35e` (has `contact_margin`) | #4646 adds `contact_margin = 0.01` | Yes | | Feb 24 | **PR #1732 renames `contact_margin` → `gap`** | — | — | | Mar 2 | pin updated to `v0.2.3` (after rename) | #4761 keeps `contact_margin` | **No — broken** | | Mar 9 | — | #4883 removes the line | Removed | | Apr 13 | — | #4822 re-adds `contact_margin` | **No — still broken** | ## Ablation: gap vs margin We conducted an ablation study to understand the impact. Note: `margin` (shape surface extension) is a different field from `gap` (broad-phase range). The original code intended to set `gap`, but setting `margin` also has a significant positive effect on training for rough terrain locomotion. | Setting | `gap` | `margin` | Go1 Reward (300 iter) | Effect | |---------|-------|----------|----------------------|--------| | Dead field (baseline) | 0.1 (default) | 0.0 | ~1.0 | — | | `gap=0.01` only | 0.01 | 0.0 | 0.66 | No training improvement | | `margin=0.01` only | 0.1 (default) | 0.01 | 18.77 | Major improvement | | `gap=0.01` + `margin=0.01` | 0.01 | 0.01 | 16.54 | Similar to margin-only | **This PR fixes the correct field migration** (`contact_margin` → `gap`). The `margin` setting for rough terrain contact quality will be addressed separately in the rough terrain env PR (#5248) via a new `default_shape_margin` config field on `NewtonCfg`. ## Test plan - [x] Verified `contact_margin` is not a field on `ShapeConfig` (Newton 1.1.0.dev0) - [x] Verified `gap` is the correct replacement field per Newton PR #1732 - [x] Confirmed by camevor (Newton developer) - [x] Ablation study: gap alone doesn't change training, so existing tests should pass Co-authored-by: Antoine RICHARD <antoiner@nvidia.com>

ooctipus requested review from kellyguo11, matthewtrepte and ossamaAhmed as code owners March 9, 2026 02:05

github-actions Bot added the isaac-lab Related to Isaac Lab team label Mar 9, 2026

ooctipus requested a review from AntoineRichard March 9, 2026 02:06

greptile-apps Bot reviewed Mar 9, 2026

View reviewed changes

ooctipus changed the title ~~fast contact filter kernel and contact tests~~ Implements fast contact filter kernel and contact tests Mar 9, 2026

AntoineRichard reviewed Mar 9, 2026

View reviewed changes

AntoineRichard mentioned this pull request Mar 9, 2026

Slow contact sensor creation newton-physics/newton#1997

Closed

AntoineRichard previously requested changes Mar 9, 2026

View reviewed changes

camevor reviewed Mar 9, 2026

View reviewed changes

Comment thread source/isaaclab_newton/isaaclab_newton/cloner/contact_filter.py Outdated

ooctipus force-pushed the contact_filter_cloning branch from 2cdac66 to 5c77631 Compare March 9, 2026 17:02

ooctipus changed the title ~~Implements fast contact filter kernel and contact tests~~ Updates contact sensor API to use Newton new API that allows fast initialization. Mar 9, 2026

ooctipus added 4 commits March 9, 2026 19:10

fast contact filter kernel and contact tests

36348cb

pass precommit

5e62174

remove timing instruments

157d5ac

cleanup

6b6e858

ooctipus force-pushed the contact_filter_cloning branch from ac7d0b6 to 6b6e858 Compare March 10, 2026 02:11

ooctipus requested a review from AntoineRichard March 10, 2026 02:11

fix precommit

0cb219c

kellyguo11 approved these changes Mar 10, 2026

View reviewed changes

fix doc

c95d4d8

ooctipus merged commit 423f73f into isaac-sim:develop Mar 10, 2026
11 of 14 checks passed

ooctipus deleted the contact_filter_cloning branch March 10, 2026 04:51

ClemensSchwarke mentioned this pull request Mar 10, 2026

[Bug Report] Trying to unpack non-iterable object in contact_sensor.py #4908

Closed

ooctipus mentioned this pull request Mar 10, 2026

Fix MockSensorContact to match Newton's nested sensing_objs structure #4925

Merged

7 tasks

hujc7 mentioned this pull request Apr 16, 2026

Fixes default shape margin field name in Newton manager #5289

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updates contact sensor API to use Newton new API that allows fast initialization.#4883

Updates contact sensor API to use Newton new API that allows fast initialization.#4883
ooctipus merged 6 commits into
isaac-sim:developfrom
ooctipus:contact_filter_cloning

ooctipus commented Mar 9, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Mar 9, 2026 •

edited

Loading

Comments Outside Diff (3)

Uh oh!

greptile-apps Bot Mar 9, 2026

Uh oh!

greptile-apps Bot Mar 9, 2026

Uh oh!

AntoineRichard left a comment

Uh oh!

Uh oh!

AntoineRichard Mar 9, 2026

Uh oh!

AntoineRichard Mar 9, 2026

Uh oh!

AntoineRichard Mar 9, 2026

Uh oh!

Uh oh!

AntoineRichard left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

ooctipus commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Screenshots

Checklist

Uh oh!

greptile-apps Bot commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 3/5

Important Files Changed

Sequence Diagram

Comments Outside Diff (3)

Uh oh!

greptile-apps Bot Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

AntoineRichard left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AntoineRichard Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

AntoineRichard Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

AntoineRichard Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AntoineRichard left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ooctipus commented Mar 9, 2026 •

edited

Loading

greptile-apps Bot commented Mar 9, 2026 •

edited

Loading