Clear self-collision filter pairs before finalizing shadow Newton model

[matthewtrepte]

Summary

• The PhysX-backend shadow Newton visualization model never runs collision detection, but USD-authored self-collision filter pairs (physxArticulation:enabledSelfCollisions) were still imported into it and replicated across every cloned env.
• At real training env counts, this filter-pair set could reach billions of entries, causing ModelBuilder.finalize() to run out of memory.
• Clears builder.shape_collision_filter_pairs before finalizing the shadow model since it has no use for them.

Test plan

• uv run python -m pytest source/isaaclab/test/sim/test_newton_manager_visualization_state.py — 23 passed, including new regression test test_ensure_visualization_model_clears_shape_collision_filter_pairs_before_finalize
• uv run python -m pytest source/isaaclab/test/sim/test_simulation_context_visualizers.py source/isaaclab_newton/test/physics/test_newton_manager_abstraction.py — 200 passed
• uv run isaaclab -f

Release backport

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

[isaaclab-review-bot]

Isaac Lab Review Bot

The change removes unused collision-filter pairs from the PhysX-backend shadow Newton visualization model before finalization, directly addressing the reported memory growth. It includes focused regression coverage and appropriate package changelog fragments.

• Design and architecture: The clearing occurs after the visualization builder is populated and immediately before finalization, and only on the PhysX shadow-model path. Newton-native simulation behavior, deformable registry population, state synchronization, and lifecycle callbacks remain unchanged.
• API: No public API, exported symbol, default, or compatibility contract changes. The modification is confined to the private shadow-model construction path, with the user-visible fix documented in the isaaclab_newton changelog and the test-only isaaclab change marked with a .skip fragment.
• Implementation: Assigning an empty collection before finalize directly prevents the unused pairs from being packed. The proposed concern about changing the container type is not supported by evidence that finalize performs set-specific mutations; the regression test confirms that the collection is removed along the intended construction path.

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 Summary

The PR prevents PhysX-backed shadow Newton models from packing unused, environment-replicated self-collision filter pairs during finalization.

• Clears shape_collision_filter_pairs immediately before shadow-model finalization.
• Adds a focused regression test and Newton changelog entry documenting the memory-exhaustion fix.

Confidence Score: 4/5

The implementation appears safe to merge, with a non-blocking opportunity to make the regression test enforce the critical pre-finalization ordering.

The functional change is confined to the PhysX visualization-only builder and occurs before finalization; the only accepted concern is that the test could remain green if that ordering later regresses.

Files Needing Attention: source/isaaclab/test/sim/test_newton_manager_visualization_state.py

Important Files Changed

Filename	Overview
source/isaaclab_newton/isaaclab_newton/physics/newton_manager.py	Clears unused collision-filter pairs on the exclusively owned PhysX shadow builder before finalization, avoiding excessive memory use without affecting the Newton physics model.
source/isaaclab/test/sim/test_newton_manager_visualization_state.py	Adds regression coverage for clearing the list, but does not enforce that clearing occurs before finalization.
source/isaaclab_newton/changelog.d/fix-shadow-model-collision-filter-oom.rst	Accurately documents the shadow-model memory-exhaustion fix and its affected visualization paths.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[PhysX USD stage] --> B[Build shadow Newton ModelBuilder]
  B --> C[Imported collision filter pairs]
  C --> D[Clear unused filter pairs]
  D --> E[Finalize shadow model]
  E --> F[Visualizers and renderers]

Loading

_{Reviews (1): Last reviewed commit: "Clear self-collision filter pairs before..." | Re-trigger Greptile}

[greptile-apps]

Finalize ordering remains untested

The assertion verifies only that the list is eventually empty, while the finalize stub never inspects it. Moving the clear after finalize() would leave this test green while restoring the production memory-exhaustion path, so the stub should assert that the list is empty when finalization begins.

[matthewtrepte]

run-ci

[ooctipus]

Thanks for fixing this. I agree with the invariant: a Newton model used only for visualization/scene queries should not carry solver-only collision data. I think the current late clear is incomplete, though, because Newton generates contact pairs during the same finalization.

I reproduced this against both Newton 1.5.1 and the exact 1.6 development commit pinned by #7453 (24bd863528d6b91137408930d0fbe8fa216ad962):

10 replicated filtered pairs, unchanged:       10 filters,  0 contacts
builder.shape_collision_filter_pairs = []:      0 filters, 10 contacts
clear filters + collision group 0:              0 filters,  0 contacts

At 1,024 worlds × 48 shapes, clearing alone generated 1,155,072 contact pairs. ModelBuilder.finalize() currently always calls _find_shape_contact_pairs(), so deleting an exclusion makes that pair eligible for the contact array. The new test mocks finalize(), which hides this transfer.

I suggest moving the policy into build_visualization_builder_from_stage_envs(), where we know every produced builder is scene-only, and applying it before composition/replication:

for visual_builder in (global_builder, *source_builders.values()):
    visual_builder.shape_collision_filter_pairs = []
    visual_builder.shape_collision_group[:] = [0] * visual_builder.shape_count

builder = ModelBuilder(up_axis=up_axis)
builder.add_builder(global_builder)
replicate_builder_mapping(builder, sources, mapping, positions, quaternions, source_builders)

The standalone branch should apply the same two assignments before returning its builder. The fresh assembled builder is important: assigning [] replaces Newton's compact filter representation with a plain list; adding already-stripped component builders to a fresh builder keeps the destination compact-empty and preserves homogeneous contact-template finalization. Please retain assignment rather than .clear(), because reading the property can materialize the compact replicated filters.

Then _ensure_visualization_model() can return to simply finalizing the already-correct builder; the late clear and its comment go away. This keeps the generic cloner unchanged and adds no cfg field, helper, fallback, or public API.

For the regression, I would replace the mocked-ordering test with a small real CPU ModelBuilder.finalize() test covering replicated filtered shapes and asserting both:

assert len(model.shape_collision_filter_pairs) == 0
assert model.shape_contact_pair_count == 0

Collision group 0 leaves shape flags and the BVH intact; I also verified the same Newton raycast hit and distance with groups 1 and 0. The existing raycaster suite should still be run, but a second large test is unnecessary.

This also ports cleanly to the planned ownership model: the same normalization belongs on source/global builders in the visualization-only NewtonReplicateContext branch before plan replication, while Newton physics resources retain collision data.

[ooctipus]

Implemented the construction-boundary fix in 4716126cc.

• Collision filters and groups are stripped from visualization-only component builders before composition/replication.
• A fresh destination builder preserves Newton's compact empty-filter representation.
• The manager-level late clear and mocked-order test are removed.
• A real in-memory USD/Newton finalization test now checks both standalone and replicated models have geometry but no filters or contact pairs.

Validation:

• 45 Newton cloner tests passed.
• 22 Newton manager/visualization-state tests passed.
• The regression test fails without the fix and passes on Newton 1.5.1 and the exact Newton 1.6 commit from [Newton] Delegate homogeneous world prefix generation to ModelBuilder.replicate() #7453.

[kellyguo11]

run-ci

[AntoineRichard]

LGTM thanks Matthew!

[ooctipus]

run-ci