Completely remove contact feature id support #1309
Conversation
…reliable and therefore the code complexity and overhead they introduce is not justified.
📝 WalkthroughWalkthroughThis PR removes per-contact feature IDs and pair-key plumbing across the collision pipeline, deletes the ContactMatcher module and its tests, simplifies support/minkowski functions to return only a point, and replaces feature-based fields with a mode/normal-only representation in contact and reduction data structures. Changes
Sequence Diagram(s)sequenceDiagram
participant NarrowPhase
participant SupportMap
participant SimplexSolver
participant Reducer
participant Writer
NarrowPhase->>SupportMap: request support point for shape(s)
SupportMap-->>NarrowPhase: support point (no feature id)
NarrowPhase->>SimplexSolver: run GJK/MPR/closest-distance using support point
SimplexSolver-->>NarrowPhase: contact (position, normal, depth)
NarrowPhase->>Reducer: export_contact_to_buffer(position, normal, depth)
Reducer-->>Writer: reduced contacts (position, normal, depth, mode)
Writer-->>[External]: emit final ContactData (mode, no feature IDs)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-09-26T05:58:21.013ZApplied to files:
📚 Learning: 2025-09-26T05:58:21.013ZApplied to files:
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
newton/_src/geometry/contact_reduction.py (1)
536-542: Remove unnecessaryint()cast.Line 537 has an unnecessary
int()cast sincewrite_idxis already declared asint(0)on line 537 (implicitly) and is used in integer operations throughout the loop.♻️ Suggested simplification
if thread_id == 0: - write_idx = int(0) + write_idx = 0 for key in range(num_slots): if buffer[key].projection > empty_marker: active_ids[write_idx] = key write_idx += 1 active_ids[num_slots] = write_idx synchronize()
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
newton/_src/geometry/collision_convex.pynewton/_src/geometry/collision_core.pynewton/_src/geometry/contact_data.pynewton/_src/geometry/contact_matcher.pynewton/_src/geometry/contact_reduction.pynewton/_src/geometry/mpr.pynewton/_src/geometry/multicontact.pynewton/_src/geometry/narrow_phase.pynewton/_src/geometry/sdf_contact.pynewton/_src/geometry/sdf_hydroelastic.pynewton/_src/geometry/simplex_solver.pynewton/_src/geometry/support_function.pynewton/_src/sim/collide_unified.pynewton/tests/test_contact_matcher.pynewton/tests/test_contact_reduction.py
💤 Files with no reviewable changes (3)
- newton/_src/geometry/contact_matcher.py
- newton/tests/test_contact_matcher.py
- newton/_src/geometry/contact_data.py
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: shi-eric
Repo: newton-physics/newton PR: 521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The Newton migration guide (docs/migration.rst) is specifically for documenting how to migrate existing warp.sim functionality to Newton equivalents. New Newton-only features that didn't exist in warp.sim do not need migration documentation.
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/_src/solvers/featherstone/kernels.py:75-75
Timestamp: 2025-08-12T18:04:06.577Z
Learning: The Newton physics framework requires nightly Warp builds, which means compatibility concerns with older stable Warp versions (like missing functions such as wp.spatial_adjoint) are not relevant for this project.
📚 Learning: 2025-10-24T07:56:14.792Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 981
File: newton/_src/geometry/collision_convex.py:180-289
Timestamp: 2025-10-24T07:56:14.792Z
Learning: In newton/_src/geometry/collision_convex.py, the single-contact solver (create_solve_convex_single_contact) intentionally returns count=1 even when shapes are separated (signed_distance > contact_threshold). The calling code is responsible for deciding whether to accept the contact based on the signed distance value. This design pushes filtering responsibility to the caller.
Applied to files:
newton/_src/geometry/collision_convex.pynewton/_src/geometry/simplex_solver.pynewton/_src/geometry/sdf_contact.pynewton/_src/geometry/sdf_hydroelastic.pynewton/_src/geometry/collision_core.pynewton/_src/geometry/contact_reduction.pynewton/_src/geometry/mpr.py
📚 Learning: 2025-09-26T05:58:21.013Z
Learnt from: WenchaoHuang
Repo: newton-physics/newton PR: 835
File: newton/_src/solvers/style3d/collision/collision.py:49-53
Timestamp: 2025-09-26T05:58:21.013Z
Learning: In newton/_src/solvers/style3d/collision/collision.py, the broad_phase buffers (broad_phase_ee, broad_phase_ef, broad_phase_vf) created with wp.array() don't need zero-initialization because they follow a write-first-read-later pattern: BVH query kernels in frame_begin() populate these buffers completely (write-only operations), and collision kernels only read from them afterward in accumulate_contact_force(), ensuring no undefined reads occur.
Applied to files:
newton/_src/geometry/collision_convex.pynewton/_src/geometry/narrow_phase.pynewton/_src/sim/collide_unified.pynewton/_src/geometry/sdf_contact.pynewton/_src/geometry/sdf_hydroelastic.pynewton/_src/geometry/collision_core.py
📚 Learning: 2025-08-12T17:51:37.474Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 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).
Applied to files:
newton/_src/geometry/collision_convex.pynewton/_src/geometry/narrow_phase.pynewton/_src/geometry/sdf_contact.pynewton/_src/geometry/sdf_hydroelastic.pynewton/_src/geometry/collision_core.pynewton/_src/geometry/contact_reduction.pynewton/_src/geometry/multicontact.py
📚 Learning: 2025-12-12T17:45:26.847Z
Learnt from: vastsoun
Repo: newton-physics/newton PR: 1019
File: newton/_src/solvers/kamino/geometry/primitives.py:0-0
Timestamp: 2025-12-12T17:45:26.847Z
Learning: In newton/_src/solvers/kamino/geometry/primitives.py, the narrow-phase kernel `_primitive_narrowphase` does not need to handle symmetric shape-pair orderings (e.g., both SPHERE-BOX and BOX-SPHERE) because `kamino.core.builder.ModelBuilder` and `kamino.geometry.primitive.broadphase` guarantee that explicit candidate geometry pairs are always defined with GIDs in ascending order.
Applied to files:
newton/_src/geometry/narrow_phase.pynewton/_src/geometry/sdf_contact.pynewton/_src/geometry/sdf_hydroelastic.pynewton/_src/geometry/collision_core.py
📚 Learning: 2025-09-26T05:58:21.013Z
Learnt from: WenchaoHuang
Repo: newton-physics/newton PR: 835
File: newton/_src/solvers/style3d/collision/collision.py:49-53
Timestamp: 2025-09-26T05:58:21.013Z
Learning: In newton/_src/solvers/style3d/collision/collision.py, the broad_phase buffers (broad_phase_ee, broad_phase_ef, broad_phase_vf) created with wp.array() don't need zero-initialization because the BVH query kernels that use them always write the count to query_results[0, tid] = 0 first before populating any data, ensuring no undefined reads occur.
Applied to files:
newton/_src/geometry/narrow_phase.pynewton/_src/sim/collide_unified.py
📚 Learning: 2025-08-12T17:52:15.009Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/_src/utils/import_mjcf.py:226-231
Timestamp: 2025-08-12T17:52:15.009Z
Learning: In the Newton codebase, when passing array-like objects (numpy arrays, lists, tuples) to wp.vec3(), the consistent pattern is to use the unpacking operator: wp.vec3(*array) rather than wp.vec3(array). This pattern is used throughout newton/_src/utils/import_mjcf.py and newton/_src/core/types.py.
Applied to files:
newton/_src/geometry/narrow_phase.py
📚 Learning: 2025-12-13T17:26:39.791Z
Learnt from: lenroe-nv
Repo: newton-physics/newton PR: 1248
File: newton/_src/geometry/contact_data.py:47-49
Timestamp: 2025-12-13T17:26:39.791Z
Learning: In ContactData (newton/_src/geometry/contact_data.py), the fields contact_stiffness, contact_damping, and contact_friction_scale use 0.0 as the "unset" sentinel value. This is intentional: Warp struct fields initialize to 0.0 by default, and solver-side code interprets 0.0 as "use default/unscaled" rather than "disable." This design avoids explicit initialization in contact generation kernels.
Applied to files:
newton/_src/geometry/narrow_phase.pynewton/_src/sim/collide_unified.pynewton/_src/geometry/sdf_contact.pynewton/_src/geometry/sdf_hydroelastic.py
📚 Learning: 2025-09-25T16:14:22.033Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_objectives.py:0-0
Timestamp: 2025-09-25T16:14:22.033Z
Learning: In NVIDIA Warp's Newton physics library, wp.transform supports direct numerical indexing (e.g., body_tf[0], body_tf[1], body_tf[2] for position and body_tf[3], body_tf[4], body_tf[5], body_tf[6] for quaternion components) to access its elements. This is the standard API used throughout the codebase.
Applied to files:
newton/_src/geometry/narrow_phase.py
📚 Learning: 2025-09-15T13:13:37.976Z
Learnt from: gdaviet
Repo: newton-physics/newton PR: 750
File: newton/_src/solvers/implicit_mpm/solve_rheology.py:772-783
Timestamp: 2025-09-15T13:13:37.976Z
Learning: In Warp's tiled launch API, using a 2D launch dimension dim=(num_blocks, tile_size) is correct for tiled kernels. The wp.tid() function can either return a 2D tuple (block_index, lane) or just discard the last dimension to return only the block_index when lane information is not needed, making `block_index = wp.tid()` a valid shortcut in tiled kernels.
Applied to files:
newton/_src/geometry/narrow_phase.py
📚 Learning: 2025-12-12T08:45:51.908Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1221
File: newton/examples/example_sdf.py:277-287
Timestamp: 2025-12-12T08:45:51.908Z
Learning: In Newton examples (e.g., example_sdf.py), computing contacts once per frame and reusing them across multiple substeps is an intentional design choice, not a bug. The contacts are computed before the substep loop and intentionally kept constant throughout all substeps within that frame.
Applied to files:
newton/_src/geometry/sdf_contact.pynewton/_src/geometry/sdf_hydroelastic.pynewton/_src/geometry/contact_reduction.py
🧬 Code graph analysis (6)
newton/_src/geometry/collision_convex.py (2)
newton/_src/geometry/mpr.py (1)
create_solve_mpr(200-487)newton/_src/geometry/simplex_solver.py (1)
create_solve_closest_distance(56-563)
newton/tests/test_contact_reduction.py (1)
newton/_src/geometry/contact_reduction.py (1)
collect_active_contacts(519-543)
newton/_src/geometry/simplex_solver.py (1)
newton/_src/geometry/mpr.py (1)
minkowski_support(118-159)
newton/_src/geometry/sdf_contact.py (1)
newton/_src/geometry/contact_reduction.py (1)
collect_active_contacts(519-543)
newton/_src/geometry/sdf_hydroelastic.py (1)
newton/_src/geometry/collision_core.py (1)
sat_box_intersection(1033-1072)
newton/_src/geometry/collision_core.py (1)
newton/_src/geometry/support_function.py (1)
support_map(112-301)
🪛 Ruff (0.14.10)
newton/_src/geometry/support_function.py
112-112: Unused function argument: data_provider
(ARG001)
newton/_src/geometry/contact_reduction.py
537-537: Value being cast to int is already an integer
Remove unnecessary int call
(RUF046)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run GPU Tests / Run GPU Unit Tests on AWS EC2
- GitHub Check: Run GPU Benchmarks / Run GPU Benchmarks on AWS EC2
🔇 Additional comments (25)
newton/_src/geometry/support_function.py (3)
24-26: Documentation accurately reflects the simplified return type.The updated docstring correctly describes the new single-value return semantics, aligning with the PR objective of removing feature ID support.
112-115: Signature simplification looks correct;data_provideris intentionally retained for extensibility.The static analysis flags
data_provideras unused (ARG001), but per theSupportMapDataProviderdocstring (lines 55-61), this parameter is a placeholder for projects like MuJoCo Warp that store convex hull data in external arrays. Keeping it in the interface maintains API compatibility and extensibility.The return type simplification from
tuple[wp.vec3, int]towp.vec3correctly implements the feature ID removal.
298-301: Default case handling is appropriate.Returning a zero vector for unhandled shape types is a safe fallback. The simplified single-value return is consistent with all other branches.
newton/_src/geometry/collision_convex.py (4)
66-67: Docstring correctly updated to reflect simplified support function contract.The documentation now accurately states the support function returns only a support point, consistent with the changes in
support_function.py.
189-191: Single-contact solver docstring updated consistently.Same documentation update as the multi-contact solver, maintaining consistency across the API.
239-249: Solver unpacking correctly simplified to 4-tuple.The MPR solver now returns
(collision, signed_distance, point, normal)without feature IDs. The unpacking matches the updated contract frommpr.py.
251-262: GJK fallback unpacking also correctly updated.The
create_solve_closest_distancecall now unpacks 4 values, consistent with the updatedsimplex_solver.pycontract.newton/_src/geometry/sdf_hydroelastic.py (1)
24-24: Import correctly simplified to remove unusedbuild_pair_key2.The import now only includes
sat_box_intersection, which is still used for OBB collision checks in the broadphase (line 832). The removal ofbuild_pair_key2is consistent with the PR objective of eliminating pair-key handling throughout the codebase.newton/_src/geometry/mpr.py (4)
73-75: Factory function docstring correctly updated.The documentation now reflects that
support_funcreturns only a support point, consistent with the updatedsupport_mapsignature.
91-115:support_map_bcorrectly simplified to return singlewp.vec3.The function now returns only the world-space support point without feature ID tracking. The internal logic (coordinate transformation, support evaluation, world-space conversion) remains unchanged and correct.
126-159:minkowski_supportcorrectly returns singleVertwithout feature IDs.The Minkowski difference computation logic is preserved. The function now returns only the
Vertstruct containingB(point on shape B) andBtoA(vector from B to A), which is sufficient for the MPR algorithm.
273-273: Allminkowski_supportcall sites in MPR core correctly updated.The calls at lines 273, 293, 328, and 375 now receive a single
Vertvalue. Previously these unpacked(Vert, feature_a_id, feature_b_id). The algorithm logic usingv1,v2,v3,v4remains unchanged.Also applies to: 293-293, 328-328, 375-375
newton/tests/test_contact_reduction.py (4)
207-207: Field rename fromfeaturetomodecorrectly applied.The test data generation now uses
c.modeinstead ofc.feature, consistent with theContactStructfield rename incontact_reduction.py.
216-216: Function reference correctly updated tocollect_active_contacts.The test kernel now uses the renamed function from
ContactReductionFunctions, aligning with the refactor fromfilter_unique_contactstocollect_active_contacts.
252-253: Function call correctly updated with matching semantics.The call to
collect_active_contactsreplacesfilter_unique_contacts, and the comment accurately describes the operation as collecting active contacts after binning.
348-348: Comment correctly describes updated reduction semantics.The explanation now accurately reflects that reduction keeps "only best contact per (bin, direction) slot" rather than filtering duplicates based on feature IDs.
newton/_src/geometry/simplex_solver.py (1)
396-396:minkowski_supportcall correctly simplified to single-value return.The GJK solver now receives a single
Vertfromminkowski_supportinstead of unpacking(Vert, feature_a_id, feature_b_id). The subsequent usage ofw.BtoA(line 401) andw.B/w.BtoA(lines 434-435) remains correct.newton/_src/geometry/collision_core.py (1)
397-418: Allsupport_mapcalls correctly simplified to single-value returns.The AABB computation now receives a single
wp.vec3from eachsupport_mapcall instead of unpacking a tuple. The six direction queries (+X, +Y, +Z, -X, -Y, -Z) and subsequent dot product calculations remain functionally correct.newton/_src/sim/collide_unified.py (1)
82-169: LGTM! Clean removal of contact matching infrastructure.The removal of
contact_pair_keyandcontact_keyhandling fromwrite_contactandUnifiedContactWriterDatais well-executed. The function now focuses solely on geometric contact data, simplifying the collision pipeline.newton/_src/geometry/multicontact.py (1)
964-993: LGTM! Support function return type simplified correctly.The change from
pt_a, feature_a = support_func(...)topt_a = support_func(...)is consistent with the removal of feature ID tracking. The manifold generation logic now focuses purely on geometric contact points.newton/_src/geometry/contact_reduction.py (2)
59-64: LGTM! Field rename fromfeaturetomodeclarifies intent.The rename from
featuretomodewith the comment "Used to track collision mode (e.g., which mesh the triangle came from)" better describes the field's purpose in the simplified contact tracking approach.
510-545: LGTM! Function rename better reflects simplified behavior.The rename from
_create_filter_unique_contactsto_create_collect_active_contactsaccurately describes the new behavior: collecting all valid contacts into a compact list rather than filtering duplicates based on feature IDs.newton/_src/geometry/sdf_contact.py (2)
1109-1113: LGTM! Mode field correctly tracks collision direction.The assignment
c.mode = mode(where mode is 0 or 1) appropriately tracks which mesh the triangle originated from in mesh-mesh collisions. This is later used correctly at lines 1155-1162 to determine thickness assignment.
1131-1167: LGTM! Consistent use of mode field in contact reduction output.The mode field is correctly used to distinguish between the two collision directions (mesh A vs SDF B, and mesh B vs SDF A), ensuring proper thickness assignment for each contact. The centered world-space coordinates are appropriately converted back to world space before writing.
newton/_src/geometry/narrow_phase.py (1)
771-771: Remove this comment; mesh-plane and SDF collision paths are independent and pose no mode semantics issue.The
c.mode = 0assignment is a placeholder in the mesh-plane collision path. Mesh-plane contacts directly assign thickness values (lines 795–796) without consulting the mode field, whereas SDF contacts use mode to distinguish between which mesh's triangles are being tested (mode=0: mesh0 vs SDF1, mode=1: mesh1 vs SDF0). These are separate, non-interacting code paths with independent thickness handling; there is no semantic inconsistency to verify.Likely an incorrect or invalid review comment.
adenzler-nvidia
left a comment
adenzler-nvidia
left a comment
There was a problem hiding this comment.
Thanks! Let's rip this out.
# Conflicts: # newton/_src/geometry/support_function.py
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
newton/tests/test_contact_reduction_global.py (1)
102-112: Tests still pass removedfeatureparameter, causing pipeline failures.The
export_and_reduce_contactfunction signature no longer includes thefeatureparameter, but this test kernel still passesfeature=42. This causes theWarpCodegenErrorseen in CI. The same issue exists in all other test kernels in this file.🐛 Proposed fix for test_basic_contact_storage
_ = export_and_reduce_contact( shape_a=0, shape_b=1, position=wp.vec3(1.0, 2.0, 3.0), normal=wp.vec3(0.0, 1.0, 0.0), depth=-0.01, - feature=42, reducer_data=reducer_data, beta0=1000.0, beta1=0.001, )Apply similar fixes to the following test kernels:
store_multiple_contacts_kernel(line 150)store_different_pairs_kernel(line 190)store_one_contact_kernel(line 227)stress_kernel(line 282)store_contact_kernelintest_clear_active(line 319)store_contacts_kernelintest_export_reduced_contacts_kernel(line 383)
🧹 Nitpick comments (1)
newton/_src/geometry/contact_reduction.py (1)
515-550: LGTM!The renamed
collect_active_contactsfunction correctly simplifies the logic by removing per-feature duplicate filtering that's no longer needed. The implementation now collects all valid contacts (projection > empty_marker) into a compact list.Minor: The
int(0)cast on line 542 is redundant since0is already an integer. Consider removing it for clarity:♻️ Optional cleanup
if thread_id == 0: - write_idx = int(0) + write_idx = 0 for key in range(num_slots):
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
asv/benchmarks/collision/benchmark_contact_reduction.pynewton/_src/geometry/contact_reduction.pynewton/_src/geometry/contact_reduction_global.pynewton/_src/geometry/narrow_phase.pynewton/_src/geometry/support_function.pynewton/tests/test_contact_reduction_global.py
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: shi-eric
Repo: newton-physics/newton PR: 521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The Newton migration guide (docs/migration.rst) is specifically for documenting how to migrate existing warp.sim functionality to Newton equivalents. New Newton-only features that didn't exist in warp.sim do not need migration documentation.
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/_src/solvers/featherstone/kernels.py:75-75
Timestamp: 2025-08-12T18:04:06.577Z
Learning: The Newton physics framework requires nightly Warp builds, which means compatibility concerns with older stable Warp versions (like missing functions such as wp.spatial_adjoint) are not relevant for this project.
📚 Learning: 2025-09-26T05:58:21.013Z
Learnt from: WenchaoHuang
Repo: newton-physics/newton PR: 835
File: newton/_src/solvers/style3d/collision/collision.py:49-53
Timestamp: 2025-09-26T05:58:21.013Z
Learning: In newton/_src/solvers/style3d/collision/collision.py, the broad_phase buffers (broad_phase_ee, broad_phase_ef, broad_phase_vf) created with wp.array() don't need zero-initialization because they follow a write-first-read-later pattern: BVH query kernels in frame_begin() populate these buffers completely (write-only operations), and collision kernels only read from them afterward in accumulate_contact_force(), ensuring no undefined reads occur.
Applied to files:
newton/_src/geometry/contact_reduction_global.pynewton/_src/geometry/narrow_phase.py
📚 Learning: 2025-09-26T05:58:21.013Z
Learnt from: WenchaoHuang
Repo: newton-physics/newton PR: 835
File: newton/_src/solvers/style3d/collision/collision.py:49-53
Timestamp: 2025-09-26T05:58:21.013Z
Learning: In newton/_src/solvers/style3d/collision/collision.py, the broad_phase buffers (broad_phase_ee, broad_phase_ef, broad_phase_vf) created with wp.array() don't need zero-initialization because the BVH query kernels that use them always write the count to query_results[0, tid] = 0 first before populating any data, ensuring no undefined reads occur.
Applied to files:
newton/_src/geometry/contact_reduction_global.pynewton/_src/geometry/narrow_phase.py
📚 Learning: 2025-10-24T07:56:14.792Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 981
File: newton/_src/geometry/collision_convex.py:180-289
Timestamp: 2025-10-24T07:56:14.792Z
Learning: In newton/_src/geometry/collision_convex.py, the single-contact solver (create_solve_convex_single_contact) intentionally returns count=1 even when shapes are separated (signed_distance > contact_threshold). The calling code is responsible for deciding whether to accept the contact based on the signed distance value. This design pushes filtering responsibility to the caller.
Applied to files:
newton/_src/geometry/contact_reduction.py
📚 Learning: 2025-08-12T17:51:37.474Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 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).
Applied to files:
newton/_src/geometry/contact_reduction.pynewton/tests/test_contact_reduction_global.pynewton/_src/geometry/narrow_phase.py
📚 Learning: 2025-12-12T08:45:51.908Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1221
File: newton/examples/example_sdf.py:277-287
Timestamp: 2025-12-12T08:45:51.908Z
Learning: In Newton examples (e.g., example_sdf.py), computing contacts once per frame and reusing them across multiple substeps is an intentional design choice, not a bug. The contacts are computed before the substep loop and intentionally kept constant throughout all substeps within that frame.
Applied to files:
newton/_src/geometry/contact_reduction.py
📚 Learning: 2025-12-12T17:45:26.847Z
Learnt from: vastsoun
Repo: newton-physics/newton PR: 1019
File: newton/_src/solvers/kamino/geometry/primitives.py:0-0
Timestamp: 2025-12-12T17:45:26.847Z
Learning: In newton/_src/solvers/kamino/geometry/primitives.py, the narrow-phase kernel `_primitive_narrowphase` does not need to handle symmetric shape-pair orderings (e.g., both SPHERE-BOX and BOX-SPHERE) because `kamino.core.builder.ModelBuilder` and `kamino.geometry.primitive.broadphase` guarantee that explicit candidate geometry pairs are always defined with GIDs in ascending order.
Applied to files:
newton/_src/geometry/narrow_phase.py
📚 Learning: 2025-10-30T07:28:13.112Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 984
File: newton/_src/geometry/broad_phase_nxn.py:318-336
Timestamp: 2025-10-30T07:28:13.112Z
Learning: In Newton's BroadPhaseAllPairs class (newton/_src/geometry/broad_phase_nxn.py), device management is the caller's responsibility. The library intentionally does not perform automatic device transfers for precomputed arrays in the launch method, even if there's a device mismatch. Users must ensure that the device specified in __init__ matches the device used in launch.
Applied to files:
newton/_src/geometry/narrow_phase.py
📚 Learning: 2025-08-12T17:52:15.009Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/_src/utils/import_mjcf.py:226-231
Timestamp: 2025-08-12T17:52:15.009Z
Learning: In the Newton codebase, when passing array-like objects (numpy arrays, lists, tuples) to wp.vec3(), the consistent pattern is to use the unpacking operator: wp.vec3(*array) rather than wp.vec3(array). This pattern is used throughout newton/_src/utils/import_mjcf.py and newton/_src/core/types.py.
Applied to files:
newton/_src/geometry/narrow_phase.py
📚 Learning: 2025-12-13T17:26:39.791Z
Learnt from: lenroe-nv
Repo: newton-physics/newton PR: 1248
File: newton/_src/geometry/contact_data.py:47-49
Timestamp: 2025-12-13T17:26:39.791Z
Learning: In ContactData (newton/_src/geometry/contact_data.py), the fields contact_stiffness, contact_damping, and contact_friction_scale use 0.0 as the "unset" sentinel value. This is intentional: Warp struct fields initialize to 0.0 by default, and solver-side code interprets 0.0 as "use default/unscaled" rather than "disable." This design avoids explicit initialization in contact generation kernels.
Applied to files:
newton/_src/geometry/narrow_phase.py
📚 Learning: 2025-09-25T16:14:22.033Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_objectives.py:0-0
Timestamp: 2025-09-25T16:14:22.033Z
Learning: In NVIDIA Warp's Newton physics library, wp.transform supports direct numerical indexing (e.g., body_tf[0], body_tf[1], body_tf[2] for position and body_tf[3], body_tf[4], body_tf[5], body_tf[6] for quaternion components) to access its elements. This is the standard API used throughout the codebase.
Applied to files:
newton/_src/geometry/narrow_phase.py
📚 Learning: 2025-09-15T13:13:37.976Z
Learnt from: gdaviet
Repo: newton-physics/newton PR: 750
File: newton/_src/solvers/implicit_mpm/solve_rheology.py:772-783
Timestamp: 2025-09-15T13:13:37.976Z
Learning: In Warp's tiled launch API, using a 2D launch dimension dim=(num_blocks, tile_size) is correct for tiled kernels. The wp.tid() function can either return a 2D tuple (block_index, lane) or just discard the last dimension to return only the block_index when lane information is not needed, making `block_index = wp.tid()` a valid shortcut in tiled kernels.
Applied to files:
newton/_src/geometry/narrow_phase.py
🧬 Code graph analysis (1)
asv/benchmarks/collision/benchmark_contact_reduction.py (1)
newton/_src/geometry/contact_reduction_global.py (1)
export_and_reduce_contact(518-534)
🪛 GitHub Actions: Pull Request
newton/tests/test_contact_reduction_global.py
[error] 111-111: WarpCodegenError: Couldn't find function overload for 'export_and_reduce_contact' that matched inputs with types: [int32, int32, vec3f, vec3f, float32, int32, GlobalContactReducerData, float32, float32]
[error] 322-322: WarpCodegenError: Couldn't find function overload for 'export_and_reduce_contact' that matched inputs with types: [int32, int32, vec3f, vec3f, float32, int32, GlobalContactReducerData, float32, float32]
[error] 230-230: WarpCodegenError: Couldn't find function overload for 'export_and_reduce_contact' that matched inputs with types: [int32, int32, vec3f, vec3f, float32, int32, GlobalContactReducerData, float32, float32]
[error] 153-153: WarpCodegenError: Couldn't find function overload for 'export_and_reduce_contact' that matched inputs with types: [int32, int32, vec3f, vec3f, float32, int32, GlobalContactReducerData, float32, float32]
[error] 193-193: WarpCodegenError: Couldn't find function overload for 'export_and_reduce_contact' that matched inputs with types: [int32, int32, vec3f, vec3f, float32, int32, GlobalContactReducerData, float32, float32]
[error] 386-386: WarpCodegenError: Couldn't find function overload for 'export_and_reduce_contact' that matched inputs with types: [int32, int32, vec3f, vec3f, float32, int32, GlobalContactReducerData, float32, float32]
[error] 285-285: WarpCodegenError: Couldn't find function overload for 'export_and_reduce_contact' that matched inputs with types: [int32, int32, vec3f, vec3f, float32, int32, GlobalContactReducerData, float32, float32]
🪛 Ruff (0.14.10)
newton/_src/geometry/support_function.py
112-112: Unused function argument: data_provider
(ARG001)
newton/_src/geometry/contact_reduction.py
542-542: Value being cast to int is already an integer
Remove unnecessary int call
(RUF046)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run GPU Tests / Run GPU Unit Tests on AWS EC2
- GitHub Check: Run GPU Benchmarks / Run GPU Benchmarks on AWS EC2
🔇 Additional comments (12)
asv/benchmarks/collision/benchmark_contact_reduction.py (1)
63-63: LGTM!The
export_and_reduce_contactcall correctly omits the removedfeatureparameter, aligning with the updated function signature from the broader refactor.newton/tests/test_contact_reduction_global.py (1)
440-440: LGTM!The field reference correctly uses
reducer.normalto match the renamed field inGlobalContactReducer.newton/_src/geometry/support_function.py (3)
24-26: LGTM!The module docstring correctly describes the simplified return value (support point only, no feature ID), aligning with the PR's goal of removing feature ID tracking.
112-126: LGTM!The function signature and docstring correctly reflect returning only
wp.vec3(the support point) without feature ID tracking. Thedata_providerparameter remains as a placeholder for extensibility in other projects as documented inSupportMapDataProvider.
298-301: LGTM!The simplified return statement correctly returns only the support point, consistent with the updated function contract.
newton/_src/geometry/narrow_phase.py (2)
731-732: LGTM!The
c.mode = 0assignment correctly replaces the previous feature-based tracking, aligning with theContactStruct.modefield introduced incontact_reduction.py.
1118-1119: LGTM!Correctly references
self.global_contact_reducer.normalto match the renamed field fromnormal_feature.newton/_src/geometry/contact_reduction.py (1)
63-64: LGTM!The field rename from
featuretomodewith updated documentation correctly reflects the shift from per-contact feature IDs to collision mode tracking.newton/_src/geometry/contact_reduction_global.py (4)
211-211: LGTM!The field type change from
wp.vec4(normal_feature) towp.vec3(normal) correctly simplifies the data structure by removing the embedded feature component.
327-330: LGTM!The buffer initialization correctly uses
wp.vec3for the normal array, matching the updated data structure.
568-590: LGTM!The
unpack_contactfunction correctly:
- Takes
normal: wp.array(dtype=wp.vec3)instead of the previous vec4 type- Returns
(position, n, depth)tuple without the feature component- Reads normal directly as vec3 without unpacking from vec4
517-534: LGTM!The
export_and_reduce_contactlegacy wrapper correctly removes thefeatureparameter and passes the updated arguments toexport_contact_to_buffer.
2 participants
Completely remove contact feature id support since they are not very reliable and therefore the code complexity and overhead they introduce is not justified.
Description
Newton Migration Guide
Please ensure the migration guide for warp.sim users is up-to-date with the changes made in this PR.
docs/migration.rstis up-to dateBefore your PR is "Ready for review"
newton/tests/test_examples.py)pre-commit run -aSummary by CodeRabbit
Bug Fixes
Chores / Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.