Convex vs Mesh collision support (allows to realize terrain meshes)#981
Conversation
📝 WalkthroughWalkthroughAdds a single-contact convex collision solver; replaces projector anchor points with scalar Changes
Sequence Diagram(s)sequenceDiagram
participant Collide as collide_unified
participant Selector as Solver Selector
participant Multi as Multi-Contact Solver
participant Single as Single-Contact Solver
participant MPRGJK as MPR / GJK routines
rect rgba(230,245,255,0.18)
Collide->>Selector: request contacts for shape pair
alt ENABLE_MULTI_CONTACT
Selector->>Multi: invoke multi-contact solver
Multi->>MPRGJK: MPR (initial)
MPRGJK-->>Multi: collision / signed distance
Multi->>MPRGJK: manifold generation (if needed)
Multi-->>Collide: multiple contacts
else
Selector->>Single: invoke single-contact solver
Single->>MPRGJK: MPR (initial)
MPRGJK-->>Single: collision / signed distance
alt separated -> fallback
Single->>MPRGJK: GJK fallback
MPRGJK-->>Single: distance & normal
end
Single-->>Collide: single contact (normal, point, distance)
end
end
sequenceDiagram
participant Pipeline as CollisionPipelineUnified
participant AABB as AABB util
participant Broad as build_contacts_kernel_gjk_mpr
participant MeshOverlap as find_mesh_triangle_overlaps_kernel
participant MeshProc as process_mesh_triangle_contacts_kernel
participant PlaneProc as process_mesh_plane_contacts_kernel
rect rgba(245,235,255,0.18)
Pipeline->>AABB: compute_tight_aabb_from_support(shape)
AABB-->>Pipeline: aabb_lower / aabb_upper
Pipeline->>Broad: build_contacts_kernel_gjk_mpr(..., shape_aabb_lower, shape_aabb_upper, ...)
Broad-->>Pipeline: shape pairs + mesh/plane pair buffers
alt mesh pairs exist
Pipeline->>MeshOverlap: find_mesh_triangle_overlaps_kernel()
MeshOverlap-->>Pipeline: triangle_pairs
Pipeline->>MeshProc: process_mesh_triangle_contacts_kernel(triangle_pairs)
Pipeline->>PlaneProc: process_mesh_plane_contacts_kernel(mesh-plane pairs)
MeshProc-->>Pipeline: triangle contacts
PlaneProc-->>Pipeline: plane contacts
end
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
…uses an older warp version
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (16)
newton/_src/geometry/simplex_solver.py (5)
84-123: Segment degeneracy handling is safe; consider clamping t to [0,1] for stability.You already branch for lambda<0 and degenerate cases. Explicit clamping of t can reduce branch sensitivity and tiny negative barycentrics under float error.
- t = -wp.dot(a, edge) / denom + t = -wp.dot(a, edge) / denom + # Numeric guard + t = wp.min(1.0, wp.max(0.0, t))
126-199: Triangle barycentric robustness: cross/denom guards look good; add small epsilon to mask edge ties.With EPSILON-level degeneracy, edge fallback is correct. To avoid flip-flopping on near-zero lambdas, bias comparisons with a tiny tolerance (e.g., 1e-8) to stabilize which edge is selected.
206-281: Inside-tetrahedron detection path is correct; consider returning a normal aligned with last_search_dir to avoid zero-normal overlaps.You currently set success=False and handle overlap outside. As a small robustness tweak, propagate a stable normal here to avoid later fallback to axis when last_search_dir is near-zero.
348-372: Early-overlap exit uses empty simplex state.When dist_sq < eps before any vertex is added, simplex_get_closest returns zeros. That’s acceptable, but consider returning the geometric center as the witness midpoint to improve debugging observability.
396-405: Duplicate-vertex detection threshold depends on COLLIDE_EPSILON; consider decoupling.A separate stall_epsilon (e.g., 1e-7 times shape scale) can reduce accidental stalls when COLLIDE_EPSILON is tuned for contact generation, not vertex spacing.
newton/_src/geometry/multicontact.py (2)
96-122: Parallel-ray handling returns input point; consider returning nearest point on plane instead.When denom≈0, ray_plane_intersection returns ray_origin. For body_projector_project this can leave points off-plane. Option: return ray_origin projected onto plane to guarantee on-plane result.
- if wp.abs(denom) < 1.0e-12: - return ray_origin + if wp.abs(denom) < 1.0e-12: + # Project origin to plane when ray is parallel + t = -(wp.dot(ray_origin, plane_normal) + plane_d) + return ray_origin + plane_normal * t
160-182: make_body_projector_from_polygon: anchor_point may not lie exactly on the polygon plane.If anchor_point is slightly off-plane, plane_d will be biased. Consider computing plane_d from any polygon vertex (e.g., poly[0]) after normalizing best_normal.
- proj.plane_d = -wp.dot(anchor_point, proj.normal) + # Prefer a vertex guaranteed on the plane + proj.plane_d = -wp.dot(poly[0], proj.normal)newton/_src/sim/collide_unified.py (9)
42-44: Feature flags: consider making these runtime-configurable.ENABLE_MULTI_CONTACT and ENABLE_TILE_BVH_QUERY as wp.static booleans require recompilation to change. Consider routing as function parameters or a config struct for flexibility.
890-919: Plane-to-cube proxy for infinite planes: pragmatic.Depth/lateral sizes based on other shape’s radius make GJK/MPR happy. Consider exposing scale multiplier (10x) as a constant/config.
964-1011: Triangle overlap kernel: mesh/non-mesh separation correct; early bail on mesh-mesh.If mesh-mesh support is on the roadmap, reserve a separate buffer and path to avoid returning silently.
1152-1152: Unused kernel arg shape_collision_radius.Remove to reduce register pressure.
1305-1305: Unused kernel arg shape_type in process_mesh_plane_contacts_kernel.Remove if not needed.
1329-1437: Mesh-plane vertex walk: robust mapping from global task_id to (pair, vertex).Binary search over inclusive cumsum is correct; good bounds checks. Only nit: consider a small negative-penetration tolerance wider than rigid_contact_margin for high-velocity cases.
1626-1640: Triangle-pair buffer size is hard-coded (1e6).Make this configurable from CollisionPipelineUnified ctor or auto-grow with reallocation on overflow; otherwise large scenes will drop contacts silently.
1920-1982: Triangle overlap/contact kernels: sequencing is correct; consider skipping launches when counts are zero.Micro-optimization: check counters host-side before launching to avoid kernel overhead.
970-970: Address static analysis hints.
- Remove unused args: shape_collision_radius (find_mesh_triangle_overlaps_kernel) and shape_type (process_mesh_plane_contacts_kernel).
- Remove redundant int(...) casts in find_pair_from_cumulative_index comparisons.
Also applies to: 1152-1152, 1305-1305
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
newton/_src/geometry/collision_convex.py(2 hunks)newton/_src/geometry/multicontact.py(6 hunks)newton/_src/geometry/simplex_solver.py(17 hunks)newton/_src/sim/collide_unified.py(17 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
newton/_src/geometry/collision_convex.py (2)
newton/_src/geometry/mpr.py (1)
create_solve_mpr(199-498)newton/_src/geometry/simplex_solver.py (1)
create_solve_closest_distance(57-550)
newton/_src/geometry/simplex_solver.py (1)
newton/_src/geometry/mpr.py (1)
Vert(53-57)
newton/_src/sim/collide_unified.py (3)
newton/_src/geometry/collision_convex.py (2)
create_solve_convex_single_contact(180-290)solve_convex_single_contact(198-288)newton/_src/geometry/support_function.py (5)
GenericShapeData(90-109)SupportMapDataProvider(56-64)support_map(113-339)pack_mesh_ptr(68-75)GeoTypeEx(51-52)newton/_src/geometry/types.py (1)
GeoType(25-67)
🪛 Ruff (0.14.1)
newton/_src/sim/collide_unified.py
970-970: Unused function argument: shape_collision_radius
(ARG001)
1114-1114: Value being cast to int is already an integer
Remove unnecessary int call
(RUF046)
1116-1116: Value being cast to int is already an integer
Remove unnecessary int call
(RUF046)
1123-1123: Value being cast to int is already an integer
Remove unnecessary int call
(RUF046)
1136-1136: Value being cast to int is already an integer
Remove unnecessary int call
(RUF046)
1152-1152: Unused function argument: shape_collision_radius
(ARG001)
1305-1305: Unused function argument: shape_type
(ARG001)
⏰ 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 Benchmarks (Pull Request)
- GitHub Check: Run GPU Unit Tests on AWS EC2 (Pull Request)
🔇 Additional comments (20)
newton/_src/geometry/simplex_solver.py (4)
54-54: Mat83f-based simplex is a clear win.Compact, cache-friendly, and simplifies vertex access. Nice cleanup.
383-395: Frank–Wolfe duality-gap check: good choice.Solid convergence criterion, guarded for fallback-direction cases.
470-489: Normal selection fallback is sensible.Preference delta -> -v -> last_search_dir -> axis covers edge cases well.
68-81: Confirm vert_v(center) type compatibility.create_support_map_function.geometric_center’s return type must match vert_v’s input. If geometric_center returns a vec3, vert_v(center) will type-mismatch at compile time; if it returns Vert, you’re good. Please verify.
Run to inspect signatures:
newton/_src/geometry/multicontact.py (5)
125-139: BodyProjector plane_d representation: LGTM.Clear docstring and 1-float storage are good improvements.
220-233: Segment projector planes: good midpoint usage; ensure normals are normalized.compute_line_segment_projector_normal returns normalized normal; all good.
240-251: Mixed polygon/segment case: stable; prefer vertex-based plane_d.Same anchor_point concern as above; using the segment midpoint is fine. If anchor passed is far off-plane, rely on midpoint as you already do.
1000-1019: Back-projection uses plane projectors correctly.Nice, consistent application across both shapes.
973-983: I'll verify the Warp allocation rules to confirm whether this review comment is valid.I need to determine which Warp version is being used in the codebase to provide a definitive verdict.
Based on my verification through official Warp documentation and release notes:
The review comment relies on outdated information.
In Warp 1.9.0+, it is now possible to allocate local arrays of a fixed size in kernels using wp.zeros(). The resulting arrays are allocated in registers, providing fast access and avoiding global memory overhead.
The code
loop_seg_ids = wp.zeros(shape=(12,), dtype=wp.uint8)is a fixed-size local allocation, which is explicitly supported and does not violate any "no-dynamic-allocation" rule. This is fundamentally different from allocating device memory arrays and is the intended use pattern.However, without visibility into the codebase's Warp version requirements, I cannot definitively confirm whether the repository uses Warp 1.9+ or an older version where this feature wasn't available.
newton/_src/geometry/collision_convex.py (1)
48-52: 1-element types: good micro-optimizations.These save registers in kernels. Nice touch.
newton/_src/sim/collide_unified.py (10)
60-63: Pre-created solvers bound to support_map_func: LGTM.Keeps kernels pure and avoids per-call factory overhead.
406-467: compute_tight_aabb_from_support: correct frame math.Using transposed rotation to express world axes in local space and dotting support points yields tight AABBs. Nice.
725-764: AABB prefilter inside kernel reduces work; good bounds checks on atomics.The early-out on non-overlapping AABBs plus pair sorting is solid.
835-853: Infinite-plane vs bounding-sphere cull: good early rejection; ensure centers/radii computed once.Uses AABB-derived spheres; efficient and conservative.
856-876: Mesh–infinite-plane path: cumulative vertex indexing is clever; watch for overflow and reset each frame.You zero mesh_plane_vertex_total_count each frame (good). For very large meshes, guard against int overflow and cap total vertices or chunk processing.
878-884: General mesh pairs buffered for later kernels: LGTM.Bounds-checked atomic writes prevent OOB.
920-961: Contact writing loop: correct use of radii/thickness with signed distances.Distance algebra aligns with offset conventions; normal orientation matches kernels.py convention.
1054-1062: Tiled vs non-tiled BVH query toggles: nice.The 2D launch uses tile_size as second dim; current code ignores _j (expected for tile intrinsics). Good.
1837-1881: Fixed-thread launches with strided loops: good for unknown pair counts.total_num_threads computed once; consistent across subsequent kernels.
1900-1918: Mesh-plane kernel always launches; early return if no pairs: good.Efficient path; nothing to change.
…d collision pipeline
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
newton/tests/test_rigid_contact.py (2)
565-575: Warmup/capture on CUDA to reduce JIT varianceConsider a short pre-run or ScopedCapture (as used earlier in this file) to avoid first-iteration JIT overhead impacting early frames on CUDA.
+ # Optional: warm up kernels (especially helpful on CUDA) + if wp.get_device(device).is_cuda: + for _ in range(2): + state_0.clear_forces() + contacts = model.collide(state_0, collision_pipeline=collision_pipeline) + solver.step(state_0, state_1, control, contacts, sim_dt / substeps) + state_0, state_1 = state_1, state_0
597-609: Velocity tolerance may be a bit tight across devicesabs(vel) < 0.01 can occasionally fail on different GPUs/precision; consider 0.02–0.03 or tie to sim_dt (e.g., 2e-2) to reduce flakiness while still catching regressions.
newton/tests/test_collision_pipeline.py (1)
270-287: Unified matrix expansion is great; keep MESH×MESH excluded until supportedThe added pairs exercise the new convex-vs-mesh paths well. Consider marking a few heaviest ones as slow to manage CI duration, or gate by env flag.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
newton/tests/test_collision_pipeline.py(3 hunks)newton/tests/test_rigid_contact.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
newton/tests/test_collision_pipeline.py (3)
newton/_src/geometry/types.py (5)
vertices(219-220)vertices(223-225)indices(228-229)indices(232-234)GeoType(25-67)newton/_src/geometry/utils.py (1)
create_box_mesh(647-709)newton/_src/utils/mesh.py (1)
create_sphere_mesh(22-83)
⏰ 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 Benchmarks (Pull Request)
- GitHub Check: Run GPU Unit Tests on AWS EC2 (Pull Request)
🔇 Additional comments (4)
newton/tests/test_rigid_contact.py (2)
520-553: Good targeted mesh-ground sanity checkTest setup is clear and exercises the unified pipeline with a minimal mesh case. Nice.
612-620: Registration looks good; keep name unique and gated per device setThe add_function_test usage matches the existing pattern and device loop. LGTM.
newton/tests/test_collision_pipeline.py (2)
96-99: Bumping rigid_contact_max_per_pair to 20 is reasonable for MESH casesMore contacts per pair helps stability with triangle meshes; acceptable trade-off for these tests.
Please monitor CI runtime; if this materially increases wall time, we can selectively reduce for fast suites.
126-131: ****The review comment misidentifies which
create_box_meshfunction is exported throughnewton.utils. The exportedcreate_box_meshinnewton.utilscomes fromnewton._src.utils.meshand accepts anextentsparameter, not fromnewton._src.geometry.utilswhich takeshalf_extents. The test code correctly callsnewton.utils.create_box_mesh(extents=(0.5, 0.5, 0.5))with the proper signature for the exported function.Likely an incorrect or invalid review comment.
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/sim/collide_unified.py (1)
1870-1905: Honor iterate_mesh_vertices flag when launching mesh‑plane kernel.The constructor exposes iterate_mesh_vertices but collide() always launches the kernel.
- # Launch mesh-plane contact processing kernel with fixed thread count - wp.launch( + # Launch mesh-plane contact processing kernel (optional) + if self.iterate_mesh_vertices: + wp.launch( kernel=process_mesh_plane_contacts_kernel, dim=total_num_threads, inputs=[ state.body_q, model.shape_transform, model.shape_body, model.shape_type, model.shape_scale, model.shape_thickness, model.shape_source_ptr, self.shape_pairs_mesh_plane, self.shape_pairs_mesh_plane_cumsum, self.shape_pairs_mesh_plane_count, self.mesh_plane_vertex_total_count, self.rigid_contact_margin, contacts.rigid_contact_max, total_num_threads, ], outputs=[ contacts.rigid_contact_count, contacts.rigid_contact_shape0, contacts.rigid_contact_shape1, contacts.rigid_contact_point0, contacts.rigid_contact_point1, contacts.rigid_contact_offset0, contacts.rigid_contact_offset1, contacts.rigid_contact_normal, contacts.rigid_contact_thickness0, contacts.rigid_contact_thickness1, contacts.rigid_contact_tids, ], - device=contacts.device, - block_dim=block_dim, - ) + device=contacts.device, + block_dim=block_dim, + )
♻️ Duplicate comments (1)
newton/_src/sim/collide_unified.py (1)
846-848: Early-out for plane–plane is correct and avoids proxy boxes.This matches the prior review ask to reject infinite plane–plane pairs early. Good.
🧹 Nitpick comments (4)
newton/_src/sim/collide_unified.py (4)
1368-1376: Normalize plane normal in mesh‑plane kernel.Transforming the axis with wp.transform_vector may carry tiny numerical drift; normalize to ensure distance is a true signed distance and consistent offsets.
- # Get plane normal in world space (plane normal is along local +Z, pointing upward) - plane_normal = wp.transform_vector(X_plane_ws, wp.vec3(0.0, 0.0, 1.0)) + # Get plane normal in world space (normalize to avoid drift) + plane_normal = wp.normalize(wp.transform_vector(X_plane_ws, wp.vec3(0.0, 0.0, 1.0)))
1618-1621: Fixed 1e6 triangle_pairs buffer is risky. Make capacity configurable or data‑driven.Preallocating one million triples can be wasteful on small scenes and insufficient on big ones. Consider:
- Sizing from mesh index counts (upper bound), or
- Passing a capacity knob, plus overflow detection.
976-976: Clean up unused kernel parameters to silence static checks.shape_collision_radius (in two kernels) and shape_type (in mesh‑plane kernel) are unused.
- Either drop them from signatures and call sites, or
- Prefix with “_” and add a brief comment.
Also applies to: 1139-1139, 1292-1292
1118-1129: Redundant int casts in find_pair_from_cumulative_index.cumulative_sums is already int-typed; you can drop explicit int() to reduce noise. Verify Warp typing doesn’t require host int here.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
newton/_src/geometry/collision_convex.py(2 hunks)newton/_src/sim/collide_unified.py(19 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-24T07:56:14.761Z
Learnt from: nvtw
PR: newton-physics/newton#981
File: newton/_src/geometry/collision_convex.py:180-289
Timestamp: 2025-10-24T07:56:14.761Z
Learning: In newton/_src/geometry/collision_convex.py, the single-contact solver (create_solve_convex_single_contact) intentionally returns count=1 even when shapes are separated (signed_distance > contact_threshold). The calling code is responsible for deciding whether to accept the contact based on the signed distance value. This design pushes filtering responsibility to the caller.
Applied to files:
newton/_src/geometry/collision_convex.py
🧬 Code graph analysis (2)
newton/_src/geometry/collision_convex.py (2)
newton/_src/geometry/mpr.py (1)
create_solve_mpr(199-498)newton/_src/geometry/simplex_solver.py (1)
create_solve_closest_distance(57-550)
newton/_src/sim/collide_unified.py (4)
newton/_src/geometry/broad_phase_common.py (1)
binary_search(42-50)newton/_src/geometry/collision_convex.py (4)
create_solve_convex_multi_contact(54-177)create_solve_convex_single_contact(180-289)solve_convex_single_contact(198-287)solve_convex_multi_contact(73-175)newton/_src/geometry/support_function.py (5)
GenericShapeData(90-109)SupportMapDataProvider(56-64)support_map(113-339)pack_mesh_ptr(68-75)GeoTypeEx(51-52)newton/_src/geometry/types.py (1)
GeoType(25-67)
🪛 Ruff (0.14.2)
newton/_src/geometry/collision_convex.py
274-274: Undefined name skip_multi_contact
(F821)
newton/_src/sim/collide_unified.py
976-976: Unused function argument: shape_collision_radius
(ARG001)
1123-1123: Value being cast to int is already an integer
Remove unnecessary int call
(RUF046)
1139-1139: Unused function argument: shape_collision_radius
(ARG001)
1292-1292: Unused function argument: shape_type
(ARG001)
⏰ 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 Unit Tests on AWS EC2 (Pull Request)
- GitHub Check: Run GPU Benchmarks (Pull Request)
🔇 Additional comments (5)
newton/_src/sim/collide_unified.py (3)
853-866: Half‑space plane overlap cull is aligned with conventions.Using center_dist <= radius implements half‑space behavior and eliminates obvious non-overlaps early. Good for perf.
969-1000: Tile BVH path is properly feature‑gated.ENABLE_TILE_BVH_QUERY=False keeps the legacy path until tile queries land. Good guard.
Please confirm the Warp MR enabling tile AABB queries before flipping the flag.
1316-1320: Verify inclusive cumsum mapping for mesh‑plane vertex work distribution.find_pair_from_cumulative_index expects shape_pairs_mesh_plane_cumsum to be inclusive (end indices). You write cumsum as “before + count” and total_vertices equals the last inclusive value—looks consistent. Please confirm no off‑by‑one when task_id == total_vertices - 1.
Also applies to: 1334-1341, 1348-1350
newton/_src/geometry/collision_convex.py (2)
48-52: Compact single‑contact types are a good register saver.Matrix/vector arities match the single‑contact path.
180-195: Single‑contact path mirrors multi‑contact prepass correctly.MPR first then GJK fallback is consistent and efficient.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
newton/tests/test_rigid_contact.py (1)
561-562: Consider removing potentially redundanteval_fkcall.The
newton.eval_fkcall may be unnecessary sincemodel.state()already initializes body positions frommodel.joint_qandmodel.joint_qd. Note thattest_shapes_on_plane(lines 44-194) andtest_mujoco_warp_newton_contacts(lines 622-736) don't calleval_fk, whiletest_mujoco_convex_on_convex(line 777) does. Consider either removing this call for consistency or documenting why it's needed here.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
newton/tests/test_rigid_contact.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
newton/tests/test_rigid_contact.py (7)
newton/_src/sim/builder.py (5)
add_ground_plane(2844-2864)add_body(1453-1525)add_joint_free(1926-1980)add_shape_mesh(3074-3110)finalize(4455-4841)newton/_src/geometry/types.py (7)
vertices(219-220)vertices(223-225)indices(228-229)indices(232-234)Mesh(116-300)finalize(102-109)finalize(237-254)newton/_src/geometry/utils.py (1)
create_box_mesh(647-709)newton/_src/sim/collide_unified.py (4)
CollisionPipelineUnified(1495-1971)from_model(1638-1702)BroadPhaseMode(47-58)collide(1704-1971)newton/_src/solvers/xpbd/solver_xpbd.py (1)
SolverXPBD(40-660)newton/_src/sim/model.py (3)
state(567-604)control(606-643)collide(667-726)newton/tests/unittest_utils.py (1)
add_function_test(312-331)
⏰ 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 Benchmarks (Pull Request)
- GitHub Check: Run GPU Unit Tests on AWS EC2 (Pull Request)
🔇 Additional comments (7)
newton/tests/test_rigid_contact.py (7)
520-525: LGTM!The function signature and docstring are clear and follow the established pattern in this test file.
526-542: LGTM!Model setup is correct. The mesh box is properly positioned with its center at z=box_half so the bottom face rests at ground level (z=0).
547-553: LGTM!The collision pipeline setup using
BroadPhaseMode.EXPLICITis correct. The pattern matches other tests in this file (e.g., lines 402-407, 653-658) and relies onmodel.finalize()to automatically build the shape contact pairs.
564-574: LGTM!The simulation loop follows the standard pattern used throughout this test file and provides adequate settling time (1 second) for the mesh box.
580-609: LGTM!The assertions correctly verify that the mesh box remains at ground level with near-zero velocities. The individual component checks provide clear error messages. Optionally, you could add a quaternion check to verify the box hasn't rotated significantly, similar to line 193 in
test_shapes_on_plane, but the angular velocity check is sufficient for this test's purpose.
613-619: LGTM!Test registration follows the established pattern and correctly registers the test for all available devices.
520-619: No duplicate test found.The AI summary claims "The test function is duplicated within the same file (two occurrences of test_mesh_box_on_ground with identical signature and logic)," but only one definition and one registration block are present in the provided code. This appears to be a false positive in the summary.
Description
Implements mesh vs convex collision support inside the alternative (unified) collision pipeline.
Should address the collision part of #596
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
Refactor
Tests