Unified collision pipeline performance improvements #1375
Conversation
…ut routing all of them through gjk/mpr
… since this parameter does not really make sense there
📝 WalkthroughWalkthroughThis PR removes ASV collision benchmarks, refactors narrow-phase collision into a primitive-first then GJK/MPR two-stage flow, introduces multi-contact capsule returns and ellipsoid SDF gradient support, adds contact-margin validation and export/reduction optimizations, and replaces per-pair contact limits with dynamic estimation plus a has_meshes flag. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (4)newton/_src/**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
newton/**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (12)📓 Common learnings📚 Learning: 2025-12-12T17:45:26.847ZApplied to files:
📚 Learning: 2025-08-12T17:51:37.474ZApplied to files:
📚 Learning: 2025-09-26T05:58:21.013ZApplied to files:
📚 Learning: 2025-08-20T18:02:36.099ZApplied to files:
📚 Learning: 2025-09-15T13:13:37.976ZApplied to files:
📚 Learning: 2025-09-10T13:03:58.591ZApplied to files:
📚 Learning: 2025-10-06T18:14:18.582ZApplied to files:
📚 Learning: 2025-09-26T05:58:21.013ZApplied to files:
📚 Learning: 2025-10-30T07:28:13.112ZApplied to files:
📚 Learning: 2025-10-10T10:47:41.082ZApplied to files:
📚 Learning: 2026-01-13T03:11:40.556ZApplied to files:
🧬 Code graph analysis (1)newton/_src/geometry/narrow_phase.py (9)
⏰ 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). (4)
🔇 Additional comments (5)
✏️ Tip: You can disable this entire section by setting 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 |
|
Removed benchmarks I added in an earlier comment as discussed in the Wednesday meeting. Will keep them locally on my machine, they are not intended to run on CI. |
|
This MR requires #1335 to be implemented before it can be merged |
Codecov Report❌ Patch coverage is
📢 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
newton/_src/geometry/narrow_phase.py (1)
212-220: Consider preventing counter overflow or clarify bounds assumptions for downstream kernels.The counter increments unconditionally while the write is conditional, so
shape_pairs_sdf_sdf_countcan exceed the buffer size. Downstream kernels insdf_hydroelastic.py(broadphase_collision_pairs_count,broadphase_collide_blocks_count) properly guard against OOB by checkingif tid >= shape_pairs_sdf_sdf_count[0]: returnbefore array access, so there's no immediate risk. However, this differs from the defensive pattern used forcontact_count(which decrements on overflow). Either apply the same defensive decrement here for consistency, or add a comment clarifying that downstream iteration bounds are safe due to the guard checks.
🤖 Fix all issues with AI agents
In `@newton/_src/geometry/narrow_phase.py`:
- Around line 410-453: The reservation can overflow the contact buffers: after
computing num_valid and calling base_index =
wp.atomic_add(writer_data.contact_count, 0, num_valid), check against
writer_data.contact_max and clamp/rollback any excess before writing; compute
end_index = base_index + num_valid, if end_index > writer_data.contact_max then
excess = end_index - writer_data.contact_max, if excess == num_valid then undo
the reservation via wp.atomic_add(writer_data.contact_count, 0, -num_valid) and
skip writes, otherwise reduce the global count by excess
(wp.atomic_add(writer_data.contact_count, 0, -excess)) and only write the first
(num_valid - excess) contacts using writer_func at indices base_index ..
base_index+allowed-1, using the existing contact_0_valid/contact_1_valid logic
to decide which ones to emit.
🧹 Nitpick comments (5)
newton/_src/geometry/contact_data.py (1)
65-96: Logic is correct but could be simplified.The computation correctly checks if the contact passes the margin threshold. However, the intermediate world-space position calculations (lines 85-90) simplify algebraically to:
d = contact_distance - (thickness_a + thickness_b)The current formulation matches the pattern in
box_plane_collisionfor consistency, which is reasonable if that's intentional. If performance is a concern, consider the simplified form.♻️ Simplified alternative (optional)
`@wp.func` def contact_passes_margin_check( contact_data: ContactData, ) -> bool: - total_separation_needed = ( - contact_data.radius_eff_a + contact_data.radius_eff_b + contact_data.thickness_a + contact_data.thickness_b - ) - - # Distance calculation matching box_plane_collision - contact_normal_a_to_b = wp.normalize(contact_data.contact_normal_a_to_b) - - a_contact_world = contact_data.contact_point_center - contact_normal_a_to_b * ( - 0.5 * contact_data.contact_distance + contact_data.radius_eff_a - ) - b_contact_world = contact_data.contact_point_center + contact_normal_a_to_b * ( - 0.5 * contact_data.contact_distance + contact_data.radius_eff_b - ) - - diff = b_contact_world - a_contact_world - distance = wp.dot(diff, contact_normal_a_to_b) - d = distance - total_separation_needed - - return d <= contact_data.margin + # Effective separation distance after accounting for radii + d = contact_data.contact_distance - (contact_data.thickness_a + contact_data.thickness_b) + return d <= contact_data.marginnewton/tests/test_hydroelastic.py (1)
132-140: Consider de-magic-ingrigid_contact_maxin this test (or justify the constants).The API update is correct, but
6000/100are hard-coded and could become flaky if hydroelastic output changes (e.g., SDF resolution tweaks). Consider deriving from expected pair count / resolution (or at least add a short rationale for the chosen values).newton/tests/test_collision_pipeline.py (1)
433-436: Thetolerance=0.1bump is very large; may mask real regressions.If the intent is to tolerate normal sign / direction variability, consider making the assertion less direction-dependent (e.g., constrain speed/energy or use a component-invariant check) so the test can stay tighter.
newton/tests/test_collision_primitives.py (1)
817-905: Test now ignores the second contact; consider asserting it for the parallel-axes cases.Right now the test only checks
distances_np[i][0]/positions_np[i][0]. To actually validate the new behavior, consider also asserting:
- parallel axes: second contact is finite and has similar distance
- non-parallel: second contact is
inf(or otherwise marked invalid)newton/_src/geometry/narrow_phase.py (1)
1120-1131:has_meshesfast-path is great; consider making the launch sizing device-aware.
min_blocks = 256can be reasonable on CUDA, but it may be unnecessary overhead on CPU runs (lots of idle threads). Consider branching onwp.get_device(device).is_cudafor the minimum block count.Also applies to: 1251-1260
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
asv/benchmarks/collision/benchmark_contact_reduction.pyasv/benchmarks/collision/benchmark_hashtable.pydocs/concepts/collisions.rstnewton/_src/geometry/collision_primitive.pynewton/_src/geometry/contact_data.pynewton/_src/geometry/contact_reduction_global.pynewton/_src/geometry/kernels.pynewton/_src/geometry/narrow_phase.pynewton/_src/sim/collide_unified.pynewton/examples/__init__.pynewton/examples/basic/example_basic_shapes.pynewton/examples/cable/example_cable_pile.pynewton/examples/robot/example_robot_panda_hydro.pynewton/tests/test_collision_pipeline.pynewton/tests/test_collision_primitives.pynewton/tests/test_hydroelastic.pynewton/tests/test_rigid_contact.pynewton/tests/test_sdf_compute.py
💤 Files with no reviewable changes (6)
- newton/examples/basic/example_basic_shapes.py
- newton/examples/init.py
- newton/tests/test_sdf_compute.py
- newton/examples/robot/example_robot_panda_hydro.py
- asv/benchmarks/collision/benchmark_contact_reduction.py
- asv/benchmarks/collision/benchmark_hashtable.py
🧰 Additional context used
📓 Path-based instructions (5)
newton/_src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
The
newton/_src/directory is internal library implementation only and must not be imported by user code (examples, documentation, or external users)
Files:
newton/_src/geometry/contact_data.pynewton/_src/geometry/contact_reduction_global.pynewton/_src/geometry/collision_primitive.pynewton/_src/geometry/narrow_phase.pynewton/_src/geometry/kernels.pynewton/_src/sim/collide_unified.py
newton/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Any user-facing class/function/object added under
_srcmust be exposed via the public Newton API through re-exports
Files:
newton/_src/geometry/contact_data.pynewton/_src/geometry/contact_reduction_global.pynewton/_src/geometry/collision_primitive.pynewton/tests/test_rigid_contact.pynewton/tests/test_hydroelastic.pynewton/examples/cable/example_cable_pile.pynewton/_src/geometry/narrow_phase.pynewton/_src/geometry/kernels.pynewton/_src/sim/collide_unified.pynewton/tests/test_collision_primitives.pynewton/tests/test_collision_pipeline.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use prefix-first naming for classes:ActuatorPD,ActuatorPID(notPDActuator,PIDActuator)
Use prefix-first naming for methods:add_shape_sphere()(notadd_sphere_shape())
Method names must usesnake_case
Prefer nested classes when self-contained: if a helper type or enum is only meaningful inside one parent class and doesn't need a public identity, define it as a nested class instead of a top-level class/module
Follow PEP 8 for Python code
Use Google-style docstrings with clear, concise explanations of what functions do, their parameters, and return values
Files:
newton/_src/geometry/contact_data.pynewton/_src/geometry/contact_reduction_global.pynewton/_src/geometry/collision_primitive.pynewton/tests/test_rigid_contact.pynewton/tests/test_hydroelastic.pynewton/examples/cable/example_cable_pile.pynewton/_src/geometry/narrow_phase.pynewton/_src/geometry/kernels.pynewton/_src/sim/collide_unified.pynewton/tests/test_collision_primitives.pynewton/tests/test_collision_pipeline.py
**/*
📄 CodeRabbit inference engine (AGENTS.md)
CLI arguments must use
kebab-case(e.g.,--use-cuda-graph, not--use_cuda_graph)
Files:
newton/_src/geometry/contact_data.pynewton/_src/geometry/contact_reduction_global.pynewton/_src/geometry/collision_primitive.pydocs/concepts/collisions.rstnewton/tests/test_rigid_contact.pynewton/tests/test_hydroelastic.pynewton/examples/cable/example_cable_pile.pynewton/_src/geometry/narrow_phase.pynewton/_src/geometry/kernels.pynewton/_src/sim/collide_unified.pynewton/tests/test_collision_primitives.pynewton/tests/test_collision_pipeline.py
newton/examples/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
newton/examples/**/*.py: Examples must not import fromnewton._src, only from the public Newton API
Examples must follow the format with anExampleclass and must be registered inREADME.mdwith auv runcommand and screenshot
Examples must implement atest_final()method that verifies the simulation state is valid after the example has run
Examples may optionally implement atest_post_step()method that is evaluated after everystep()call
Files:
newton/examples/cable/example_cable_pile.py
🧠 Learnings (18)
📓 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.
📚 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_data.pynewton/_src/geometry/collision_primitive.pydocs/concepts/collisions.rstnewton/tests/test_collision_primitives.py
📚 Learning: 2026-01-13T03:11:40.556Z
Learnt from: jumyungc
Repo: newton-physics/newton PR: 1333
File: newton/_src/sim/builder.py:0-0
Timestamp: 2026-01-13T03:11:40.556Z
Learning: Ensure all VBD damping semantics follow Rayleigh-style damping and are unitless. This convention, demonstrated in SolverVBD, should be applied consistently across all VBD-related constraints in the codebase. In newton/_src/sim/builder.py, validate that any damping terms use the same normalization and are treated as dimensionless; add comments and unit tests to enforce consistency across VBD components.
Applied to files:
newton/_src/geometry/contact_data.pynewton/_src/geometry/contact_reduction_global.pynewton/_src/geometry/collision_primitive.pynewton/tests/test_rigid_contact.pynewton/tests/test_hydroelastic.pynewton/examples/cable/example_cable_pile.pynewton/_src/geometry/narrow_phase.pynewton/_src/geometry/kernels.pynewton/_src/sim/collide_unified.pynewton/tests/test_collision_primitives.pynewton/tests/test_collision_pipeline.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/contact_reduction_global.pynewton/tests/test_rigid_contact.pynewton/_src/geometry/narrow_phase.pynewton/_src/sim/collide_unified.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_global.pynewton/tests/test_hydroelastic.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/contact_reduction_global.pynewton/_src/geometry/narrow_phase.pynewton/tests/test_collision_primitives.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.pynewton/_src/sim/collide_unified.py
📚 Learning: 2025-08-20T03:30:16.037Z
Learnt from: eric-heiden
Repo: newton-physics/newton PR: 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.
Applied to files:
newton/_src/geometry/collision_primitive.pynewton/examples/cable/example_cable_pile.pynewton/tests/test_collision_primitives.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_primitive.pynewton/examples/cable/example_cable_pile.pynewton/_src/geometry/narrow_phase.pynewton/_src/sim/collide_unified.pynewton/tests/test_collision_primitives.pynewton/tests/test_collision_pipeline.py
📚 Learning: 2025-12-25T21:44:17.047Z
Learnt from: camevor
Repo: newton-physics/newton PR: 1161
File: docs/concepts/sensors.rst:12-12
Timestamp: 2025-12-25T21:44:17.047Z
Learning: Future API standardization: The sensor.update() method should be standardized to a single-argument form update(state) across all sensor types. The redundant model parameter will be removed from sensors like SensorFrameTransform. In code reviews, check for API changes that move toward a single-argument update, ensure all sensor implementations conform, and update user-facing docs to reflect the change. If reviewing related code, verify that calls using the old two-argument form are refactored and that any dependent serialization or tests are updated accordingly.
Applied to files:
docs/concepts/collisions.rst
📚 Learning: 2026-01-12T10:11:47.988Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py:6061-6091
Timestamp: 2026-01-12T10:11:47.988Z
Learning: Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py (ModelBuilder.finalize)
Learning: The project prefers to keep finalize() validation lightweight; do not add union-find/connected-components validation for loop joints. Current behavior (allow non-articulated joints when the child is in any articulation) is acceptable; avoid overengineering here.
Applied to files:
newton/tests/test_rigid_contact.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/tests/test_rigid_contact.pynewton/_src/geometry/narrow_phase.py
📚 Learning: 2025-09-18T07:05:56.836Z
Learnt from: gdaviet
Repo: newton-physics/newton PR: 750
File: newton/_src/solvers/implicit_mpm/solve_rheology.py:969-986
Timestamp: 2025-09-18T07:05:56.836Z
Learning: In newton/_src/solvers/implicit_mpm/solve_rheology.py, transposed_strain_mat parameter cannot be None - the type signature was corrected to reflect this guarantee, eliminating the need for None checks when accessing transposed_strain_mat.offsets.
Applied to files:
newton/tests/test_hydroelastic.py
📚 Learning: 2025-10-29T14:55:48.171Z
Learnt from: gdaviet
Repo: newton-physics/newton PR: 940
File: newton/examples/mpm/example_mpm_granular.py:92-101
Timestamp: 2025-10-29T14:55:48.171Z
Learning: In newton/examples/mpm/example_mpm_granular.py, the capture() method intentionally advances the simulation state once during CUDA graph capture. This is expected behavior because CUDA graph capture records exact memory addresses, so the state arrays (state_0, state_1) cannot be reset or reallocated after capture without invalidating the graph.
Applied to files:
newton/examples/cable/example_cable_pile.py
📚 Learning: 2025-12-12T08:45:43.428Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1221
File: newton/examples/example_sdf.py:277-287
Timestamp: 2025-12-12T08:45:43.428Z
Learning: In Newtown (Newton) example code, specifically files under newton/examples, computing contacts once per frame and reusing them across all substeps is an intentional design choice, not a bug. Reviewers should verify that contacts are computed before the substep loop and reused for every substep within the same frame. This pattern reduces redundant work and preserves frame-consistency; do not flag as a regression unless the behavior is changed for correctness or performance reasons.
Applied to files:
newton/examples/cable/example_cable_pile.py
📚 Learning: 2025-10-10T10:47:41.082Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 899
File: newton/tests/test_rigid_contact.py:214-268
Timestamp: 2025-10-10T10:47:41.082Z
Learning: In `newton/tests/test_rigid_contact.py`, using `add_shape_plane(width=0, length=0)` for an infinite plane works correctly with the collision system and does not cause degenerate AABB issues in the test scenario `test_shape_collisions_gjk_mpr_multicontact`.
Applied to files:
newton/_src/geometry/narrow_phase.py
📚 Learning: 2025-09-10T13:03:58.591Z
Learnt from: gdaviet
Repo: newton-physics/newton PR: 750
File: newton/_src/solvers/implicit_mpm/solve_rheology.py:799-804
Timestamp: 2025-09-10T13:03:58.591Z
Learning: In Warp's tiled launch API, the launch dimension should include the tile size as the last dimension, matching the CUDA block dimension. For tiled operations, use dim=(num_blocks, tile_size) where tile_size equals block_dim, not just dim=num_blocks like regular kernel launches.
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 (7)
newton/_src/geometry/contact_data.py (1)
newton/_src/sim/builder.py (1)
dot(5249-5250)
newton/_src/geometry/collision_primitive.py (1)
newton/_src/sim/builder.py (1)
dot(5249-5250)
newton/tests/test_rigid_contact.py (2)
newton/_src/sim/collide_unified.py (2)
CollisionPipelineUnified(273-715)from_model(439-524)newton/_src/sim/collide.py (1)
from_model(146-190)
newton/examples/cable/example_cable_pile.py (3)
newton/examples/__init__.py (1)
create_collision_pipeline(398-467)newton/_src/sim/collide_unified.py (1)
collide(526-705)newton/_src/sim/model.py (1)
collide(658-719)
newton/_src/geometry/kernels.py (1)
newton/_src/geometry/types.py (1)
GeoType(25-67)
newton/_src/sim/collide_unified.py (2)
newton/_src/sim/builder.py (1)
shape_count(1135-1139)newton/_src/geometry/types.py (1)
GeoType(25-67)
newton/tests/test_collision_primitives.py (1)
newton/_src/geometry/collision_primitive.py (1)
collide_capsule_capsule(199-293)
🪛 Ruff (0.14.11)
newton/_src/geometry/contact_reduction_global.py
716-716: Value being cast to int is already an integer
Remove unnecessary int call
(RUF046)
🔇 Additional comments (23)
newton/_src/geometry/contact_reduction_global.py (3)
558-560: LGTM!Good early-exit optimization. Avoids unnecessary grid-stride loop iterations when there are no contacts to process.
688-690: LGTM!Consistent early-exit pattern matching the other kernel's optimization.
712-721: The current implementation is correct; do not use type annotations in place of explicit casts.The explicit type casts (
j = int(0),already_exported = bool(False)) are required by Warp's JIT compiler for variables that are mutated in kernels. Type annotations alone (e.g.,j: int = 0) are compile-time hints in Python and do not provide the runtime type information Warp needs. The retrieved learning from this repository confirms: "variables must be explicitly cast to be mutable" in Warp kernels.The Ruff linter warning about redundancy is technically correct from a static analysis perspective, but the explicit casts are not redundant in this context—they are functionally necessary for Warp's kernel compilation. The code is correct as written.
newton/examples/cable/example_cable_pile.py (3)
34-36: LGTM - Module-level toggle for unified collision pipeline.The flag provides a clean way to switch between collision pipeline implementations for testing and comparison purposes.
253-260: LGTM - Conditional collision pipeline initialization.The logic correctly creates and uses the unified collision pipeline when enabled, falling back to the standard pipeline otherwise. The pattern aligns with the
create_collision_pipelinehelper fromnewton.examples.
284-287: LGTM - Consistent collision pipeline usage in simulation loop.The conditional collision detection correctly mirrors the initialization logic, ensuring the same pipeline is used throughout the simulation.
newton/_src/geometry/kernels.py (3)
274-288: LGTM - Robust ellipsoid SDF computation.The radii clamping and fallback handling for near-center points are well-designed for numerical stability.
291-314: LGTM - Ellipsoid SDF gradient with appropriate fallbacks.The gradient computation correctly derives from the SDF approximation formula. The fallback to
(0, 0, 1)for degenerate cases (near center or zero gradient) is a reasonable choice that prevents NaN propagation.
820-822: LGTM - Ellipsoid case now uses analytic gradient.This correctly integrates the new
ellipsoid_sdf_gradfunction for soft contact normal computation, replacing any prior generic fallback.newton/_src/geometry/collision_primitive.py (3)
209-230: LGTM - Multi-contact return signature for capsule-capsule collision.The updated return type correctly supports up to 2 contacts with
wp.infmarking unused slots. The initialization is appropriate.
246-272: LGTM - Single contact generation for non-parallel capsules.The non-parallel case correctly finds the closest points using the determinant-based approach and stores the single contact in the first slot.
274-293: Well-designed parallel capsule handling with two endpoint contacts.For parallel capsules, generating contacts at both ends of capsule 1 provides better stability for cable-like simulations where capsules often align. Using the first contact's normal for both contacts is a reasonable simplification since parallel capsules share the same contact direction.
docs/concepts/collisions.rst (1)
639-639: LGTM - Clear documentation of pipeline buffer sizing differences.The clarification accurately describes how the standard pipeline uses
rigid_contact_max_per_pairwhile the unified pipeline automatically estimates buffer sizes based on the model configuration.newton/tests/test_rigid_contact.py (1)
491-496: Goodrigid_contact_maxmigration; verify500is comfortably above worst-case.This aligns with the API change; just ensure
500won’t truncate contacts on different backends / mesh tessellations (which could make the “at rest” assertions flaky).newton/tests/test_collision_primitives.py (1)
185-213: Kernel signature update matches the multi-contact capsule–capsule API.
distances: wp.vec2andcontact_positions: matrix((2,3))align with returning up to 2 contacts, and the assignments look correct.newton/_src/geometry/narrow_phase.py (2)
1395-1416: Verify the tiled launch dimensions fornarrow_phase_find_mesh_triangle_overlaps_kernelwhenENABLE_TILE_BVH_QUERYis False.You switch
second_dimto1but keepblock_dim=self.tile_size_mesh_convex. If this kernel relies on Warp’s “tiled”(block_id, lane)semantics (tid, j = wp.tid()), double-check the(dim, block_dim)combination is valid across Warp versions/configs. Based on learnings, tiled kernels typically wantdim=(num_blocks, tile_size)withblock_dim == tile_size.
581-597: Bounding-sphere early rejection: please validate no false negatives.The new
compute_bounding_sphere_from_aabb+check_infinite_plane_bsphere_overlap(...)gate is a nice perf win, but it’s easy to accidentally cull true contacts (especially with planes/SDF fallbacks). This likely needs targeted tests (or at least an invariant/assert path in debug) to ensure correctness.newton/_src/sim/collide_unified.py (6)
304-346: LGTM!The
has_meshesparameter is well-designed with a safe default ofTrueand clear documentation explaining its purpose as an optimization flag for skipping mesh-related kernel launches.
384-400: LGTM!The contact buffer estimation heuristics are well-reasoned:
- EXPLICIT mode leverages known pair count with a 10x multiplier for mesh contacts
- Dynamic modes (NXN/SAP) use a per-shape estimate that scales linearly rather than quadratically
- The floor of 1000 contacts prevents under-allocation for small scenes
The comment appropriately guides users toward
model.rigid_contact_maxfor memory-efficient configurations.
413-423: LGTM!The
has_meshesflag is correctly propagated toNarrowPhase, enabling the mesh kernel skip optimization.
477-479: LGTM!The fallback logic correctly chains: user-provided value →
model.rigid_contact_max→ heuristic estimation in__init__. Ifmodel.rigid_contact_maxisNone, it propagates to__init__where the heuristics kick in.
497-502: LGTM!The mesh detection logic is well-implemented:
- Defensive checks for attribute existence
- Efficient numpy-based comparison
- Correctly handles empty arrays (
.any()returnsFalse)- The
.numpy()call is acceptable here since it runs only once during pipeline construction
503-522: LGTM!The constructor call correctly passes all parameters including the auto-detected
has_meshesflag.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
newton/examples/cable/example_cable_pile.py (1)
116-117: Register this example inREADME.mdwith auv runcommand and screenshot.The example file is properly structured with an
Exampleclass and implements the requiredtest_final()method for state validation. However, it is not registered inREADME.mdas required by the coding guidelines fornewton/examples/**/*.pyfiles.newton/_src/geometry/kernels.py (1)
269-315: Fix ellipsoid gradient fallback: returning +Z can produce contact points not on the ellipsoid surface.
When the ellipsoid's minimum radius is not along the Z-axis,ellipsoid_sdf_grad()returning(0,0,1)in fallback paths causes the reconstructed contact pointx_local - n*dto lie inside the ellipsoid instead of on its surface. The SDF and gradient fallbacks must agree on the same axis (the minimum-radius axis) to ensure correct contact point/normal computation.Proposed fix (fallback to min-radius axis, signed by p)
`@wp.func` def ellipsoid_sdf_grad(radii: wp.vec3, p: wp.vec3): @@ - if k1 < eps: - return wp.vec3(0.0, 0.0, 1.0) + if k1 < eps: + # Gradient is undefined near the center; pick the minimum-radius axis so that + # the reconstructed surface point `p - n*d` lies on the ellipsoid for the + # corresponding deep-inside fallback distance. + if r[0] <= r[1] and r[0] <= r[2]: + s = wp.sign(p[0]) + if wp.abs(s) < eps: + s = 1.0 + return wp.vec3(s, 0.0, 0.0) + if r[1] <= r[2]: + s = wp.sign(p[1]) + if wp.abs(s) < eps: + s = 1.0 + return wp.vec3(0.0, s, 0.0) + s = wp.sign(p[2]) + if wp.abs(s) < eps: + s = 1.0 + return wp.vec3(0.0, 0.0, s) @@ - if grad_len > eps: - return grad / grad_len - return wp.vec3(0.0, 0.0, 1.0) + if grad_len > eps: + return grad / grad_len + # Same fallback as above (min-radius axis) + if r[0] <= r[1] and r[0] <= r[2]: + return wp.vec3(1.0, 0.0, 0.0) + if r[1] <= r[2]: + return wp.vec3(0.0, 1.0, 0.0) + return wp.vec3(0.0, 0.0, 1.0)
🤖 Fix all issues with AI agents
In `@newton/examples/cable/example_cable_pile.py`:
- Around line 150-152: The global default rigid contact margin set via
builder.rigid_contact_margin = 0.05 is too large for 1.2cm-radius cables and
inflates broadphase AABBs (margins are summed per pair); change the default to a
small fraction of the cable radius (e.g., O(0.01–0.1) * cable_radius) by
computing that fraction from the cable_radius variable instead of the hardcoded
0.05, and only override builder.rigid_contact_margin with larger values for
specific shapes/contacts where you intentionally need extra tolerance.
In `@newton/tests/test_collision_pipeline.py`:
- Around line 431-437: The test test_mesh_mesh_sdf_vs_sdf currently calls
test_mesh_mesh_sdf_modes with a very permissive tolerance=0.1; instead, keep
strict velocity/component tolerances and address only the known flakiness by
relaxing or removing assertions that depend on the exact collision normal
direction inside test_mesh_mesh_sdf_modes (e.g., per-component
velocity/normal-sign comparisons). Modify test_mesh_mesh_sdf_vs_sdf to pass a
tighter tolerance (or default) and update test_mesh_mesh_sdf_modes to: (1) avoid
sign-sensitive assertions on contact normals, (2) assert on invariant quantities
such as speed/magnitude, kinetic energy, or bounds on normal-aligned velocity
magnitudes, and (3) optionally allow an explicit flag/parameter from
test_mesh_mesh_sdf_vs_sdf to enable the relaxed normal checks when SDF↔SDF
behavior is expected.
🧹 Nitpick comments (5)
newton/_src/geometry/contact_data.py (1)
78-96: Consider whether re-normalization is necessary.Line 83 re-normalizes
contact_normal_a_to_b, but according to theContactDatadocstring (line 36), this should already be a unit normal vector. While defensive normalization is safe, it adds computational overhead.Additionally, the distance calculation could be simplified. The current approach:
- Computes
a_contact_worldandb_contact_worldfromcontact_point_center- Computes
diffbetween them- Projects
diffonto the normalThis is equivalent to
distance = contact_data.contact_distance + contact_data.radius_eff_a + contact_data.radius_eff_b, which would avoid the redundant computation.♻️ Simplified implementation (optional)
`@wp.func` def contact_passes_margin_check( contact_data: ContactData, ) -> bool: - total_separation_needed = ( - contact_data.radius_eff_a + contact_data.radius_eff_b + contact_data.thickness_a + contact_data.thickness_b - ) - - # Distance calculation matching box_plane_collision - contact_normal_a_to_b = wp.normalize(contact_data.contact_normal_a_to_b) - - a_contact_world = contact_data.contact_point_center - contact_normal_a_to_b * ( - 0.5 * contact_data.contact_distance + contact_data.radius_eff_a - ) - b_contact_world = contact_data.contact_point_center + contact_normal_a_to_b * ( - 0.5 * contact_data.contact_distance + contact_data.radius_eff_b - ) - - diff = b_contact_world - a_contact_world - distance = wp.dot(diff, contact_normal_a_to_b) - d = distance - total_separation_needed + # d = surface_distance - total_thickness + # where surface_distance = contact_distance + radius_eff_a + radius_eff_b + d = contact_data.contact_distance - (contact_data.thickness_a + contact_data.thickness_b) return d <= contact_data.marginnewton/examples/cable/example_cable_pile.py (2)
34-36: Prefer driving unified/standard selection viaargs(avoid module-levelUSE_UNIFIED_COLLISION).
Right now the example has two sources of truth:USE_UNIFIED_COLLISIONandnewton.examples.create_collision_pipeline(self.model, args)(which already reads args and can returnNonefor"standard"). Consider removing the global flag and letting CLI/config decide consistently.Proposed simplification
-# Global flag to enable/disable unified collision pipeline -USE_UNIFIED_COLLISION = True +USE_UNIFIED_COLLISION = None # Optional: keep only if you *must* force behavior in-code- # Create collision pipeline (unified if enabled, otherwise standard) - if USE_UNIFIED_COLLISION: - self.collision_pipeline = newton.examples.create_collision_pipeline(self.model, args) - self.contacts = self.model.collide(self.state_0, collision_pipeline=self.collision_pipeline) - else: - self.collision_pipeline = None - self.contacts = self.model.collide(self.state_0) + # Create collision pipeline from args (None => standard pipeline) + self.collision_pipeline = newton.examples.create_collision_pipeline(self.model, args) + self.contacts = self.model.collide(self.state_0, collision_pipeline=self.collision_pipeline)
283-288: Re-check intent: colliding every substep vs once-per-frame reuse.
This example callsmodel.collide()inside the substep loop. If the goal is to showcase unified-pipeline perf, consider computing contacts once per rendered frame and reusing for all substeps (common pattern in other Newton examples), unless this scene needs per-substep contact refresh for correctness. Based on learnings, examples often intentionally reuse contacts per frame—worth confirming here.newton/tests/test_rigid_contact.py (1)
490-496: Consider derivingrigid_contact_maxfrom geometry (avoid brittle magic number).
If this test needs a larger buffer, it may be more robust to base it on the mesh’s vertex count / expected contact upper bound (or setmodel.rigid_contact_maxonce) so future mesh/contact changes don’t silently require tweaking500.newton/tests/test_collision_primitives.py (1)
817-905: Test currently validates only the first capsule-capsule contact; add coverage for the second contact in the parallel case.
For parallel axes, the implementation returns 2 contacts—this test should assert that the second contact is finite and meaningfully different (e.g., positions differ along the capsule axis) to prevent regressions where contact#1silently breaks.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
asv/benchmarks/collision/benchmark_contact_reduction.pyasv/benchmarks/collision/benchmark_hashtable.pydocs/concepts/collisions.rstnewton/_src/geometry/collision_primitive.pynewton/_src/geometry/contact_data.pynewton/_src/geometry/contact_reduction_global.pynewton/_src/geometry/kernels.pynewton/_src/geometry/narrow_phase.pynewton/_src/sim/collide_unified.pynewton/examples/__init__.pynewton/examples/basic/example_basic_shapes.pynewton/examples/cable/example_cable_pile.pynewton/examples/robot/example_robot_panda_hydro.pynewton/tests/test_collision_pipeline.pynewton/tests/test_collision_primitives.pynewton/tests/test_hydroelastic.pynewton/tests/test_rigid_contact.pynewton/tests/test_sdf_compute.py
💤 Files with no reviewable changes (6)
- newton/examples/robot/example_robot_panda_hydro.py
- newton/examples/init.py
- newton/examples/basic/example_basic_shapes.py
- asv/benchmarks/collision/benchmark_contact_reduction.py
- asv/benchmarks/collision/benchmark_hashtable.py
- newton/tests/test_sdf_compute.py
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
CLI arguments must use
kebab-case(e.g.,--use-cuda-graph, not--use_cuda_graph)
Files:
docs/concepts/collisions.rstnewton/tests/test_rigid_contact.pynewton/_src/geometry/contact_reduction_global.pynewton/_src/geometry/collision_primitive.pynewton/_src/geometry/kernels.pynewton/tests/test_collision_pipeline.pynewton/tests/test_collision_primitives.pynewton/examples/cable/example_cable_pile.pynewton/tests/test_hydroelastic.pynewton/_src/geometry/contact_data.pynewton/_src/geometry/narrow_phase.pynewton/_src/sim/collide_unified.py
newton/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Any user-facing class/function/object added under
_srcmust be exposed via the public Newton API through re-exports
Files:
newton/tests/test_rigid_contact.pynewton/_src/geometry/contact_reduction_global.pynewton/_src/geometry/collision_primitive.pynewton/_src/geometry/kernels.pynewton/tests/test_collision_pipeline.pynewton/tests/test_collision_primitives.pynewton/examples/cable/example_cable_pile.pynewton/tests/test_hydroelastic.pynewton/_src/geometry/contact_data.pynewton/_src/geometry/narrow_phase.pynewton/_src/sim/collide_unified.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use prefix-first naming for classes:ActuatorPD,ActuatorPID(notPDActuator,PIDActuator)
Use prefix-first naming for methods:add_shape_sphere()(notadd_sphere_shape())
Method names must usesnake_case
Prefer nested classes when self-contained: if a helper type or enum is only meaningful inside one parent class and doesn't need a public identity, define it as a nested class instead of a top-level class/module
Follow PEP 8 for Python code
Use Google-style docstrings with clear, concise explanations of what functions do, their parameters, and return values
Files:
newton/tests/test_rigid_contact.pynewton/_src/geometry/contact_reduction_global.pynewton/_src/geometry/collision_primitive.pynewton/_src/geometry/kernels.pynewton/tests/test_collision_pipeline.pynewton/tests/test_collision_primitives.pynewton/examples/cable/example_cable_pile.pynewton/tests/test_hydroelastic.pynewton/_src/geometry/contact_data.pynewton/_src/geometry/narrow_phase.pynewton/_src/sim/collide_unified.py
newton/_src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
The
newton/_src/directory is internal library implementation only and must not be imported by user code (examples, documentation, or external users)
Files:
newton/_src/geometry/contact_reduction_global.pynewton/_src/geometry/collision_primitive.pynewton/_src/geometry/kernels.pynewton/_src/geometry/contact_data.pynewton/_src/geometry/narrow_phase.pynewton/_src/sim/collide_unified.py
newton/examples/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
newton/examples/**/*.py: Examples must not import fromnewton._src, only from the public Newton API
Examples must follow the format with anExampleclass and must be registered inREADME.mdwith auv runcommand and screenshot
Examples must implement atest_final()method that verifies the simulation state is valid after the example has run
Examples may optionally implement atest_post_step()method that is evaluated after everystep()call
Files:
newton/examples/cable/example_cable_pile.py
🧠 Learnings (16)
📚 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:
docs/concepts/collisions.rstnewton/tests/test_rigid_contact.pynewton/_src/geometry/collision_primitive.pynewton/tests/test_collision_primitives.pynewton/_src/geometry/contact_data.py
📚 Learning: 2025-12-25T21:44:17.047Z
Learnt from: camevor
Repo: newton-physics/newton PR: 1161
File: docs/concepts/sensors.rst:12-12
Timestamp: 2025-12-25T21:44:17.047Z
Learning: Future API standardization: The sensor.update() method should be standardized to a single-argument form update(state) across all sensor types. The redundant model parameter will be removed from sensors like SensorFrameTransform. In code reviews, check for API changes that move toward a single-argument update, ensure all sensor implementations conform, and update user-facing docs to reflect the change. If reviewing related code, verify that calls using the old two-argument form are refactored and that any dependent serialization or tests are updated accordingly.
Applied to files:
docs/concepts/collisions.rst
📚 Learning: 2025-10-10T10:47:41.082Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 899
File: newton/tests/test_rigid_contact.py:214-268
Timestamp: 2025-10-10T10:47:41.082Z
Learning: In `newton/tests/test_rigid_contact.py`, using `add_shape_plane(width=0, length=0)` for an infinite plane works correctly with the collision system and does not cause degenerate AABB issues in the test scenario `test_shape_collisions_gjk_mpr_multicontact`.
Applied to files:
newton/tests/test_rigid_contact.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/tests/test_rigid_contact.pynewton/_src/geometry/contact_reduction_global.pynewton/tests/test_hydroelastic.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/tests/test_rigid_contact.pynewton/_src/geometry/contact_reduction_global.pynewton/tests/test_collision_primitives.pynewton/_src/geometry/narrow_phase.pynewton/_src/sim/collide_unified.py
📚 Learning: 2026-01-12T10:11:47.988Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py:6061-6091
Timestamp: 2026-01-12T10:11:47.988Z
Learning: Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py (ModelBuilder.finalize)
Learning: The project prefers to keep finalize() validation lightweight; do not add union-find/connected-components validation for loop joints. Current behavior (allow non-articulated joints when the child is in any articulation) is acceptable; avoid overengineering here.
Applied to files:
newton/tests/test_rigid_contact.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/tests/test_rigid_contact.pynewton/_src/geometry/narrow_phase.py
📚 Learning: 2026-01-13T03:11:40.556Z
Learnt from: jumyungc
Repo: newton-physics/newton PR: 1333
File: newton/_src/sim/builder.py:0-0
Timestamp: 2026-01-13T03:11:40.556Z
Learning: Ensure all VBD damping semantics follow Rayleigh-style damping and are unitless. This convention, demonstrated in SolverVBD, should be applied consistently across all VBD-related constraints in the codebase. In newton/_src/sim/builder.py, validate that any damping terms use the same normalization and are treated as dimensionless; add comments and unit tests to enforce consistency across VBD components.
Applied to files:
newton/tests/test_rigid_contact.pynewton/_src/geometry/contact_reduction_global.pynewton/_src/geometry/collision_primitive.pynewton/_src/geometry/kernels.pynewton/tests/test_collision_pipeline.pynewton/tests/test_collision_primitives.pynewton/examples/cable/example_cable_pile.pynewton/tests/test_hydroelastic.pynewton/_src/geometry/contact_data.pynewton/_src/geometry/narrow_phase.pynewton/_src/sim/collide_unified.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/contact_reduction_global.pynewton/tests/test_collision_primitives.pynewton/_src/geometry/narrow_phase.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_global.pynewton/_src/geometry/collision_primitive.pynewton/examples/cable/example_cable_pile.pynewton/_src/geometry/narrow_phase.pynewton/_src/sim/collide_unified.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.pynewton/_src/sim/collide_unified.py
📚 Learning: 2025-08-20T03:30:16.037Z
Learnt from: eric-heiden
Repo: newton-physics/newton PR: 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.
Applied to files:
newton/_src/geometry/collision_primitive.pynewton/tests/test_collision_primitives.pynewton/examples/cable/example_cable_pile.py
📚 Learning: 2025-10-29T14:55:48.171Z
Learnt from: gdaviet
Repo: newton-physics/newton PR: 940
File: newton/examples/mpm/example_mpm_granular.py:92-101
Timestamp: 2025-10-29T14:55:48.171Z
Learning: In newton/examples/mpm/example_mpm_granular.py, the capture() method intentionally advances the simulation state once during CUDA graph capture. This is expected behavior because CUDA graph capture records exact memory addresses, so the state arrays (state_0, state_1) cannot be reset or reallocated after capture without invalidating the graph.
Applied to files:
newton/examples/cable/example_cable_pile.py
📚 Learning: 2025-12-12T08:45:43.428Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1221
File: newton/examples/example_sdf.py:277-287
Timestamp: 2025-12-12T08:45:43.428Z
Learning: In Newtown (Newton) example code, specifically files under newton/examples, computing contacts once per frame and reusing them across all substeps is an intentional design choice, not a bug. Reviewers should verify that contacts are computed before the substep loop and reused for every substep within the same frame. This pattern reduces redundant work and preserves frame-consistency; do not flag as a regression unless the behavior is changed for correctness or performance reasons.
Applied to files:
newton/examples/cable/example_cable_pile.py
📚 Learning: 2025-09-10T13:03:58.591Z
Learnt from: gdaviet
Repo: newton-physics/newton PR: 750
File: newton/_src/solvers/implicit_mpm/solve_rheology.py:799-804
Timestamp: 2025-09-10T13:03:58.591Z
Learning: In Warp's tiled launch API, the launch dimension should include the tile size as the last dimension, matching the CUDA block dimension. For tiled operations, use dim=(num_blocks, tile_size) where tile_size equals block_dim, not just dim=num_blocks like regular kernel launches.
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 (6)
newton/tests/test_rigid_contact.py (2)
newton/_src/sim/collide_unified.py (2)
CollisionPipelineUnified(273-715)from_model(439-524)newton/_src/sim/collide.py (1)
from_model(146-190)
newton/_src/geometry/collision_primitive.py (1)
newton/_src/sim/builder.py (1)
dot(5249-5250)
newton/tests/test_collision_pipeline.py (1)
newton/_src/sim/collide.py (1)
device(334-341)
newton/tests/test_collision_primitives.py (1)
newton/_src/geometry/collision_primitive.py (1)
collide_capsule_capsule(199-293)
newton/examples/cable/example_cable_pile.py (3)
newton/examples/__init__.py (1)
create_collision_pipeline(398-467)newton/_src/sim/collide_unified.py (1)
collide(526-705)newton/_src/sim/model.py (1)
collide(658-719)
newton/_src/sim/collide_unified.py (2)
newton/_src/sim/builder.py (1)
shape_count(1135-1139)newton/_src/geometry/types.py (1)
GeoType(25-67)
🪛 Ruff (0.14.11)
newton/_src/geometry/contact_reduction_global.py
716-716: 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 Tests / Start self-hosted EC2 runner
🔇 Additional comments (23)
docs/concepts/collisions.rst (1)
639-639: Documentation update accurately reflects API changes.The updated description correctly distinguishes between the two pipelines:
- Standard pipeline: uses
rigid_contact_max_per_pairviaModel.collide- Unified pipeline: automatically estimates buffer sizes based on the model
This aligns with the broader PR changes that remove
rigid_contact_max_per_pairfrom the unified pipeline's public API.newton/_src/geometry/contact_reduction_global.py (3)
558-560: Good early-exit optimization.This prevents unnecessary work when there are no contacts to process, which is a valid fast path for empty collision scenarios.
688-690: Good early-exit optimization for empty hashtables.This avoids iterating over the grid stride loop when there are no active entries to export.
712-721: Duplicate check optimization with while-loop is correct.The while-loop with early exit on duplicate detection is more efficient than previous approaches, avoiding unnecessary checks when a duplicate is found early. The explicit type declarations (
bool(False)andint(0)) on lines 716-717 are indeed required by Warp for variables mutated in while loops within kernel code, so the static analysis warning can be safely ignored.newton/_src/geometry/collision_primitive.py (2)
209-230: Multi-contact return type change is well-structured.The updated signature correctly returns:
contact_dist(vec2): distances for up to 2 contacts, withwp.inffor invalid slotscontact_pos(mat23f): 2x3 matrix with contact positionscontact_normal(vec3): shared normal vectorThe initialization with
wp.inffor distances follows the convention documented at line 36-37.
274-292: Parallel-axis case correctly generates two contacts.The parallel case logic appropriately generates contacts at both endpoints of capsule 1, projecting each to the nearest point on capsule 2. Using the first contact's normal for both (line 283) is a reasonable simplification since parallel capsules share the same separation direction.
However, there's a potential edge case: if
mc(line 278, 287) is very small (degenerate capsule with near-zero half-length), the division(v - mb) / mcand(v + mb) / mccould produce very large or undefined values before clamping.Consider whether degenerate capsules (near-zero half-length) need explicit handling, or if upstream code guarantees valid capsule dimensions.
newton/tests/test_hydroelastic.py (1)
132-140: Test correctly updated for the new API.The parameter rename from
rigid_contact_max_per_pairtorigid_contact_maxaligns with the broader API changes in this PR. The conditional buffer sizing (6000 for unreduced contacts vs 100 for reduced) appropriately accounts for the significantly higher contact count when contact reduction is disabled.newton/_src/geometry/kernels.py (1)
820-823: Ellipsoid soft-contacts wiring looks good (once gradient fallback is fixed).
Nice to seeGeoType.ELLIPSOIDhandled in the same SDF/grad path as other primitives.newton/tests/test_collision_primitives.py (1)
185-213: Good update for the newcollide_capsule_capsule()return shape. Matrix types likewp.array(dtype=wp.types.matrix((2, 3), wp.float32))andwp.array(dtype=wp.vec2)are supported across the codebase in the pinned nightly Warp version (≥1.11.0.dev20251205) and already used in multiple collision and solver modules—no additional verification needed.newton/_src/geometry/narrow_phase.py (9)
132-146: Well-structured kernel factory with clear documentation.The docstring clearly explains the kernel's purpose and the routing strategy for different shape types. The two-stage approach (analytical primitives first, then GJK/MPR for complex shapes) is a sound optimization.
186-211: Early exit and safety checks are well-implemented.The strided loop pattern with safety checks for self-collision and invalid indices is correct. The type sorting ensures consistent collision pair ordering (type_a ≤ type_b), which simplifies downstream collision handling.
257-269: Mesh-plane pair ordering is intentionally reversed.The code stores
wp.vec2i(shape_b, shape_a)(mesh, plane) rather than the sorted order. This is intentional to match the mesh-plane kernel's expectations where the first element is the mesh shape. The comment on line 265 clarifies this.
288-296: Effective radii correctly limited to sphere/capsule shapes.The Minkowski sphere decomposition (effective radius) is only applied to spheres and capsules. For other primitives like boxes and cylinders, the effective radius remains 0, which is correct since they don't have a uniform rounding radius.
436-454: Efficient batch contact allocation.Using a single atomic add for multiple valid contacts and then writing sequentially is more efficient than separate atomic operations per contact. The reuse of
contact_datastruct between writes is safe since operations are sequential within the same thread.
466-500: Clear documentation of kernel contract.The docstring explicitly states the preconditions (pairs pre-sorted, mesh/hydroelastic/primitives already handled), which helps maintain correctness when modifying the primitive kernel.
1337-1359: Two-stage kernel launch correctly chains outputs to inputs.The primitive kernel writes to
gjk_candidate_pairsandgjk_candidate_pairs_count, which are then used as inputs to the GJK/MPR kernel. This clean handoff between stages is well-structured.
1361-1363: Mesh kernel skip optimization improves primitive-only scene performance.Skipping all mesh-related kernel launches when
has_meshes=Falseeliminates unnecessary kernel launch overhead for scenes with only primitive shapes. This aligns with the PR objective of performance improvements.
1209-1224: Consolidated counter array reduces kernel launch overhead.Using sliced views into a single counter array allows clearing all counters with a single
zero_()call instead of multiple separate calls. The sliced views work correctly with atomic operations in kernels, as evidenced by their use throughout the primitive and GJK collision detection kernels.newton/_src/sim/collide_unified.py (5)
384-400: Simplified contact estimation improves API usability.The new estimation logic (
shape_count * 20for dynamic broad phase,len(shape_pairs_filtered) * 10for explicit pairs) is simpler and avoids the confusion aroundrigid_contact_max_per_pair. The minimum of 1000 contacts prevents underallocation for small scenes.
497-501: Mesh detection via numpy conversion is acceptable for initialization.The
numpy()call triggers a device-to-host copy, but since this runs once during pipeline construction (not per-frame), the overhead is negligible. A device-side check would add complexity without meaningful performance benefit.
413-423: has_meshes flag correctly propagated to NarrowPhase.The mesh detection result from
from_modelflows through to theNarrowPhaseconstructor, enabling the kernel skip optimization for primitive-only scenes.
315-319: Clear documentation for new parameters.The docstrings explain the
rigid_contact_maxestimation strategy and thehas_meshesoptimization, helping users understand when to provide explicit values vs. relying on defaults.Also applies to: 344-346
438-453: API simplification: rigid_contact_max_per_pair removed.The parameter rename from
rigid_contact_max_per_pairtorigid_contact_maxsimplifies the API. This is a breaking change but intentional per the PR discussion—the per-pair factor was confusing and led to overallocation.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
adenzler-nvidia
left a comment
adenzler-nvidia
left a comment
There was a problem hiding this comment.
some comments, hard to judge this. Will have another look tomorrow morning with a fresh head.
adenzler-nvidia
left a comment
adenzler-nvidia
left a comment
There was a problem hiding this comment.
Thanks! some more comments
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/_src/geometry/narrow_phase.py (1)
675-682: Bug:returnshould becontinueinside strided loop.Inside the strided loop (line 657), using
returnexits the entire kernel rather than moving to the next iteration. If a mesh-mesh pair or null mesh_id is encountered, subsequent pairs assigned to this thread won't be processed.🐛 Proposed fix
elif type_b == int(GeoType.MESH) and type_a != int(GeoType.MESH): mesh_shape = shape_b non_mesh_shape = shape_a else: # Mesh-mesh collision not supported yet - return + continue # Get mesh BVH ID and mesh transform mesh_id = shape_source[mesh_shape] if mesh_id == wp.uint64(0): - return + continue
🧹 Nitpick comments (1)
newton/_src/geometry/narrow_phase.py (1)
521-523: Safety checks are defensive but redundant.These validity checks duplicate the primitive kernel's filtering (line 200-201). While safe to keep for robustness, they add minor overhead. Consider documenting that these are intentional defensive checks since pairs should already be validated.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
newton/_src/geometry/narrow_phase.py
🧰 Additional context used
📓 Path-based instructions (4)
newton/_src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
The
newton/_src/directory is internal library implementation only and must not be imported by user code (examples, documentation, or external users)
Files:
newton/_src/geometry/narrow_phase.py
newton/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Any user-facing class/function/object added under
_srcmust be exposed via the public Newton API through re-exports
Files:
newton/_src/geometry/narrow_phase.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use prefix-first naming for classes:ActuatorPD,ActuatorPID(notPDActuator,PIDActuator)
Use prefix-first naming for methods:add_shape_sphere()(notadd_sphere_shape())
Method names must usesnake_case
Prefer nested classes when self-contained: if a helper type or enum is only meaningful inside one parent class and doesn't need a public identity, define it as a nested class instead of a top-level class/module
Follow PEP 8 for Python code
Use Google-style docstrings with clear, concise explanations of what functions do, their parameters, and return values
Files:
newton/_src/geometry/narrow_phase.py
**/*
📄 CodeRabbit inference engine (AGENTS.md)
CLI arguments must use
kebab-case(e.g.,--use-cuda-graph, not--use_cuda_graph)
Files:
newton/_src/geometry/narrow_phase.py
🧠 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-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-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/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 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/narrow_phase.py
📚 Learning: 2025-08-20T18:02:36.099Z
Learnt from: eric-heiden
Repo: newton-physics/newton PR: 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.
Applied to files:
newton/_src/geometry/narrow_phase.py
📚 Learning: 2025-10-10T10:47:41.082Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 899
File: newton/tests/test_rigid_contact.py:214-268
Timestamp: 2025-10-10T10:47:41.082Z
Learning: In `newton/tests/test_rigid_contact.py`, using `add_shape_plane(width=0, length=0)` for an infinite plane works correctly with the collision system and does not cause degenerate AABB issues in the test scenario `test_shape_collisions_gjk_mpr_multicontact`.
Applied to files:
newton/_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/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-09-10T13:03:58.591Z
Learnt from: gdaviet
Repo: newton-physics/newton PR: 750
File: newton/_src/solvers/implicit_mpm/solve_rheology.py:799-804
Timestamp: 2025-09-10T13:03:58.591Z
Learning: In Warp's tiled launch API, the launch dimension should include the tile size as the last dimension, matching the CUDA block dimension. For tiled operations, use dim=(num_blocks, tile_size) where tile_size equals block_dim, not just dim=num_blocks like regular kernel launches.
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: 2026-01-13T03:11:40.556Z
Learnt from: jumyungc
Repo: newton-physics/newton PR: 1333
File: newton/_src/sim/builder.py:0-0
Timestamp: 2026-01-13T03:11:40.556Z
Learning: Ensure all VBD damping semantics follow Rayleigh-style damping and are unitless. This convention, demonstrated in SolverVBD, should be applied consistently across all VBD-related constraints in the codebase. In newton/_src/sim/builder.py, validate that any damping terms use the same normalization and are treated as dimensionless; add comments and unit tests to enforce consistency across VBD components.
Applied to files:
newton/_src/geometry/narrow_phase.py
🧬 Code graph analysis (1)
newton/_src/geometry/narrow_phase.py (9)
newton/_src/geometry/collision_core.py (2)
check_infinite_plane_bsphere_overlap(504-556)compute_bounding_sphere_from_aabb(428-438)newton/_src/geometry/collision_primitive.py (7)
collide_capsule_capsule(199-293)collide_plane_capsule(297-348)collide_plane_sphere(119-125)collide_sphere_box(1105-1158)collide_sphere_capsule(162-195)collide_sphere_cylinder(443-512)collide_sphere_sphere(129-158)newton/_src/geometry/contact_data.py (1)
ContactData(26-62)newton/_src/geometry/types.py (2)
GeoType(25-67)SDF(74-113)newton/_src/geometry/support_function.py (1)
extract_shape_data(305-347)newton/_src/sim/contacts.py (1)
device(110-114)newton/_src/sim/collide.py (1)
device(334-341)newton/_src/geometry/broad_phase_nxn.py (2)
launch(295-361)launch(378-435)newton/_src/geometry/broad_phase_sap.py (1)
launch(506-650)
⏰ 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). (1)
- GitHub Check: Run GPU Benchmarks / Run GPU Benchmarks on AWS EC2
🔇 Additional comments (8)
newton/_src/geometry/narrow_phase.py (8)
23-64: LGTM!The new imports for collision primitives (
collide_capsule_capsule,collide_plane_sphere, etc.) and utilities (check_infinite_plane_bsphere_overlap,compute_bounding_sphere_from_aabb,contact_passes_margin_check) are well-organized and align with the two-stage primitive-first pipeline design.
78-130: LGTM!The dual-path write logic is correct:
output_index < 0triggers atomic allocation with margin and bounds checks, whileoutput_index >= 0trusts the caller's pre-validated index. The primitive kernel properly validates indices before passing them, making this design safe.
1216-1231: Good optimization: consolidated counter array with sliced views.Using a single array with sliced views (
self._counter_array[0:1], etc.) minimizes kernel launches for zeroing and ensures all counters are cleared atomically with_counter_array.zero_(). This is a clean pattern for managing multiple related counters.
1535-1616: LGTM!The public
launchmethod correctly wrapslaunch_custom_writewith the standardContactWriterDatastruct. The comment at line 1589 properly notes that internal counters are cleared inlaunch_custom_write.
278-408: LGTM!The primitive collision handlers are correctly designed with the type sorting invariant (type_a ≤ type_b). This ensures:
- Planes (type 0) are always shape_a
- Spheres/capsules can be either position
- Cylinders/boxes are always shape_b when paired with smaller types
- Other combinations (box-box, etc.) are routed to GJK/MPR
The multi-contact support for plane-capsule and capsule-capsule pairs is well implemented.
436-460: Bounds checking correctly implemented per prior review.The atomic allocation with bounds checking and rollback (lines 439-445) properly prevents out-of-bounds writes. The margin pre-check before allocation is efficient.
1401-1422: The kernel launch dimensions and parameters are correct. The 2D launch withdim=[num_tile_blocks, tile_size_mesh_convex]is the proper pattern for tiled kernels in Warp. Thetidunpacks to(block_index, lane), whereblock_indexdrives the strided loop across mesh pairs (stride =num_tile_blocks), andlaneis passed tomesh_vs_convex_midphase()for tiled BVH query operations. Usingnum_tile_blocksastotal_num_threadsis correct for this distribution pattern.
376-383: Sentinel value pattern is documented and correctly used.The
dists[1] < wp.infcheck is properly implemented.collide_capsule_capsuleexplicitly documents in its docstring that contact distances usewp.infto mark invalid contacts (line 224 in collision_primitive.py). In the non-parallel case, onlydists[0]is set whiledists[1]remainswp.inf, and the check at line 378 correctly detects this. The pattern is consistent: plane-capsule always returns both contacts (so no sentinel check needed), while capsule-capsule conditionally returns one or two contacts using the documented sentinel pattern.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
# Conflicts: # newton/examples/cable/example_cable_pile.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@newton/_src/geometry/narrow_phase.py`:
- Around line 1401-1422: The wp.launch call for
narrow_phase_find_mesh_triangle_overlaps_kernel must use a 1D launch when
ENABLE_TILE_BVH_QUERY is False: wrap the launch in a conditional so that if
ENABLE_TILE_BVH_QUERY is True you keep dim=[self.num_tile_blocks, second_dim]
and block_dim=self.tile_size_mesh_convex, otherwise call wp.launch with
dim=self.num_tile_blocks and block_dim=1 (and pass the same inputs/outputs), so
the kernel’s wp.tid() unpacking (tid, j) and tiled launch requirements are
satisfied; update the launch site in
narrow_phase_find_mesh_triangle_overlaps_kernel invocation accordingly.
🧹 Nitpick comments (1)
newton/_src/geometry/narrow_phase.py (1)
1367-1369: Consider a safety check forhas_meshesto avoid silent contact loss.When
has_meshes=False, mesh kernels are skipped even if mesh pairs are produced by the primitive stage; those contacts are silently dropped. If this flag can be misconfigured by callers, consider a cheap guard (e.g., debug-only assert onshape_pairs_mesh_count[0] == 0/shape_pairs_mesh_plane_count[0] == 0after stage 1) or auto-detection fromshape_types.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
newton/_src/geometry/narrow_phase.py
🧰 Additional context used
📓 Path-based instructions (4)
newton/_src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
The
newton/_src/directory is internal library implementation only and must not be imported by user code (examples, documentation, or external users)
Files:
newton/_src/geometry/narrow_phase.py
newton/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Any user-facing class/function/object added under
_srcmust be exposed via the public Newton API through re-exports
Files:
newton/_src/geometry/narrow_phase.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use prefix-first naming for classes:ActuatorPD,ActuatorPID(notPDActuator,PIDActuator)
Use prefix-first naming for methods:add_shape_sphere()(notadd_sphere_shape())
Method names must usesnake_case
Prefer nested classes when self-contained: if a helper type or enum is only meaningful inside one parent class and doesn't need a public identity, define it as a nested class instead of a top-level class/module
Follow PEP 8 for Python code
Use Google-style docstrings with clear, concise explanations of what functions do, their parameters, and return values
Files:
newton/_src/geometry/narrow_phase.py
**/*
📄 CodeRabbit inference engine (AGENTS.md)
CLI arguments must use
kebab-case(e.g.,--use-cuda-graph, not--use_cuda_graph)
Files:
newton/_src/geometry/narrow_phase.py
🧠 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: 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.
📚 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-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/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 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/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-20T18:02:36.099Z
Learnt from: eric-heiden
Repo: newton-physics/newton PR: 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.
Applied to files:
newton/_src/geometry/narrow_phase.py
📚 Learning: 2025-10-10T10:47:41.082Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 899
File: newton/tests/test_rigid_contact.py:214-268
Timestamp: 2025-10-10T10:47:41.082Z
Learning: In `newton/tests/test_rigid_contact.py`, using `add_shape_plane(width=0, length=0)` for an infinite plane works correctly with the collision system and does not cause degenerate AABB issues in the test scenario `test_shape_collisions_gjk_mpr_multicontact`.
Applied to files:
newton/_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/narrow_phase.py
📚 Learning: 2025-09-10T13:03:58.591Z
Learnt from: gdaviet
Repo: newton-physics/newton PR: 750
File: newton/_src/solvers/implicit_mpm/solve_rheology.py:799-804
Timestamp: 2025-09-10T13:03:58.591Z
Learning: In Warp's tiled launch API, the launch dimension should include the tile size as the last dimension, matching the CUDA block dimension. For tiled operations, use dim=(num_blocks, tile_size) where tile_size equals block_dim, not just dim=num_blocks like regular kernel launches.
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: 2026-01-13T03:11:40.556Z
Learnt from: jumyungc
Repo: newton-physics/newton PR: 1333
File: newton/_src/sim/builder.py:0-0
Timestamp: 2026-01-13T03:11:40.556Z
Learning: Ensure all VBD damping semantics follow Rayleigh-style damping and are unitless. This convention, demonstrated in SolverVBD, should be applied consistently across all VBD-related constraints in the codebase. In newton/_src/sim/builder.py, validate that any damping terms use the same normalization and are treated as dimensionless; add comments and unit tests to enforce consistency across VBD components.
Applied to files:
newton/_src/geometry/narrow_phase.py
🧬 Code graph analysis (1)
newton/_src/geometry/narrow_phase.py (6)
newton/_src/geometry/collision_core.py (2)
check_infinite_plane_bsphere_overlap(504-556)compute_bounding_sphere_from_aabb(428-438)newton/_src/geometry/collision_primitive.py (2)
collide_capsule_capsule(199-293)collide_sphere_sphere(129-158)newton/_src/geometry/contact_data.py (1)
ContactData(26-62)newton/_src/geometry/types.py (2)
GeoType(25-67)SDF(74-113)newton/_src/geometry/support_function.py (1)
extract_shape_data(305-347)newton/_src/geometry/contact_reduction_global.py (4)
clear_active(351-385)get_data_struct(387-404)reduce_buffered_contacts_kernel(548-567)export_reduced_contacts_kernel(658-760)
⏰ 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). (4)
- GitHub Check: Run GPU Tests / Run GPU Unit Tests on AWS EC2
- GitHub Check: Run GPU Benchmarks / Run GPU Benchmarks on AWS EC2
- GitHub Check: run-newton-tests / newton-unittests (windows-latest)
- GitHub Check: run-newton-tests / newton-unittests (ubuntu-latest)
🔇 Additional comments (1)
newton/_src/geometry/narrow_phase.py (1)
212-219: The pattern is correct as-is and follows established Warp conventions for optional arrays. PassingNonefor optional arrays and checking withif array:inside kernels is valid and idiomatic in Warp (see prior work in PR 1047 and PR 818). Theshape_pairs_sdf_sdfarray can safely beNonewhen hydroelastic is disabled, and the truthiness check at line 215 is the proper way to guard its usage.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
The benchmarks seem to show the right trend: https://github.com/newton-physics/newton/actions/runs/21061587041/job/60569091244?pr=1375 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@newton/_src/sim/contacts.py`:
- Around line 108-138: The clear method's conservative path (clear_buffers=True)
doesn't actually clear per-contact geometry fields despite the docstring; update
clear to also reset the remaining per-contact buffers (e.g.,
rigid_contact_point, rigid_contact_normal, rigid_contact_thickness,
soft_contact_point, soft_contact_normal, soft_contact_thickness and any other
per-contact arrays not currently handled) using .fill_(-1) for index/sentinel
arrays and .zero_() for numeric arrays, and keep the existing resets for
rigid_contact_shape0, rigid_contact_shape1, rigid_contact_tids,
rigid_contact_force, rigid_contact_stiffness, rigid_contact_damping,
rigid_contact_friction, soft_contact_particle, soft_contact_shape,
soft_contact_tids; alternatively if you choose not to clear them, update the
clear docstring in the clear method to explicitly list which buffers are and are
not zeroed when clear_buffers=True.
♻️ Duplicate comments (1)
newton/_src/geometry/narrow_phase.py (1)
1410-1431: (Dup) Switch to 1D launch whenENABLE_TILE_BVH_QUERYis False.
This is the same Warp tiled-launch mismatch previously flagged:dim=[num_tile_blocks, 1]withblock_dim=tile_size_mesh_convexviolates the tiled dimension requirement when tiling is disabled.
🧹 Nitpick comments (1)
newton/_src/geometry/narrow_phase.py (1)
1258-1275: Clampnum_blockstomax_blocks_limitfor small GPUs.
Current formula can exceed the intended upper bound whensm_count * 4 < min_blocks, which undermines the cap and may overlaunch on smaller devices.♻️ Suggested refactor
- num_blocks = max(min_blocks, min(candidate_blocks, max_blocks_limit)) + num_blocks = min(max_blocks_limit, max(min_blocks, candidate_blocks))
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
newton/_src/geometry/narrow_phase.pynewton/_src/sim/contacts.py
🧰 Additional context used
📓 Path-based instructions (4)
newton/_src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
The
newton/_src/directory is internal library implementation only and must not be imported by user code (examples, documentation, or external users)
Files:
newton/_src/sim/contacts.pynewton/_src/geometry/narrow_phase.py
newton/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Any user-facing class/function/object added under
_srcmust be exposed via the public Newton API through re-exports
Files:
newton/_src/sim/contacts.pynewton/_src/geometry/narrow_phase.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use prefix-first naming for classes:ActuatorPD,ActuatorPID(notPDActuator,PIDActuator)
Use prefix-first naming for methods:add_shape_sphere()(notadd_sphere_shape())
Method names must usesnake_case
Prefer nested classes when self-contained: if a helper type or enum is only meaningful inside one parent class and doesn't need a public identity, define it as a nested class instead of a top-level class/module
Follow PEP 8 for Python code
Use Google-style docstrings with clear, concise explanations of what functions do, their parameters, and return values
Files:
newton/_src/sim/contacts.pynewton/_src/geometry/narrow_phase.py
**/*
📄 CodeRabbit inference engine (AGENTS.md)
CLI arguments must use
kebab-case(e.g.,--use-cuda-graph, not--use_cuda_graph)
Files:
newton/_src/sim/contacts.pynewton/_src/geometry/narrow_phase.py
🧠 Learnings (13)
📓 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: 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.
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.
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.
📚 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/sim/contacts.pynewton/_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/sim/contacts.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/sim/contacts.pynewton/_src/geometry/narrow_phase.py
📚 Learning: 2026-01-13T03:11:40.556Z
Learnt from: jumyungc
Repo: newton-physics/newton PR: 1333
File: newton/_src/sim/builder.py:0-0
Timestamp: 2026-01-13T03:11:40.556Z
Learning: Ensure all VBD damping semantics follow Rayleigh-style damping and are unitless. This convention, demonstrated in SolverVBD, should be applied consistently across all VBD-related constraints in the codebase. In newton/_src/sim/builder.py, validate that any damping terms use the same normalization and are treated as dimensionless; add comments and unit tests to enforce consistency across VBD components.
Applied to files:
newton/_src/sim/contacts.pynewton/_src/geometry/narrow_phase.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-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/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-20T18:02:36.099Z
Learnt from: eric-heiden
Repo: newton-physics/newton PR: 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.
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-09-10T13:03:58.591Z
Learnt from: gdaviet
Repo: newton-physics/newton PR: 750
File: newton/_src/solvers/implicit_mpm/solve_rheology.py:799-804
Timestamp: 2025-09-10T13:03:58.591Z
Learning: In Warp's tiled launch API, the launch dimension should include the tile size as the last dimension, matching the CUDA block dimension. For tiled operations, use dim=(num_blocks, tile_size) where tile_size equals block_dim, not just dim=num_blocks like regular kernel launches.
Applied to files:
newton/_src/geometry/narrow_phase.py
📚 Learning: 2025-10-06T18:14:18.582Z
Learnt from: daedalus5
Repo: newton-physics/newton PR: 868
File: newton/examples/diffsim/example_diffsim_bear.py:82-96
Timestamp: 2025-10-06T18:14:18.582Z
Learning: Warp's tile operations (`wp.tile_store()` and `wp.tile_load()`) have built-in boundary checking. Out-of-bounds writes are automatically skipped and out-of-bounds reads are zero-filled, so tiled kernels are safe even when the array size is not a multiple of the tile size.
Applied to files:
newton/_src/geometry/narrow_phase.py
📚 Learning: 2025-10-10T10:47:41.082Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 899
File: newton/tests/test_rigid_contact.py:214-268
Timestamp: 2025-10-10T10:47:41.082Z
Learning: In `newton/tests/test_rigid_contact.py`, using `add_shape_plane(width=0, length=0)` for an infinite plane works correctly with the collision system and does not cause degenerate AABB issues in the test scenario `test_shape_collisions_gjk_mpr_multicontact`.
Applied to files:
newton/_src/geometry/narrow_phase.py
🧬 Code graph analysis (2)
newton/_src/sim/contacts.py (2)
newton/_src/solvers/solver.py (1)
device(169-176)newton/_src/sim/collide.py (1)
device(334-341)
newton/_src/geometry/narrow_phase.py (11)
newton/_src/geometry/collision_core.py (3)
check_infinite_plane_bsphere_overlap(504-556)compute_bounding_sphere_from_aabb(428-438)compute_tight_aabb_from_support(365-424)newton/_src/geometry/collision_primitive.py (5)
collide_capsule_capsule(199-293)collide_plane_capsule(297-348)collide_plane_sphere(119-125)collide_sphere_capsule(162-195)collide_sphere_sphere(129-158)newton/_src/geometry/contact_data.py (2)
ContactData(26-62)contact_passes_margin_check(66-96)newton/_src/geometry/types.py (2)
GeoType(25-67)SDF(74-113)newton/_src/geometry/support_function.py (2)
extract_shape_data(305-347)SupportMapDataProvider(55-63)newton/_src/sim/contacts.py (1)
device(143-147)newton/_src/sim/collide.py (1)
device(334-341)newton/_src/geometry/sdf_hydroelastic.py (1)
launch(386-449)newton/_src/geometry/broad_phase_nxn.py (2)
launch(295-361)launch(378-435)newton/_src/geometry/broad_phase_sap.py (1)
launch(506-650)newton/_src/geometry/contact_reduction_global.py (4)
clear_active(351-385)get_data_struct(387-404)reduce_buffered_contacts_kernel(548-567)export_reduced_contacts_kernel(658-760)
⏰ 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). (4)
- GitHub Check: Run GPU Tests / Run GPU Unit Tests on AWS EC2
- GitHub Check: Run GPU Benchmarks / Run GPU Benchmarks on AWS EC2
- GitHub Check: run-newton-tests / newton-unittests (windows-latest)
- GitHub Check: run-newton-tests / newton-unittests (ubuntu-latest)
🔇 Additional comments (3)
newton/_src/sim/contacts.py (1)
61-66: Nice consolidation of contact counters.
Single zero for both rigid/soft counts is a clean perf win and keeps views lightweight.Also applies to: 95-95
newton/_src/geometry/narrow_phase.py (2)
1376-1399: Good guard to skip mesh kernels whenhas_meshes=False.
Skipping those launches is a clean win for primitive-only scenes.
212-219: No changes required—current code correctly handles optional arrays per Warp conventions.The truthiness check on
shape_pairs_sdf_sdfis the idiomatic pattern for optional Warp arrays: passNonewhen the feature is disabled, and useif array:inside the kernel to gate access. This avoids unnecessary allocations and dummy arrays. The code is correct as-is.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
2 participants
Description
Unified collision pipeline performance improvements
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
New Features
Documentation
Refactor
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.