Add triangle mesh to example_robot_anymal_c_walk#1010
Conversation
📝 WalkthroughPre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (4)📚 Learning: 2025-08-12T17:51:37.474ZApplied to files:
📚 Learning: 2025-08-12T17:58:00.815ZApplied to files:
📚 Learning: 2025-08-14T17:38:36.106ZApplied to files:
📚 Learning: 2025-10-24T07:56:14.792ZApplied to files:
🧬 Code graph analysis (2)newton/_src/geometry/__init__.py (1)
newton/_src/sim/collide_unified.py (1)
⏰ 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)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
newton/_src/sim/collide_unified.py(2 hunks)newton/examples/robot/example_robot_anymal_c_walk.py(5 hunks)newton/examples/robot/terrain_generator.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-12T17:51:37.474Z
Learnt from: nvlukasz
PR: newton-physics/newton#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.pynewton/examples/robot/terrain_generator.py
📚 Learning: 2025-10-24T07:56:14.792Z
Learnt from: nvtw
PR: newton-physics/newton#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/sim/collide_unified.py
🧬 Code graph analysis (2)
newton/_src/sim/collide_unified.py (1)
newton/_src/solvers/style3d/collision/kernels.py (1)
triangle_normal(22-25)
newton/examples/robot/example_robot_anymal_c_walk.py (6)
newton/examples/robot/terrain_generator.py (1)
generate_terrain_grid(209-283)newton/_src/geometry/types.py (1)
Mesh(116-300)newton/_src/sim/builder.py (1)
add_shape_mesh(3074-3110)newton/_src/sim/collide_unified.py (4)
CollisionPipelineUnified(1519-1995)from_model(1662-1726)BroadPhaseMode(47-58)collide(1728-1995)newton/_src/solvers/mujoco/solver_mujoco.py (1)
step(1355-1378)newton/_src/sim/articulation.py (1)
eval_fk(407-463)
🪛 Ruff (0.14.2)
newton/examples/robot/terrain_generator.py
90-90: Unused function argument: platform_width
(ARG001)
209-209: Unused function argument: seed
(ARG001)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run GPU Unit Tests on AWS EC2 (Pull Request)
- GitHub Check: Run GPU Benchmarks (Pull Request)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
newton/examples/robot/terrain_generator.py (1)
89-127: Drop the unusedplatform_widthparameter
random_grid_terrain()doesn’t use itsplatform_widthargument at all, so we’re exposing a knob that has no effect (Ruff also flags ARG001 here). Please trim it to avoid a misleading API surface:- def random_grid_terrain(size, grid_width=0.5, grid_height_range=(-0.15, 0.15), platform_width=None, seed=None): + def random_grid_terrain(size, grid_width=0.5, grid_height_range=(-0.15, 0.15), seed=None):If you intended a platform, wire it up; otherwise removing the parameter keeps things clear.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
newton/_src/sim/collide_unified.py(2 hunks)newton/examples/robot/example_robot_anymal_c_walk.py(5 hunks)newton/examples/robot/terrain_generator.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-24T07:56:14.792Z
Learnt from: nvtw
PR: newton-physics/newton#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/sim/collide_unified.py
📚 Learning: 2025-08-12T17:51:37.474Z
Learnt from: nvlukasz
PR: newton-physics/newton#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.pynewton/examples/robot/terrain_generator.py
🧬 Code graph analysis (2)
newton/_src/sim/collide_unified.py (1)
newton/_src/solvers/style3d/collision/kernels.py (1)
triangle_normal(22-25)
newton/examples/robot/example_robot_anymal_c_walk.py (6)
newton/examples/robot/terrain_generator.py (1)
generate_terrain_grid(209-283)newton/_src/geometry/types.py (1)
Mesh(116-300)newton/_src/sim/builder.py (1)
add_shape_mesh(3074-3110)newton/_src/sim/collide_unified.py (4)
CollisionPipelineUnified(1519-1995)from_model(1662-1726)BroadPhaseMode(47-58)collide(1728-1995)newton/_src/solvers/mujoco/solver_mujoco.py (1)
step(1347-1370)newton/_src/sim/articulation.py (1)
eval_fk(407-463)
🪛 Ruff (0.14.2)
newton/examples/robot/terrain_generator.py
90-90: Unused function argument: platform_width
(ARG001)
209-209: Unused function argument: seed
(ARG001)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run GPU Benchmarks (Pull Request)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
newton/_src/geometry/__init__.py(2 hunks)newton/_src/geometry/terrain_generator.py(1 hunks)newton/examples/robot/example_robot_anymal_c_walk.py(5 hunks)newton/geometry.py(2 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common 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).
📚 Learning: 2025-08-12T17:51:37.474Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/geometry.py:16-22
Timestamp: 2025-08-12T17:51:37.474Z
Learning: In the Newton public API refactor, common geometry symbols like ParticleFlags and ShapeFlags are now exposed at the top level in newton/__init__.py rather than through newton.geometry. The newton/geometry.py module is intentionally focused only on broad-phase collision detection classes (BroadPhaseAllPairs, BroadPhaseExplicit, BroadPhaseSAP).
Applied to files:
newton/_src/geometry/__init__.pynewton/geometry.pynewton/_src/geometry/terrain_generator.py
📚 Learning: 2025-08-12T17:58:00.815Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/_src/solvers/featherstone/kernels.py:18-20
Timestamp: 2025-08-12T17:58:00.815Z
Learning: In newton/_src/core/__init__.py, the function transform_twist is properly re-exported from the spatial module via "from .spatial import (..., transform_twist, ...)" and included in __all__, making it available for import as "from ...core import transform_twist" from other modules.
Applied to files:
newton/_src/geometry/__init__.pynewton/geometry.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/geometry/__init__.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/geometry.py
🧬 Code graph analysis (3)
newton/_src/geometry/__init__.py (1)
newton/_src/geometry/terrain_generator.py (1)
generate_terrain_grid(287-371)
newton/geometry.py (1)
newton/_src/geometry/terrain_generator.py (1)
generate_terrain_grid(287-371)
newton/examples/robot/example_robot_anymal_c_walk.py (6)
newton/_src/geometry/terrain_generator.py (1)
generate_terrain_grid(287-371)newton/_src/geometry/types.py (1)
Mesh(116-300)newton/_src/sim/builder.py (1)
add_shape_mesh(3074-3110)newton/_src/sim/collide_unified.py (4)
CollisionPipelineUnified(1519-1995)from_model(1662-1726)BroadPhaseMode(47-58)collide(1728-1995)newton/_src/solvers/mujoco/solver_mujoco.py (1)
step(1361-1384)newton/_src/sim/articulation.py (1)
eval_fk(407-463)
🪛 Ruff (0.14.2)
newton/_src/geometry/terrain_generator.py
117-117: Unused function argument: platform_width
(ARG001)
292-292: Unused function argument: seed
(ARG001)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run GPU Benchmarks (Pull Request)
- GitHub Check: Run GPU Unit Tests on AWS EC2 (Pull Request)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
newton/_src/geometry/terrain_generator.py (2)
478-480: Consider improving parameter lookup for callable terrain functions.When
terrain_nameis a callable, the currentterrain_params.get(terrain_name, {})lookup requires users to pass the callable object itself as a dictionary key, which is awkward. Consider using an index-based key or a separate parameter for callable terrain configurations.Example improvement:
- # Get parameters for this terrain type - params = terrain_params.get(terrain_name, {}) + # Get parameters for this terrain type + if callable(terrain_name): + # For callables, allow index-based params or use a special key + params = terrain_params.get(f"block_{row}_{col}", {}) + else: + params = terrain_params.get(terrain_name, {})
481-485: Consider a more extensible approach for forwarding seeds to stochastic functions.Currently only
_random_grid_terrainreceives the seed. If you add more stochastic terrain types in the future, they won't automatically benefit from deterministic generation.Consider checking if the terrain function accepts a
seedparameter using introspection:import inspect # Forward seed to any function that accepts it if rng is not None and "seed" not in params: sig = inspect.signature(terrain_func) if "seed" in sig.parameters: params = dict(params) params["seed"] = int(rng.integers(0, 2**32))
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
newton/_src/geometry/terrain_generator.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-12T17:51:37.474Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/geometry.py:16-22
Timestamp: 2025-08-12T17:51:37.474Z
Learning: In the Newton public API refactor, common geometry symbols like ParticleFlags and ShapeFlags are now exposed at the top level in newton/__init__.py rather than through newton.geometry. The newton/geometry.py module is intentionally focused only on broad-phase collision detection classes (BroadPhaseAllPairs, BroadPhaseExplicit, BroadPhaseSAP).
Applied to files:
newton/_src/geometry/terrain_generator.py
🪛 Ruff (0.14.2)
newton/_src/geometry/terrain_generator.py
249-249: Unused function argument: platform_width
(ARG001)
417-417: Unused function argument: seed
(ARG001)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run GPU Benchmarks (Pull Request)
- GitHub Check: Run GPU Unit Tests on AWS EC2 (Pull Request)
🔇 Additional comments (4)
newton/_src/geometry/terrain_generator.py (4)
30-162: LGTM! Well-structured box generation.The per-face vertex duplication correctly ensures sharp edges, and the face winding appears consistent.
169-405: Terrain generation functions look solid.The implementations correctly generate their respective terrain types, and the wave terrain boundary handling is elegant.
512-553: LGTM! Helper functions are correct.The mesh combination logic properly offsets indices, and the dtype conversion ensures Newton compatibility.
249-249: Static analysis findings are acceptable.
- Line 249:
platform_widthis intentionally unused per the docstring ("kept for API compatibility")- Line 417:
seedIS used on line 455 to create the RNG—this is a false positiveAlso applies to: 417-417
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
newton/_src/geometry/terrain_generator.py (2)
249-249: Remove unused parameter or document retention rationale.The
platform_widthparameter is documented as "kept for API compatibility" but is never used in the function. If there's no concrete compatibility requirement, remove it to simplify the API. Otherwise, document why it must remain (e.g., if other code expects this signature).
541-553: Remove unused_to_newton_meshhelper function.The ripgrep search confirms
_to_newton_meshis defined but never called anywhere in the codebase. Remove it from lines 541–553 to reduce maintenance burden.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
newton/_src/geometry/__init__.py(2 hunks)newton/_src/geometry/terrain_generator.py(1 hunks)newton/examples/robot/example_robot_anymal_c_walk.py(5 hunks)newton/geometry.py(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 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/geometry.pynewton/_src/geometry/__init__.pynewton/_src/geometry/terrain_generator.py
📚 Learning: 2025-08-12T17:58:00.815Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/_src/solvers/featherstone/kernels.py:18-20
Timestamp: 2025-08-12T17:58:00.815Z
Learning: In newton/_src/core/__init__.py, the function transform_twist is properly re-exported from the spatial module via "from .spatial import (..., transform_twist, ...)" and included in __all__, making it available for import as "from ...core import transform_twist" from other modules.
Applied to files:
newton/geometry.pynewton/_src/geometry/__init__.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/geometry.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/geometry.py
🧬 Code graph analysis (3)
newton/geometry.py (1)
newton/_src/geometry/terrain_generator.py (1)
generate_terrain_grid(412-496)
newton/examples/robot/example_robot_anymal_c_walk.py (5)
newton/_src/geometry/terrain_generator.py (1)
generate_terrain_grid(412-496)newton/_src/geometry/types.py (1)
Mesh(116-300)newton/_src/sim/builder.py (1)
add_shape_mesh(3074-3110)newton/_src/sim/collide_unified.py (4)
CollisionPipelineUnified(1519-1995)from_model(1662-1726)BroadPhaseMode(47-58)collide(1728-1995)newton/_src/sim/articulation.py (1)
eval_fk(407-463)
newton/_src/geometry/__init__.py (1)
newton/_src/geometry/terrain_generator.py (1)
generate_terrain_grid(412-496)
🪛 Ruff (0.14.2)
newton/_src/geometry/terrain_generator.py
249-249: Unused function argument: platform_width
(ARG001)
417-417: Unused function argument: seed
(ARG001)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run GPU Benchmarks (Pull Request)
🔇 Additional comments (7)
newton/_src/geometry/terrain_generator.py (1)
454-484: LGTM: Seed forwarding is correctly implemented.The deterministic seeding logic properly:
- Creates an RNG from the top-level seed (line 455)
- Forwards unique per-block seeds to
_random_grid_terrain(lines 482-484)- Preserves user-supplied per-terrain seed overrides via the
"seed" not in paramschecknewton/_src/geometry/__init__.py (1)
35-35: LGTM: Clean API exposure.The import and
__all__entry correctly exposegenerate_terrain_gridfrom the internal module, following the established pattern for other geometry utilities.Also applies to: 69-69
newton/geometry.py (1)
32-32: LGTM: Public API exposure follows project conventions.The re-export of
generate_terrain_gridaligns with Newton's public API structure, making the terrain generator accessible asnewton.geometry.generate_terrain_grid.Based on learnings.
Also applies to: 54-54
newton/examples/robot/example_robot_anymal_c_walk.py (4)
106-121: Verify whether ground plane is still needed.Line 121 adds a ground plane immediately after adding the procedural terrain mesh (lines 107-120). If the terrain mesh provides full ground coverage, the ground plane may be redundant. Alternatively, it might serve as a safety fallback if the robot moves beyond the terrain bounds.
Please clarify: Is the ground plane intentionally kept as a fallback, or can it be removed now that procedural terrain is in place?
156-169: LGTM: Collision pipeline and solver setup is correct.The collision pipeline is properly created after
model.finalize()usingCollisionPipelineUnified.from_model(), and the solver is correctly configured withuse_mujoco_contacts=Falseto leverage Newton's contact handling for the terrain mesh.
188-192: LGTM: Proper initialization order for FK and contacts.Forward kinematics is evaluated first to ensure body poses match the initial joint configuration, then initial contacts are computed using the collision pipeline. This sequence ensures the simulation starts with a consistent state.
232-235: LGTM: Contact computation integrated correctly.Each simulation substep now computes contacts through the terrain-aware collision pipeline and passes them to the solver, enabling proper physics interaction with the procedural terrain.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
newton/examples/robot/example_robot_anymal_c_walk.py (1)
158-164: Consider using NXN or SAP broad phase mode instead of EXPLICIT.The collision pipeline is configured with
BroadPhaseMode.EXPLICIT, which requires precomputed shape pairs. For a robot walking on dynamic terrain,BroadPhaseMode.NXNorBroadPhaseMode.SAPwould be more appropriate as they automatically detect collisions between all eligible shape pairs.Unless EXPLICIT mode is specifically required for performance optimization with known shape pairs, consider:
self.collision_pipeline = newton.CollisionPipelineUnified.from_model( self.model, rigid_contact_max_per_pair=10, rigid_contact_margin=0.01, - broad_phase_mode=newton.BroadPhaseMode.EXPLICIT, + broad_phase_mode=newton.BroadPhaseMode.SAP, )
SAP(Sweep and Prune) is generally more efficient thanNXN(all-pairs) for scenarios with many shapes.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
newton/examples/robot/example_robot_anymal_c_walk.py(7 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-19T12:27:30.629Z
Learnt from: preist-nvidia
Repo: newton-physics/newton PR: 579
File: newton/examples/example_mujoco.py:350-354
Timestamp: 2025-08-19T12:27:30.629Z
Learning: In Newton examples, there's a distinction between solver parameters and Example class attributes. The Example class can have its own use_mujoco attribute for controlling example-level behavior (like CUDA graphs, rendering logic), while the solver uses use_mujoco_cpu for backend selection. These serve different purposes and should not be conflated during API renames.
Applied to files:
newton/examples/robot/example_robot_anymal_c_walk.py
📚 Learning: 2025-08-25T21:41:45.795Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 634
File: newton/tests/test_examples.py:329-333
Timestamp: 2025-08-25T21:41:45.795Z
Learning: Newton examples use a centralized CLI argument parsing system in newton/examples/__init__.py. The create_parser() function defines common arguments like --device, --viewer, --output-path, and --num-frames, while init(parser) creates viewers based on parsed arguments. Individual example scripts don't need to define these flags themselves - they inherit them from the centralized system via the example_map and runpy execution.
Applied to files:
newton/examples/robot/example_robot_anymal_c_walk.py
🧬 Code graph analysis (1)
newton/examples/robot/example_robot_anymal_c_walk.py (4)
newton/_src/geometry/terrain_generator.py (1)
generate_terrain_grid(412-504)newton/_src/geometry/types.py (1)
Mesh(116-300)newton/_src/sim/collide_unified.py (4)
CollisionPipelineUnified(1519-1995)from_model(1662-1726)BroadPhaseMode(47-58)collide(1728-1995)newton/_src/sim/articulation.py (1)
eval_fk(407-463)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run GPU Benchmarks (Pull Request)
- GitHub Check: Run GPU Unit Tests on AWS EC2 (Pull Request)
🔇 Additional comments (6)
newton/examples/robot/example_robot_anymal_c_walk.py (6)
36-36: LGTM!The import is correctly placed and the
generate_terrain_gridfunction is properly exposed through the geometry API.
107-123: Verify if ground plane is still needed with terrain mesh.The code adds a ground plane (line 123) regardless of whether the procedural terrain is generated. This might be intentional as a fallback, but it could also cause unexpected collisions or z-fighting if the terrain overlaps with the ground plane at z=0.
Please confirm:
- Is the ground plane intended as a safety fallback when
is_test=True?- Does the terrain mesh offset (
z=0.01) prevent conflicts with the ground plane atz=0?If the ground plane is only needed during tests, consider:
- if not self.is_test: - vertices, indices = generate_terrain_grid(...) - terrain_mesh = newton.Mesh(vertices, indices) - terrain_offset = wp.transform(p=wp.vec3(-5, -2.0, 0.01), q=wp.quat_identity()) - builder.add_shape_mesh(body=-1, mesh=terrain_mesh, xform=terrain_offset) - builder.add_ground_plane() + if self.is_test: + builder.add_ground_plane() + else: + vertices, indices = generate_terrain_grid(...) + terrain_mesh = newton.Mesh(vertices, indices) + terrain_offset = wp.transform(p=wp.vec3(-5, -2.0, 0.01), q=wp.quat_identity()) + builder.add_shape_mesh(body=-1, mesh=terrain_mesh, xform=terrain_offset)
166-171: LGTM!The solver is correctly configured to use Newton contacts from the collision pipeline (
use_mujoco_contacts=False). The comment clearly explains this choice.
190-194: LGTM!The initialization sequence is correct:
- Evaluate forward kinematics to compute initial body poses from joint configuration
- Compute initial contacts using the collision pipeline
This ensures the simulation starts with consistent state and contact information.
234-237: LGTM!The collision pipeline is correctly integrated into the simulation loop:
- Contacts are computed after applying forces but before solving
- Contacts are passed to
solver.stepfor constraint resolutionThe simulation flow is correct and well-commented.
339-339: LGTM!The example instantiation correctly passes
argsto match the updated constructor signature, following Newton's centralized CLI argument pattern.Based on learnings
Description
This PR adds two main improvements:
Terrain Generator Module: Adds a compact procedural terrain generator (
terrain_generator.py, partly taken from IsaacLab) for Newton examples with support for multiple terrain types (flat, pyramid stairs, random grid, wave, box, gap) and configurable grid-based terrain generation. Updatesexample_robot_anymal_c_walk.pyto use procedurally generated terrain instead of a flat ground plane, including necessary collision pipeline setup withuse_mujoco_contacts=Falseand forward kinematics initialization.Triangle Contact Normal Correction: Fixes mesh-triangle contacts in
collide_unified.pyto prevent objects from being pushed into triangles. When a contact normal is nearly parallel (within 10°) to the triangle normal but pointing in the opposite direction, the penetration depth sign is flipped to push objects out through the triangle surface instead of further into it. This ensures proper contact behavior for terrain meshes where objects can end up on the wrong side of triangles.Newton Migration Guide
Please ensure the migration guide for warp.sim users is up-to-date with the changes made in this PR.
docs/migration.rstis up-to dateBefore your PR is "Ready for review"
newton/tests/test_examples.py)pre-commit run -aSummary by CodeRabbit
New Features
Improvements