change all occurences of wp.inf to MAXVAL #1391
Conversation
Signed-off-by: Alain Denzler <[email protected]>
Signed-off-by: Alain Denzler <[email protected]>
Signed-off-by: Alain Denzler <[email protected]>
Signed-off-by: Alain Denzler <[email protected]>
Signed-off-by: Alain Denzler <[email protected]>
📝 WalkthroughWalkthroughAdds a finite sentinel constant MAXVAL (1e10), exposes it in the core public API, and replaces prior uses of mathematical infinity (wp.inf or JOINT_LIMIT_UNLIMITED) with MAXVAL across collision, geometry/SDF, raytracing, sensors, viewer, sim/joints, solver, builder, and tests. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
newton/_src/viewer/viewer.py (1)
270-270: Validity mask will not work correctly with MAXVAL.The code initializes bounds with
MAXVAL(1e10) but then usesnp.isinf()to detect invalid bounds. SinceMAXVALis finite,np.isinf()will always returnFalse, causing the mask to incorrectly treat all worlds as valid.Proposed fix
# Find maximum extents across all worlds - # Mask out invalid bounds (inf values) - valid_mask = ~np.isinf(bounds_min_np[:, 0]) + # Mask out invalid bounds (MAXVAL sentinel values) + valid_mask = bounds_min_np[:, 0] < MAXVAL * 0.99
🧹 Nitpick comments (2)
newton/_src/geometry/sdf_hydroelastic.py (1)
23-24: Reduce duplication of the MAXVAL validity threshold.Both overloads repeat
MAXVAL * 0.99; consider a shared constant to keep the threshold consistent and easier to tune.♻️ Proposed refactor
MIN_FRICTION = 1e-4 EPS_LARGE = 1e-8 EPS_SMALL = 1e-20 +SDF_VALID_LIMIT = MAXVAL * 0.99 @@ - is_valid = not ( - valA >= wp.static(MAXVAL * 0.99) or wp.isnan(valA) or valB >= wp.static(MAXVAL * 0.99) or wp.isnan(valB) - ) + is_valid = not ( + valA >= wp.static(SDF_VALID_LIMIT) or wp.isnan(valA) or valB >= wp.static(SDF_VALID_LIMIT) or wp.isnan(valB) + ) @@ - is_valid = not ( - valA >= wp.static(MAXVAL * 0.99) or wp.isnan(valA) or valB >= wp.static(MAXVAL * 0.99) or wp.isnan(valB) - ) + is_valid = not ( + valA >= wp.static(SDF_VALID_LIMIT) or wp.isnan(valA) or valB >= wp.static(SDF_VALID_LIMIT) or wp.isnan(valB) + )Also applies to: 968-970, 1002-1004
newton/_src/sensors/warp_raytrace/ray.py (1)
188-188: Consider using MAXVAL for internal sentinel consistency.The internal
min_t = 1.0e10sentinel at lines 188 and 311 happens to equal MAXVAL, but using the named constant would improve maintainability. The boundary checks (min_t < 1.0e9) also use a magic number.This is optional since the current logic is correct and the internal sentinel is distinct from the return value.
Optional refactor for consistency
- min_t = 1.0e10 + min_t = MAXVAL ... - if min_t < 1.0e9: + if min_t < MAXVAL * 0.99:Also applies to: 311-311
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
newton/_src/core/__init__.pynewton/_src/core/types.pynewton/_src/geometry/collision_primitive.pynewton/_src/geometry/kernels.pynewton/_src/geometry/narrow_phase.pynewton/_src/geometry/sdf_contact.pynewton/_src/geometry/sdf_hydroelastic.pynewton/_src/geometry/sdf_mc.pynewton/_src/geometry/sdf_utils.pynewton/_src/sensors/sensor_tiled_camera.pynewton/_src/sensors/warp_raytrace/bvh.pynewton/_src/sensors/warp_raytrace/lighting.pynewton/_src/sensors/warp_raytrace/ray.pynewton/_src/sensors/warp_raytrace/ray_cast.pynewton/_src/viewer/viewer.pynewton/tests/test_collision_primitives.py
🧰 Additional context used
📓 Path-based instructions (4)
newton/_src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
The
newton/_src/directory is internal library implementation only and must not be imported by user code (examples, documentation, or external users)
Files:
newton/_src/core/__init__.pynewton/_src/sensors/sensor_tiled_camera.pynewton/_src/geometry/narrow_phase.pynewton/_src/geometry/sdf_hydroelastic.pynewton/_src/geometry/kernels.pynewton/_src/core/types.pynewton/_src/geometry/sdf_contact.pynewton/_src/geometry/collision_primitive.pynewton/_src/geometry/sdf_utils.pynewton/_src/sensors/warp_raytrace/lighting.pynewton/_src/sensors/warp_raytrace/ray.pynewton/_src/sensors/warp_raytrace/bvh.pynewton/_src/geometry/sdf_mc.pynewton/_src/sensors/warp_raytrace/ray_cast.pynewton/_src/viewer/viewer.py
newton/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Any user-facing class/function/object added under
_srcmust be exposed via the public Newton API through re-exports
Files:
newton/_src/core/__init__.pynewton/_src/sensors/sensor_tiled_camera.pynewton/_src/geometry/narrow_phase.pynewton/_src/geometry/sdf_hydroelastic.pynewton/_src/geometry/kernels.pynewton/_src/core/types.pynewton/tests/test_collision_primitives.pynewton/_src/geometry/sdf_contact.pynewton/_src/geometry/collision_primitive.pynewton/_src/geometry/sdf_utils.pynewton/_src/sensors/warp_raytrace/lighting.pynewton/_src/sensors/warp_raytrace/ray.pynewton/_src/sensors/warp_raytrace/bvh.pynewton/_src/geometry/sdf_mc.pynewton/_src/sensors/warp_raytrace/ray_cast.pynewton/_src/viewer/viewer.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use prefix-first naming for classes:ActuatorPD,ActuatorPID(notPDActuator,PIDActuator)
Use prefix-first naming for methods:add_shape_sphere()(notadd_sphere_shape())
Method names must usesnake_case
Prefer nested classes when self-contained: if a helper type or enum is only meaningful inside one parent class and doesn't need a public identity, define it as a nested class instead of a top-level class/module
Follow PEP 8 for Python code
Use Google-style docstrings with clear, concise explanations of what functions do, their parameters, and return values
Files:
newton/_src/core/__init__.pynewton/_src/sensors/sensor_tiled_camera.pynewton/_src/geometry/narrow_phase.pynewton/_src/geometry/sdf_hydroelastic.pynewton/_src/geometry/kernels.pynewton/_src/core/types.pynewton/tests/test_collision_primitives.pynewton/_src/geometry/sdf_contact.pynewton/_src/geometry/collision_primitive.pynewton/_src/geometry/sdf_utils.pynewton/_src/sensors/warp_raytrace/lighting.pynewton/_src/sensors/warp_raytrace/ray.pynewton/_src/sensors/warp_raytrace/bvh.pynewton/_src/geometry/sdf_mc.pynewton/_src/sensors/warp_raytrace/ray_cast.pynewton/_src/viewer/viewer.py
**/*
📄 CodeRabbit inference engine (AGENTS.md)
CLI arguments must use
kebab-case(e.g.,--use-cuda-graph, not--use_cuda_graph)
Files:
newton/_src/core/__init__.pynewton/_src/sensors/sensor_tiled_camera.pynewton/_src/geometry/narrow_phase.pynewton/_src/geometry/sdf_hydroelastic.pynewton/_src/geometry/kernels.pynewton/_src/core/types.pynewton/tests/test_collision_primitives.pynewton/_src/geometry/sdf_contact.pynewton/_src/geometry/collision_primitive.pynewton/_src/geometry/sdf_utils.pynewton/_src/sensors/warp_raytrace/lighting.pynewton/_src/sensors/warp_raytrace/ray.pynewton/_src/sensors/warp_raytrace/bvh.pynewton/_src/geometry/sdf_mc.pynewton/_src/sensors/warp_raytrace/ray_cast.pynewton/_src/viewer/viewer.py
🧠 Learnings (29)
📚 Learning: 2026-01-13T07:40:13.635Z
Learnt from: CR
Repo: newton-physics/newton PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T07:40:13.635Z
Learning: Applies to newton/**/*.py : Any user-facing class/function/object added under `_src` must be exposed via the public Newton API through re-exports
Applied to files:
newton/_src/core/__init__.pynewton/_src/geometry/narrow_phase.pynewton/_src/core/types.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/core/__init__.pynewton/_src/geometry/narrow_phase.pynewton/_src/geometry/kernels.pynewton/tests/test_collision_primitives.pynewton/_src/geometry/collision_primitive.py
📚 Learning: 2026-01-13T07:40:13.635Z
Learnt from: CR
Repo: newton-physics/newton PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T07:40:13.635Z
Learning: Applies to newton/examples/**/*.py : Examples must not import from `newton._src`, only from the public Newton API
Applied to files:
newton/_src/core/__init__.pynewton/_src/geometry/narrow_phase.py
📚 Learning: 2025-08-14T17:38:36.106Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/__init__.py:25-29
Timestamp: 2025-08-14T17:38:36.106Z
Learning: The Newton project prefers incremental __all__ building using __all__ += [...] pattern to group exports with their related imports, rather than a single consolidated __all__ at the end of the file.
Applied to files:
newton/_src/core/__init__.py
📚 Learning: 2025-08-12T18:04:06.287Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/_src/utils/import_usd.py:364-365
Timestamp: 2025-08-12T18:04:06.287Z
Learning: The quat_between_axes function in Newton accepts AxisType parameters, which includes strings like "X", "Y", "Z" as well as Axis enum values and integers. The function internally handles conversion using Axis.from_any(), so passing strings directly to quat_between_axes is valid and correct behavior.
Applied to files:
newton/_src/core/__init__.py
📚 Learning: 2025-08-12T18:04:06.287Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/_src/utils/import_usd.py:364-365
Timestamp: 2025-08-12T18:04:06.287Z
Learning: The quat_between_axes function in Newton accepts AxisType parameters, which includes both Axis enum values and strings. The function internally handles string-to-Axis conversion using Axis.from_any(), so passing strings directly to quat_between_axes is valid and correct.
Applied to files:
newton/_src/core/__init__.py
📚 Learning: 2026-01-13T03:11:40.556Z
Learnt from: jumyungc
Repo: newton-physics/newton PR: 1333
File: newton/_src/sim/builder.py:0-0
Timestamp: 2026-01-13T03:11:40.556Z
Learning: Ensure all VBD damping semantics follow Rayleigh-style damping and are unitless. This convention, demonstrated in SolverVBD, should be applied consistently across all VBD-related constraints in the codebase. In newton/_src/sim/builder.py, validate that any damping terms use the same normalization and are treated as dimensionless; add comments and unit tests to enforce consistency across VBD components.
Applied to files:
newton/_src/core/__init__.pynewton/_src/sensors/sensor_tiled_camera.pynewton/_src/geometry/narrow_phase.pynewton/_src/geometry/sdf_hydroelastic.pynewton/_src/geometry/kernels.pynewton/_src/core/types.pynewton/tests/test_collision_primitives.pynewton/_src/geometry/sdf_contact.pynewton/_src/geometry/collision_primitive.pynewton/_src/geometry/sdf_utils.pynewton/_src/sensors/warp_raytrace/lighting.pynewton/_src/sensors/warp_raytrace/ray.pynewton/_src/sensors/warp_raytrace/bvh.pynewton/_src/geometry/sdf_mc.pynewton/_src/sensors/warp_raytrace/ray_cast.pynewton/_src/viewer/viewer.py
📚 Learning: 2025-11-07T01:42:36.906Z
Learnt 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.
Applied to files:
newton/_src/sensors/sensor_tiled_camera.py
📚 Learning: 2025-12-12T17:45:26.847Z
Learnt from: vastsoun
Repo: newton-physics/newton PR: 1019
File: newton/_src/solvers/kamino/geometry/primitives.py:0-0
Timestamp: 2025-12-12T17:45:26.847Z
Learning: In newton/_src/solvers/kamino/geometry/primitives.py, the narrow-phase kernel `_primitive_narrowphase` does not need to handle symmetric shape-pair orderings (e.g., both SPHERE-BOX and BOX-SPHERE) because `kamino.core.builder.ModelBuilder` and `kamino.geometry.primitive.broadphase` guarantee that explicit candidate geometry pairs are always defined with GIDs in ascending order.
Applied to files:
newton/_src/geometry/narrow_phase.py
📚 Learning: 2025-10-24T07:56:14.792Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 981
File: newton/_src/geometry/collision_convex.py:180-289
Timestamp: 2025-10-24T07:56:14.792Z
Learning: In newton/_src/geometry/collision_convex.py, the single-contact solver (create_solve_convex_single_contact) intentionally returns count=1 even when shapes are separated (signed_distance > contact_threshold). The calling code is responsible for deciding whether to accept the contact based on the signed distance value. This design pushes filtering responsibility to the caller.
Applied to files:
newton/_src/geometry/narrow_phase.pynewton/_src/geometry/kernels.pynewton/tests/test_collision_primitives.pynewton/_src/geometry/sdf_contact.pynewton/_src/geometry/collision_primitive.py
📚 Learning: 2026-01-13T07:40:13.635Z
Learnt from: CR
Repo: newton-physics/newton PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T07:40:13.635Z
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)
Applied to files:
newton/_src/geometry/narrow_phase.py
📚 Learning: 2025-10-06T18:14:18.582Z
Learnt from: daedalus5
Repo: newton-physics/newton PR: 868
File: newton/examples/diffsim/example_diffsim_bear.py:82-96
Timestamp: 2025-10-06T18:14:18.582Z
Learning: Warp's tile operations (`wp.tile_store()` and `wp.tile_load()`) have built-in boundary checking. Out-of-bounds writes are automatically skipped and out-of-bounds reads are zero-filled, so tiled kernels are safe even when the array size is not a multiple of the tile size.
Applied to files:
newton/_src/geometry/narrow_phase.py
📚 Learning: 2025-09-26T05:58:21.013Z
Learnt from: WenchaoHuang
Repo: newton-physics/newton PR: 835
File: newton/_src/solvers/style3d/collision/collision.py:49-53
Timestamp: 2025-09-26T05:58:21.013Z
Learning: In newton/_src/solvers/style3d/collision/collision.py, the broad_phase buffers (broad_phase_ee, broad_phase_ef, broad_phase_vf) created with wp.array() don't need zero-initialization because the BVH query kernels that use them always write the count to query_results[0, tid] = 0 first before populating any data, ensuring no undefined reads occur.
Applied to files:
newton/_src/geometry/narrow_phase.pynewton/_src/sensors/warp_raytrace/bvh.pynewton/_src/viewer/viewer.py
📚 Learning: 2025-09-26T05:58:21.013Z
Learnt from: WenchaoHuang
Repo: newton-physics/newton PR: 835
File: newton/_src/solvers/style3d/collision/collision.py:49-53
Timestamp: 2025-09-26T05:58:21.013Z
Learning: In newton/_src/solvers/style3d/collision/collision.py, the broad_phase buffers (broad_phase_ee, broad_phase_ef, broad_phase_vf) created with wp.array() don't need zero-initialization because they follow a write-first-read-later pattern: BVH query kernels in frame_begin() populate these buffers completely (write-only operations), and collision kernels only read from them afterward in accumulate_contact_force(), ensuring no undefined reads occur.
Applied to files:
newton/_src/geometry/narrow_phase.pynewton/_src/geometry/sdf_hydroelastic.pynewton/_src/geometry/kernels.pynewton/_src/geometry/collision_primitive.pynewton/_src/sensors/warp_raytrace/bvh.py
📚 Learning: 2025-08-12T18:04:06.577Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/_src/solvers/featherstone/kernels.py:75-75
Timestamp: 2025-08-12T18:04:06.577Z
Learning: The Newton physics framework requires nightly Warp builds, which means compatibility concerns with older stable Warp versions (like missing functions such as wp.spatial_adjoint) are not relevant for this project.
Applied to files:
newton/_src/geometry/narrow_phase.pynewton/_src/geometry/sdf_hydroelastic.pynewton/_src/geometry/kernels.pynewton/_src/geometry/sdf_contact.pynewton/_src/sensors/warp_raytrace/lighting.pynewton/_src/sensors/warp_raytrace/bvh.py
📚 Learning: 2025-11-24T10:44:09.613Z
Learnt from: gdaviet
Repo: newton-physics/newton PR: 1108
File: newton/_src/solvers/implicit_mpm/rasterized_collisions.py:94-129
Timestamp: 2025-11-24T10:44:09.613Z
Learning: In Warp, wp.normalize() returns a zero vector (or zero quaternion) when given a zero-length input, so explicit zero-length checks before normalization are not required.
Applied to files:
newton/_src/geometry/narrow_phase.pynewton/tests/test_collision_primitives.pynewton/_src/sensors/warp_raytrace/lighting.pynewton/_src/sensors/warp_raytrace/ray.py
📚 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:
newton/_src/geometry/sdf_hydroelastic.pynewton/_src/geometry/sdf_contact.pynewton/_src/geometry/sdf_utils.pynewton/_src/geometry/sdf_mc.py
📚 Learning: 2025-09-22T21:08:31.901Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/examples/ik/example_ik_franka.py:121-123
Timestamp: 2025-09-22T21:08:31.901Z
Learning: In the newton physics framework, when creating warp arrays for IK solver joint variables using wp.array(self.model.joint_q, shape=(1, coord_count)), the resulting array acts as a reference/pointer to the original model's joint coordinates, so updates from the IK solver automatically reflect in the model's joint_q buffer used for rendering.
Applied to files:
newton/_src/geometry/sdf_hydroelastic.pynewton/_src/sensors/warp_raytrace/bvh.pynewton/_src/sensors/warp_raytrace/ray_cast.py
📚 Learning: 2025-09-22T21:08:31.901Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/examples/ik/example_ik_franka.py:121-123
Timestamp: 2025-09-22T21:08:31.901Z
Learning: In the newton physics framework, when creating warp arrays for IK solver joint variables using wp.array(self.model.joint_q, shape=(1, coord_count)), the resulting array acts as a reference/pointer to the original model's joint coordinates, so updates from the IK solver automatically reflect in the model's joint_q buffer used for rendering, despite the general warp documentation suggesting copies are made by default.
Applied to files:
newton/_src/geometry/sdf_hydroelastic.pynewton/_src/geometry/sdf_contact.pynewton/_src/sensors/warp_raytrace/bvh.pynewton/_src/sensors/warp_raytrace/ray_cast.py
📚 Learning: 2025-12-13T17:26:39.791Z
Learnt from: lenroe-nv
Repo: newton-physics/newton PR: 1248
File: newton/_src/geometry/contact_data.py:47-49
Timestamp: 2025-12-13T17:26:39.791Z
Learning: In ContactData (newton/_src/geometry/contact_data.py), the fields contact_stiffness, contact_damping, and contact_friction_scale use 0.0 as the "unset" sentinel value. This is intentional: Warp struct fields initialize to 0.0 by default, and solver-side code interprets 0.0 as "use default/unscaled" rather than "disable." This design avoids explicit initialization in contact generation kernels.
Applied to files:
newton/_src/geometry/sdf_hydroelastic.pynewton/_src/geometry/sdf_contact.pynewton/_src/geometry/collision_primitive.py
📚 Learning: 2025-08-12T23:27:11.727Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/_src/solvers/vbd/solver_vbd.py:1219-1223
Timestamp: 2025-08-12T23:27:11.727Z
Learning: ParticleFlags in the Newton codebase is an IntEnum (not IntFlag), and Warp can handle IntEnum directly in kernel code without requiring explicit casting to int.
Applied to files:
newton/_src/geometry/kernels.py
📚 Learning: 2025-10-10T10:47:41.082Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 899
File: newton/tests/test_rigid_contact.py:214-268
Timestamp: 2025-10-10T10:47:41.082Z
Learning: In `newton/tests/test_rigid_contact.py`, using `add_shape_plane(width=0, length=0)` for an infinite plane works correctly with the collision system and does not cause degenerate AABB issues in the test scenario `test_shape_collisions_gjk_mpr_multicontact`.
Applied to files:
newton/tests/test_collision_primitives.py
📚 Learning: 2025-08-12T17:58:16.929Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/tests/test_ik.py:25-26
Timestamp: 2025-08-12T17:58:16.929Z
Learning: In the Newton physics engine codebase, it's acceptable for tests to import and use private `_src` internal modules and functions when needed for testing purposes, even though this breaks the public API boundary. This is an intentional project decision for test code.
Applied to files:
newton/tests/test_collision_primitives.py
📚 Learning: 2025-09-25T16:14:22.033Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_objectives.py:0-0
Timestamp: 2025-09-25T16:14:22.033Z
Learning: In NVIDIA Warp's Newton physics library, wp.transform supports direct numerical indexing (e.g., body_tf[0], body_tf[1], body_tf[2] for position and body_tf[3], body_tf[4], body_tf[5], body_tf[6] for quaternion components) to access its elements. This is the standard API used throughout the codebase.
Applied to files:
newton/_src/geometry/sdf_contact.py
📚 Learning: 2025-08-20T03:30:16.037Z
Learnt from: eric-heiden
Repo: newton-physics/newton PR: 584
File: newton/_src/utils/import_urdf.py:159-167
Timestamp: 2025-08-20T03:30:16.037Z
Learning: In the URDF importer (newton/_src/utils/import_urdf.py), cylinders are intentionally created using add_shape_capsule() instead of add_shape_cylinder() because cylinder collisions are not fully supported yet. This is a deliberate workaround until proper cylinder collision support is implemented.
Applied to files:
newton/_src/geometry/collision_primitive.py
📚 Learning: 2025-08-12T17:52:15.009Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/_src/utils/import_mjcf.py:226-231
Timestamp: 2025-08-12T17:52:15.009Z
Learning: In the Newton codebase, when passing array-like objects (numpy arrays, lists, tuples) to wp.vec3(), the consistent pattern is to use the unpacking operator: wp.vec3(*array) rather than wp.vec3(array). This pattern is used throughout newton/_src/utils/import_mjcf.py and newton/_src/core/types.py.
Applied to files:
newton/_src/sensors/warp_raytrace/ray.pynewton/_src/geometry/sdf_mc.py
📚 Learning: 2025-12-01T17:00:28.889Z
Learnt from: camevor
Repo: newton-physics/newton PR: 1161
File: newton/_src/solvers/mujoco/kernels.py:1268-1268
Timestamp: 2025-12-01T17:00:28.889Z
Learning: MuJoCo Warp spatial vectors (e.g., cacc, cvel) use the convention (angular, linear), which is opposite to Newton's (linear, angular) convention. When converting from MuJoCo Warp to Newton format, spatial_top() on MuJoCo Warp data extracts the angular component, and spatial_bottom() extracts the linear component.
Applied to files:
newton/_src/sensors/warp_raytrace/ray.py
📚 Learning: 2025-09-09T10:29:35.521Z
Learnt from: gdaviet
Repo: newton-physics/newton PR: 750
File: newton/_src/solvers/implicit_mpm/rasterized_collisions.py:86-90
Timestamp: 2025-09-09T10:29:35.521Z
Learning: In Warp's mesh query system, wp.mesh_eval_face_normal returns face normals that point outward from the collider surface by definition. The query.sign from wp.mesh_query_point_sign_normal indicates inside/outside but should not be used to flip the face normal direction, as this would make outward-pointing normals point inward incorrectly.
Applied to files:
newton/_src/sensors/warp_raytrace/ray.py
📚 Learning: 2025-08-25T20:10:51.009Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 634
File: newton/examples/ik/example_ik_h1.py:0-0
Timestamp: 2025-08-25T20:10:51.009Z
Learning: In Warp, creating a new wp.array() with an existing array and different shape (e.g., wp.array(existing_array, shape=new_shape)) creates a view into the same underlying memory buffer, not a copy. This means modifications through either view are automatically reflected in both arrays.
Applied to files:
newton/_src/sensors/warp_raytrace/bvh.pynewton/_src/sensors/warp_raytrace/ray_cast.py
⏰ 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 (58)
newton/_src/geometry/sdf_utils.py (1)
21-21: LGTM! Appropriate sentinel value for SDF background.The switch from
wp.inftoMAXVALforSDF_BACKGROUND_VALUEis well-motivated. Using a finite sentinel ensures trilinear interpolation produces detectable large values for unallocated voxels while avoidingverify_fpfalse positives. The updated comment clearly documents the rationale.Also applies to: 55-58
newton/_src/sensors/sensor_tiled_camera.py (1)
24-24: LGTM! Correct sentinel pattern for bounds computation.The initialization of
min_point = wp.vec3(MAXVAL)andmax_point = wp.vec3(-MAXVAL)follows the standard pattern for computing AABB bounds via iterative min/max reduction. The sentinel values will be replaced by actual mesh extents when points exist, or remain as sentinels for empty meshes.Also applies to: 52-67
newton/_src/core/types.py (2)
192-207: LGTM! MAXVAL correctly added to__all__.The constant is properly exported, making it available to other modules that import from this types module.
68-76: LGTM! Well-documented sentinel constant with proper public API re-export.The
MAXVAL = 1e10definition with its comprehensive docstring provides clear guidance for users. The note about using>= wp.static(MAXVAL * 0.99)for volume-sampled data is helpful for avoiding interpolation-related edge cases. MAXVAL is correctly re-exported fromnewton.core, making it accessible to users as part of the public API.newton/_src/sensors/warp_raytrace/ray_cast.py (2)
18-19: LGTM! Appropriate sentinel for ray-hit distance initialization.Initializing
hit_disttoMAXVALbefore intersection testing ensures no false positives. The value will be replaced by actual hit distances when intersections occur.Also applies to: 80-80
324-324: LGTM! Consistent sentinel usage in first_hit_shape.The
distinitialization toMAXVALfollows the same pattern, ensuring that only actual intersections (withdist < max_dist) trigger a hit result.newton/_src/geometry/kernels.py (1)
2117-2119: No issues found.collide_plane_cylindercorrectly initializes invalid contacts toMAXVAL(1e10), consistent with all other primitive collision functions in the same module. The threshold change from1.0e5toMAXVALis correct and properly aligns the validity check with the actual sentinel values returned by the primitive function.newton/_src/sensors/warp_raytrace/lighting.py (1)
18-60: MAXVAL sentinel usage looks good here.Finite initialization for
dist_to_lightaligns with the PR-wide sentinel strategy and avoids inf usage in downstream comparisons.newton/_src/sensors/warp_raytrace/bvh.py (3)
18-32: Finite sentinel initialization for mesh bounds looks good.Using MAXVAL/-MAXVAL keeps bounds finite and consistent with the PR-wide sentinel change.
70-72: Finite sentinel initialization for box bounds looks good.Consistent MAXVAL-based initialization avoids wp.inf use and matches the new sentinel strategy.
122-124: Finite sentinel initialization for plane bounds looks good.Consistent MAXVAL usage keeps bounds finite even for large plane extents.
newton/_src/geometry/narrow_phase.py (1)
23-23: No action required. The import and usage ofMAXVALat line 23 and the second contact validation check at lines 379-383 are correct. Thecollide_capsule_capsulefunction explicitly initializes both contacts withMAXVAL(line 230 in collision_primitive.py) and only overwrites invalid contacts with actual distance values. The checkif dists[1] < MAXVAL:correctly identifies valid second contacts.Likely an incorrect or invalid review comment.
newton/_src/core/__init__.py (1)
28-36: MAXVAL correctly remains internal-only; no public re-export required.MAXVAL is a sentinel constant for internal use (markers for "no limit", "no hit", "invalid") and is not part of the user-facing API. It is properly scoped to
newton._src.core.__init__.pyand correctly excluded fromnewton/__init__.py. This follows the coding guidelines.newton/_src/viewer/viewer.py (2)
38-38: LGTM!The MAXVAL import is correctly added from the internal core.types module.
242-243: LGTM!Using
MAXVALand-MAXVALas initial sentinel values for bounds computation is consistent with the PR's objective to replace infinity with finite sentinels.newton/_src/geometry/sdf_mc.py (2)
30-31: LGTM!MAXVAL import is correctly placed.
188-192: LGTM!The depth validity check correctly uses
wp.static(MAXVAL * 0.99)to detect invalid/background samples. Usingwp.static()ensures the threshold is computed at kernel compilation time, and the 0.99 multiplier appropriately handles interpolation rounding as noted in the PR description.newton/_src/geometry/sdf_contact.py (4)
20-21: LGTM!MAXVAL import is correctly placed.
205-210: LGTM!The validity check correctly detects background samples using the MAXVAL threshold and falls back to the coarse grid. The pattern is consistent with other SDF sampling code in this PR.
213-214: LGTM!Comment accurately reflects that the epsilon shrinking is to avoid sampling the background value (rather than NaN).
265-271: LGTM!The gradient sampling validity check mirrors the non-gradient version, maintaining consistency.
newton/tests/test_collision_primitives.py (12)
22-22: LGTM!Importing from internal
_srcmodule is acceptable in test code. Based on learnings, this is an intentional project decision.
1334-1340: LGTM!The validity check correctly uses
MAXVAL * 0.99threshold to identify valid contacts, consistent with the new sentinel convention.
1416-1416: LGTM!Contact counting logic correctly updated to use MAXVAL threshold.
1429-1435: LGTM!Valid contact distance check is consistent with the new convention.
1599-1599: LGTM!Box-box contact counting uses correct MAXVAL threshold.
1609-1610: LGTM!Normal validation skip condition correctly uses MAXVAL threshold.
1737-1737: LGTM!Margin test contact counting is consistent.
1761-1762: LGTM!Normal validation skip uses correct threshold.
1845-1845: LGTM!Capsule-box valid distance filtering uses correct threshold.
1879-1880: LGTM!Contact validation skip condition is correct.
2116-2117: LGTM!Box-box penetration depth test contact filtering is consistent.
2168-2169: LGTM!Normal validation skip uses correct threshold.
newton/_src/sensors/warp_raytrace/ray.py (16)
20-21: LGTM!MAXVAL import correctly added.
71-71: LGTM!Quadratic solver returns MAXVAL for no valid solution, consistent with the new convention.
86-86: LGTM!Returns MAXVAL when both solutions are negative (ray pointing away).
104-104: LGTM!
ray_planecorrectly returns MAXVAL for all no-intersection cases.Also applies to: 108-109, 119-119
132-133: LGTM!
ray_plane_with_normalcorrectly checks against MAXVAL and returns appropriate sentinel.
161-162: LGTM!
ray_sphere_with_normalcorrectly handles no-hit case.
175-176: LGTM!
ray_capsulecorrectly uses MAXVAL for early-out bounding sphere test and final no-hit return.Also applies to: 183-183, 249-249
258-259: LGTM!
ray_capsule_with_normalcorrectly handles no-hit case.
302-303: LGTM!
ray_cylindercorrectly uses MAXVAL for bounding sphere early-out and t_hit initialization.Also applies to: 310-310
370-371: LGTM!
ray_cylinder_with_normalcorrectly handles no-hit case.
394-395: LGTM!
ray_conecorrectly returns MAXVAL for all no-intersection cases including degenerate cone.Also applies to: 440-440, 443-443, 449-449
460-461: LGTM!
ray_cone_with_normalcorrectly handles no-hit case.
500-501: LGTM!
ray_boxcorrectly uses MAXVAL for bounding sphere early-out and t_hit initialization.Also applies to: 507-507
543-544: LGTM!
ray_box_with_normalcorrectly handles no-hit case.
581-581: LGTM!
ray_meshcorrectly returns MAXVAL for no-hit case.
613-613: LGTM!
ray_mesh_with_bvhcorrectly returns MAXVAL for no-hit case.newton/_src/geometry/collision_primitive.py (9)
35-38: LGTM!The docstring accurately documents the new sentinel convention for unpopulated contact slots.
44-45: LGTM!The import correctly brings in
MAXVALfrom the internal types module.
226-232: LGTM!The initialization with
MAXVALcorrectly establishes the sentinel value for unpopulated contact slots, and the docstring accurately reflects this convention.
405-413: LGTM!The
wp.vec4(MAXVAL)initialization correctly sets all four contact distance slots to the sentinel value. The subsequent loop fills valid contacts while leaving unpopulated slots atMAXVAL.
544-546: LGTM!The initialization to
MAXVALis consistent with other collision functions, even though this function populates all four contact slots unconditionally.
673-684: LGTM!The explicit loop initialization ensures all 8 contact distance slots start at
MAXVAL. This correctly handles the multiple early-return paths where contacts may be partially or fully unpopulated.
1185-1190: LGTM!The docstring and the early return for invalid collision type correctly use
MAXVALas the sentinel value for unpopulated contact distances.Also applies to: 1361-1362
1492-1496: LGTM!When no second contact point exists,
dist2is correctly set toMAXVALto mark the slot as unpopulated.
44-45: MAXVAL threshold is appropriate and consistently applied downstream.Verification confirms that all code paths properly check for invalid contacts using
MAXVAL * 0.99(for volume-sampled data) or< MAXVALcomparisons. The migration fromwp.infis complete with no remaining infinity comparisons, and the finite sentinel value (1e10) effectively avoids verify_fp false positives as intended.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
Can you also check if the constant |
Signed-off-by: Alain Denzler <[email protected]>
2 participants
This makes it much easier to use verify_fp, because we don't get all of the false-positives anymore. Functionally, this should be equivalent.
Note that any comparisons involving volume-sampled data need to add a bit of epsilon around the threshold to account for rounding behavior during interpolation.
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
Improvements
Chores
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.