newton/examples/robot/example_robot_ur10.py (4)

36-59: Add bounds checking for trajectory array access.

The kernel accesses joint_target_trajectory[step + 1, ...] without ensuring that step + 1 is within bounds. While the modulo operation should prevent overflow, adding an explicit bounds check would improve robustness.

 @wp.kernel
 def update_joint_target_trajectory_kernel(
     joint_target_trajectory: wp.array3d(dtype=wp.float32),
     time: wp.array(dtype=wp.float32),
     dt: wp.float32,
     # output
     joint_target: wp.array2d(dtype=wp.float32),
 ):
     env_idx = wp.tid()
     t = time[env_idx]
     t = wp.mod(t + dt, float(joint_target_trajectory.shape[0] - 1))
     step = int(t)
+    step = wp.min(step, joint_target_trajectory.shape[0] - 2)  # Ensure step+1 is valid
     time[env_idx] = t
 
     num_dofs = joint_target.shape[1]
     for dof in range(num_dofs):
         # add env_idx here to make the sequence of dofs different for each env
         di = (dof + env_idx) % num_dofs
         joint_target[env_idx, dof] = wp.lerp(
             joint_target_trajectory[step, env_idx, di],
             joint_target_trajectory[step + 1, env_idx, di],
             wp.frac(t),
         )

---

114-119: Add validation for articulation count mismatch.

The assertion provides a clear error message, but consider handling this case more gracefully or providing additional context about which articulations were found.

 self.articulation_view = ArticulationView(
     self.model, "*ur10*", exclude_joint_types=[newton.JointType.FREE, newton.JointType.DISTANCE]
 )
-assert self.articulation_view.count == self.num_envs, (
-    "Number of environments must match the number of articulations"
-)
+if self.articulation_view.count != self.num_envs:
+    raise ValueError(
+        f"Expected {self.num_envs} UR10 articulations but found {self.articulation_view.count}. "
+        f"This may indicate an issue with the USD import or replication."
+    )

---

131-135: Consider making the joint limit fallback configurable.

The hardcoded fallback to [-π, π] for joints without limits might not be appropriate for all joint types. Consider making this configurable or documenting the assumption.

+        # Default limits for joints without explicit limits (assuming angular joints)
+        DEFAULT_ANGULAR_LIMITS = (-wp.pi, wp.pi)
+        
         for i in range(dof_count):
             # generate sinusoidal control signal for this dof
             lower = dof_lower[i]
             upper = dof_upper[i]
             if not np.isfinite(lower) or abs(lower) > 6.0:
                 # no limits; assume the joint dof is angular
-                lower = -wp.pi
-                upper = wp.pi
+                lower, upper = DEFAULT_ANGULAR_LIMITS
+                if i == 0:  # Log once to avoid spam
+                    print(f"Using default angular limits {DEFAULT_ANGULAR_LIMITS} for joints without explicit limits")

---

143-143: Trajectory generation creates large arrays for wide joint ranges.

The trajectory uses int(limit_range * 50) points, which could create very large arrays for joints with wide ranges. Consider capping the maximum number of points or using a fixed trajectory length.

-            traj = np.sin(np.linspace(phase_shift, 2 * np.pi + phase_shift, int(limit_range * 50)))
+            # Cap trajectory points to avoid excessive memory usage
+            num_points = min(int(limit_range * 50), 500)
+            traj = np.sin(np.linspace(phase_shift, 2 * np.pi + phase_shift, num_points))

newton/_src/utils/import_usd.py (4)

532-533: Consider using a more specific warning category.

While the warnings are helpful, consider using a more specific warning category like UserWarning or creating a custom warning class for USD import issues to allow users to filter these warnings if needed.

-                warnings.warn(f"Skipping joint {joint_desc.primPath}: both bodies unresolved", stacklevel=2)
+                warnings.warn(f"Skipping joint {joint_desc.primPath}: both bodies unresolved", UserWarning, stacklevel=2)

-                    warnings.warn(
-                        f"Body {body_path} has zero mass and zero inertia despite having the MassAPI USD schema applied.",
-                        stacklevel=2,
-                    )
+                    warnings.warn(
+                        f"Body {body_path} has zero mass and zero inertia despite having the MassAPI USD schema applied.",
+                        UserWarning,
+                        stacklevel=2,
+                    )

Also applies to: 1246-1248

---

1224-1225: Consider making the default density configurable.

The sphere approximation uses default_shape_density for the inertia calculation. Consider allowing this to be overridden via a parameter or using the body's actual density if available from body_density.

-                    density = default_shape_density  # kg/m³
+                    # Use body-specific density if available, otherwise fall back to default
+                    density = body_density.get(body_path, default_shape_density)  # kg/m³

---

538-538: Consider improving the verbose message for world connections.

The message could be clearer about which body is being connected to the world frame.

-                print(f"Joint {joint_desc.primPath} connects {parent_path} to world")
+                print(f"Joint {joint_desc.primPath}: connecting body '{parent_path}' to world frame (child unresolved)")

---

1241-1243: Consider adding units to the verbose output.

The verbose message about inertia diagonal elements would be clearer with units.

-                            f"Applied default inertia matrix for body {body_path}: diagonal elements = [{I_default[0, 0]}, {I_default[1, 1]}, {I_default[2, 2]}]"
+                            f"Applied default inertia matrix for body {body_path}: diagonal elements = [{I_default[0, 0]:.6f}, {I_default[1, 1]:.6f}, {I_default[2, 2]:.6f}] kg⋅m²"

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: Milad-Rakhsha-NV
PR: newton-physics/newton#535
File: newton/tests/test_examples.py:320-414
Timestamp: 2025-08-27T19:05:44.697Z
Learning: In newton/examples/__init__.py, the robot policy example is registered with the key "robot_policy" (not "robot.example_robot_policy"), so tests should reference it as name="robot_policy".

Learnt from: shi-eric
PR: newton-physics/newton#660
File: pyproject.toml:54-59
Timestamp: 2025-08-27T23:32:48.670Z
Learning: In the newton project's pyproject.toml, self-referential extras (e.g., `examples = ["newton[sim]", "newton[importers]", ...]`) are normal and encouraged for dependency management, rather than inlining the dependencies.

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).

Learnt from: nvlukasz
PR: newton-physics/newton#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.

newton/tests/test_import_usd.py (1)

116-119: Strengthen assertions: verify no FREE root and parent/child of the revolute joint.

Good update to 2 joints. To harden this test against regressions:

• assert that no FREE joint exists
• assert the revolute joint connects CenterPivot → Arm

         # Should have 2 joints:
         # 1. Fixed joint with only body0 specified (CenterPivot to world)
         # 2. Revolute joint between CenterPivot and Arm (normal joint with both bodies)
-        self.assertEqual(builder.joint_count, 2)
+        self.assertEqual(builder.joint_count, 2)
+        # Ensure no free root joint is present
+        self.assertNotIn(newton.JointType.FREE, builder.joint_type)
+
+        # Validate revolute joint connectivity explicitly
+        arm_idx = builder.body_key.index("/Articulation/Arm")
+        self.assertEqual(builder.joint_parent[revolute_joint_idx], center_pivot_idx)
+        self.assertEqual(builder.joint_child[revolute_joint_idx], arm_idx)

newton/tests/assets/revolute_articulation.usda (1)

52-61: Normalize or remove localRot1; add a comment explaining missing body1.

The missing body1 is intentional, but localRot1=(0.6324555, 0, 0, 0) is not unit-length and can cause downstream normalization differences. Prefer identity and document intent.

         def PhysicsFixedJoint "FixedJoint"
         {
             rel physics:body0 = </Articulation/CenterPivot>
             float physics:breakForce = inf
             float physics:breakTorque = inf
             point3f physics:localPos0 = (0, 0, 0)
             point3f physics:localPos1 = (0, 0, 0)
             quatf physics:localRot0 = (1, 0, 0, 0)
-            quatf physics:localRot1 = (0.6324555, 0, 0, 0)
+            # Intentionally omit physics:body1 to exercise importer fallback to world.
+            quatf physics:localRot1 = (1, 0, 0, 0)
         }

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.