newton/_src/sim/builder.py (6)

364-364: List replaces set for collision filters: enforce canonical pairs to avoid bloat.

Now that this is a list, duplicates/non‑canonical ordering will accumulate. Please ensure all insertions use (min, max) ordering to keep find_shape_contact_pairs() set‑conversion cheap.

---

977-981: Type mixing in particle offset.

pos_offset = xform.p may be a Warp vec; adding it to np.array(builder.particle_q) can incur dtype coercions. Cast once.

Apply:

-                pos_offset = xform.p
+                pos_offset = np.asarray(xform.p, dtype=np.float32)

---

1060-1062: Use captured start index for clarity and robustness.

Using start_shape_idx avoids relying on current shape_count invariants.

-        shape_count_offset = self.shape_count
-        self.shape_collision_filter_pairs.extend(
-            [(i + shape_count_offset, j + shape_count_offset) for i, j in builder.shape_collision_filter_pairs]
-        )
+        self.shape_collision_filter_pairs.extend(
+            [(i + start_shape_idx, j + start_shape_idx) for i, j in builder.shape_collision_filter_pairs]
+        )

---

2416-2417: Canonicalize same‑body shape filter pairs.

Ensure (a, b) is ordered to match find_shape_contact_pairs() checks and prevent duplicates.

-                self.shape_collision_filter_pairs.append((same_body_shape, shape))
+                a, b = (same_body_shape, shape)
+                if a > b:
+                    a, b = b, a
+                self.shape_collision_filter_pairs.append((a, b))

---

2441-2442: Canonicalize parent–child filter pairs added from add_shape.

Same rationale as above.

-                        self.shape_collision_filter_pairs.append((parent_shape, shape))
+                        a, b = (parent_shape, shape)
+                        if a > b:
+                            a, b = b, a
+                        self.shape_collision_filter_pairs.append((a, b))

---

4431-4465: Set conversion on filters: good safeguard; consider pruning on insert.

Converting to a set at runtime avoids duplicates, but the list can still grow large. With canonicalization fixes above, consider deduping opportunistically if perf/memory becomes a concern.

newton/_src/utils/import_mjcf.py (2)

923-924: Filter pairs for visuals: canonicalize and skip self‑pairs.

Avoid (i, i) and enforce (min, max).

-            builder.shape_collision_filter_pairs.append((i, j))
+            if i != j:
+                a, b = (i, j) if i < j else (j, i)
+                builder.shape_collision_filter_pairs.append((a, b))

---

926-929: Self‑collision disable loop: canonicalize pair order.

Same rationale as above.

-                builder.shape_collision_filter_pairs.append((i, j))
+                a, b = (i, j) if i < j else (j, i)
+                builder.shape_collision_filter_pairs.append((a, b))

newton/examples/robot/example_robot_allegro_hand.py (1)

111-112: Canonicalize appended filter pairs.

Keep (min, max) ordering to match filtering semantics elsewhere.

-                    allegro_hand.shape_collision_filter_pairs.append((shape1, shape2))
+                    a, b = (shape1, shape2) if shape1 < shape2 else (shape2, shape1)
+                    allegro_hand.shape_collision_filter_pairs.append((a, b))

newton/_src/utils/import_usd.py (1)

1253-1271: Canonicalize and skip world–world pairs to avoid duplicates with list-backed filters

Now that shape_collision_filter_pairs is a list, appending unordered pairs introduces duplicates and unnecessary entries (especially (a,b) vs (b,a)). Also, per our previous learning, world-attached shapes (-1) already don’t collide; skip those pairs to keep the list compact.

Apply this diff in the three places where pairs are appended:

@@
-        builder.shape_collision_filter_pairs.append((shape1, shape2))
+        if not (builder.shape_body[shape1] == -1 and builder.shape_body[shape2] == -1):
+            a, b = sorted((shape1, shape2))
+            builder.shape_collision_filter_pairs.append((a, b))
@@
-            if other_shape_id != shape_id:
-                builder.shape_collision_filter_pairs.append((shape_id, other_shape_id))
+            if other_shape_id != shape_id:
+                if not (builder.shape_body[shape_id] == -1 and builder.shape_body[other_shape_id] == -1):
+                    a, b = sorted((shape_id, other_shape_id))
+                    builder.shape_collision_filter_pairs.append((a, b))
@@
-                        builder.shape_collision_filter_pairs.append((shape1, shape2))
+                        if not (builder.shape_body[shape1] == -1 and builder.shape_body[shape2] == -1):
+                            a, b = sorted((shape1, shape2))
+                            builder.shape_collision_filter_pairs.append((a, b))

(Background: collisions between body == -1 shapes are auto-skipped; no need to list them.)

newton/tests/test_model.py (1)

570-571: Use tolerant comparison for transforms to avoid float-equality flakiness

Direct equality on warp transforms can be brittle due to FP rounding. Use the helper already imported.

-        self.assertEqual(builder.body_q[0], offset_xform * orig_xform)
-        self.assertEqual(builder.body_q[1], offset_xform * orig_xform)
+        assert_np_equal(np.array(builder.body_q[0]), np.array(offset_xform * orig_xform), tol=1.0e-6)
+        assert_np_equal(np.array(builder.body_q[1]), np.array(offset_xform * orig_xform), tol=1.0e-6)

newton/_src/utils/import_urdf.py (1)

533-541: Canonicalize filter pairs (and guard i==j) to reduce duplicates with list-backed storage

Small cleanup to keep the list minimal.

-    for i in range(start_shape_count, end_shape_count):
-        for j in visual_shapes:
-            builder.shape_collision_filter_pairs.append((i, j))
+    for i in range(start_shape_count, end_shape_count):
+        for j in visual_shapes:
+            if i != j:
+                a, b = sorted((i, j))
+                builder.shape_collision_filter_pairs.append((a, b))
@@
-        for i in range(start_shape_count, end_shape_count):
-            for j in range(i + 1, end_shape_count):
-                builder.shape_collision_filter_pairs.append((i, j))
+        for i in range(start_shape_count, end_shape_count):
+            for j in range(i + 1, end_shape_count):
+                a, b = sorted((i, j))
+                builder.shape_collision_filter_pairs.append((a, b))

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Learnt from: eric-heiden
PR: newton-physics/newton#587
File: newton/_src/sim/builder.py:656-661
Timestamp: 2025-08-20T18:02:36.099Z
Learning: In Newton physics, collisions between shapes with body index -1 (world-attached/global shapes) are automatically skipped by the collision system, so no manual collision filter pairs need to be added between global shapes from different builders.

Learnt from: nvlukasz
PR: newton-physics/newton#519
File: newton/geometry.py:16-22
Timestamp: 2025-08-12T17:51:37.474Z
Learning: In the Newton public API refactor, common geometry symbols like ParticleFlags and ShapeFlags are now exposed at the top level in newton/__init__.py rather than through newton.geometry. The newton/geometry.py module is intentionally focused only on broad-phase collision detection classes (BroadPhaseAllPairs, BroadPhaseExplicit, BroadPhaseSAP).

Learnt from: eric-heiden
PR: newton-physics/newton#584
File: newton/_src/utils/import_urdf.py:159-167
Timestamp: 2025-08-20T03:30:16.037Z
Learning: In the URDF importer (newton/_src/utils/import_urdf.py), cylinders are intentionally created using add_shape_capsule() instead of add_shape_cylinder() because cylinder collisions are not fully supported yet. This is a deliberate workaround until proper cylinder collision support is implemented.

newton/_src/sim/builder.py (4)

364-365: List-based collision-filter storage: OK; consider dedup before use.

Switching from a set to a list is fine for amortized appends. To keep memory bounded across many add_builder/replicate calls, dedup the list at a choke point (e.g., before assigning to the Model).

You can do this near finalize:

unique_pairs = sorted(set(self.shape_collision_filter_pairs))
m.shape_collision_filter_pairs = unique_pairs

---

639-646: Avoid item-assignment on wp.transform; set .p directly.

Index-based writes on wp.transform may not be supported and could fail on some Warp versions. Set the translation via .p (or construct a new transform).

-        xform = wp.transform_identity()
+        xform = wp.transform_identity()
         for i in range(num_copies):
             offset = offsets[i]
-            # assign translation
-            for j in range(3):
-                xform[j] = offset[j]
+            # assign translation
+            xform.p = wp.vec3(offset[0], offset[1], offset[2])
             self.add_builder(builder, xform=xform)

---

980-983: Normalize pos_offset to NumPy for vectorized add.

xform.p may be a Warp vec3; converting once avoids dtype surprises and object arrays during np math.

-            if xform is not None:
-                pos_offset = xform.p
+            if xform is not None:
+                pos_offset = np.array(xform.p, dtype=np.float32)
             else:
                 pos_offset = np.zeros(3)

---

1061-1063: Offsetting imported filter pairs: OK; consider canonicalization + dedup.

Pairs are appended with offsets. To prevent accidental duplicates from different code paths, canonicalize and dedup once (see earlier comment).

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Learnt from: eric-heiden
PR: newton-physics/newton#587
File: newton/_src/sim/builder.py:656-661
Timestamp: 2025-08-20T18:02:36.099Z
Learning: In Newton physics, collisions between shapes with body index -1 (world-attached/global shapes) are automatically skipped by the collision system, so no manual collision filter pairs need to be added between global shapes from different builders.

Learnt from: eric-heiden
PR: newton-physics/newton#587
File: newton/_src/sim/builder.py:656-661
Timestamp: 2025-08-20T18:02:36.099Z
Learning: In Newton physics, collisions between shapes with body index -1 (world-attached/global shapes) are automatically skipped by the collision system, so no manual collision filter pairs need to be added between global shapes from different builders.

newton/_src/sim/builder.py (1)

967-974: Unsafe from_buffer and reliance on private Warp core API.

Creating a transform via from_buffer over a temporary NumPy array risks lifetime/aliasing; also wp.context.runtime.core.* is private and brittle. Allocate a proper wp.transform and add a safe fallback to Python-level multiply.

Apply this diff:

-        transform_mul_cfunc = wp.context.runtime.core.wp_builtin_mul_transformf_transformf
-
-        # dispatches two transform multiplies to the native implementation
-        def transform_mul(a, b):
-            out = wp.transform.from_buffer(np.empty(7, dtype=np.float32))
-            transform_mul_cfunc(a, b, ctypes.byref(out))
-            return out
+        # best-effort: use native cfunc if present; otherwise fall back to Python operator *
+        try:
+            _mul_tf_tf = wp.context.runtime.core.wp_builtin_mul_transformf_transformf  # type: ignore[attr-defined]
+        except Exception:
+            _mul_tf_tf = None
+
+        def transform_mul(a, b):
+            if _mul_tf_tf is not None:
+                out = wp.transform()  # owns its memory
+                try:
+                    _mul_tf_tf(a, b, ctypes.byref(out))
+                    return out
+                except Exception:
+                    pass
+            return a * b

newton/_src/sim/builder.py (1)

978-983: Ensure NumPy addition with pos_offset uses an array.

xform.p is a Warp vec3; add explicit conversion to avoid dtype surprises.

-            if xform is not None:
-                pos_offset = xform.p
+            if xform is not None:
+                pos_offset = np.array(xform.p, dtype=np.float32)
             else:
                 pos_offset = np.zeros(3)
-            self.particle_q.extend((np.array(builder.particle_q) + pos_offset).tolist())
+            self.particle_q.extend((np.array(builder.particle_q, dtype=np.float32) + pos_offset).tolist())

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Learnt from: eric-heiden
PR: newton-physics/newton#587
File: newton/_src/sim/builder.py:656-661
Timestamp: 2025-08-20T18:02:36.099Z
Learning: In Newton physics, collisions between shapes with body index -1 (world-attached/global shapes) are automatically skipped by the collision system, so no manual collision filter pairs need to be added between global shapes from different builders.

Learnt from: eric-heiden
PR: newton-physics/newton#587
File: newton/_src/sim/builder.py:656-661
Timestamp: 2025-08-20T18:02:36.099Z
Learning: In Newton physics, collisions between shapes with body index -1 (world-attached/global shapes) are automatically skipped by the collision system, so no manual collision filter pairs need to be added between global shapes from different builders.

Learnt from: nvlukasz
PR: newton-physics/newton#519
File: newton/geometry.py:16-22
Timestamp: 2025-08-12T17:51:37.474Z
Learning: In the Newton public API refactor, common geometry symbols like ParticleFlags and ShapeFlags are now exposed at the top level in newton/__init__.py rather than through newton.geometry. The newton/geometry.py module is intentionally focused only on broad-phase collision detection classes (BroadPhaseAllPairs, BroadPhaseExplicit, BroadPhaseSAP).

newton/_src/sim/builder.py (1)

967-974: Stop using private Warp cfunc + from_buffer; allocate a proper transform with a safe fallback.

Accessing wp.context.runtime.core is private and out may alias temporary memory. Use wp.transform() and fall back to public operator overload if the cfunc isn’t available. This was flagged previously and remains unresolved.

Apply this diff:

-        # explicitly resolve the transform multiplication function to avoid
-        # repeatedly resolving builtin overloads during shape transformation
-        transform_mul_cfunc = wp.context.runtime.core.wp_builtin_mul_transformf_transformf
-
-        # dispatches two transform multiplies to the native implementation
-        def transform_mul(a, b):
-            out = wp.transform.from_buffer(np.empty(7, dtype=np.float32))
-            transform_mul_cfunc(a, b, ctypes.byref(out))
-            return out
+        # Try to use the native transform mul; fall back to the public overload
+        try:
+            _mul_cfunc = wp.context.runtime.core.wp_builtin_mul_transformf_transformf  # private; may not exist
+        except Exception:
+            _mul_cfunc = None
+
+        def transform_mul(a, b):
+            if _mul_cfunc is not None:
+                out = wp.transform()
+                _mul_cfunc(a, b, ctypes.byref(out))
+                return out
+            return a * b

newton/_src/sim/builder.py (2)

1025-1040: Joint offsetting logic mostly fine; consider slice assign for FREE joints.

Minor micro-opt: replace the 7‑element loop with a slice assign to reduce Python overhead.

-                        qi += start_q
-                        for j in range(7):
-                            self.joint_q[qi + j] = tf[j]
+                        qi += start_q
+                        self.joint_q[qi : qi + 7] = list(tf)

---

2217-2220: Robustness: guard static_shapes retrieval when clearing.

If invariant breaks in future, this can KeyError. Cheap to harden.

-        static_shapes = self.body_shapes[-1]
+        static_shapes = self.body_shapes.get(-1, [])
         self.body_shapes.clear()
-        # restore static shapes
-        self.body_shapes[-1] = static_shapes
+        # restore static shapes
+        self.body_shapes[-1] = static_shapes

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Learnt from: eric-heiden
PR: newton-physics/newton#587
File: newton/_src/sim/builder.py:656-661
Timestamp: 2025-08-20T18:02:36.099Z
Learning: In Newton physics, collisions between shapes with body index -1 (world-attached/global shapes) are automatically skipped by the collision system, so no manual collision filter pairs need to be added between global shapes from different builders.

Learnt from: eric-heiden
PR: newton-physics/newton#587
File: newton/_src/sim/builder.py:656-661
Timestamp: 2025-08-20T18:02:36.099Z
Learning: In Newton physics, collisions between shapes with body index -1 (world-attached/global shapes) are automatically skipped by the collision system, so no manual collision filter pairs need to be added between global shapes from different builders.

newton/_src/sim/builder.py (2)

639-645: Don’t mutate wp.transform via indexing; assign translation via .p.

Index assignment on Warp transforms is not supported. Set xform.p explicitly.

Apply this diff:

-        xform = wp.transform_identity()
+        xform = wp.transform()
         for i in range(num_copies):
             offset = offsets[i]
-            # assign translation
-            for j in range(3):
-                xform[j] = offset[j]
+            # assign translation
+            xform.p = wp.vec3(offset[0], offset[1], offset[2])
             self.add_builder(builder, xform=xform)

---

969-974: Unsafe from_buffer + private C symbol; use proper transform and add fallback.

Avoid from_buffer on a temp NumPy array and reliance on private runtime symbols without guard.

Apply this diff:

-        def transform_mul(a, b):
-            out = wp.transform.from_buffer(np.empty(7, dtype=np.float32))
-            transform_mul_cfunc(a, b, ctypes.byref(out))
-            return out
+        def transform_mul(a, b):
+            out = wp.transform()
+            try:
+                transform_mul_cfunc(a, b, ctypes.byref(out))
+                return out
+            except Exception:
+                # safe public fallback
+                return a * b

newton/_src/sim/builder.py (1)

364-364: List-based collision filters: enforce invariants or dedup to bound growth.

Switching to list is fine for determinism, but duplicates can accumulate. Guarantee (a <= b) ordering at all append sites (looks mostly true) and consider dedup once (e.g., in finalize) if lists get large.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Learnt from: eric-heiden
PR: newton-physics/newton#587
File: newton/_src/sim/builder.py:656-661
Timestamp: 2025-08-20T18:02:36.099Z
Learning: In Newton physics, collisions between shapes with body index -1 (world-attached/global shapes) are automatically skipped by the collision system, so no manual collision filter pairs need to be added between global shapes from different builders.

Learnt from: eric-heiden
PR: newton-physics/newton#587
File: newton/_src/sim/builder.py:656-661
Timestamp: 2025-08-20T18:02:36.099Z
Learning: In Newton physics, collisions between shapes with body index -1 (world-attached/global shapes) are automatically skipped by the collision system, so no manual collision filter pairs need to be added between global shapes from different builders.

newton/_src/sim/builder.py (5)

1059-1061: Fix validated: use start_shape_idx for filter-pair offset.

This resolves the over-shift bug when copying filter pairs.

---

4162-4163: Model expects a set — good cast at finalize.

Matches solver expectations noted in prior review.

---

4427-4463: Contact-pair generation is now set-aware and canonical; minor optional skip for world–world.

• Using model.shape_collision_filter_pairs (set) is right.
• Canonicalization avoids s1/s2 mutation bugs from earlier versions.
• Optional: skip pairs where both shapes are world-attached (body == -1); collision is auto-skipped, so this trims work.

Apply:

         for i1 in range(len(sorted_indices)):
             s1 = sorted_indices[i1]
+            b1 = self.shape_body[s1]
             env1 = self.shape_group[s1]
             collision_group1 = self.shape_collision_group[s1]
             for i2 in range(i1 + 1, len(sorted_indices)):
                 s2 = sorted_indices[i2]
+                b2 = self.shape_body[s2]
                 env2 = self.shape_group[s2]
+                # skip world–world pairs (auto-ignored by collision)
+                if b1 == -1 and b2 == -1:
+                    continue

Also safe to place the world–world check after the env check to retain the early-break behavior.

---

639-646: Don’t mutate wp.transform via indexing; set translation via .p.

Index assignment on wp.transform isn’t supported; assign the position vector directly.

Apply:

-        xform = wp.transform_identity()
+        xform = wp.transform_identity()
         for i in range(num_copies):
             offset = offsets[i]
-            # assign translation
-            for j in range(3):
-                xform[j] = offset[j]
+            # assign translation safely
+            xform.p = wp.vec3(offset[0], offset[1], offset[2])
             self.add_builder(builder, xform=xform)

---

969-974: Unsafe transform buffer + private Warp API; allocate a real transform and add fallback.

Avoid from_buffer on a temp NumPy array and hard reliance on wp.context.runtime.core.*.

Apply:

-        transform_mul_cfunc = wp.context.runtime.core.wp_builtin_mul_transformf_transformf
+        try:
+            transform_mul_cfunc = wp.context.runtime.core.wp_builtin_mul_transformf_transformf
+        except AttributeError:
+            transform_mul_cfunc = None
@@
-        def transform_mul(a, b):
-            out = wp.transform.from_buffer(np.empty(7, dtype=np.float32))
-            transform_mul_cfunc(a, b, ctypes.byref(out))
-            return out
+        def transform_mul(a, b):
+            if transform_mul_cfunc is None:
+                return a * b
+            out = wp.transform()  # proper Warp-owned storage
+            try:
+                transform_mul_cfunc(a, b, ctypes.byref(out))
+                return out
+            except Exception:
+                return a * b

newton/_src/sim/builder.py (2)

979-983: Ensure pos_offset is NumPy-compatible.

xform.p is a wp.vec3; make the dtype explicit before vectorized add.

Apply:

-                pos_offset = xform.p
+                pos_offset = np.array([xform.p[0], xform.p[1], xform.p[2]], dtype=np.float32)

---

940-949: Docs/migration note needed for users of Shape filter API change.

Public API now stores filter pairs as list in the builder; document the cast to set at finalize and that callers should not rely on set semantics pre-finalize.

I can draft a short migration snippet for docs/migration.rst if helpful.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Learnt from: eric-heiden
PR: newton-physics/newton#587
File: newton/_src/sim/builder.py:656-661
Timestamp: 2025-08-20T18:02:36.099Z
Learning: In Newton physics, collisions between shapes with body index -1 (world-attached/global shapes) are automatically skipped by the collision system, so no manual collision filter pairs need to be added between global shapes from different builders.

Learnt from: eric-heiden
PR: newton-physics/newton#587
File: newton/_src/sim/builder.py:656-661
Timestamp: 2025-08-20T18:02:36.099Z
Learning: In Newton physics, collisions between shapes with body index -1 (world-attached/global shapes) are automatically skipped by the collision system, so no manual collision filter pairs need to be added between global shapes from different builders.

newton/_src/sim/builder.py (2)

639-646: Don’t mutate wp.transform via indexing; set translation via .p.

Index-based mutation on wp.transform is brittle and was previously flagged. Assign the position once via xform.p.

Apply:

-        xform = wp.transform_identity()
+        xform = wp.transform_identity()
         for i in range(num_copies):
             offset = offsets[i]
-            # assign translation
-            for j in range(3):
-                xform[j] = offset[j]
+            # assign translation
+            xform.p = wp.vec3(offset[0], offset[1], offset[2])
             self.add_builder(builder, xform=xform)

---

967-974: Unsafe from_buffer + private API usage; allocate a real transform and add a safe fallback.

Creating a transform from a temporary NumPy buffer risks lifetime issues, and accessing wp.context.runtime.core is private/fragile. Use a proper wp.transform for the out param and fall back to Python-level multiply if the cfunc is unavailable.

Apply:

-        transform_mul_cfunc = wp.context.runtime.core.wp_builtin_mul_transformf_transformf
+        # Try to resolve native mul; fall back to Python operator if unavailable
+        transform_mul_cfunc = getattr(getattr(getattr(wp.context.runtime, "core", object()),
+                                              "wp_builtin_mul_transformf_transformf", None),
+                                      "__call__", None)

         # dispatches two transform multiplies to the native implementation
         def transform_mul(a, b):
-            out = wp.transform.from_buffer(np.empty(7, dtype=np.float32))
-            transform_mul_cfunc(a, b, ctypes.byref(out))
-            return out
+            # allocate a proper Warp transform; C kernel will fill it
+            out = wp.transform()
+            if transform_mul_cfunc is not None:
+                try:
+                    transform_mul_cfunc(a, b, ctypes.byref(out))
+                    return out
+                except Exception:
+                    pass
+            # safe fallback using public operator
+            return a * b

newton/_src/sim/builder.py (5)

364-364: LGTM: list-backed collision-filter store with set materialization at finalize.

Storing as a list here and converting to a set in finalize is fine. Keep an eye on growth if many duplicates are appended in hot import paths.

If duplicates become an issue, we can keep a small shadow set for O(1) membership checks at append sites while still storing the list for order/debuggability.

---

1025-1040: FREE-joint rebase logic looks correct; minor robustness nit.

The loop writes 7 scalars via tf[j]. If Warp ever changes transform indexing, consider slicing with .p/.q to be explicit.

Would you like a follow-up patch to assign self.joint_q[qi:qi+3] = tf.p and self.joint_q[qi+3:qi+7] = tf.q?

---

2411-2415: Skip adding filter pairs for world (-1) shapes; canonicalize order.

Per our learnings, collisions between shapes with body index -1 are already skipped by the collision system; adding pairs here bloats the list. Also make the order explicit.

Apply:

         if body in self.body_shapes:
             # no contacts between shapes of the same body
-            for same_body_shape in self.body_shapes[body]:
-                self.shape_collision_filter_pairs.append((same_body_shape, shape))
+            if body != -1:
+                for same_body_shape in self.body_shapes[body]:
+                    a, b = (same_body_shape, shape) if same_body_shape < shape else (shape, same_body_shape)
+                    self.shape_collision_filter_pairs.append((a, b))
             self.body_shapes[body].append(shape)
         else:
             self.body_shapes[body] = [shape]

Context: “collisions between shapes with body index -1 ... are automatically skipped,” so we can avoid emitting those pairs. [Used retrieved learnings]

---

2434-2438: Canonicalize parent/child shape filter pairs for consistency.

Guarantees (min, max) ordering like other sites.

Apply:

-                    for parent_shape in self.body_shapes[parent_body]:
-                        self.shape_collision_filter_pairs.append((parent_shape, shape))
+                    for parent_shape in self.body_shapes[parent_body]:
+                        a, b = (parent_shape, shape) if parent_shape < shape else (shape, parent_shape)
+                        self.shape_collision_filter_pairs.append((a, b))

---

4427-4460: LGTM: contact pair generation with env and group gating; stable ordering fixed.

The canonical pair formation without mutating loop vars is correct. Minor doc nit: the comment says “Copy the set …” but we only alias it.

Change the comment to “Alias the set of filtered-out shape pairs …” or actually copy via filters = set(model.shape_collision_filter_pairs) if you plan to mutate.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Learnt from: eric-heiden
PR: newton-physics/newton#587
File: newton/_src/sim/builder.py:656-661
Timestamp: 2025-08-20T18:02:36.099Z
Learning: In Newton physics, collisions between shapes with body index -1 (world-attached/global shapes) are automatically skipped by the collision system, so no manual collision filter pairs need to be added between global shapes from different builders.

Learnt from: eric-heiden
PR: newton-physics/newton#587
File: newton/_src/sim/builder.py:656-661
Timestamp: 2025-08-20T18:02:36.099Z
Learning: In Newton physics, collisions between shapes with body index -1 (world-attached/global shapes) are automatically skipped by the collision system, so no manual collision filter pairs need to be added between global shapes from different builders.

newton/_src/sim/builder.py (2)

639-643: Don’t mutate Warp transforms via slicing; assign translation through .p.

Index assignment on wp.transform is unreliable. Set the position explicitly while reusing the same object.

Apply:

-        xform = wp.transform_identity()
+        xform = wp.transform()
         for i in range(num_copies):
-            xform[:3] = offsets[i]
+            xform.p = wp.vec3(offsets[i][0], offsets[i][1], offsets[i][2])
             self.add_builder(builder, xform=xform)

---

964-971: Critical: from_buffer() out-param risks dangling memory; avoid private cfunc hard dependency.

Creating a transform from a temporary NumPy buffer can leave out pointing at freed memory; also wp.context.runtime.core is private and may change.

Apply:

-        transform_mul_cfunc = wp.context.runtime.core.wp_builtin_mul_transformf_transformf
+        transform_mul_cfunc = getattr(getattr(wp.context.runtime, "core", None), "wp_builtin_mul_transformf_transformf", None)
...
-        def transform_mul(a, b):
-            out = wp.transform.from_buffer(np.empty(7, dtype=np.float32))
-            transform_mul_cfunc(a, b, ctypes.byref(out))
-            return out
+        def transform_mul(a, b):
+            if transform_mul_cfunc:
+                out = wp.transform()  # proper Warp-owned storage
+                transform_mul_cfunc(a, b, ctypes.byref(out))
+                return out
+            # safe public fallback
+            return a * b

newton/_src/sim/builder.py (3)

972-980: Normalize xform input before use.

If callers pass a tuple/list instead of a Warp transform, xform.p will fail.

Apply:

-            self.particle_max_velocity = builder.particle_max_velocity
-            if xform is not None:
-                pos_offset = xform.p
+            self.particle_max_velocity = builder.particle_max_velocity
+            if xform is not None:
+                if not hasattr(xform, "p"):
+                    xform = wp.transform(*xform)
+                pos_offset = xform.p

---

2405-2411: Skip generating same-body filter pairs for world (-1) to reduce pair count.

Per your prior guidance, world-attached shapes are auto-excluded; explicit (-1, -1) pairs are redundant and can bloat the list for large scenes.
[retrieved_learnings used]

Apply:

-        if body in self.body_shapes:
+        if body in self.body_shapes:
             # no contacts between shapes of the same body
-            for same_body_shape in self.body_shapes[body]:
-                self.shape_collision_filter_pairs.append((same_body_shape, shape))
+            if body != -1:
+                for same_body_shape in self.body_shapes[body]:
+                    self.shape_collision_filter_pairs.append((same_body_shape, shape))

---

4423-4455: Edge case: ensure empty contact pair array has shape (0,2).

When no pairs exist, np.array([]) yields shape (0,), which can trip Warp dtype/shape checks.

Apply:

-        model.shape_contact_pairs = wp.array(np.array(contact_pairs), dtype=wp.vec2i, device=model.device)
+        if contact_pairs:
+            arr = np.asarray(contact_pairs, dtype=np.int32)
+        else:
+            arr = np.empty((0, 2), dtype=np.int32)
+        model.shape_contact_pairs = wp.array(arr, dtype=wp.vec2i, device=model.device)

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Learnt from: eric-heiden
PR: newton-physics/newton#587
File: newton/_src/sim/builder.py:656-661
Timestamp: 2025-08-20T18:02:36.099Z
Learning: In Newton physics, collisions between shapes with body index -1 (world-attached/global shapes) are automatically skipped by the collision system, so no manual collision filter pairs need to be added between global shapes from different builders.

Learnt from: eric-heiden
PR: newton-physics/newton#587
File: newton/_src/sim/builder.py:656-661
Timestamp: 2025-08-20T18:02:36.099Z
Learning: In Newton physics, collisions between shapes with body index -1 (world-attached/global shapes) are automatically skipped by the collision system, so no manual collision filter pairs need to be added between global shapes from different builders.