Rewrite and extend collision detection documentation #1311
Conversation
📝 WalkthroughWalkthroughRewrites collision reference into dual pipelines (Standard vs. Unified), expands ShapeConfig and SDF/hydroelastic options and parameters, adds USD mappings and performance guidance, and updates CollisionPipelineUnified docstring; no public API signatures changed. (≤50 words) Changes
Sequence Diagram(s)(Section skipped — changes are documentation and a docstring update; no new multi-component control flow introduced.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
docs/concepts/collisions.rst (4)
354-384: Provide explicitCollisionPipelineUnifiedusage example.The example at lines 376–384 shows how to create the pipeline with different broad-phase modes, which is good. Consider adding a note clarifying that
model.collide()without acollision_pipelineargument defaults to the standardCollisionPipelinefor backward compatibility, so users opting for the unified pipeline must explicitly pass it tomodel.collide().
515-525: Add emphasis on SDF configuration for mesh-heavy workloads.The warning about mesh-mesh collisions falling back to BVH without SDF is excellent. Consider strengthening the recommendation with a note that all mesh shapes in production scenes with many potential mesh-mesh contacts should have SDF precomputed to avoid performance cliffs.
589-591: Clarify contact generation note for clarity.The note at lines 589–591 explains contact generation criteria (
d = surface_distance - (thickness_a + thickness_b)). This is precise, but consider adding an example with concrete values (e.g., "For two objects with thickness=0.001m and margin=0.01m, contact is generated when surfaces are within 0.01m") to make it more accessible.
463-464: Add note on Cone and Ellipsoid in Unified Pipeline table.Line 463 states "Cone, Ellipsoid, and ConvexMesh are also fully supported," but these shape types are not explicitly shown in the Unified Pipeline shape compatibility table (lines 393–461). Either add rows for these shapes or clarify that the table covers the most common types and full support includes these additional geometries.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/concepts/collisions.rst
🧰 Additional context used
🧠 Learnings (6)
📓 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-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:
docs/concepts/collisions.rst
📚 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.rst
📚 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:
docs/concepts/collisions.rst
📚 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:
docs/concepts/collisions.rst
📚 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
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run GPU Tests / Run GPU Unit Tests on AWS EC2
- GitHub Check: Run GPU Benchmarks / Run GPU Benchmarks on AWS EC2
🔇 Additional comments (5)
docs/concepts/collisions.rst (5)
119-120: Clarify world index -1 collision behavior.The description states index -1 "collides with all worlds," but this needs clarification. Per the codebase learnings, Newton automatically skips collisions between shapes with body index -1 (world-attached shapes). Refine the description to clarify:
- Index -1 shapes collide with bodies in any world
- But two index -1 shapes do not collide with each other (auto-skipped)
This prevents unintended interactions between globally-placed obstacles across worlds.
622-627: Thebuilder.rigid_contact_marginattribute exists in the ModelBuilder class with the default value of 0.1, and the code snippet in the documentation is accurate.
954-954: No issues found. The import statementfrom newton.geometry import SDFHydroelasticConfigat line 954 is correct.SDFHydroelasticConfigis properly exported bynewton/geometry.pyand this import pattern is used consistently throughout the codebase in tests, examples, and internal modules.
144-144: No action needed. The Model attributesshape_worldandbody_worldreferenced at line 144 are properly defined in the Model class with comprehensive docstrings documenting their purpose, shape, type, and semantics. The documentation reference is accurate.
652-663:builder.default_shape_cfgand its parameters are correctly implemented and supported.Verification confirms:
- The
default_shape_cfgattribute exists on ModelBuilder as a ShapeConfig instance- All parameters referenced are valid and properly documented:
ke(contact elastic stiffness)kd(contact damping coefficient)mu(coefficient of friction)is_hydroelastic(enables SDF-based hydroelastic collisions)sdf_max_resolution(maximum dimension for sparse SDF grid)The documentation example is accurate and matches the actual API implementation.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @docs/concepts/collisions.rst:
- Around line 119-122: Update the wording for "Index -1" in the world indices
section to explicitly state that while Index -1 represents global entities that
can collide with entities in all worlds, the physics system skips collisions
between two global shapes themselves (i.e., global vs global collisions are not
processed); modify the line describing "**Index -1**: Global entities that
collide with all worlds (e.g., ground plane)" to append or replace with a
clarification sentence that global shapes do not collide with other global
shapes.
- Around line 20-35: Update the documentation example to use the correct
parameter name for CollisionPipelineUnified.from_model(): replace the incorrect
sdf_hydro_config with sdf_hydroelastic_config in the call to
CollisionPipelineUnified.from_model(), ensuring any nearby examples or parameter
lists use the full sdf_hydroelastic_config identifier so the docs match the
actual API.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/concepts/collisions.rst
🧰 Additional context used
🧠 Learnings (7)
📓 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-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:
docs/concepts/collisions.rst
📚 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.rst
📚 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:
docs/concepts/collisions.rst
📚 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:
docs/concepts/collisions.rst
📚 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:
docs/concepts/collisions.rst
📚 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
⏰ 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 (ubuntu-latest)
- GitHub Check: run-newton-tests / newton-unittests (windows-latest)
🔇 Additional comments (2)
docs/concepts/collisions.rst (2)
1-1098: Documentation structure and clarity are excellent.The rewrite successfully expands and clarifies the collision detection system across multiple dimensions: pipeline choice, shape configuration, filtering rules, contact generation, material properties, and integration with USD. The use of tables, code examples, and progressive detail levels makes the documentation accessible to both beginners and advanced users. The distinction between Standard and Unified pipelines, and explicit guidance on when to use each, is well-presented.
273-390: The shape compatibility matrices in the documentation cannot be verified against explicit pair support definitions in the codebase.The Standard Pipeline implementation filters shape contact pairs based on collision groups, collision masks, and world assignments—not by shape type compatibility. The narrow-phase collision kernel in
collision_core.pyhandles all shape types (SPHERE, CAPSULE, BOX, PLANE, CYLINDER, CONE, ELLIPSOID, MESH, etc.) through a generic GJK/MPR algorithm with support maps and does not explicitly reject or validate shape type pairs.To verify the accuracy of the compatibility matrices (empty cells,
⚠️ warnings for Mesh-Mesh and Mesh-SDF, Cylinder/Cone/Ellipsoid claims), the following should be confirmed:
- Whether empty cells in the Standard Pipeline table reflect actual unsupported pairs or conservative documentation
- Which pairs (if any) produce invalid contacts despite being accepted by the collision pipeline
- Whether Cylinder and Cone support is complete or limited to specific pairings (the code shows rolling stabilization only for discrete vs. axial configurations)
- Whether particle pairs are genuinely unsupported or undocumented in the Standard Pipeline
Consider testing all documented shape pairs or reviewing contact generation tests to confirm matrix accuracy.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/concepts/collisions.rst (1)
249-253: Consider clarifying world-attached shape collision behavior.Line 252 states "Same-body shapes" are auto-filtered, which includes all shapes with
body=-1(world-attached). This means multiple static shapes (e.g., multiple ground planes from different builders) won't collide with each other. Consider making this explicit, as it's a common pattern in multi-world scenarios.Example addition:
- **World-attached shapes**: All shapes with ``body=-1`` share the same body index, so they won't collide with each otherBased on learnings: Newton automatically skips collisions between world-attached shapes (body=-1).
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/concepts/collisions.rstnewton/_src/sim/collide_unified.py
🧰 Additional context used
🧠 Learnings (7)
📓 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-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:
docs/concepts/collisions.rst
📚 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.rst
📚 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:
docs/concepts/collisions.rst
📚 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:
docs/concepts/collisions.rst
📚 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:
docs/concepts/collisions.rst
📚 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
⏰ 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 (16)
newton/_src/sim/collide_unified.py (2)
282-290: LGTM - Docstring accurately reflects implementation.The updated class docstring correctly describes CollisionPipelineUnified as a full-featured pipeline with GJK/MPR narrow phase and pluggable broad phase modes. All listed key features (multiple broad phase modes, SDF-based mesh collision, hydroelastic contacts, contact matching) are supported by the implementation.
482-482: LGTM - Return type documentation is correct.The docstring correctly indicates that
from_model()returns aCollisionPipelineUnifiedinstance, matching the implementation at line 505.docs/concepts/collisions.rst (14)
37-83: LGTM - Shape types documentation is clear and accurate.The Supported Shape Types section provides a comprehensive list with helpful notes:
- HFIELD limitation is clearly documented with workaround
- SDF auxiliary data nature is well explained, clarifying a common misconception
85-105: LGTM - Clear explanation of shape-body relationship.The section correctly explains shape attributes and world-space transformations. The distinction between
shape_source(Python reference) andshape_source_ptr(Warp kernel pointer) is helpful for users.
273-397: LGTM - Standard pipeline documentation is clear.The section correctly documents the standard pipeline's characteristics:
- Precomputed pairs during finalization
- Static collision topology
- Shape compatibility matrix showing limitations (e.g., limited Cylinder and SDF support)
The compatibility table clearly distinguishes the standard pipeline's more limited shape support from the unified pipeline's full support.
398-535: LGTM - Comprehensive unified pipeline documentation.The Unified Pipeline section effectively documents:
- Three broad phase modes with performance characteristics
- Complete shape compatibility matrix with performance warnings (
⚠️ )- Helpful notes about SDF auxiliary data and development status
The documentation provides clear guidance on when to use each broad phase mode.
536-665: LGTM - Thorough narrow phase and contact reduction documentation.The section provides excellent technical depth:
- Clear explanation of MPR algorithm usage
- Helpful performance warning for mesh-mesh without SDF (lines 576-586)
- Detailed contact reduction mechanics with beta scoring explanation
- Comprehensive SDFHydroelasticConfig parameter table
The documentation balances technical accuracy with practical guidance.
666-745: LGTM - Comprehensive shape configuration reference.The Shape Configuration section provides essential details:
- Clear distinction between thickness (creates visible gap) and contact_margin (no gap)
- Important constraint: contact_margin must be >= thickness
- Precise contact generation formula (lines 705-708)
- Complete SDF configuration parameters with defaults
This is valuable reference material for users tuning collision behavior.
746-813: LGTM - Practical common patterns with correct examples.The Common Patterns section provides useful real-world examples:
- Static geometry creation (body=-1)
- Default configuration setup
- Collision frequency optimization (matches learned design pattern)
- Soft contact handling
Based on learnings: The collision reuse pattern (lines 782-795) is intentional and correct.
814-880: LGTM - Clear contact data structure documentation.The Contact Data section provides:
- Comprehensive tables of rigid and soft contact attributes
- Important clarification about contact forces (use SensorContact)
- Correct usage example for accessing contact data
The documentation aligns with the implementation in collide_unified.py.
881-908: LGTM - Complete Model.collide() parameter reference.The parameters table provides a clear reference for all
Model.collide()options with appropriate defaults and descriptions.
909-969: LGTM - Clear hydroelastic contacts documentation.The Hydroelastic Contacts section effectively explains:
- Opt-in nature (both shapes must have is_hydroelastic=True)
- Requirements and use cases
- Marching cubes process for contact surface generation
- Configuration options consistent with earlier sections
The emphasis on opt-in behavior and the requirement for both shapes is helpful.
970-1022: LGTM - Comprehensive material properties reference.The Contact Materials section provides a useful table showing:
- All available material properties
- Which solvers support each property
- Important notes (e.g., restitution requires enable_restitution=True)
This helps users configure materials appropriately for their chosen solver.
1024-1046: LGTM - USD integration example is clear.The USD Integration section shows correct syntax for custom collision properties in USD format, with appropriate cross-references to related documentation.
1047-1098: LGTM - Performance guidance and API reference are comprehensive.The final sections provide:
- Practical performance recommendations for choosing broad phase modes
- Complete import statements and API class listings
- Comprehensive model attribute references
- Appropriate cross-references to related documentation
The performance guidance is particularly valuable for users selecting the right collision configuration.
1076-1076: No action required. The import pathfrom newton.geometry import SDFHydroelasticConfigis correct. The class is properly re-exported in the public API atnewton/geometry.pyand is used consistently throughout tests and examples.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @docs/concepts/collisions.rst:
- Around line 670-745: The ShapeConfig docs are missing the is_hydroelastic and
k_hydro parameters and the contact_margin default is incorrect; update the
tables to include is_hydroelastic (bool) and k_hydro (float) under Collision
control/Geometry parameters with brief descriptions matching their
implementation in ShapeConfig, and change the contact_margin default note to
state "Default: None (uses builder.rigid_contact_margin when None)" and
optionally mention builder.rigid_contact_margin's default value elsewhere as
done; reference ShapeConfig, is_hydroelastic, k_hydro, and
builder.rigid_contact_margin when making these edits.
🧹 Nitpick comments (2)
docs/concepts/collisions.rst (2)
121-146: Consider clarifying automatic collision filtering for global shapes (body=-1).The learnings indicate that shapes with body index -1 (world-attached) automatically skip collision with each other, so explicit filter pairs are unnecessary between global shapes from different builders. The current documentation doesn't explicitly mention this behavior. Consider adding a note to clarify this automatic filtering.
This could help users avoid confusion about why collisions between global shapes don't occur without explicit configuration.
Suggested addition to World Indices section
World indices enable multi-world simulations where independent instances coexist without interacting: - **Index -1**: Global entities that collide with all worlds (e.g., ground plane) - **Index 0, 1, 2, ...**: World-specific entities that only interact within their world +.. note:: + **Global-global filtering**: Shapes with ``world=-1`` automatically skip collision with each other. + This is implicit in the collision system, so no manual filter pairs need to be added between + global shapes from different builders.
206-217: Clarify parent-child collision filtering and self-collision interaction.The section explains that parent-child collisions are disabled by default via
collision_filter_parent=True, but the relationship between this setting andenable_self_collisionswhen loading USD/MJCF files could be clearer.Consider adding a brief note explaining the distinction between per-shape filtering and full self-collision control, and how they interact.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/concepts/collisions.rst
🧰 Additional context used
🧠 Learnings (7)
📓 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-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:
docs/concepts/collisions.rst
📚 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.rst
📚 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:
docs/concepts/collisions.rst
📚 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:
docs/concepts/collisions.rst
📚 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:
docs/concepts/collisions.rst
📚 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
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run GPU Benchmarks / Run GPU Benchmarks on AWS EC2
- GitHub Check: Run GPU Tests / Run GPU Unit Tests on AWS EC2
- GitHub Check: run-newton-tests / newton-unittests (windows-latest)
🔇 Additional comments (5)
docs/concepts/collisions.rst (5)
1069-1076: Verify import statements for public API location.The import example shows
SDFHydroelasticConfigimported fromnewton.geometry, but the learnings indicate that common Newton classes are typically exposed at the top-levelnewtonmodule rather than sub-packages. Verify that all listed imports work as documented and update the example if necessary.# Current example (lines 1069-1076): from newton.geometry import SDFHydroelasticConfig # Verify this is the correct import path
818-825: All attributes mentioned are correctly implemented. TheContactsclass contains the expected attributes:rigid_contact_count,rigid_contact_shape0,soft_contact_particle, and others are all properly defined with appropriate types (Warp tensors with int32, float32, or vec3 dtypes). Naming is consistent across rigid and soft contact attributes.
29-29: All API references in the documentation are accurate and consistent with the actual implementation. TheCollisionPipelineUnified.from_model()method signature correctly includes bothbroad_phase_modeandsdf_hydroelastic_configparameters with appropriate types and defaults. All imported classes (CollisionPipelineUnified,BroadPhaseMode,SDFHydroelasticConfig,Contacts) exist and are properly defined in the codebase.
886-908: All parameter documentation is accurate. The method signature, parameter names, and default values match the implementation:
soft_contact_margindefaults to0.01✓edge_sdf_iterdefaults to10✓requires_graddefaults toNoneand resolves tomodel.requires_gradviaif requires_grad is None: requires_grad = self.requires_grad✓No discrepancies found.
292-389: Shape compatibility table is accurate.The table correctly reflects limited cylinder support. Only
collide_sphere_cylinderandcollide_plane_cylindercollision functions are implemented incollision_primitive.py; no collision handlers exist for cylinder-box, cylinder-capsule, or cylinder-cylinder pairs. This is confirmed by the absence of corresponding test cases intest_collision_pipeline.pyand the deliberate use of capsule substitutes in URDF imports due to incomplete cylinder collision support.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/concepts/collisions.rst (2)
85-106: Shape description is clear, with minor clarity opportunity.The body/shape/transform mechanics are well-explained. For completeness, you might explicitly clarify that body=-1 (world-attached) shapes collide with dynamic bodies (body>=0) but not with other body=-1 shapes. This helps users understand the filtering semantics.
107-272: Filtering section is comprehensive and well-organized.World indices, collision groups, and filter pairs are clearly explained with good examples. The USD integration notes are valuable for asset authoring. One minor consideration: the orthogonality between body index (dynamic vs static) and world index (multi-world separation) is implicit but not explicitly called out—clarifying that shapes can have body=-1 with world=0 (static but world-specific) might prevent confusion.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/concepts/collisions.rst
🧰 Additional context used
🧠 Learnings (7)
📓 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-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:
docs/concepts/collisions.rst
📚 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.rst
📚 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:
docs/concepts/collisions.rst
📚 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:
docs/concepts/collisions.rst
📚 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:
docs/concepts/collisions.rst
📚 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
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- 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)
🔇 Additional comments (7)
docs/concepts/collisions.rst (7)
1-36: Introduction and pipelines overview look solid.The dual-pipeline narrative is clearly structured with practical examples. The distinction between default (implicit) and explicit pipeline usage is well-explained.
37-84: Shape types table and notes are accurate.The SDF note at lines 77-83 appropriately clarifies that SDFs are auxiliary data generated from primary geometry, providing O(1) queries without forcing their use—important for managing user expectations about SDF behavior.
273-397: Standard pipeline description and compatibility table are clear.The motivation (static precomputed pairs) and limitations are well-articulated. The shape compatibility table clearly shows the gap vs the Unified pipeline, with appropriate direction to use Unified for advanced features.
Please verify that line 289 ("Does not support hydroelastic contacts") is accurate for CollisionPipeline. If hydroelastic support is a hard limitation of the Standard pipeline (vs just not being available by default), this should be confirmed against the implementation in collide.py or collide_unified.py.
398-535: Unified pipeline and broad phase documentation is clear and practical.The broad phase mode descriptions (NxN, SAP, EXPLICIT) and their complexity profiles are well-explained. The shape compatibility table is comprehensive and appropriately flags mesh-mesh/mesh-SDF pairs with
⚠️ . The caveat at line 534 about active development status manages expectations appropriately.
536-586: Narrow phase and mesh collision sections are detailed and practical.The MPR explanation, mesh collision strategies (BVH vs SDF), and the prominent warning about performance without SDF precomputation are all valuable. However, line 549 states that contact limits are controlled in the "collision pipeline constructor"—please verify this parameter's actual location (it may be on Model.collide() instead, as shown in the parameters table at lines 890-912).
588-665: Contact reduction section is thorough and appropriately cautious about API stability.The binning strategy, betas scoring explanation, and configuration options are all well-documented. The note at lines 640-642 appropriately flags that beta behavior may change—good for API maturity management. The distinction between multiple configurations (normal_matching, moment_matching) helps advanced users understand their options.
666-1101: Shape Configuration through See Also sections are comprehensive and well-organized.The Shape Configuration section (lines 666-749) clearly documents pairwise behaviors (thickness summing, contact_margin max) and the interaction with rest distance. Common Patterns provide practical guidance. Contact Data documentation is clear. Hydroelastic Contacts appropriately explain the opt-in design. Material properties table includes solver compatibility—a helpful detail. USD Integration shows authoring examples. Performance guidelines are actionable. See Also provides good import and reference information.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
docs/concepts/collisions.rst (6)
15-21: Clarify pipeline selection guidance.The description states that Standard pipeline is "efficient when the number of potentially colliding pairs is limited." However, per the AI summary and code description, the distinction is also about feature support: the Standard pipeline doesn't support advanced contact models (SDF, hydroelastic). A clearer guideline would help users choose. Consider adding that Standard is also simpler to use for scenes with limited, known collision topology.
295-403: Consider adding CONVEX_MESH to shape compatibility tables.Lines 69 and 557 mention CONVEX_MESH as a supported shape type, but the compatibility tables (both Standard and Unified) do not include a CONVEX_MESH column. Add this column to provide complete compatibility information, or clarify in a note that CONVEX_MESH follows the same pattern as MESH.
720-734: Clarify contact generation and rest distance behavior.Lines 720-734 explain thickness and contact_margin in ShapeConfig. The note on lines 731-734 defines contact generation as: "A contact is created when d < max(margin_a, margin_b)". However, the description of
contact_marginon lines 722-723 says "Margin only affects contact generation; effective rest distance is not affected and is only governed by thickness." This could be clearer: objects rest with surfaces separated bythickness_a + thickness_b, and contacts are detected when surfaces are withinmax(margin_a, margin_b)of each other, enabling "speculative" contacts before actual penetration. Consider a visual diagram or more explicit explanation.
948-952: Clarify shape requirements for hydroelastic contacts.Line 952 states hydroelastic supports "only volumetric shapes (not planes, heightfields, or non-watertight meshes)". This is good guidance, but consider adding: what about implicit surfaces (SDF as primary shape)? Are SDF shapes considered "volumetric" for this purpose? Can you enable hydroelastic on an SDF shape if it was generated from a non-watertight mesh?
86-101: Clarify automatic collision filtering for static/world-attached shapes.Line 88 states: "Use body=-1 for static/world-fixed shapes." The learning context indicates that "collisions between shapes with body index -1 are automatically skipped by the collision system, so no manual collision filter pairs need to be added between global shapes from different builders." The documentation should explicitly state that multiple global shapes (body=-1) do not collide with each other, or clarify if they do when multiple builders are composed. This is important for users adding multiple static objects to a scene.
698-728: Consolidate overlapping parameter sections.The ShapeConfig documentation spans lines 696-728 with overlapping sections: "Collision control" (lines 694-709) documents
collision_group,collision_filter_parent,has_shape_collision,has_particle_collision, while "Geometry parameters" (lines 711-729) documentsthickness,contact_margin,is_solid,is_hydroelastic,k_hydro. Consider reorganizing so these related parameters are grouped more logically, or add a note explaining the distinction between collision control and geometry parameters.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/concepts/collisions.rst
🧰 Additional context used
🧠 Learnings (9)
📓 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-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:
docs/concepts/collisions.rst
📚 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.rst
📚 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:
docs/concepts/collisions.rst
📚 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:
docs/concepts/collisions.rst
📚 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:
docs/concepts/collisions.rst
📚 Learning: 2025-07-21T19:11:04.077Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 450
File: newton/sim/ik.py:1286-1296
Timestamp: 2025-07-21T19:11:04.077Z
Learning: In Newton's IK system, the JointLimitObjective class intentionally only handles joints where DOF count equals coordinate count (1-1 mapping). This is appropriate because joint limits are typically only meaningful for simple joint types like revolute and prismatic joints, not complex joints like ball or free joints where DOF count != coordinate count.
Applied to files:
docs/concepts/collisions.rst
📚 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:
docs/concepts/collisions.rst
📚 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
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- 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)
🔇 Additional comments (7)
docs/concepts/collisions.rst (7)
562-563: Verify particle collision support statement.Line 563 states: "CollisionPipelineUnified does not currently support particle (soft body) collisions." However, the shape compatibility table (rows 544-553, "Particle" row) shows empty cells for all shape types, suggesting particles don't interact with anything in the Unified pipeline. Clarify: do particles work with the Standard pipeline, and is this a known limitation of Unified? If so, consider recommending which pipeline to use for particle-heavy scenes.
1029-1050: Verify USD collision attribute names and scope.Lines 1043-1047 document USD custom attributes for collision properties (newton:contact_ke, newton:contact_kd, newton:contact_kf, newton:contact_ka, newton:contact_thickness). These should be cross-referenced with the actual USD import implementation. Additionally, verify that these attributes are applied per-shape (as shown in the example) and clarify whether they override ShapeConfig defaults or work alongside them.
1069-1104: Update SDFHydroelasticConfig import path.Line 1081 imports SDFHydroelasticConfig from
newton.geometry, but according to the AI summary, this class should be part of the public API. Verify that this import path is correct and accessible, or clarify whether it's an internal implementation detail that should not be documented in the public API reference.
852-865: Document contact point ordering and matching.The Contacts class documents
rigid_contact_point0andrigid_contact_point1on lines 858-859, and similarly for soft contacts. The documentation should clarify: are these always in the same order relative torigid_contact_shape0andrigid_contact_shape1? That is, isrigid_contact_point0always onrigid_contact_shape0andrigid_contact_point1onrigid_contact_shape1? This is important for correct contact handling in the solver.Also applies to: 873-885
934-974: Clarify hydroelastic contact point distribution and reduction interaction.Lines 965-969 explain hydroelastic contact generation (SDF intersection → marching cubes → point distribution → optional reduction). However, the interaction with contact reduction parameters in SDFHydroelasticConfig (lines 632-685) could be more explicit. Does contact reduction always apply to hydroelastic contacts? What is the relationship between
reduce_contactsand the distributed points? Clarify whether contact reduction is a post-processing step after marching cubes.
122-138: Verify multi-world API usage in example.The multi-world example (lines 122-138) uses
builder.add_world(robot_builder)to instantiate separate worlds. Verify that this API exists and that callingadd_worldtwice indeed creates world index 0 and 1 respectively. The example is important for RL use cases, so ensure the API is documented correctly and that the world assignment is deterministic.
754-800: Verify ShapeConfig parameter names and availability.Lines 754-760 and 793-800 use ShapeConfig parameters:
collision_group,thickness,contact_margin,sdf_max_resolution,is_hydroelastic,k_hydro,ke,kd,mu,is_hydroelastic. These should be cross-referenced with the actualModelBuilder.ShapeConfigclass definition to ensure:
- All parameter names are correct
- Default values match documentation
- Parameters are available in the public API
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/concepts/collisions.rst (1)
557-557: Clarify Ellipsoid and ConvexMesh table representation.This statement mentions "Ellipsoid and ConvexMesh are also fully supported" but neither type appears in the compatibility tables above (lines 295-402, 445-554). Consider either adding them to the tables or clarifying here that they follow the same rules as Box/Mesh respectively.
Suggested clarification
-Ellipsoid and ConvexMesh are also fully supported. The only unsupported type is ``HFIELD`` (heightfield) - convert to mesh instead. +Ellipsoid and ConvexMesh are also fully supported (Ellipsoid follows Box collision rules, ConvexMesh follows Mesh rules). The only unsupported type is ``HFIELD`` (heightfield) - convert to mesh instead.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/concepts/collisions.rstnewton/_src/sim/collide_unified.py
🚧 Files skipped from review as they are similar to previous changes (1)
- newton/_src/sim/collide_unified.py
🧰 Additional context used
🧠 Learnings (10)
📓 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-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:
docs/concepts/collisions.rst
📚 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.rst
📚 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:
docs/concepts/collisions.rst
📚 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:
docs/concepts/collisions.rst
📚 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:
docs/concepts/collisions.rst
📚 Learning: 2025-07-21T19:11:04.077Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 450
File: newton/sim/ik.py:1286-1296
Timestamp: 2025-07-21T19:11:04.077Z
Learning: In Newton's IK system, the JointLimitObjective class intentionally only handles joints where DOF count equals coordinate count (1-1 mapping). This is appropriate because joint limits are typically only meaningful for simple joint types like revolute and prismatic joints, not complex joints like ball or free joints where DOF count != coordinate count.
Applied to files:
docs/concepts/collisions.rst
📚 Learning: 2025-12-12T08:46:29.875Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1221
File: newton/examples/example_sdf.py:189-191
Timestamp: 2025-12-12T08:46:29.875Z
Learning: In Newton, SDF data structures are shared across replicated world instances (e.g., when using ModelBuilder.replicate()). Therefore, the memory cost of SDF generation scales with sdf_max_resolution but does not scale with the number of worlds—100 worlds with sdf_max_resolution=512 uses the same SDF memory as 1 world with sdf_max_resolution=512.
Applied to files:
docs/concepts/collisions.rst
📚 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:
docs/concepts/collisions.rst
📚 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
⏰ 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 Benchmarks / Run GPU Benchmarks on AWS EC2
- GitHub Check: Run GPU Tests / Run GPU Unit Tests on AWS EC2
- GitHub Check: run-newton-tests / newton-unittests (windows-latest)
- GitHub Check: run-newton-tests / newton-unittests (ubuntu-latest)
🔇 Additional comments (10)
docs/concepts/collisions.rst (10)
6-38: Clear pipeline introduction with good examples.The dual-pipeline architecture (Standard vs. Unified) is well-explained with practical usage examples showing both default and explicit pipeline creation patterns.
75-79: Excellent clarification about SDF.This note effectively prevents confusion by clearly stating that SDF is a collision optimization option (via
sdf_max_resolutionorsdf_target_voxel_size) rather than a standalone shape type. The explanation of O(1) distance queries and efficiency benefits is helpful.
115-146: Clear explanation of multi-world collision isolation.The world indices feature is well-documented with a practical example showing how to instantiate multiple robot instances that don't collide with each other. The distinction between global entities (index -1) and world-specific entities (0, 1, 2, ...) is clear.
Based on learnings, this aligns with Newton's design where shapes with body index -1 automatically skip mutual collisions.
605-616: Critical performance warning with actionable solution.The warning about mesh-mesh BVH fallback being "significantly slower" is essential, and the code example provides a clear solution. This will help users avoid a common performance pitfall.
934-974: Well-structured opt-in feature documentation.The documentation clearly distinguishes between default behavior (hard SDF point contacts) and opt-in hydroelastic behavior (distributed contact areas). The requirements are explicit, and the example code demonstrates proper configuration. The harmonic mean formula for effective stiffness is a helpful detail.
983-1037: Comprehensive material properties reference.This table provides an excellent mapping between material properties, applicable solvers, configuration fields, and runtime arrays. The solver applicability column helps users understand which properties are relevant for their chosen solver.
1072-1085: Actionable performance guidance.The performance section provides clear decision criteria for pipeline/mode selection:
- EXPLICIT/standard for limited pairs (<100 shapes)
- SAP for >100 shapes with spatial coherence
- NxN for small/uniform scenes
The additional tips about minimizing global entities, using positive collision groups, and tuning contact limits are practical and useful.
1086-1125: Complete API reference section.The See Also section provides a comprehensive list of relevant imports, classes, methods, and model attributes. The organization into Imports, API Reference, Model attributes, and Related documentation makes this an effective reference.
650-663: Verify beta scoring formulas against implementation.The beta scoring formulas documented in lines 650-663 are specific, but their accuracy and the definitions of
spatial_positionanddepthshould be confirmed against the actual implementation in the contact reduction code. While line 662 acknowledges this is "subject to refinement," clarifying whether the formulas and variable names match the implementation would improve documentation accuracy.
731-733: Verify the contact generation formula against the solver implementation.The documented formula
d < max(margin_a, margin_b)may not fully represent the actual contact generation behavior. Evidence suggests the solver creates contacts regardless of the threshold and delegates filtering to the caller. Confirm this formula accurately reflects what the implementation does.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/concepts/collisions.rst (1)
719-723: Consider clarifying thecontact_marginconstraint.The documentation states "Must be >=
thickness" but doesn't explain what happens if this constraint is violated. Consider adding a brief note about the behavior (e.g., whether it's clamped, raises an error, or causes incorrect collision detection).
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/concepts/collisions.rstnewton/_src/sim/collide_unified.py
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: shi-eric
Repo: newton-physics/newton PR: 521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The Newton migration guide (docs/migration.rst) is specifically for documenting how to migrate existing warp.sim functionality to Newton equivalents. New Newton-only features that didn't exist in warp.sim do not need migration documentation.
📚 Learning: 2025-08-12T05:17:34.423Z
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.
Applied to files:
newton/_src/sim/collide_unified.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/sim/collide_unified.pydocs/concepts/collisions.rst
📚 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:
docs/concepts/collisions.rst
📚 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.rst
📚 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:
docs/concepts/collisions.rst
📚 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:
docs/concepts/collisions.rst
📚 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:
docs/concepts/collisions.rst
📚 Learning: 2025-07-21T19:11:04.077Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 450
File: newton/sim/ik.py:1286-1296
Timestamp: 2025-07-21T19:11:04.077Z
Learning: In Newton's IK system, the JointLimitObjective class intentionally only handles joints where DOF count equals coordinate count (1-1 mapping). This is appropriate because joint limits are typically only meaningful for simple joint types like revolute and prismatic joints, not complex joints like ball or free joints where DOF count != coordinate count.
Applied to files:
docs/concepts/collisions.rst
📚 Learning: 2025-12-12T08:46:29.875Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1221
File: newton/examples/example_sdf.py:189-191
Timestamp: 2025-12-12T08:46:29.875Z
Learning: In Newton, SDF data structures are shared across replicated world instances (e.g., when using ModelBuilder.replicate()). Therefore, the memory cost of SDF generation scales with sdf_max_resolution but does not scale with the number of worlds—100 worlds with sdf_max_resolution=512 uses the same SDF memory as 1 world with sdf_max_resolution=512.
Applied to files:
docs/concepts/collisions.rst
📚 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
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run GPU Benchmarks / Run GPU Benchmarks on AWS EC2
- GitHub Check: Run GPU Tests / Run GPU Unit Tests on AWS EC2
- GitHub Check: run-newton-tests / newton-unittests (windows-latest)
🔇 Additional comments (15)
newton/_src/sim/collide_unified.py (2)
273-282: LGTM! Docstring accurately reflects the implementation.The updated docstring correctly describes the key features:
- GJK/MPR is handled via
NarrowPhase(line 397)- Multiple broad phase modes are implemented (lines 350-376)
- SDF hydroelastic integration exists (line 405, 479)
- Contact matching support via
sdf_hydroelasticconfiguration
436-459: LGTM! Return type documentation now correctly statesCollisionPipelineUnified.The return type annotation on line 436 is
CollisionPipelineUnifiedand the docstring now matches at line 458. This fixes a previous inconsistency where it likely saidCollisionPipeline.docs/concepts/collisions.rst (13)
6-37: LGTM! Clear introduction with accurate code examples.The dual-pipeline overview and basic usage examples accurately reflect the API design shown in
collide_unified.py. The import paths and method signatures are correct.
46-79: LGTM! Shape types documentation is clear and accurate.The table correctly lists the supported geometry types, and the notes about HFIELD (not implemented) and SDF (collision option, not shape type) provide important clarifications for users.
94-101: LGTM! Transform computation matches implementation.The world-space transform formula
body_q[shape_body] * shape_transform[shape_id]correctly matches the kernel implementation incompute_shape_aabbs(lines 199-202 incollide_unified.py).
147-189: LGTM! Collision group rules are clearly documented.The table provides an excellent reference for understanding collision group behavior. The rules (group 0 disables, positive groups collide with same or negative, negative groups collide with everything except same negative) are well-explained.
295-404: LGTM! Standard pipeline shape compatibility table is well-structured.The compatibility matrix clearly shows which shape pairs are supported in the standard pipeline, with appropriate notes about unsupported combinations (Cylinder, Cone, SDF) directing users to
CollisionPipelineUnified.
559-563: Important limitation clearly documented.The note about
CollisionPipelineUnifiednot supporting particle (soft body) collisions is critical for users choosing between pipelines. The recommendation to use the standard pipeline for particle-shape contacts is helpful.
605-616: Good warning about mesh-mesh performance.The warning about SDF not being precomputed leading to slow BVH distance queries is important guidance. The code example showing how to enable SDF via
sdf_max_resolutionprovides a clear solution.
802-815: LGTM! Useful performance pattern documented.The example showing collision detection running less frequently than simulation substeps is a valuable optimization pattern. This aligns with the design choice mentioned in the retrieved learnings about computing contacts once per frame and reusing across substeps.
901-928: LGTM! Parameters are accurately documented.The
Model.collide()parameters table correctly reflects the available options with appropriate default values and descriptions.
929-974: LGTM! Hydroelastic contacts section is comprehensive.The documentation clearly explains:
- Opt-in nature (both shapes need
is_hydroelastic=True)- Requirements (SDF enabled, volumetric shapes)
- How the algorithm works (SDF intersection → marching cubes → contact distribution)
- Stiffness computation (harmonic mean)
983-1037: LGTM! Contact materials table is well-organized.The table effectively documents each material property with:
- Description
- Which solvers support it
- ShapeConfig attribute name
- Model array name
This cross-referencing helps users understand the full data flow.
1072-1084: LGTM! Performance recommendations are practical and actionable.The performance tips provide clear guidance for choosing between pipeline modes based on scene complexity, with specific thresholds (e.g., <100 shapes) that help users make informed decisions.
1089-1101: No action required—the import path is correct.The documentation correctly shows
from newton.geometry import SDFHydroelasticConfig. This class is properly re-exported innewton/geometry.py(line 36) and included in its__all__list (line 45), making it a valid public API import.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @docs/concepts/collisions.rst:
- Line 8: Replace the incorrect capitalization "MJWarp" with the official type
alias capitalization "MjWarp" in the documentation occurrences (the sentence
about Newton's collision system and the other occurrence), so the docs match the
codebase type aliases such as MjWarpData and MjWarpModel; ensure the string is
exactly "MjWarp".
🧹 Nitpick comments (1)
docs/concepts/collisions.rst (1)
821-837: Clarify which pipeline supports soft contacts.This example shows soft contact (particle-shape) usage, but line 567 states that
CollisionPipelineUnifieddoes not support particle collisions. Add a note here clarifying that soft contacts require the standardCollisionPipeline(which is the default when no pipeline is specified).📝 Suggested clarification
**Soft contacts (particle-shape)** -Soft contacts are generated automatically when particles are present. They use a separate margin: +Soft contacts are generated automatically when particles are present and using the standard +CollisionPipeline (default). They use a separate margin:
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/concepts/collisions.rst
🧰 Additional context used
🧠 Learnings (10)
📓 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-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:
docs/concepts/collisions.rst
📚 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:
docs/concepts/collisions.rst
📚 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.rst
📚 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:
docs/concepts/collisions.rst
📚 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:
docs/concepts/collisions.rst
📚 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:
docs/concepts/collisions.rst
📚 Learning: 2025-07-21T19:11:04.077Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 450
File: newton/sim/ik.py:1286-1296
Timestamp: 2025-07-21T19:11:04.077Z
Learning: In Newton's IK system, the JointLimitObjective class intentionally only handles joints where DOF count equals coordinate count (1-1 mapping). This is appropriate because joint limits are typically only meaningful for simple joint types like revolute and prismatic joints, not complex joints like ball or free joints where DOF count != coordinate count.
Applied to files:
docs/concepts/collisions.rst
📚 Learning: 2025-12-12T08:46:29.875Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1221
File: newton/examples/example_sdf.py:189-191
Timestamp: 2025-12-12T08:46:29.875Z
Learning: In Newton, SDF data structures are shared across replicated world instances (e.g., when using ModelBuilder.replicate()). Therefore, the memory cost of SDF generation scales with sdf_max_resolution but does not scale with the number of worlds—100 worlds with sdf_max_resolution=512 uses the same SDF memory as 1 world with sdf_max_resolution=512.
Applied to files:
docs/concepts/collisions.rst
📚 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
⏰ 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 (7)
docs/concepts/collisions.rst (7)
6-38: Excellent documentation structure.The introduction clearly establishes the dual-pipeline architecture (Standard vs. Unified) with clear guidance on when to use each. The basic usage examples are concise and immediately actionable.
299-407: Clear and comprehensive compatibility tables.The shape compatibility tables effectively communicate the differences between Standard and Unified pipelines. The use of checkmarks (✅) for supported pairs and empty cells for unsupported pairs is intuitive. The note directing users to the Unified pipeline for advanced features is helpful.
933-979: Well-documented hydroelastic contacts feature.This section clearly explains the opt-in nature of hydroelastic contacts, the requirements for enabling them, and the practical benefits. The inclusion of the harmonic mean formula for
k_effis a valuable technical detail.
152-189: Excellent collision group rules reference.The collision group rules table with explicit examples is very helpful. The use of checkmarks (✅) and crosses (❌) with clear reasoning makes the group interaction logic easy to understand.
1076-1089: Concise and actionable performance guidance.The performance recommendations are clear and practical. The specific thresholds (<100 shapes) help users make informed decisions about which broad-phase algorithm to use.
609-620: Critical performance warning appropriately highlighted.The warning about mesh-mesh collision performance without SDF is crucial information that could save users significant debugging time. The code example showing how to enable SDF is immediately actionable.
1-1129: Comprehensive and well-structured collision documentation.This documentation rewrite successfully achieves the PR objectives. The dual-pipeline structure is clearly explained, shape compatibility is well-documented through tables, and advanced features (SDF, hydroelastic) are thoroughly covered. The addition of USD integration notes, performance guidance, and practical examples makes this a valuable reference for users.
The documentation is internally consistent and appropriately flags experimental features and limitations. Great work on this comprehensive rewrite!
|
@preist-nvidia there are a couple of API design questions here. Somehow I can't reply to the last few points where you tagged me. I think we should handle those separately in the architecture meetings. For now what @nvtw has documented I think is accurate. |
|
@preist-nvidia @eric-heiden - all your comment should be addressed. Should we come to an end on this MR and put future improvements into a new MR? It got messy and time consuming to answer all of the threads which are probably close to 100 by now. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @docs/concepts/collisions.rst:
- Around line 349-354: The note overstates a firm replacement plan for
CollisionPipeline by saying CollisionPipelineUnified "will replace" it; soften
the language to reflect intent rather than a committed timeline by rewording to
something like "Development is focused on CollisionPipelineUnified; it is
intended to eventually replace CollisionPipeline once feature and performance
parity is achieved." Update the text referencing CollisionPipelineUnified,
CollisionPipeline, and BroadPhaseMode.EXPLICIT accordingly, and if you later
want to signal a formal deprecation follow the project pattern used for
CollisionHandler and compute_world_offsets (DeprecationWarning decorators)
rather than asserting a definite replacement now.
- Line 8: Remove the incorrect examples/mjwarp/ reference and clarify that SDF
and hydroelastic are features of Newton's collision system, not of MJWarp;
update the sentence mentioning "Newton's collision system" and "MJWarp" to state
that MJWarp is a GPU-accelerated MuJoCo backend for contact conversion, that
SDF/hydroelastic are Newton features, and either remove the examples/mjwarp/
path or replace it with the correct example location (or a generic "see
examples" pointer) so the docs no longer reference a non-existent directory.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/concepts/collisions.rstnewton/_src/sim/collide_unified.py
🚧 Files skipped from review as they are similar to previous changes (1)
- newton/_src/sim/collide_unified.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
CLI arguments must use
kebab-case(e.g.,--use-cuda-graph, not--use_cuda_graph)
Files:
docs/concepts/collisions.rst
🧠 Learnings (14)
📓 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-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:
docs/concepts/collisions.rst
📚 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:
docs/concepts/collisions.rst
📚 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.rst
📚 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:
docs/concepts/collisions.rst
📚 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:
docs/concepts/collisions.rst
📚 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:
docs/concepts/collisions.rst
📚 Learning: 2025-07-21T19:11:04.077Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 450
File: newton/sim/ik.py:1286-1296
Timestamp: 2025-07-21T19:11:04.077Z
Learning: In Newton's IK system, the JointLimitObjective class intentionally only handles joints where DOF count equals coordinate count (1-1 mapping). This is appropriate because joint limits are typically only meaningful for simple joint types like revolute and prismatic joints, not complex joints like ball or free joints where DOF count != coordinate count.
Applied to files:
docs/concepts/collisions.rst
📚 Learning: 2025-12-12T08:46:29.875Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1221
File: newton/examples/example_sdf.py:189-191
Timestamp: 2025-12-12T08:46:29.875Z
Learning: In Newton, SDF data structures are shared across replicated world instances (e.g., when using ModelBuilder.replicate()). Therefore, the memory cost of SDF generation scales with sdf_max_resolution but does not scale with the number of worlds—100 worlds with sdf_max_resolution=512 uses the same SDF memory as 1 world with sdf_max_resolution=512.
Applied to files:
docs/concepts/collisions.rst
📚 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: Repo: newton-physics/newton — VBD damping semantics
- Joint/cable damping in SolverVBD is Rayleigh-style and treated as unitless; same convention applies across VBD constraints.
Applied to files:
docs/concepts/collisions.rst
📚 Learning: 2025-12-12T17:41:15.097Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1221
File: newton/_src/sim/builder.py:5556-5587
Timestamp: 2025-12-12T17:41:15.097Z
Learning: newton._src.geometry.sdf_utils.compute_sdf accepts max_resolution=None (e.g., when sdf_target_voxel_size is provided), so passing None from ModelBuilder.finalize is valid and should not be flagged.
Applied to files:
docs/concepts/collisions.rst
📚 Learning: 2025-12-03T09:33:27.083Z
Learnt from: camevor
Repo: newton-physics/newton PR: 1019
File: newton/_src/solvers/kamino/examples/sim/example_sim_basics_cartpole.py:388-392
Timestamp: 2025-12-03T09:33:27.083Z
Learning: In the Kamino ModelBuilder class located at newton/_src/solvers/kamino/core/builder.py, the attributes `gravity`, `bodies`, `joints`, `collision_geoms`, and `physical_geoms` are implemented as property decorated methods, so they should be accessed as attributes (e.g., `builder.gravity`) rather than called as methods (e.g., `builder.gravity()`).
Applied to files:
docs/concepts/collisions.rst
📚 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:
docs/concepts/collisions.rst
📚 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
🔇 Additional comments (4)
docs/concepts/collisions.rst (4)
1-1186: Comprehensive collision documentation rewrite looks excellent.This is a thorough and well-structured rewrite of the collision detection documentation. The dual-pipeline approach is clearly explained, compatibility tables are comprehensive, and the numerous code examples make concepts concrete.
Strengths:
- Clear differentiation between Standard and Unified pipelines with guidance on when to use each
- Comprehensive filtering documentation (world indices, collision groups, USD integration)
- Detailed shape configuration tables with all parameters documented
- Practical "Common Patterns" section with real-world examples
- Good performance guidance and warnings (especially mesh-mesh without SDF)
- Extensive cross-references to API documentation
The documentation successfully addresses the PR objectives and provides users with the information needed to effectively use Newton's collision system.
620-624: Particle collision support status is accurate.The documentation correctly states that CollisionPipelineUnified does not currently support particle (soft body) collisions. The standard CollisionPipeline does support them—its
collide()method explicitly launches thecreate_soft_contactskernel when particles and shapes are present. CollisionPipelineUnified'scollide()method only handles rigid (shape-to-shape) collisions through its narrow phase. The note about the unified pipeline being under active development is appropriate context.
699-699: Fix incorrect imports for SDFHydroelasticConfig.Lines 699 and 709 attempt to import
SDFHydroelasticConfigfrom the top-levelnewtonmodule, but it is not exported there. The class is defined innewton/_src/geometry/sdf_hydroelastic.pyand exposed through the public API atnewton.geometry.The correct import is:
from newton.geometry import SDFHydroelasticConfigThis matches the existing working examples in the codebase and is consistent with line 1162. Additionally, update the Sphinx reference on line 695 from
:class:~newton.SDFHydroelasticConfigto `:class:`~newton.geometry.SDFHydroelasticConfig.Without these fixes, the code examples at lines 699 and 709 would fail with an ImportError at runtime.
⛔ Skipped due to learnings
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).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.Learnt from: CR Repo: newton-physics/newton PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-01-13T07:40:13.624Z Learning: Applies to newton/examples/**/*.py : Examples must not import from `newton._src`, only from the public Newton APILearnt from: daniela-hase Repo: newton-physics/newton PR: 1044 File: newton/_src/sensors/tiled_camera_sensor.py:224-256 Timestamp: 2025-11-07T01:42:36.906Z Learning: In newton/_src/sensors/tiled_camera_sensor.py, the `# noqa: PLC0415` directives on the local PIL imports (lines 224 and 256 in save_color_image and save_depth_image methods) should be kept because they are required by the pre-commit configuration. The local imports are intentional for optional dependency handling.Learnt from: CR Repo: newton-physics/newton PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-01-13T07:40:13.624Z Learning: Applies to newton/_src/**/*.py : The `newton/_src/` directory is internal library implementation only and must not be imported by user code (examples, documentation, or external users)
142-143: Confirm the MJWarp heterogeneous environment limitation is accurately documented.The note correctly states that MJWarp (via SolverMuJoCo) does not support heterogeneous environments. The solver explicitly validates and enforces homogeneous worlds, requiring all simulation worlds to have identical body, joint, shape, and constraint counts with matching types.
|
Yes ok to rework when we have the collision library further along - but please make sure you record the open API questions and any follow-up issues where applicable when you resolve the threads before merging. |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
4 participants
Description
Rewrite and extend collision detection documentation
I would recommend to check out the branch, build the html and then review the html file
The MR should cover issues #853, #1038 and #1308
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
✏️ Tip: You can customize this high-level summary in your review settings.