newton/_src/viewer/viewer_usd.py (2)

20-26: Docstring default contradicts code (fps default 24 vs 60).

The Args section says fps default is 24, but the constructor uses 60. Align the docs or the code.

Apply this diff to fix the docstring:

-        fps: Frames per second for time sampling (default: 24)
+        fps: Frames per second for time sampling (default: 60)

---

207-214: PointInstancer ids/protoIndices authored only once — can desync when instance count changes across frames.

If num_instances varies over time, fixed ids/protoIndices authored “once” will mismatch attribute lengths and cause viewer issues. Re-author when counts differ.

Apply this diff:

-            # Initialize ids and protoIndices once for count stability
-            ids_attr = instancer.GetIdsAttr()
-            if ids_attr.GetNumTimeSamples() == 0 and not ids_attr.HasAuthoredValueOpinion():
-                instancer.CreateIdsAttr().Set(list(range(num_instances)))
-                instancer.CreateProtoIndicesAttr().Set([0] * num_instances)
+            # Ensure ids/protoIndices length matches current instance count
+            ids_attr = instancer.GetIdsAttr()
+            existing_ids = ids_attr.Get() if ids_attr.IsValid() else None
+            if existing_ids is None or len(existing_ids) != num_instances:
+                instancer.CreateIdsAttr().Set(list(range(num_instances)))
+                instancer.CreateProtoIndicesAttr().Set([0] * num_instances)

newton/_src/sim/builder.py (1)

575-582: Nit: Ensure transform arg types are explicit vec3 to avoid Warp coercion surprises.

Passing a NumPy row into wp.transform usually works, but being explicit improves readability and type clarity.

Apply this diff:

-        for i in range(num_copies):
-            self.add_builder(builder, xform=wp.transform(offsets[i], wp.quat_identity()))
+        for i in range(num_copies):
+            p = wp.vec3(float(offsets[i][0]), float(offsets[i][1]), float(offsets[i][2]))
+            self.add_builder(builder, xform=wp.transform(p, wp.quat_identity()))

newton/examples/__init__.py (1)

50-56: Deprecation warning: good placement and stacklevel

The explicit warning and stacklevel are appropriate. Consider adding a brief pointer to docs/migration.rst in the message to aid users migrating from compute_env_offsets to builder.replicate().

-    warnings.warn(
-        "compute_env_offsets is deprecated and will be removed in a future version. Use the builder.replicate() function instead.",
+    warnings.warn(
+        "compute_env_offsets is deprecated and will be removed in a future version. Use builder.replicate() instead. See docs/migration.rst for details.",
         stacklevel=2,
     )

newton/examples/basic/example_basic_pendulum.py (3)

16-26: Header comment is incorrect for this example

The header says “Example Basic URDF” and references a quadruped/URDF, but this file implements a pendulum example. Please correct to avoid confusion.

-###########################################################################
-# Example Basic URDF
+###########################################################################
+# Example Basic Pendulum
@@
-# Shows how to set up a simulation of a rigid-body quadruped articulation
-# from a URDF using the newton.ModelBuilder().
+# Shows how to set up and simulate a simple two-link pendulum using
+# newton.ModelBuilder().
@@
-# Users can pick bodies by right-clicking and dragging with the mouse.
+# Users can pick bodies by right-clicking and dragging with the mouse (in ViewerGL).

---

59-64: Consider specifying body mass to ensure dynamic behavior

By default, ModelBuilder.add_body() may create kinematic bodies if mass is 0. If that’s the case in this codebase, the pendulum won’t move. Consider setting a positive mass per link or using builder defaults to ensure dynamic simulation.

Would you like a patch that sets per-link masses and inertia approximations based on the box dimensions?

---

99-101: Minor grammar: “its” vs “it’s”

“It’s” is a contraction; possessive is “its.”

-        # put graph capture into it's own function
+        # put graph capture into its own function

newton/_src/viewer/viewer_rerun.py (2)

204-210: is_running may never terminate in run(viewer, example) flows

Without a spawned viewer process or an external stop signal, is_running returns self._running which is never toggled, causing infinite loops under newton.examples.run. Consider adding an optional num_frames limit (parity with ViewerNull) or respecting rr.is_recording/connection status.

Would you like a patch to add a num_frames parameter and increment an internal frame counter in begin_frame()/end_frame()?

---

216-225: _viewer_process lifecycle handling appears unused

You never assign to self._viewer_process; terminate/kill paths won’t execute. Either store the process handle from rr.serve_web_viewer (if available) or remove the dead code.

-        self._viewer_process = None
+        self._viewer_process = None  # Consider capturing a handle if rr exposes it; otherwise remove related code.

If rr.serve_web_viewer doesn’t return a Popen-like handle, remove the terminate/kill branches to avoid confusion.

newton/_src/viewer/viewer_null.py (1)

46-51: Update begin_frame to retain ViewerBase time

For consistency with other backends, call super().begin_frame(time) so self.time is tracked (useful for logs or future extensions).

-    def begin_frame(self, time):
-        pass
+    def begin_frame(self, time):
+        super().begin_frame(time)

newton/examples/basic/example_basic_viewer.py (4)

82-85: Fix viewer type check (always false currently).

viewer is now an object, not a string. The condition on Line 82 never triggers, so the usage hint is never printed for the GL backend.

Apply this diff to correctly detect the GL backend:

-        if viewer == "gl":
-            print("Viewer running. WASD/Arrow keys to move, drag to orbit, scroll to zoom. Close window to exit.")
+        # Only print usage hints for interactive GL viewer
+        try:
+            from newton.viewer import ViewerGL as _ViewerGL
+            if isinstance(viewer, _ViewerGL):
+                print("Viewer running. WASD/Arrow keys to move, drag to orbit, scroll to zoom. Close window to exit.")
+        except Exception:
+            pass

---

170-177: Align plane geo_scale with ViewerBase.log_geo contract.

log_shapes(..., GeoType.PLANE, (0.0, 0.0, 1.0, 0.0), ...) passes 4 values that are ignored by the plane path (only width/length are used). This is confusing and suggests plane-equation parameters, which aren’t supported here.

Use a 2-tuple (or a single float) to convey “infinite” plane intent:

-        self.viewer.log_shapes(
-            "/plane_instance",
-            newton.GeoType.PLANE,
-            (0.0, 0.0, 1.0, 0.0),
-            wp.array([wp.transform_identity()], dtype=wp.transform),
-            self.col_plane,
-            self.mat_plane,
-        )
+        self.viewer.log_shapes(
+            "/plane_instance",
+            newton.GeoType.PLANE,
+            (0.0, 0.0),  # non-positive -> treated as a large ground plane
+            wp.array([wp.transform_identity()], dtype=wp.transform),
+            self.col_plane,
+            self.mat_plane,
+        )

---

94-106: Comment/axis mismatch (Y used, comment says X).

The layout uses the Y coordinate (Line 106 uses base_left as Y), but comments say “arrange along X-axis” and “Sphere at x = -6”. Consider aligning comments with the actual axis used to avoid confusion.

Apply this diff to fix comments:

-        # Clean layout: arrange objects in a line along X-axis
+        # Clean layout: arrange objects in a line along Y-axis
...
-        # Sphere: gentle bounce at x = -6
+        # Sphere: gentle bounce at y ≈ -4

---

179-179: Clarify log_lines path name.

"coordinate_self.axes" looks like a typo and is inconsistent with other log paths that use a leading slash.

Rename for consistency:

-        self.viewer.log_lines("coordinate_self.axes", self.axes_begins, self.axes_ends, self.axes_colors)
+        self.viewer.log_lines("/axes", self.axes_begins, self.axes_ends, self.axes_colors)

newton/_src/viewer/viewer.py (1)

160-163: Fix comment color mismatch.

Comment says orange-red, but the color is green.

Minimal correction:

-        # Use orange-red color for contact normals
+        # Use green color for contact normals
         line_colors = (0.0, 1.0, 0.0)

newton/tests/test_examples_new.py (2)

100-107: Ensure viewer cleanup even on exceptions.

Wrap the frame loop in try/finally to guarantee viewer.close() runs. Reduces resource leaks and stray open windows/files on failure.

Apply this diff:

-        # run for N frames
-        for _ in range(num_frames):
-            example.step()
-            example.render()
-            example.test()
-
-        viewer.close()
+        try:
+            # run for N frames
+            for _ in range(num_frames):
+                example.step()
+                example.render()
+                example.test()
+        finally:
+            try:
+                viewer.close()
+            except Exception:
+                pass

---

85-88: Remove or use unused env_vars.

env_vars is computed but never used. Either set os.environ["WARP_CACHE_PATH"] inline or drop the block.

One-liner fix:

-        env_vars = os.environ.copy()
-        if warp_cache_path is not None:
-            env_vars["WARP_CACHE_PATH"] = warp_cache_path
+        if warp_cache_path is not None:
+            os.environ["WARP_CACHE_PATH"] = warp_cache_path

Configuration used: .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: 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.

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

71-77: Replication is fine; consider explicit spacing or per-env filtering if environments could interact

Using replicate(quadruped, self.num_envs) with default spacing likely avoids cross-env interactions. If you plan to reduce spacing or increase num_envs significantly, consider making spacing explicit or adding per-environment collision filtering to prevent inter-env collisions.

Example (optional):

-        scene.replicate(quadruped, self.num_envs)
+        scene.replicate(quadruped, self.num_envs, spacing=(5.0, 5.0, 0.0))  # explicit grid spacing

---

93-95: Fix minor grammar: "it's" → "its"

Nitpick on comment wording.

-        # put graph capture into it's own function
+        # put graph capture into its own function

---

96-103: Capture device should match model.device to avoid mismatches on multi-device setups

Using wp.get_device() reads the default device, which may differ from model.device. Tie capture to the model’s device for correctness.

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

---

104-116: Contacts logged for rendering can lag one substep; refresh after the loop

You compute contacts before stepping, then swap states; render() logs contacts with the post-step state, which can be visually out of sync by one substep. Refresh contacts after the loop for accurate visualization.

     def simulate(self):
         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.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
+        # refresh contacts for the post-step state used for rendering
+        self.contacts = self.model.collide(self.state_0)

---

125-127: Provide a minimal smoke test implementation or remove the stub

Empty test() can hinder automated example tests. Implement a short smoke step or remove the method if not used by the harness.

-    def test(self):
-        pass
+    def test(self):
+        # Smoke test: run a couple of substeps to validate stepping without rendering
+        for _ in range(2):
+            self.simulate()

If the test harness doesn’t call test(), consider removing the method. I can wire this to newton/tests/test_examples*.py if you’d like.

---

35-37: Make num_envs optional in example_basic_urdf.py for consistency

Update the URDF example’s constructor so it matches the other basic examples (which take only viewer) and avoids breaking calls that omit num_envs.

• File: newton/examples/basic/example_basic_urdf.py
Change the __init__ signature as follows:

 class Example:
-    def __init__(self, viewer, num_envs):
+    def __init__(self, viewer, num_envs=1):
     # setup simulation parameters first
     self.fps = 100

This lets you call Example(viewer) just like in example_basic_pendulum.py and example_basic_viewer.py, while still allowing explicit overrides (e.g. num_envs=100).

---

136-139: Add headless fallback to ViewerNull in example_basic_urdf.py

Creating ViewerGL can fail on servers without an OpenGL context (e.g., CI or headless environments). Wrap its construction in a try/except and fall back to ViewerNull, which is already exported in newton.viewer.

• File: newton/examples/basic/example_basic_urdf.py (around lines 136–139)
• Change:

-if __name__ == "__main__":
-    viewer = newton.viewer.ViewerGL()
-    example = Example(viewer, num_envs=100)
-
-    newton.examples.run(example)
+if __name__ == "__main__":
+    try:
+        viewer = newton.viewer.ViewerGL()
+    except Exception:
+        # Headless fallback for CI / non-GL environments
+        viewer = newton.viewer.ViewerNull()
+    example = Example(viewer, num_envs=100)
+    newton.examples.run(example)

newton/examples/basic/example_basic_viewer.py (5)

94-99: Fix axis mentions in comments (objects are laid out along Y, not X)

Comments say X-axis and “x = …”, but base_left increments along Y in transforms.

Apply this diff:

-        # Clean layout: arrange objects in a line along X-axis
+        # Clean layout: arrange objects in a line along Y-axis
@@
-        # Sphere: gentle bounce at x = -6
+        # Sphere: gentle bounce at y = -4 (initial)
@@
-        # Box: rocking rotation at x = -3
+        # Box: rocking rotation at y = -2
@@
-        # Cone: spinning at origin (x = 0)
+        # Cone: spinning at origin (y = 0)
@@
-        # Cylinder: spinning on different axis at x = 3
+        # Cylinder: spinning on different axis at y = +2
@@
-        # Capsule: gentle sway at x = 6
+        # Capsule: gentle sway at y = +4

Also applies to: 105-126

---

179-179: Tweak log_lines path for readability

Name “coordinate_self.axes” reads odd. Consider a clearer path-like name.

-        self.viewer.log_lines("coordinate_self.axes", self.axes_begins, self.axes_ends, self.axes_colors)
+        self.viewer.log_lines("/debug/axes", self.axes_begins, self.axes_ends, self.axes_colors)

---

186-187: Optional: add a tiny smoke test to exercise rendering without a window

A short headless smoke in test() helps CI run examples (e.g., with ViewerUSD or ViewerNull). Not required, but useful.

Example implementation (adjust if your test harness provides a viewer):

def test(self):
    # Render a handful of frames deterministically
    for _ in range(5):
        self.render()

If you prefer a Null backend, I can open a follow-up PR to add a CLI --viewer null option and wire this example into the example tests.

---

192-193: Optional: Add “null” viewer backend for headless CI

ViewerNull is already implemented and exported in newton.viewer. Adding a "null" choice to the example’s --viewer flag enables faster, window-free CI runs.

In newton/examples/basic/example_basic_viewer.py:

• Update the argument parser to include "null" in the choices.
• Add an elif args.viewer == "null": branch to import and instantiate ViewerNull.

Proposed diff:

-    parser.add_argument("--viewer", choices=["gl", "usd", "rerun"], default="gl", help="Viewer backend to use.")
+    parser.add_argument("--viewer", choices=["gl", "usd", "rerun", "null"], default="gl", help="Viewer backend to use.")

     elif args.viewer == "rerun":
         from newton.viewer import ViewerRerun
         viewer = ViewerRerun(server=True, launch_viewer=True)
+    elif args.viewer == "null":
+        from newton.viewer import ViewerNull
+        viewer = ViewerNull(num_frames=120)

---

170-177: Plane geo_scale should be width/length or float, not a 4-tuple

The log_shapes doc and implementation expect for GeoType.PLANE a 2-tuple (width, length) or a single float (both dimensions), with non-positive values signalling an “infinite” plane. Passing a 4-tuple (nx, ny, nz, d) (a plane equation) is unsupported—only the first two entries are used (and extra values will pollute the mesh cache).

• File: newton/examples/basic/example_basic_viewer.py
Lines 172–174: replace the 4-tuple with either a 2-tuple or a float.

Example diffs:

Option A – infinite plane (default checkerboard):

-            newton.GeoType.PLANE,
-            (0.0, 0.0, 1.0, 0.0),
+            newton.GeoType.PLANE,
+            (0.0, 0.0),  # (width, length); zeros → infinite plane

Option B – finite plane size:

-            newton.GeoType.PLANE,
-            (0.0, 0.0, 1.0, 0.0),
+            newton.GeoType.PLANE,
+            12.0,  # width=length=12 units

Configuration used: .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: 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.

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

56-70: Correct Warp API usage for transforms — past issue addressed

Using wp.transform with wp.quat_identity() is the right API (vs. non-existent wp.transformf). Hinge frames for both joints look consistent.

newton/_src/viewer/viewer_gl.py (1)

92-101: Set a default Warp device when no model is provided

If set_model(None) is used (as here), self.device stays None and log_mesh/log_lines will fail when allocating arrays. Initialize a sensible default device for model-less flows.

Apply this diff:

     def set_model(self, model):
-        super().set_model(model)
+        super().set_model(model)
+
+        # Ensure a valid device for model-less workflows (e.g., viewer-only)
+        if model is None and self.device is None:
+            self.device = wp.get_device()

newton/examples/basic/example_basic_viewer.py (2)

36-46: Allocate all wp.arrays on the viewer device (GPU/CPU) to prevent device-mismatch bugs

Create a local device handle from the viewer and pass device=self.device to every wp.array (colors, materials, axes, and per-frame transform arrays). This avoids GL backend crashes when Warp’s default device differs from the viewer device.

Apply this diff:

 def __init__(self, viewer):
-        self.viewer = viewer
+        self.viewer = viewer
+        self.device = viewer.device

         # self.colors and materials per instance
-        self.col_sphere = wp.array([wp.vec3(0.9, 0.1, 0.1)], dtype=wp.vec3)
-        self.col_box = wp.array([wp.vec3(0.1, 0.9, 0.1)], dtype=wp.vec3)
-        self.col_cone = wp.array([wp.vec3(0.1, 0.4, 0.9)], dtype=wp.vec3)
-        self.col_capsule = wp.array([wp.vec3(0.9, 0.9, 0.1)], dtype=wp.vec3)
-        self.col_cylinder = wp.array([wp.vec3(0.8, 0.5, 0.2)], dtype=wp.vec3)
-        self.col_plane = wp.array([wp.vec3(0.125, 0.125, 0.15)], dtype=wp.vec3)
+        self.col_sphere = wp.array([wp.vec3(0.9, 0.1, 0.1)], dtype=wp.vec3, device=self.device)
+        self.col_box = wp.array([wp.vec3(0.1, 0.9, 0.1)], dtype=wp.vec3, device=self.device)
+        self.col_cone = wp.array([wp.vec3(0.1, 0.4, 0.9)], dtype=wp.vec3, device=self.device)
+        self.col_capsule = wp.array([wp.vec3(0.9, 0.9, 0.1)], dtype=wp.vec3, device=self.device)
+        self.col_cylinder = wp.array([wp.vec3(0.8, 0.5, 0.2)], dtype=wp.vec3, device=self.device)
+        self.col_plane = wp.array([wp.vec3(0.125, 0.125, 0.15)], dtype=wp.vec3, device=self.device)

         # material = (metallic, roughness, checker, unused)
-        self.mat_default = wp.array([wp.vec4(0.0, 0.7, 0.0, 0.0)], dtype=wp.vec4)
-        self.mat_plane = wp.array([wp.vec4(0.5, 0.5, 1.0, 0.0)], dtype=wp.vec4)
+        self.mat_default = wp.array([wp.vec4(0.0, 0.7, 0.0, 0.0)], dtype=wp.vec4, device=self.device)
+        self.mat_plane = wp.array([wp.vec4(0.5, 0.5, 1.0, 0.0)], dtype=wp.vec4, device=self.device)

         # Demonstrate log_lines() with animated debug/visualization lines
         axis_eps = 0.01
         axis_length = 2.0
-        self.axes_begins = wp.array(
+        self.axes_begins = wp.array(
             [
                 wp.vec3(0.0, 0.0, axis_eps),  # X axis start
                 wp.vec3(0.0, 0.0, axis_eps),  # Y axis start
                 wp.vec3(0.0, 0.0, axis_eps),  # Z axis start
             ],
-            dtype=wp.vec3,
+            dtype=wp.vec3,
+            device=self.device,
         )

-        self.axes_ends = wp.array(
+        self.axes_ends = wp.array(
             [
                 wp.vec3(axis_length, 0.0, axis_eps),  # X axis end
                 wp.vec3(0.0, axis_length, axis_eps),  # Y axis end
                 wp.vec3(0.0, 0.0, axis_length + axis_eps),  # Z axis end
             ],
-            dtype=wp.vec3,
+            dtype=wp.vec3,
+            device=self.device,
         )

-        self.axes_colors = wp.array(
+        self.axes_colors = wp.array(
             [
                 wp.vec3(1.0, 0.0, 0.0),  # Red X
                 wp.vec3(0.0, 1.0, 0.0),  # Green Y
                 wp.vec3(0.0, 0.0, 1.0),  # Blue Z
             ],
-            dtype=wp.vec3,
+            dtype=wp.vec3,
+            device=self.device,
         )

And in render (per-frame transform arrays):

-        x_sphere_anim = wp.array([wp.transform(sphere_pos, qy_slow)], dtype=wp.transform)
+        x_sphere_anim = wp.array([wp.transform(sphere_pos, qy_slow)], dtype=wp.transform, device=self.device)

-        x_box_anim = wp.array([wp.transform([0.0, base_left, base_height], qx_slow)], dtype=wp.transform)
+        x_box_anim = wp.array([wp.transform([0.0, base_left, base_height], qx_slow)], dtype=wp.transform, device=self.device)

-        x_cone_anim = wp.array([wp.transform([0.0, base_left, base_height], qy_slow)], dtype=wp.transform)
+        x_cone_anim = wp.array([wp.transform([0.0, base_left, base_height], qy_slow)], dtype=wp.transform, device=self.device)

-        x_cyl_anim = wp.array([wp.transform([0.0, base_left, base_height], qz_slow)], dtype=wp.transform)
+        x_cyl_anim = wp.array([wp.transform([0.0, base_left, base_height], qz_slow)], dtype=wp.transform, device=self.device)

-        x_cap_anim = wp.array([wp.transform(capsule_pos, qy_slow)], dtype=wp.transform)
+        x_cap_anim = wp.array([wp.transform(capsule_pos, qy_slow)], dtype=wp.transform, device=self.device)

Also applies to: 47-49, 54-61, 63-70, 72-79, 106-106, 111-111, 115-115, 119-119, 124-124

---

81-82: Remove stale string check for viewer type; print GL help in main instead

viewer is an object, so viewer == "gl" will never be True. Move the guidance print into main based on args.viewer.

Apply these diffs:

-        if viewer == "gl":
-            print("Viewer running. WASD/Arrow keys to move, drag to orbit, scroll to zoom. Close window to exit.")

 if __name__ == "__main__":
     # Parse arguments and initialize viewer
     viewer, args = newton.examples.init()

-    # Create viewer and run
+    # Print GL-only help when using the GL viewer backend
+    if getattr(args, "viewer", None) == "gl":
+        print("Viewer running. WASD/Arrow keys to move, drag to orbit, scroll to zoom. Close window to exit.")
+
+    # Create viewer and run
     example = Example(viewer)
     newton.examples.run(example)

Also applies to: 189-196

newton/tests/test_examples.py (1)

179-182: Reduce noisy logs: print stderr only on failure (optional)

Currently prints stderr even on success, which can spam CI logs. Consider printing stderr/stdout only when returncode != 0.

Apply this diff:

-        # print any error messages (e.g.: module not found)
-        if result.stderr != "":
-            print(result.stderr)
+        # Print process output on failure to aid debugging
+        if result.returncode != 0:
+            if result.stdout:
+                print(result.stdout)
+            if result.stderr:
+                print(result.stderr)

newton/examples/__init__.py (1)

122-134: Docstring parameter mismatch (nit): clarify the ‘parser’ argument

The function expects an argparse.ArgumentParser (or None) and internally calls parse_known_args, but the docstring says “Parsed arguments.” Recommend updating the docstring for accuracy.

Apply this diff:

-    Args:
-        parser: Parsed arguments from argparse (should include arguments from
-              create_parser())
+    Args:
+        parser: An argparse.ArgumentParser (or None). If None, a base parser from
+                create_parser() is constructed and parse_known_args() is used.

newton/examples/basic/example_basic_viewer.py (3)

178-178: Typo in log_lines name

Use a clearer identifier without “self.” in the literal string.

-        self.viewer.log_lines("coordinate_self.axes", self.axes_begins, self.axes_ends, self.axes_colors)
+        self.viewer.log_lines("coordinate_axes", self.axes_begins, self.axes_ends, self.axes_colors)

---

89-124: Optional: Avoid per-frame allocations by reusing xform arrays

Allocate one-element wp.arrays for each x_*_anim in init and update their element each frame instead of constructing new arrays. This reduces Python/allocator overhead and GPU allocations.

Example outline (outside current ranges):

• In init: create x_sphere_anim = wp.zeros(1, dtype=wp.transform, device=self.device) for each.
• In render(): write x_sphere_anim[0] = wp.transform(...).

---

185-186: Implement a tiny smoke test for test harnesses that call Example.test()

If newton/tests/test_examples.py expects examples to expose a test() entrypoint, calling render once is usually sufficient and works with ViewerNull.

Apply this diff:

-    def test(self):
-        pass
+    def test(self):
+        # Single render step for CI smoke testing (works with ViewerNull)
+        self.render()

Please confirm how test_examples.py discovers and executes example tests; if it expects a different contract, I can adjust accordingly.

Configuration used: .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: 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: adenzler-nvidia
PR: newton-physics/newton#552
File: newton/_src/solvers/mujoco/solver_mujoco.py:0-0
Timestamp: 2025-08-18T15:56:26.540Z
Learning: In Newton's MuJoCo solver, when transforming joint axes from Newton's internal frame to MuJoCo's expected frame, use wp.quat_rotate(joint_rot, axis) not wp.quat_rotate_inv(joint_rot, axis). The joint_rot represents rotation from joint-local to body frame, so forward rotation is correct.

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

56-70: Correct Warp API usage for transforms.

Using wp.transform with wp.quat_identity() for parent/child frames is correct and resolves the earlier ‘wp.transformf’ issue noted in prior reviews.

docs/images/examples/.gitattributes (1)

1-1: Broaden LFS patterns to cover .jpeg and uppercase variants.

Some systems or tools may emit .jpeg or uppercase .JPG/.JPEG. Adding these prevents large binaries from slipping outside LFS.

Apply this diff to extend coverage:

 *.jpg filter=lfs diff=lfs merge=lfs -text
+*.jpeg filter=lfs diff=lfs merge=lfs -text
+*.JPG filter=lfs diff=lfs merge=lfs -text
+*.JPEG filter=lfs diff=lfs merge=lfs -text

docs/images/examples/resize.sh (1)

1-2: Add shebang and make robust when no PNGs are present (ShellCheck SC2148).

Without a shebang, shells vary. Also avoid iterating the literal pattern when there are no matches.

Apply this diff:

-# resize all pngs in a directory by 2x in dimension + convert to jpg
-for f in *.png; do ffmpeg -y -i "$f" -vf "scale=iw/2:ih/2" -q:v 3 "${f%.png}.jpg"; done
+#!/usr/bin/env bash
+# resize all pngs in a directory by 2x in dimension + convert to jpg
+set -euo pipefail
+shopt -s nullglob
+for f in *.png; do
+  ffmpeg -y -i "$f" -vf "scale=iw/2:ih/2" -q:v 3 "${f%.png}.jpg"
+done

newton/examples/__init__.py (3)

36-45: Guarantee viewer.close() runs even if step/render raises.

Wrap the loop in try/finally to ensure proper cleanup.

Apply this diff:

-def run(example):
-    while example.viewer.is_running():
-        with wp.ScopedTimer("step"):
-            example.step()
-
-        with wp.ScopedTimer("render"):
-            example.render()
-
-    example.viewer.close()
+def run(example):
+    try:
+        while example.viewer.is_running():
+            with wp.ScopedTimer("step"):
+                example.step()
+            with wp.ScopedTimer("render"):
+                example.render()
+    finally:
+        example.viewer.close()

---

50-57: Optional guard for invalid num_envs to avoid min/max on empty arrays.

If num_envs <= 0, np.min/np.max will raise. Consider a clearer error.

Apply this diff:

     warnings.warn(
         "compute_env_offsets is deprecated and will be removed in a future version. Use the builder.replicate() function instead.",
         stacklevel=2,
     )
 
+    if num_envs <= 0:
+        raise ValueError("num_envs must be > 0")

---

122-134: Fix docstring: ‘parser’ is an ArgumentParser (or None), not parsed args.

The current docstring is misleading.

Apply this diff:

-def init(parser=None):
-    """Initialize Newton example components from parsed arguments.
-
-    Args:
-        parser: Parsed arguments from argparse (should include arguments from
-              create_parser())
+def init(parser=None):
+    """Initialize Newton example components from CLI arguments.
+
+    Args:
+        parser: An argparse.ArgumentParser to parse arguments (if None, a default
+                parser from create_parser() is used)

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

100-112: Minor: consider a scoped timer for simulate() for consistent profiling.

Optional, but adding wp.ScopedTimer around the substep loop aligns with the timers used in newton.examples.run.

Apply this diff:

-    def simulate(self):
-        for _ in range(self.sim_substeps):
+    def simulate(self):
+        with wp.ScopedTimer("simulate"):
+            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.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

README.md (4)

63-63: Grammar: “command-line”

Use “command-line” (or “CLI”) instead of “common line arguments”.

Apply this change:

-The examples support the following common line arguments:
+The examples support the following common command-line arguments:

---

67-69: Headless environments: default gl may fail

Defaulting to gl is fine locally, but can fail in headless CI/servers. A short note will prevent confusion.

Append after the table:

-| `--output-path`         | Output path for USD files (required if `--viewer usd` is used).                            | `None`          |
+| `--output-path`         | Output path for USD files (required if `--viewer usd` is used).                            | `None`          |
+
+Tip: On headless systems, prefer `--viewer usd` (with `--output-path`) or `--viewer null` instead of the default `gl`.

---

79-81: Clarify “uv” and link to project

Nice inclusion. Consider linking to uv for discoverability.

-    # With uv
-    uv run python -m newton.examples basic_pendulum
+    # With uv (https://github.com/astral-sh/uv)
+    uv run python -m newton.examples basic_pendulum

---

65-71: Add note about ReRun SDK requirement

The rerun viewer backend is fully supported (ViewerRerun in newton/_src/viewer/viewer_rerun.py will raise an ImportError with “Install with: pip install rerun-sdk” if the SDK is missing). Please append the following under the argument table in README.md (around lines 65–71):

 Some examples may add additional arguments (see their respective source files for details).

+Note:
+- The `rerun` viewer requires the ReRun SDK to be installed (e.g., `pip install rerun-sdk`).

Configuration used: .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: 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: adenzler-nvidia
PR: newton-physics/newton#552
File: newton/_src/solvers/mujoco/solver_mujoco.py:0-0
Timestamp: 2025-08-18T15:56:26.540Z
Learning: In Newton's MuJoCo solver, when transforming joint axes from Newton's internal frame to MuJoCo's expected frame, use wp.quat_rotate(joint_rot, axis) not wp.quat_rotate_inv(joint_rot, axis). The joint_rot represents rotation from joint-local to body frame, so forward rotation is correct.

docs/images/examples/resize.bat (1)

7-7: Quality setting inconsistent with the Bash script

Windows uses -q:v 2 while the Bash script uses -q:v 3, leading to different file sizes/quality. Pick one value for consistency; the diff above sets both to 3.

README.md (1)

68-74: Clarify --num-frames applies to all viewers, not just USD

Align with parser behavior and prior discussion.

-| `--num-frames`          | Number of frames to simulate (for USD output).                                             | `100`          |
+| `--num-frames`          | Number of frames to simulate.                                                              | `100`          |

newton/examples/__init__.py (1)

164-199: CLI entrypoint is clear and robust; sys.argv rewriting is correct

The argv rewrite keeps a list and preserves argv[0] as the target module, fixing the prior tuple bug reported earlier.

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

59-72: Correct Warp API usage for transforms and quaternions

Good fix replacing wp.transformf(...) with wp.transform(...), and using wp.quat_identity(). This matches usage elsewhere (e.g., URDF example) and avoids runtime AttributeError.

docs/images/examples/.gitattributes (1)

1-1: Consider tracking PNGs (and JPEG aliases) with LFS as well

If you plan to commit source PNGs alongside the generated JPGs, add PNG/JPEG aliases to avoid large blobs in Git history. Keeping this file scoped to docs/images/examples is fine.

Apply:

 *.jpg filter=lfs diff=lfs merge=lfs -text
+*.jpeg filter=lfs diff=lfs merge=lfs -text
+*.png filter=lfs diff=lfs merge=lfs -text

Follow-up: Confirm whether PNGs in this folder will be committed or only used locally as inputs. If they’re not committed, the current rule is sufficient.

docs/images/examples/resize.sh (1)

1-2: Add a shebang, fix the comment vs. behavior mismatch, and harden the loop

• The comment says “2x” but the filter halves the dimensions.
• No shebang; ShellCheck SC2148 flags target shell unknown.
• If no PNGs exist, the glob can pass a literal “*.png” to ffmpeg. Enable nullglob.

Apply:

-# resize all pngs in a directory by 2x in dimension, crop to square, + convert to jpg
-for f in *.png; do ffmpeg -y -i "$f" -vf "crop=min(iw\,ih):min(iw\,ih),scale=iw/2:ih/2,format=yuv420p" -update 1 -q:v 3 "${f%.png}.jpg"; done
+#!/usr/bin/env bash
+# resize all PNGs in a directory to half size, crop to square, and convert to JPG
+set -euo pipefail
+shopt -s nullglob
+
+for f in *.png; do
+  ffmpeg -y -i "$f" -vf "crop=min(iw\,ih):min(iw\,ih),scale=iw/2:ih/2,format=yuv420p" -q:v 3 "${f%.png}.jpg"
+done

Notes:

• Removed -update 1 since we’re writing unique outputs, not updating a single target.
• Consider aligning quality with the Windows script (see next comment) so both produce comparable outputs.

docs/images/examples/resize.bat (1)

3-9: Avoid temp files: combine crop+scale in a single FFmpeg call; align quality and drop -update

The current two-pass flow is slower and temporarily writes PNGs. A single filter chain is simpler and faster. Also, -update is intended for single image sequences and is unnecessary here.

 for %%f in (*.png) do (
     echo Processing %%f...
-    REM First crop to square (keep as PNG), then resize to half and convert to JPG
-    ffmpeg -y -i "%%f" -vf "crop=min(iw\,ih):min(iw\,ih)" -update 1 "%%~nf_temp.png"
-    ffmpeg -y -i "%%~nf_temp.png" -vf "scale=iw/2:ih/2,format=yuv420p" -update 1 -q:v 2 "%%~nf.jpg"
-    del "%%~nf_temp.png"
+    REM Crop to square, resize to half, and convert to JPG in one step
+    ffmpeg -y -i "%%f" -vf "crop=min(iw,ih):min(iw,ih),scale=iw/2:ih/2,format=yuv420p" -q:v 3 "%%~nf.jpg"
 )

Notes:

• The backslash before commas is not needed on Windows; keep the filter in double quotes.
• Set -q:v 3 to match the Bash script for parity across platforms (see next comment).

newton/examples/__main__.py (1)

6-9: LGTM; minor aliasing nit to avoid any ambiguity

This is correct and concise. As a tiny readability tweak, alias the import to make it explicit you’re calling the function exported by the package, not a submodule.

-from . import main
+from . import main as _examples_main

 if __name__ == "__main__":
-    main()
+    _examples_main()

README.md (1)

66-66: Grammar: “command-line arguments”

Tweak phrasing.

-The examples support the following common line arguments:
+The examples support the following common command-line arguments:

newton/examples/__init__.py (6)

36-45: Close the viewer even if step/render raises: wrap loop in try/finally

Prevents resource leaks when exceptions occur inside the loop.

Apply this diff:

-def run(example):
-    while example.viewer.is_running():
-        with wp.ScopedTimer("step"):
-            example.step()
-
-        with wp.ScopedTimer("render"):
-            example.render()
-
-    example.viewer.close()
+def run(example):
+    try:
+        while example.viewer.is_running():
+            with wp.ScopedTimer("step"):
+                example.step()
+
+            with wp.ScopedTimer("render"):
+                example.render()
+    finally:
+        example.viewer.close()

---

50-56: Use DeprecationWarning for deprecations

Use a specific warning category so downstream can filter appropriately.

Apply this diff:

-    warnings.warn(
-        "compute_env_offsets is deprecated and will be removed in a future version. Use the builder.replicate() function instead.",
-        stacklevel=2,
-    )
+    warnings.warn(
+        "compute_env_offsets is deprecated and will be removed in a future version. Use the builder.replicate() function instead.",
+        category=DeprecationWarning,
+        stacklevel=2,
+    )

---

58-64: Guard against num_envs <= 0 to avoid reductions on empty arrays

np.min/max on an empty env_offsets array will raise. Either validate or early-return zero rows.

Apply this diff:

-    # compute positional offsets per environment
+    # compute positional offsets per environment
+    if num_envs <= 0:
+        return np.zeros((0, 3))

---

122-134: Docstring parameter description is misleading: this takes a parser, not parsed args

Clarify that the function accepts an argparse.ArgumentParser (or None), not a parsed Namespace.

Apply this diff:

-def init(parser=None):
-    """Initialize Newton example components from parsed arguments.
-
-    Args:
-        parser: Parsed arguments from argparse (should include arguments from
-              create_parser())
+def init(parser=None):
+    """Initialize Newton example components from parsed arguments.
+
+    Args:
+        parser (argparse.ArgumentParser | None): Parent parser that includes
+            the common arguments from create_parser(). If None, a default
+            parser is created via create_parser().

---

169-175: Optional: allow dotted module names in addition to short aliases

This lets users run python -m newton.examples newton.examples.basic.example_basic_pendulum without updating the map.

Apply this diff:

-    # Set up sys.argv for the target script
-    target_module = example_map[example_name]
+    # Set up sys.argv for the target script
+    if "." in example_name:
+        target_module = example_name
+    else:
+        target_module = example_map[example_name]

Also applies to: 192-198

---

205-205: Optional: export main in all

Useful if other tooling wants to import and forward the CLI routinely.

Apply this diff:

-__all__ = ["create_parser", "init", "run"]
+__all__ = ["create_parser", "init", "run", "main"]

Configuration used: .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: 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: adenzler-nvidia
PR: newton-physics/newton#552
File: newton/_src/solvers/mujoco/solver_mujoco.py:0-0
Timestamp: 2025-08-18T15:56:26.540Z
Learning: In Newton's MuJoCo solver, when transforming joint axes from Newton's internal frame to MuJoCo's expected frame, use wp.quat_rotate(joint_rot, axis) not wp.quat_rotate_inv(joint_rot, axis). The joint_rot represents rotation from joint-local to body frame, so forward rotation is correct.

README.md (1)

105-105: Clarify --num-frames applies to all viewers, not just USD output

This repeats a previously raised point; the flag controls simulation length across viewers.

Apply this diff:

-| `--num-frames`          | Number of frames to simulate (for USD output).                                             | `100`          |
+| `--num-frames`          | Number of frames to simulate.                                                               | `100`          |

newton/tests/test_examples.py (1)

141-166: Fix UnboundLocalError: stage_path undefined when use_viewer=True and USD is unavailable

In the null-viewer branch, stage_path is never assigned but is used later for cleanup (Line 191). Initialize it to None.

Apply this diff:

         else:
             # new-style example, setup viewer type and output path
             if USD_AVAILABLE:
                 stage_path = os.path.join(
                     os.path.dirname(__file__), f"outputs/{name}_{sanitize_identifier(device)}.usd"
                 )
                 command.extend(["--viewer", "usd", "--output-path", stage_path])
             else:
                 command.extend(["--viewer", "null"])
+                stage_path = None

newton/_src/viewer/viewer_gl.py (1)

95-103: Initialize a default Warp device when no model is set (prevents None device usage).

Model-less flows call set_model(None) in init, leaving self.device=None. That can leak into GL resource creation (e.g., LinesGL) and allocations with an invalid device. Set a sensible default when model is None.

Apply this diff:

 def set_model(self, model):
     super().set_model(model)
 
+    # Ensure we have a valid Warp device for model-less flows too
+    if model is None and self.device is None:
+        self.device = wp.get_device()
+
     self.picking = Picking(model, pick_stiffness=10000.0, pick_damping=1000.0)
     self.wind = Wind(model)
 
     fb_w, fb_h = self.renderer.window.get_framebuffer_size()
     self.camera = Camera(width=fb_w, height=fb_h)

newton/examples/cloth/example_cloth_bending.py (1)

129-133: Consider rendering contacts for debugging consistency

Other cloth examples log contact normals; adding them here helps with consistency and debugging.

Apply this diff:

     def render(self):
         self.viewer.begin_frame(self.sim_time)
         self.viewer.log_state(self.state_0)
+        # Optional: visualize contact normals for consistency with other cloth examples
+        self.viewer.log_contacts(self.contacts, self.state_0)
         self.viewer.end_frame()

newton/examples/cloth/example_cloth_style3d.py (2)

171-172: Remove placeholder test() or wire it into CI

An empty test method is dead code here.

Apply this diff:

-    def test(self):
-        pass

---

46-50: Nit: fix comment typo “Grament” → “Garment”

Minor readability improvement.

Apply this diff:

-            # Grament
+            # Garment

README.md (1)

99-99: Use “command-line arguments” terminology

Minor wording improvement.

Apply this diff:

-The examples support the following common line arguments:
+The examples support the following common command-line arguments:

newton/_src/viewer/wind.py (3)

63-66: Guard against period <= 0 to avoid division by zero.

If period is set programmatically to 0 (UI constrains to [1,30] but not guaranteed elsewhere), time_phase = 2*pi*time/period will crash or produce NaNs.

Apply this diff:

-    time_phase = 2.0 * wp.pi * params.time / params.period
+    # Guard against zero/near-zero period
+    if params.period <= 1.0e-6:
+        return
+    time_phase = 2.0 * wp.pi * params.time / params.period

---

186-202: Always normalize direction; docstring promises normalization but wp.vec3 path skips it.

The tuple/list path normalizes, but the “vector-like” branch copies values without normalization. This violates the contract and can scale forces unintentionally.

Apply this diff:

     @direction.setter
     def direction(self, value):
         """Set wind direction (will be normalized)."""
-        if isinstance(value, (list, tuple)) and len(value) == 3:
-            # Normalize the direction vector
-            import math  # noqa: PLC0415
-
-            length = math.sqrt(value[0] ** 2 + value[1] ** 2 + value[2] ** 2)
-            if length > 0:
-                self._wind_params_host.direction = wp.vec3(value[0] / length, value[1] / length, value[2] / length)
-            else:
-                self._wind_params_host.direction = wp.vec3(1.0, 0.0, 0.0)
-        elif hasattr(value, "__len__") and len(value) == 3:
-            # Handle wp.vec3 or similar
-            self._wind_params_host.direction = wp.vec3(float(value[0]), float(value[1]), float(value[2]))
+        if hasattr(value, "__len__") and len(value) == 3:
+            import math  # noqa: PLC0415
+            x, y, z = float(value[0]), float(value[1]), float(value[2])
+            length = math.sqrt(x * x + y * y + z * z)
+            if length > 0.0:
+                self._wind_params_host.direction = wp.vec3(x / length, y / length, z / length)
+            else:
+                # Default to +X if zero vector is passed
+                self._wind_params_host.direction = wp.vec3(1.0, 0.0, 0.0)
         else:
             raise ValueError("Direction must be a 3-element vector")

---

55-58: Consider handling negative amplitudes explicitly or clamping at zero.

The UI slider allows negative amplitudes but the kernel early-returns for amplitude <= 0, turning half the slider range into a no-op. Either clamp on set or document that only positive values have effect.

newton/examples/cloth/example_cloth_hanging.py (1)

190-195: Contacts logging is fine; default visibility gated by UI.

Calling viewer.log_contacts even on cloth-only scenes is safe: the viewer-side option guards actual rendering and allocations.

newton/_src/viewer/viewer_gl.py (2)

229-236: Guard picking line rendering for models without bodies.

On cloth-only scenes, state.body_q may be None. Accessing numpy() will raise. Add a defensive check.

Apply this diff:

-        # Get the body's COM position
-        body_transforms = state.body_q.numpy()
+        # Get the body's COM position (guard cloth-only scenes)
+        if not hasattr(state, "body_q") or state.body_q is None:
+            self.log_lines("picking_line", None, None, None)
+            return
+        body_transforms = state.body_q.numpy()

---

169-177: Minor: verify wp.array.fill_ availability or use wp.fill().

Using line_colors.fill_(color_vec) assumes Warp arrays expose fill_. If unsupported on some versions, prefer wp.fill(line_colors, color_vec) to avoid API discrepancies.

Configuration used: .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: 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.