newton/_src/geometry/kernels.py (1)

1582-1583: Consider verbosity-gating instead of fully silencing this diagnostic

The generic unsupported-pair message can be useful when debugging new geometry types. Suggest gating it behind wp.config.verbose with wp.printf instead of commenting it out.

-        # print("Unsupported geometry pair in collision handling")
+        # if wp.static(wp.config.verbose):
+        #     wp.printf("handle_contact_pairs: unsupported geometry pair %i and %i\n", geo_type_a, geo_type_b)

newton/examples/robot/example_quadruped.py (1)

68-75: Use math for host-side grid math (avoid Warp scalars on host)

Minor: wp.sqrt/wp.ceil are unnecessary here and can be slower on host. Use math for clarity and to avoid any device-type ambiguity.

-        spacing = 3.0
-        sqn = int(wp.ceil(wp.sqrt(float(self.num_envs))))
+        import math
+        spacing = 3.0
+        sqn = int(math.ceil(math.sqrt(float(self.num_envs))))

Additionally, remember to add import math at the top of the file if you prefer that style:

@@
-import warp as wp
+import math
+import warp as wp

newton/examples/__init__.py (1)

36-43: Optional: Make timing scopes user-toggleable

ScopedTimer(..., active=False) disables timing by default. Consider wiring this to a CLI flag or env var (e.g., NEWTON_PROFILE=1) so developers can enable profiling without code edits.

newton/examples/robot/example_h1.py (3)

117-129: Recompute contacts each substep to avoid stale contact pairs

Currently, contacts are computed once per frame and reused across substeps, which can miss new contacts after integration within the same frame. Recompute inside the loop.

-        self.contacts = self.model.collide(self.state_0)
-        for _ in range(self.sim_substeps):
+        for _ in range(self.sim_substeps):
             self.state_0.clear_forces()
 
             # apply forces to the model for picking, wind, etc
             self.viewer.apply_forces(self.state_0)
 
+            # refresh contacts per substep
+            self.contacts = self.model.collide(self.state_0)
             self.solver.step(self.state_0, self.state_1, self.control, self.contacts, self.sim_dt)
 
             # swap states
             self.state_0, self.state_1 = self.state_1, self.state_0

---

111-115: Use cached device handle

You already cache the device as self.device; use it here for consistency and micro-avoidance of API calls.

-        if wp.get_device().is_cuda:
+        if self.device.is_cuda:
             with wp.ScopedCapture() as capture:
                 self.simulate()
             self.graph = capture.graph

---

82-89: Use example helper to place environments

newton.examples.compute_env_offsets provides a consistent grid placement and keeps layout logic centralized.

-        spacing = 3.0
-        sqn = int(wp.ceil(wp.sqrt(float(self.num_envs))))
-
-        builder = newton.ModelBuilder(up_axis=newton.Axis.Z)
-        for i in range(self.num_envs):
-            pos = wp.vec3((i % sqn) * spacing, (i // sqn) * spacing, 0)
-            builder.add_builder(articulation_builder, xform=wp.transform(pos, wp.quat_identity()))
+        spacing = 3.0
+        positions = newton.examples.compute_env_offsets(self.num_envs, env_offset=(spacing, spacing, 0.0))
+
+        builder = newton.ModelBuilder(up_axis=newton.Axis.Z)
+        for i in range(self.num_envs):
+            builder.add_builder(articulation_builder, xform=wp.transform(positions[i], wp.quat_identity()))

newton/examples/robot/example_anymal_d.py (3)

118-129: Refresh contacts within substeps

Same rationale as other examples: compute contacts after applying forces in each substep.

-        self.contacts = self.model.collide(self.state_0)
-        for _ in range(self.sim_substeps):
+        for _ in range(self.sim_substeps):
             self.state_0.clear_forces()
 
             # apply forces to the model for picking, wind, etc
             self.viewer.apply_forces(self.state_0)
 
+            self.contacts = self.model.collide(self.state_0)
             self.solver.step(self.state_0, self.state_1, self.control, self.contacts, self.sim_dt)
 
             # swap states
             self.state_0, self.state_1 = self.state_1, self.state_0

---

111-116: Use cached device handle

Minor consistency/clarity improvement.

-        if wp.get_device().is_cuda:
+        if self.device.is_cuda:
             with wp.ScopedCapture() as capture:
                 self.simulate()
             self.graph = capture.graph

---

91-95: Verify cone fallback mapping and module dependency

• The SolverMuJoCo constructor already accepts both the strings "pyramidal" and "elliptic" (in addition to the integer constants) and maps them via _resolve_mj_opt, so passing "elliptic" will work as expected.
• However, solver_mujoco.py still does a top-level import mujoco and directly references mujoco.mjtCone.mjCONE_PYRAMIDAL/mjCONE_ELLIPTIC in its mapping dict, so the module cannot be imported without MuJoCo installed.
• If the goal is truly to avoid a hard dependency on MuJoCo at import time (even when using string values), consider deferring the import or attribute lookup for mujoco.mjtCone inside __init__ (e.g. within a try/except) so that consumers who only refer to SolverMuJoCo by string names aren’t forced to install MuJoCo.

newton/examples/robot/example_g1.py (2)

71-75: Be explicit about up_axis on the aggregate builder

Other examples pass up_axis=Axis.Z explicitly; do the same here to avoid surprises if the default changes.

-        builder = newton.ModelBuilder()
+        builder = newton.ModelBuilder(up_axis=newton.Axis.Z)

---

114-125: Recompute contacts in the substep loop

Keeps contact pairs/normals consistent with the latest state.

-        self.contacts = self.model.collide(self.state_0)
-        for _ in range(self.sim_substeps):
+        for _ in range(self.sim_substeps):
             self.state_0.clear_forces()
 
             # apply forces to the model for picking, wind, etc
             self.viewer.apply_forces(self.state_0)
 
+            self.contacts = self.model.collide(self.state_0)
             self.solver.step(self.state_0, self.state_1, self.control, self.contacts, self.sim_dt)
 
             # swap states
             self.state_0, self.state_1 = self.state_1, self.state_0

newton/examples/robot/example_humanoid.py (2)

19-21: Comment mismatch: this example uses MJCF, not USD

Update the header to avoid confusing readers.

-# Shows how to set up a simulation of a humanoid articulation
-# from a USD stage using newton.ModelBuilder().
+# Shows how to set up a simulation of a humanoid articulation
+# from an MJCF asset using newton.ModelBuilder().

---

112-124: Refresh contacts each substep

Ensure contact computation reflects the latest integrated configuration.

-        self.contacts = self.model.collide(self.state_0)
-        for _ in range(self.sim_substeps):
+        for _ in range(self.sim_substeps):
             self.state_0.clear_forces()
 
             # apply forces to the model for picking, wind, etc
             self.viewer.apply_forces(self.state_0)
 
+            self.contacts = self.model.collide(self.state_0)
             self.solver.step(self.state_0, self.state_1, self.control, self.contacts, self.sim_dt)
 
             # swap states
             self.state_0, self.state_1 = self.state_1, self.state_0

newton/examples/robot/example_anymal_c_walk.py (2)

151-153: Strip trailing whitespace on blank lines (Ruff W293)

Minor formatting cleanup to satisfy linters.

-        
-        
-        
+
+
+

---

198-207: Avoid redundant collide call; compute once per substep with consistent margin

You call collide before the loop and again inside. Drop the outer call and keep a single, consistent collide inside the loop (preserving the margin you used initially).

-        self.contacts = self.model.collide(self.state_0, rigid_contact_margin=0.1)
-        for _ in range(self.sim_substeps):
+        # contacts are recomputed within the substep loop
+        for _ in range(self.sim_substeps):
             self.state_0.clear_forces()
 
             # apply forces to the model
             self.viewer.apply_forces(self.state_0)
 
-            self.contacts = self.model.collide(self.state_0)
+            self.contacts = self.model.collide(self.state_0, rigid_contact_margin=0.1)
             self.solver.step(self.state_0, self.state_1, self.control, self.contacts, self.sim_dt)

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: 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: preist-nvidia
PR: newton-physics/newton#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.

newton/_src/geometry/kernels.py (1)

1110-1111: LGTM: Unsupported geometry pair message is now verbosity-gated and kernel-safe

Good move switching to wp.printf under wp.static(wp.config.verbose). This avoids noisy logs while remaining device-compatible.

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

1026-1027: Plumbing is_visible into collision shape config is correct

Passing is_visible=not hide_collision_shapes provides the expected toggle without impacting collision behavior.

---

53-55: No API break risk found: hide_collision_shapes insertion is safe
Scan of all parse_usd call sites (in tests and examples) shows that every invocation either uses only the two required positional arguments or specifies additional parameters (e.g. collapse_fixed_joints, mesh_maxhullvert, xform, ignore_paths, enable_self_collisions) by keyword. No calls pass mesh_maxhullvert or any later parameter positionally, so inserting hide_collision_shapes before mesh_maxhullvert will not affect existing callers.

newton/examples/robot/example_quadruped.py (1)

96-101: LGTM: CUDA graph capture is correctly isolated

Clean separation into capture() and guarded by is_cuda. This aligns with the new example pattern.

newton/examples/robot/example_cartpole.py (1)

32-47: LGTM: Viewer-centric API and timing fields are consistent with the new pattern

Constructor signature, time management, and storing the viewer reference look good and match the refactor across examples.

newton/examples/__init__.py (1)

186-193: LGTM: Example map extended with robot examples

Nice consolidation. The new entries align with the viewer-driven workflow.

newton/examples/robot/example_h1.py (1)

148-161: Viewer-driven CLI wiring looks good

Entrypoint aligns with the unified runner (create_parser/init/run). Clean and consistent with other examples.

newton/examples/robot/example_anymal_d.py (1)

149-162: Viewer-centric main path LGTM

The CLI hooks and run loop match the new examples infrastructure.

newton/examples/robot/example_g1.py (2)

57-67: USD import and visibility flags look correct

Using hide_collision_shapes=True with approximate_meshes("bounding_box") aligns with the updated visibility flags and mesh simplification pipeline.

---

79-90: All SolverMuJoCo parameters validated – no changes required

• The solver="newton" string is accepted and mapped via _resolve_mj_opt to mujoco.mjtSolver.mjSOL_NEWTON (lines 1722–1724).
• The cone="elliptic" string is accepted and mapped via _resolve_mj_opt to mujoco.mjtCone.mjCONE_ELLIPTIC (lines 1735–1737).
• Both nefc_per_env (default 100) and ncon_per_env (default None) are parameters in the constructor (lines 1162–1163) and correctly drive the maximum constraint counts (njmax and nconmax) in the MuJoCo-Warp data upload (lines 2351–2356).

No further action needed.

newton/examples/robot/example_humanoid.py (1)

143-156: Unified CLI/viewer flow LGTM

Constructor signature and main follow the standard pattern and integrate cleanly with newton.examples.

newton/examples/robot/example_anymal_c_walk.py (1)

246-253: Viewer-centered main path LGTM

Modernized entrypoint using newton.examples.init/run is consistent and clear.

newton/examples/__init__.py (2)

186-192: Future-proof discovery: consider plugin-style example registration.

Manual upkeep of example_map doesn’t scale. Consider discovering examples via entry points (setuptools) or a lightweight registry (e.g., modules exporting EXAMPLE_NAME or an register_example() call in module import). This decouples updates in newton/examples/robot from this central map and simplifies community contributions.

---

38-42: Gate and de-conflict outer-loop timers across all examples

Outer timers in newton/examples/__init__.py are currently hard-disabled (active=False), but if ever enabled they will collide and double-count with the per-example ScopedTimer("step")/("render") calls scattered throughout the examples. To avoid misleading timings:

• Introduce a single toggle (e.g. via NEWTON_TIMING or example.enable_timing)
• Rename the outer-loop timers to distinct names (e.g. "frame.step" and "frame.render")
• Ensure the viewer always closes by wrapping the loop in a try…finally

Pinpointed collisions (you’ll need to apply the same flag/renaming in each):

• newton/examples/robot/example_anymal_c_walk.py (lines 213–214)
• newton/examples/example_robot_manipulating_cloth.py (lines 572–573)
• newton/examples/example_rigid_force.py (lines 84 and 95–96)
• newton/examples/example_contact_sensor.py (lines 260–261 and 315–316)
• newton/examples/example_cloth_self_contact.py (line 266)
• newton/examples/example_anymal_c_walk_on_sand.py (lines 236–237 and 251–252)

Suggested patch for newton/examples/__init__.py (apply analogous changes in the other files):

 def run(example):
-    while example.viewer.is_running():
-        with wp.ScopedTimer("step", active=False):
-            example.step()
-
-        with wp.ScopedTimer("render", active=False):
-            example.render()
-
-    example.viewer.close()
+    enable_timing = (
+        os.environ.get("NEWTON_TIMING", "0").lower() in ("1","true","yes")
+        or getattr(example, "enable_timing", False)
+    )
+    try:
+        while example.viewer.is_running():
+            with wp.ScopedTimer("frame.step", active=enable_timing):
+                example.step()
+            with wp.ScopedTimer("frame.render", active=enable_timing):
+                example.render()
+    finally:
+        example.viewer.close()

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.

newton/examples/robot/example_cartpole.py (1)

119-123: contacts=None but still passed to viewer.log_contacts; this will raise at runtime.

Viewer.log_contacts dereferences contacts.rigid_contact_count. When contacts is None (as set in init), this will throw. Either compute contacts or guard the call.

Apply this diff to guard logging:

     def render(self):
-        self.viewer.begin_frame(self.sim_time)
-        self.viewer.log_state(self.state_0)
-        self.viewer.log_contacts(self.contacts, self.state_0)
-        self.viewer.end_frame()
+        self.viewer.begin_frame(self.sim_time)
+        self.viewer.log_state(self.state_0)
+        if self.contacts is not None:
+            self.viewer.log_contacts(self.contacts, self.state_0)
+        self.viewer.end_frame()

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

314-319: Instance handling skips distinct instances; resolve to instance proxies

Current code iterates prototype children and reuses prototype child paths. Keys/path_name collide across instances, causing dropped visual shapes.

Apply this diff to remap prototype child paths via instance proxies:

-        if prim.IsInstance():
-            proto = prim.GetPrototype()
-            for child in proto.GetChildren():
-                load_visual_shapes(parent_body_id, child, xform)
-            return
+        if prim.IsInstance():
+            proto = prim.GetPrototype()
+            for child in proto.GetChildren():
+                inst_path = child.GetPath().ReplacePrefix(proto.GetPath(), prim.GetPath())
+                inst_child = stage.GetPrimAtPath(inst_path)
+                load_visual_shapes(parent_body_id, inst_child, xform)
+            return

newton/examples/robot/example_humanoid.py (1)

48-56: Bug: re-initializing the builder drops configured defaults before add_mjcf()

You configure articulation_builder (up_axis, joint/shape cfg), then overwrite it with a new ModelBuilder(), losing those settings.

Apply this diff to keep the configured builder:

-        articulation_builder = newton.ModelBuilder()
-
-        articulation_builder.add_mjcf(
+        articulation_builder.add_mjcf(
             mjcf_filename,
             ignore_names=["floor", "ground"],
             up_axis="Z",
         )

If you intended a fresh builder, re-apply the defaults to the new instance before add_mjcf.

Also applies to: 59-66

newton/examples/example_ik_interactive.py (5)

161-164: API migration looks correct; minor nit: pass str() to add_mjcf to avoid potential pathlib issues.

ModelBuilder.add_mjcf docs show a str parameter. pathlib.Path typically works, but explicitly converting keeps the call-site robust across different backends.

-        articulation_builder.add_mjcf(
-            newton.utils.download_asset("unitree_h1") / "mjcf/h1_with_hand.xml",
+        articulation_builder.add_mjcf(
+            str(newton.utils.download_asset("unitree_h1") / "mjcf/h1_with_hand.xml"),
             floating=floating,
         )

---

275-277: Guard quaternion normalization against divide-by-zero.

tf[3:7] should be a valid quaternion, but numerical issues or malformed assets could yield near-zero norms, producing NaNs.

-        quat = tf[3:7] / np.linalg.norm(tf[3:7])
+        q = tf[3:7]
+        n = np.linalg.norm(q)
+        quat = (q / n) if n > 1.0e-12 else np.array([0.0, 0.0, 0.0, 1.0], dtype=np.float32)

---

399-421: Tied-position drag reinitializes every event; verify this is intended.

Because _is_dragging_pos is set back to False at the end of the handler, each subsequent drag event re-captures _drag_start_positions. This yields incremental deltas frame-to-frame rather than a delta from the original mouse-down. If you intended a "one capture on press, accumulate until release" behavior, keep _is_dragging_pos True until mouse release (see _mouse_release_handler). Otherwise, ignore this note.

---

423-476: Same observation for rotation drag: state resets every event.

As above, _is_dragging_rot is immediately reset to False, which re-captures start state on the next event. Confirm that per-frame incremental rotation is desired. If not, track the drag session across events and clear flags on mouse release only.

---

520-556: Optional: align entrypoint with centralized examples init/run.

Per the repository’s examples pattern (newton.examples.create_parser/init/run), consider migrating this script’s main() to that framework for consistency, device handling, and viewer selection parity with other examples.

If desired, I can draft a minimal refactor that keeps gizmo behavior while using newton.examples.init() to create the viewer.

newton/examples/robot/example_anymal_c_walk.py (1)

178-189: Avoid hard-coding joint_target length (18); derive it from the model/control.

Hard-coding 18 couples the example to a specific URDF. If the articulation changes, this breaks graph capture. Use control.joint_target length (or model.joint_coord_count) to size the tensor.

     def capture(self):
         if self.device.is_cuda:
-            torch_tensor = torch.zeros(18, device=self.torch_device, dtype=torch.float32)
-            self.control.joint_target = wp.from_torch(torch_tensor, dtype=wp.float32, requires_grad=False)
+            n = int(self.control.joint_target.shape[0])  # robust to model changes
+            torch_tensor = torch.zeros(n, device=self.torch_device, dtype=torch.float32)
+            self.control.joint_target = wp.from_torch(torch_tensor, dtype=wp.float32, requires_grad=False)
             with wp.ScopedCapture() as capture:
                 self.simulate()
             self.graph = capture.graph
         else:
             self.graph = None

newton/examples/robot/example_g1.py (1)

113-125: Update contacts each substep for accurate contact visualization.

Currently, contacts are computed once per frame, before substeps. Viewer.log_contacts will then render normals from a pre-step state. If you want contact lines to reflect the latest state at frame end, recompute inside the substep loop.

-    def simulate(self):
-        self.contacts = self.model.collide(self.state_0)
-        for _ in range(self.sim_substeps):
+    def simulate(self):
+        for _ in range(self.sim_substeps):
             self.state_0.clear_forces()
 
             # apply forces to the model for picking, wind, etc
             self.viewer.apply_forces(self.state_0)
 
-            self.solver.step(self.state_0, self.state_1, self.control, self.contacts, self.sim_dt)
+            # refresh contacts for the current substep state
+            self.contacts = self.model.collide(self.state_0)
+            self.solver.step(self.state_0, self.state_1, self.control, self.contacts, self.sim_dt)
 
             # swap states
             self.state_0, self.state_1 = self.state_1, self.state_0

newton/tests/test_kinematics.py (1)

31-31: Prefer enum over string for up_axis for type-safety and consistency

Minor nit: pass the Axis enum instead of a raw string to align with the typed signature and other tests.

-        builder.add_mjcf(newton.examples.get_asset("nv_ant.xml"), up_axis="Y")
+        builder.add_mjcf(newton.examples.get_asset("nv_ant.xml"), up_axis=newton.Axis.Y)

newton/tests/test_import_urdf.py (1)

239-242: Migration to builder.add_urdf looks good

The switch to the builder-centric API is correct. Defaults (e.g., up_axis=Axis.Z) look intentional for cartpole; if the asset is Y-up, consider specifying up_axis explicitly to avoid implicit assumptions.

newton/examples/basic/example_basic_urdf.py (1)

62-67: Nice API migration; consider making up_axis explicit (enum) to document intent

Not required, but explicitly setting up_axis improves readability and prevents accidental regressions if default changes later. If the URDF is Z-up (common), this keeps behavior unchanged.

         quadruped.add_urdf(
             newton.examples.get_asset("quadruped.urdf"),
             xform=wp.transform([0.0, 0.0, 0.7], wp.quat_identity()),
             floating=True,
-            enable_self_collisions=False,
+            enable_self_collisions=False,
+            up_axis=newton.Axis.Z,
         )

newton/examples/selection/example_selection_cartpole.py (1)

70-75: Restrict USD import to the articulation root for faster loads; optionally pass verbose flag

If the cartpole prim lives under “/cartPole” (it’s later used by ArticulationView), importing only that subtree avoids scanning the full stage. Passing verbose=VERBOSE can help during example runs.

         env.add_usd(
             newton.examples.get_asset("cartpole.usda"),
             xform=wp.transform((0.0, 0.0, 2.0), wp.quat_identity()),
             collapse_fixed_joints=COLLAPSE_FIXED_JOINTS,
             enable_self_collisions=False,
+            root_path="/cartPole",
+            verbose=VERBOSE,
         )

If “/cartPole” is not the correct root in this asset, replace it with the actual root path or drop the argument.

newton/tests/test_import_mjcf.py (1)

35-39: Migration to ModelBuilder.add_mjcf is solid; minor axis-style consistency nit

All call sites now target the builder method and preserve intent/options. One minor consistency tweak: prefer the enum over a string for up_axis to align with the signature and avoid future type drift.

Apply this small diff in the first test (optional but recommended):

-        builder.add_mjcf(
-            mjcf_filename,
-            ignore_names=["floor", "ground"],
-            up_axis="Z",
-        )
+        builder.add_mjcf(
+            mjcf_filename,
+            ignore_names=["floor", "ground"],
+            up_axis=newton.Axis.Z,
+        )

If you agree, I can sweep-convert other tests that pass axis strings.

Also applies to: 107-108, 148-149, 162-163, 226-227, 263-264, 300-301, 349-350, 398-399, 455-456, 499-500

newton/tests/test_examples.py (1)

299-304: Likely module path + viewer mode update needed for example_h1

If h1 also moved under newton/examples/robot/ and uses the viewer API (as with other robot examples), update the import path and enable use_viewer.

-add_example_test(
-    TestRobotExamples,
-    name="example_h1",
-    devices=test_devices,
-    test_options={"usd_required": True, "num_frames": 500, "headless": True},
-    test_options_cpu={"num_frames": 10},
-)
+add_example_test(
+    TestRobotExamples,
+    name="robot.example_h1",
+    devices=test_devices,
+    test_options={"usd_required": True, "num_frames": 500, "headless": True},
+    test_options_cpu={"num_frames": 10},
+    use_viewer=True,
+)

If example_g1 similarly moved under robot/, make the same path/viewer adjustments there as well.

newton/examples/example_robot_manipulating_cloth.py (3)

360-373: URDF importer migration looks good; prefer enum for up_axis

Using builder.add_urdf(...) directly is correct and options map 1:1. For up_axis, pass the enum for type clarity and future safety.

-        builder.add_urdf(
-            str(asset_path / "urdf" / "fr3_franka_hand.urdf"),
-            up_axis=self.up_axis,
+        builder.add_urdf(
+            str(asset_path / "urdf" / "fr3_franka_hand.urdf"),
+            up_axis=newton.Axis.Y,

---

50-67: Type hint/docstring mismatch: function returns a quaternion, not a transform

vec_rotation(...) returns wp.quat(...) but is annotated/documented as returning a wp.transform. Tighten the API to avoid confusion.

-def vec_rotation(x: float, y: float, z: float) -> wp.transform:
-    """Convert plane coordinates given by the plane normal and its offset along the normal to a transform."""
+def vec_rotation(x: float, y: float, z: float) -> wp.quat:
+    """Return a quaternion rotating +Z to the given normal direction."""

---

422-425: Duplicate assignment to self.targets

self.targets = self.robot_key_poses[:, 1:] appears twice back-to-back; the first is redundant.

-        self.targets = self.robot_key_poses[:, 1:]
-        self.targets = self.robot_key_poses[:, 1:]
+        self.targets = self.robot_key_poses[:, 1:]

newton/examples/example_ik_benchmark.py (2)

61-61: Nit: The partial(..., scale=1.0) is unnecessary.

scale=1.0 is already the default for ModelBuilder.add_urdf; you can drop the partial without changing behavior. This slightly simplifies the config and avoids confusion about whether the method is bound.

Apply this diff if you prefer the simpler form:

-        parser=partial(newton.ModelBuilder.add_urdf, scale=1.0),
+        parser=newton.ModelBuilder.add_urdf,

---

138-141: Pass a string path to the importer for consistency and robustness.

Elsewhere (e.g., example_mujoco.py), paths are passed as str(...). While many stdlib functions accept PathLike, downstream XML loaders and helpers can be stricter in some environments. Converting to str here avoids edge cases and aligns examples.

Apply this diff:

-        asset_path = newton.utils.download_asset(self.cfg.asset) / self.cfg.file
-        self.cfg.parser(builder, asset_path, floating=False)
+        asset_path = newton.utils.download_asset(self.cfg.asset) / self.cfg.file
+        self.cfg.parser(builder, str(asset_path), floating=False)

newton/examples/example_mujoco.py (1)

79-83: Optional: Use Axis.Z instead of "Z" for clarity.

The API accepts either, but using the enum improves readability and type clarity across examples.

Apply this diff if you like:

-    articulation_builder.add_mjcf(
-        newton.examples.get_asset("nv_humanoid.xml"),
-        ignore_names=["floor", "ground"],
-        up_axis="Z",
-    )
+    articulation_builder.add_mjcf(
+        newton.examples.get_asset("nv_humanoid.xml"),
+        ignore_names=["floor", "ground"],
+        up_axis=newton.Axis.Z,
+    )

(Repeat similarly for other "Z" usages in this file.)

newton/tests/test_equality_constraints.py (2)

32-37: Minor readability: prefer pathlib for test asset paths.

Pathlib avoids os.path.join verbosity and is cross-platform by construction.

Apply this diff if desired:

-        builder.add_mjcf(
-            os.path.join(os.path.dirname(__file__), "assets", "constraints.xml"),
+        from pathlib import Path
+        builder.add_mjcf(
+            str(Path(__file__).with_name("assets") / "constraints.xml"),
             ignore_names=["floor", "ground"],
             up_axis="Z",
             skip_equality_constraints=False,
         )

---

16-17: Optional: remove unused import(s) if flagged by linters.

If your linting pipeline flags unused imports, consider dropping numpy as np here (only used later; keep if your test harness expects it). Otherwise, ignore this note.

newton/_src/utils/import_urdf.py (1)

53-60: Consider broadening the type hint for urdf_filename to accept os.PathLike[str].

Most stdlib consumers accept pathlikes; explicitly supporting them in the signature improves ergonomics. If you keep it str, ensure call sites pass str(...) (as suggested in the IK benchmark).

Proposed change:

-def parse_urdf(
-    builder: ModelBuilder,
-    urdf_filename: str,
+def parse_urdf(
+    builder: ModelBuilder,
+    urdf_filename: str,  # or: os.PathLike[str] | str

If you adopt PathLike, also wrap uses with os.fspath(urdf_filename) where string paths are required.

newton/tests/test_import_usd.py (1)

80-82: Nit: bitwise checks on flags — cast consistently or rely on IntFlag.

You sometimes use int(newton.ShapeFlags.COLLIDE_SHAPES) and sometimes not. Both work, but pick one style for consistency (prefer no cast if ShapeFlags is an IntFlag).

newton/tests/test_api.py (3)

30-44: Good parity harness; one brittle assertion can cause noisy failures across minor API edits

The structure is solid. The strict length equality is fine, but coupling to “excluding the first param” makes assumptions about the builder-first convention. If any parser/method gains a positional-only or keyword-only boundary change (e.g., adding a trailing “/” or “*”), this will fail even if it’s semantically compatible. Consider enhancing the error to print both full signatures for debuggability.

Apply this diff to include the full signatures in failure output:

-    assert len(func_params) == len(method_params), (
-        f"Parameter count mismatch (excluding first): "
-        f"{len(func_params)} ({func_name}) != {len(method_params)} ({method_name})"
-    )
+    assert len(func_params) == len(method_params), (
+        f"Parameter count mismatch (excluding first): "
+        f"{len(func_params)} ({func_name}) != {len(method_params)} ({method_name}); "
+        f"func={sig_func!s}, method={sig_method!s}"
+    )

---

63-67: Default-value equality check is too strict (e.g., tuple vs list, np arrays, NaNs)

Parser defaults and builder defaults sometimes use equivalent-but-not-identical objects ((), []), or float("nan"). A strict “==” will falsely fail parity. Normalize before compare.

Apply this diff to compare defaults more robustly:

-        assert pf.default == pm.default, (
+        def _norm_default(v):
+            import math
+            if v is inspect._empty:
+                return v
+            if isinstance(v, (list, tuple)):
+                return tuple(v)
+            if isinstance(v, float) and math.isnan(v):
+                return ("nan",)
+            return v
+        assert _norm_default(pf.default) == _norm_default(pm.default), (
             f"Param {pf.name!r} default mismatch: {pf.default!r} ({func_name}) != {pm.default!r} ({method_name})"
         )

---

85-89: Docstring equality is brittle; prefer normalized semantic checks or allow references

Requiring exact docstring equality will churn on harmless edits (punctuation, Sphinx markup). Recommend either:

• Allowing docstrings to match if one contains a “See also” reference to the other, or
• Normalizing by lowercasing, removing multiple spaces, and stripping Sphinx roles before compare.

Apply this diff to make the doc check less brittle while preserving parity intent:

-    doc_func = "\n".join(line.strip() for line in (func.__doc__ or "").splitlines()).strip()
-    doc_method = "\n".join(line.strip() for line in (method.__doc__ or "").splitlines()).strip()
-    assert doc_func == doc_method, f"Docstring mismatch between {func_name} and {method_name}"
+    import re
+    def _normalize_doc(s: str) -> str:
+        s = "\n".join(line.strip() for line in (s or "").splitlines()).strip().lower()
+        s = re.sub(r":\w+:`[^`]+`", "", s)  # drop Sphinx roles
+        s = re.sub(r"\s+", " ", s)
+        return s
+    doc_func = _normalize_doc(func.__doc__ or "")
+    doc_method = _normalize_doc(method.__doc__ or "")
+    # allow parity if either references the other
+    if not (doc_func == doc_method or func_name in doc_method or method_name in doc_func):
+        raise AssertionError(f"Docstring mismatch between {func_name} and {method_name}")

newton/_src/utils/import_mjcf.py (2)

331-384: Mesh path: assertion and normals access look inconsistent with comment

Comment says “ignore geom size attribute” but asserts len(geom_size) == 3. Also, trimesh.Scene meshes use per-geometry vertex_normals; for single mesh case, we access m.vertex_normals which may be empty until computed. Consider calling m.vertex_normals or m.vertex_normals if available; otherwise compute via m.vertex_normals = m.vertex_normals (trimesh computes lazily).

Would you like me to draft a guarded normals extraction that computes normals when missing? I can also convert the assertion into a warning and ignore geom_size as intended.

---

754-764: Nonzero inertia guard uses any() on a mat33; clarify intent

Using any(x for x in I_m) on a mat33 is unclear. It’s safer to check determinant or sum of absolute values.

Apply this diff:

-            if any(x for x in I_m):
+            if float(abs(I_m[0,0]) + abs(I_m[1,1]) + abs(I_m[2,2])) > 0.0:
                 builder.body_inv_inertia[link] = wp.inverse(I_m)
             else:
                 builder.body_inv_inertia[link] = I_m

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

430-447: Face index dtype should be integer; minor cleanup

Indices are loaded as float32 then converted later. Load as int32 upfront to avoid shape/dtype confusion when building faces.

Apply this diff:

-                indices = np.array(mesh.GetFaceVertexIndicesAttr().Get(), dtype=np.float32)
+                indices = np.array(mesh.GetFaceVertexIndicesAttr().Get(), dtype=np.int32)

And likewise in the collision-shape mesh path:

-                    indices = np.array(mesh.GetFaceVertexIndicesAttr().Get(), dtype=np.float32)
+                    indices = np.array(mesh.GetFaceVertexIndicesAttr().Get(), dtype=np.int32)

---

1185-1197: Inverse inertia computation mixes mat33 and constructor args

For the “else” path, constructing wp.mat33 with star-expansion of a numpy array is error-prone. Prefer wp.mat33(numpy_array) consistently, then inverse on the same.

Apply this diff:

-                if inertia.any():
-                    builder.body_inv_inertia[body_id] = wp.inverse(wp.mat33(*inertia))
-                else:
-                    builder.body_inv_inertia[body_id] = wp.mat33(*np.zeros((3, 3), dtype=np.float32))
+                M = wp.mat33(inertia)
+                builder.body_inertia[body_id] = M
+                builder.body_inv_inertia[body_id] = wp.inverse(M) if inertia.any() else wp.mat33(np.zeros((3, 3), dtype=np.float32))

newton/examples/robot/example_humanoid.py (1)

71-75: Controller gains hardcoded uniformly; consider centralizing per-DOF defaults

Minor, but if other robot examples follow, factor these target gains into a helper to ensure consistency across examples and simplify tuning.

I can draft a small utility (e.g., newton.examples.set_pd_targets(builder, mode, ke, kd)) if you want to use it here and in anymal/cartpole examples.

newton/examples/robot/example_anymal_d.py (2)

118-130: Contacts computed once per frame; may under-resolve high-frequency contacts

In the basic example, contacts are updated every substep. Here you compute before the loop. For stiff contact setups, consider moving collide() inside the substep loop like the basic example to match solver expectations.

Apply this diff if you want parity with the basic example:

-    def simulate(self):
-        self.contacts = self.model.collide(self.state_0)
-        for _ in range(self.sim_substeps):
+    def simulate(self):
+        for _ in range(self.sim_substeps):
+            self.contacts = self.model.collide(self.state_0)
             self.state_0.clear_forces()
             # apply forces to the model for picking, wind, etc
             self.viewer.apply_forces(self.state_0)
             self.solver.step(self.state_0, self.state_1, self.control, self.contacts, self.sim_dt)
             # swap states
             self.state_0, self.state_1 = self.state_1, self.state_0

If MuJoCo’s internal contact update cadence already matches your step, feel free to keep as-is.

---

91-95: Solver parameters are explicit; consider exposing via CLI flags for quick tuning

Optional: add example-level args (iterations, ls_iterations) to experiment without code changes.

I can thread these through newton.examples.create_parser via an example-specific parent parser if desired.

docs/migration.rst (1)

27-37: Good mapping; verify public surface for resolve_usd_from_url

The table accurately maps the importer entry points. One concern: pointing users to newton._src.utils.import_usd.resolve_usd_from_url exposes an internal, underscore-prefixed module path.

• If this utility is intended to be public, consider re-exporting it under a stable public namespace (e.g., newton.utils.resolve_usd_from_url) and update the table accordingly.
• If it’s not intended to be public, consider removing it from the migration guide or labeling it as “advanced/internal”.

I can patch the docs once you confirm which direction you prefer.

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: nvlukasz
PR: newton-physics/newton#519
File: newton/utils.py:30-33
Timestamp: 2025-08-12T18:07:17.267Z
Learning: In newton/_src/utils/import_usd.py, the PXR/USD modules are lazily loaded using try-except blocks within the functions rather than at module import time, making it safe to import parse_usd at the module level without causing import-time failures for users without USD dependencies.

Learnt from: nvlukasz
PR: newton-physics/newton#519
File: newton/utils.py:30-33
Timestamp: 2025-08-12T18:07:17.267Z
Learning: In newton/_src/utils/import_usd.py, the PXR/USD modules are lazily loaded using try-except blocks within the functions (lines 109-111 and 1289-1291) rather than at module import time, making it safe to import parse_usd at the module level without causing import-time failures for users without USD dependencies.

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

Learnt from: dylanturpin
PR: newton-physics/newton#634
File: newton/tests/test_examples.py:329-333
Timestamp: 2025-08-25T21:41:45.769Z
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.

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

Learnt from: shi-eric
PR: newton-physics/newton#521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The STYLE3D solver in Newton is a new solver that did not exist in warp.sim, so it does not require any migration documentation.