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

82-84: Double‑check capture invariance of update().

update() early‑returns when not picking. If update() is part of a captured CUDA graph in any viewer path, this conditional could break capture determinism. If applicable, consider always launching update_pick_target_kernel with a no‑op ray (e.g., zero dir) and gating in the kernel. Otherwise, ignore.

---

162-163: Host sync to read pick_body is acceptable given pick() frequency.

Reading pick_body back to host once per pick is fine. If pick() is called per-mouse‑move and profiling shows stalls on CUDA, consider keeping a small host mirror updated via a trivial 1‑element memcpy kernel or using wp.copy to a CPU scalar. Not required unless measured.

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

731-732: Consider buffer orphaning or SubData for CPU path to reduce reallocations.

glBufferData with DYNAMIC_DRAW each frame can cause reallocations. Two lighter options:

• Orphaning: glBufferData(GL_ARRAY_BUFFER, size, None, GL_DYNAMIC_DRAW) followed by glBufferSubData with data.
• Or persistent mapping with glMapBufferRange if available.

Optional unless profiling shows overhead.

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

291-297: Avoid double parsing of CLI args.

You parse args before and after init(). Keep the init() path only to avoid divergence.

Apply:

-args = parser.parse_known_args()[0]
-
-viewer, args = newton.examples.init(parser)
+viewer, args = newton.examples.init(parser)

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

262-273: Avoid double parsing of CLI args.

Same as articulations: remove the pre-init parse to rely on init() as the single source of truth.

-args = parser.parse_known_args()[0]
-
-viewer, args = newton.examples.init(parser)
+viewer, args = newton.examples.init(parser)

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

118-125: Unnecessary/mis-scoped FK evaluation; remove or use state arrays

The comment says this is for non‑MuJoCo solvers, but the code always calls newton.eval_fk using model.joint_q/joint_qd while you’ve randomized state_0’s joint_q. This is redundant (you already eval FK via ArticulationView for the non‑MuJoCo case) and potentially inconsistent with the state.

Two options (prefer removing to avoid duplication):

Apply this diff to remove the block:

-        # Ensure FK evaluation (for non-MuJoCo solvers):
-        newton.eval_fk(
-            self.model,
-            self.model.joint_q,
-            self.model.joint_qd,
-            self.state_0,
-        )

If you keep it, at least use the state arrays and guard by solver type:

+        if not isinstance(self.solver, newton.solvers.SolverMuJoCo):
+            newton.eval_fk(self.model, self.state_0.joint_q, self.state_0.joint_qd, self.state_0)

---

196-201: Torch device setup is likely incorrect API

torch.set_device(...) isn’t a public PyTorch API. Use torch.cuda.set_device for CUDA and torch.set_default_device for CPU (if desired), guarding for availability.

Apply this diff:

-    if USE_TORCH:
-        import torch
-
-        torch.set_device(args.device)
+    if USE_TORCH:
+        import torch
+        # Set device only when using CUDA; otherwise default CPU is fine
+        if args.device and isinstance(args.device, str) and args.device.startswith("cuda"):
+            idx = int(args.device.split(":")[1]) if ":" in args.device else 0
+            torch.cuda.set_device(idx)
+        elif hasattr(torch, "set_default_device"):
+            # Optional: explicitly pin to CPU if needed
+            try:
+                torch.set_default_device("cpu")
+            except Exception:
+                pass

---

16-26: Update usage hint to reflect viewer-based CLI (nit)

The “Command” line doesn’t reflect that this example expects a viewer argument now. Consider showing a concrete invocation that works with both GL and USD:

• python -m newton.examples selection_cartpole --viewer gl
• python -m newton.examples selection_cartpole --viewer usd --output-path out.usd

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/viewer/picking.py (2)

56-57: Explicit picking_active flag is a clean separation from pick_body.

Good addition. It avoids device reads in hot paths that only need a boolean and makes the intent clearer.

---

85-88: Release path resets both device and host picking state.

Setting pick_body = -1 and picking_active = False keeps CPU/GPU state consistent. Looks good.

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

630-631: Use of GL_DYNAMIC_DRAW for per‑instance transforms is appropriate.

This matches the per‑frame update pattern for instance transforms and avoids STATIC allocation thrash.

newton/examples/selection/example_selection_articulations.py (4)

86-96: Viewer-based constructor and timing fields look good.

Clear separation of sim_dt vs frame_dt and support for multi-env is consistent with other examples.

---

97-104: Scene replication pattern is correct; minor verification on ignore_names.

Building one env then replicating via scene.replicate is the right approach. Verify that "floor"/"ground" filtering in both MJCFs is sufficient to avoid duplicate planes across envs.

Also applies to: 105-111, 113-117

---

192-193: Explicit viewer.set_model(model) is necessary here.

Good to see this wired up to enable viewer-driven logging and picking.

---

200-206: CUDA graph capture is scoped correctly.

Capturing simulate() once and replaying in step() is the right balance. Ensure no host-side allocations happen inside simulate() to keep capture reusable.

newton/examples/selection/example_selection_materials.py (3)

71-81: Viewer-based constructor and sim timing look consistent.

Matches the articulations example; good for test harness uniformity.

---

151-152: viewer.set_model(model) is correctly placed.

Critical for picking/forces to bind to the active model.

---

153-155: FK precompute for non-MuJoCo solvers is a good safety.

This avoids stale transforms when switching solvers.

newton/examples/__init__.py (1)

186-189: Selection examples importability failed locally

We attempted to run the provided import-verification script but encountered a ModuleNotFoundError for the warp dependency when loading the new example modules:

ModuleNotFoundError: No module named 'warp'

Please ensure that:

• The warp package is declared and installed in your CI environments.
• All three modules under newton.examples.selection import cleanly once warp is available.

Once warp is installed, rerun:

python - <<'PY'
import importlib
for m in [
  "newton.examples.selection.example_selection_articulations",
  "newton.examples.selection.example_selection_cartpole",
  "newton.examples.selection.example_selection_materials",
]:
    importlib.import_module(m)
    print("OK:", m)
PY

to confirm that each prints “OK: ”.

newton/tests/test_examples.py (3)

333-336: Selection articulations: migrated test wiring to viewer path looks good

• use_viewer=True correctly routes through the new viewer initialization in newton.examples.init().
• Explicit num_frames keeps runtime bounded; CPU override at Line 334 is reasonable for CI speed.

---

341-344: Selection cartpole: viewer-based execution enabled

Switching this example to use_viewer=True aligns it with the refactored viewer-driven runtime and matches the example’s new CLI. No issues spotted.

---

349-352: Selection materials: consistent test options and viewer flow

Consistent with the other selection tests; the viewer mode integration and frame budgeting look appropriate.

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

128-134: CUDA graph capture includes viewer.apply_forces; confirm it remains interactive

simulate() calls viewer.apply_forces(self.state_0), and capture() records simulate() inside a CUDA graph. If viewer.apply_forces depends on per-frame host state (e.g., picking), ensure that the data it writes is fed via device buffers updated between launches so that interaction isn’t “frozen” at capture time.

Would you like me to add a minimal toggle to re-capture on interaction state changes, or document why capture is safe here?

---

139-141: Nice: forces routed through viewer for picking/wind

Integrating viewer.apply_forces keeps interaction logic consolidated and makes the example consistent with the other selection demos.

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

158-162: Fix: kernel launch dimension must be 1D integer (not a tuple).

apply_forces_kernel uses a 1D thread id (wp.tid()), but the launch passes dim=joint_f.shape (a tuple). This will compile/launch incorrectly. Use the batch size only.

Apply this diff:

-            wp.launch(
-                apply_forces_kernel,
-                dim=joint_f.shape,
-                inputs=[joint_q, joint_f],
-            )
+            wp.launch(
+                apply_forces_kernel,
+                dim=joint_f.shape[0],
+                inputs=[joint_q, joint_f],
+            )

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

118-125: Redundant FK evaluation; remove or guard to non-MuJoCo solvers.

You already call self.cartpoles.eval_fk(self.state_0) for non-MuJoCo solvers. The unconditional newton.eval_fk block duplicates work and is unnecessary for MuJoCo (and double-computes for other solvers).

Apply one of these:

Option A — remove redundant block:

-        # Ensure FK evaluation (for non-MuJoCo solvers):
-        newton.eval_fk(
-            self.model,
-            self.model.joint_q,
-            self.model.joint_qd,
-            self.state_0,
-        )

Option B — guard consistently:

-        # Ensure FK evaluation (for non-MuJoCo solvers):
-        newton.eval_fk(
-            self.model,
-            self.model.joint_q,
-            self.model.joint_qd,
-            self.state_0,
-        )
+        if not isinstance(self.solver, newton.solvers.SolverMuJoCo):
+            newton.eval_fk(self.model, self.model.joint_q, self.model.joint_qd, self.state_0)

---

192-192: Remove dead code: duplicate args parsing.

args is parsed here and immediately overwritten by init(parser). Safe to delete this line.

-    args = parser.parse_known_args()[0]

---

103-111: Consistency: prefer self.num_envs over local num_envs.

Keeps initialization consistent and resilient to signature refactors.

-            cart_positions = 2.0 - 4.0 * torch.rand(num_envs)
-            pole1_angles = torch.pi / 8.0 - torch.pi / 4.0 * torch.rand(num_envs)
-            pole2_angles = torch.pi / 8.0 - torch.pi / 4.0 * torch.rand(num_envs)
+            cart_positions = 2.0 - 4.0 * torch.rand(self.num_envs)
+            pole1_angles = torch.pi / 8.0 - torch.pi / 4.0 * torch.rand(self.num_envs)
+            pole2_angles = torch.pi / 8.0 - torch.pi / 4.0 * torch.rand(self.num_envs)
             joint_q = torch.stack([cart_positions, pole1_angles, pole2_angles], dim=1)
 ...
-            wp.launch(randomize_states_kernel, dim=num_envs, inputs=[joint_q, 42])
+            wp.launch(randomize_states_kernel, dim=self.num_envs, inputs=[joint_q, 42])

---

60-66: Optional: honor USD fps if available.

parse_usd returns metadata including fps. Consider reading it to set self.fps/frame_dt so the sim time aligns with the asset’s timing.

Example (conceptual):

ret = newton.utils.parse_usd(...)

fps = ret.get("fps")
if fps:
    self.fps = fps
    self.frame_dt = 1.0 / self.fps
    self.sim_dt = self.frame_dt / self.sim_substeps

Also applies to: 69-83

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: 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/examples/selection/example_selection_cartpole.py (5)

116-117: Viewer migration looks correct.

set_model + the render path (begin_frame/log_state/end_frame) follow the new viewer guidelines and are consistent with other updated examples.

---

78-83: Good use of builder.replicate for multi-env instancing.

Replicating a single env builder into a scene builder keeps USD parsing simple and aligns with the new replication API.

---

84-85: Solver choice is explicit and not conflated with example-level flags.

Using SolverMuJoCo(disable_contacts=True) directly aligns with the “solver param vs example attribute” distinction from prior learnings. No issues.

---

126-134: Verify CUDA graph capture safety with dynamic viewer forces.

You capture simulate(), which calls viewer.apply_forces(self.state_0) each substep. Ensure this function uses GPU-side writes or mapped buffers that are compatible with CUDA graph replay (e.g., GL interop with WRITE_DISCARD and no implicit host sync), otherwise graph replay may bake in stale inputs or incur sync.

Would you confirm that viewer.apply_forces avoids CPU-GPU sync and is implemented with CUDA-GL interop suitable for capture (matching the changes mentioned in this PR)?

Also applies to: 139-141, 166-171

---

23-23: Update “Command” doc string to the new runner.

The usage hint may be outdated. If the new entrypoint is via the example runner registry, it’s likely either:

• python -m newton.examples selection.example_selection_cartpole
  or
• python -m newton.examples selection_cartpole

Please update to the correct, supported form.

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

158-162: Kernel launch dimension is incorrect (tuple passed to 1D kernel)

apply_forces_kernel uses a 1D thread id (wp.tid()), but the launch passes dim=joint_f.shape (a tuple). Launch with the batch size only.

Apply this diff:

-            wp.launch(
-                apply_forces_kernel,
-                dim=joint_f.shape,
-                inputs=[joint_q, joint_f],
-            )
+            wp.launch(
+                apply_forces_kernel,
+                dim=joint_f.shape[0],
+                inputs=[joint_q, joint_f],
+            )

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

118-124: Unconditional FK evaluation contradicts the comment and is redundant

The comment says “for non-MuJoCo solvers,” but the code runs unconditionally and duplicates the earlier FK update via ArticulationView for that branch. Either guard this block with the same condition or remove it to avoid extra work.

Apply this diff to remove the redundant call (preferred, since the initial FK is already handled above for non-MuJoCo):

-        # Ensure FK evaluation (for non-MuJoCo solvers):
-        newton.eval_fk(
-            self.model,
-            self.model.joint_q,
-            self.model.joint_qd,
-            self.state_0,
-        )

If you want full-model FK instead of the articulation-only call, keep this block but move it under the existing if not isinstance(self.solver, newton.solvers.SolverMuJoCo): and drop the earlier self.cartpoles.eval_fk(...).

---

192-192: Redundant parse — args are parsed again in init(parser)

This parse result is immediately overwritten by the subsequent init(parser) call. It’s safe to drop to avoid confusion.

-    args = parser.parse_known_args()[0]
+    # args are parsed within newton.examples.init(parser)

---

79-79: Spacing between replicated envs might be tight for longer poles

Minor: with spacing=(2.0, 0.0, 0.0), neighboring carts could overlap visuals if assets change. Not a blocker, just a heads-up.

If you anticipate longer links, consider a slightly larger x-spacing (e.g., 2.5–3.0).

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

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

You can enable these sources in your CodeRabbit configuration.

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/examples/selection/example_selection_cartpole.py (6)

16-26: Clear example header and CLI guidance — looks good

The header block is concise and the command matches the new viewer-based entrypoint pattern.

---

69-83: Viewer-driven pipeline and scene replication are correctly applied

• Using a per-env builder (env) and replicating into scene before finalize() follows the new guidelines.
• Assigning the viewer via constructor injection and calling viewer.set_model(self.model) at init time is the right ordering for allocating renderer/interop resources.

Also applies to: 86-88, 116-116

---

128-134: CUDA graph capture placement is fine; double-check capture safety of viewer.apply_forces

Capturing simulate() once in init mirrors the other selection examples. Assuming viewer.apply_forces is purely GPU-side (no implicit host sync), the graph should be safe to replay each frame with updated inputs.

Would you like me to run a quick repo scan to confirm viewer.apply_forces launches kernels only and doesn’t issue host-side syncs?

---

139-141: Using viewer.apply_forces removes CPU/GPU sync in picking path

Good integration point; this aligns with the PR goal to avoid unnecessary synchronization while keeping per-frame forces on-GPU.

---

167-171: Graph replay fallback is correct

Cleanly falls back to simulate() when graphs aren’t available. Matches patterns in the sibling examples.

---

58-66: Time-step configuration is clear and conventional

fps/frame_dt with sim_substeps/sim_dt is straightforward and consistent with other examples.

newton/examples/selection/example_selection_materials.py (3)

60-67: RNG stride fix (shape_count) is correct and resolves prior collision issue.

Switching the kernel arg from num_envs to shape_count and using i*shape_count + j prevents per-shape RNG collisions within an environment. Call site passes self.ants.shape_count accordingly.

Also applies to: 224-228

---

173-175: Ensure apply_forces is implemented (or a no-op) for all viewers.

Given apply_forces() is called unconditionally, confirm the base viewer provides a concrete no-op so non-GL viewers don’t error.

#!/bin/bash
set -euo pipefail
echo "✓ Confirm apply_forces exists on viewer base/backends and is not abstract"
rg -nP 'class\s+Viewer\w+' newton/_src/viewer -C2
rg -nP 'def\s+apply_forces\s*\(' newton/_src/viewer -C3
rg -nP '@abstractmethod.*apply_forces' newton/_src/viewer || echo "No abstract apply_forces found (good)"

---

274-279: Fix PyTorch device selection (unguarded, wrong API).

torch.set_device(...) is not a public API and this call is unguarded. Use set_default_device when available or cuda.set_device for CUDA, and only when args.device is provided and valid.

-    import torch
-
-    torch.set_device(args.device)
+    import torch
+    if getattr(args, "device", None):
+        # Prefer modern API when available
+        if hasattr(torch, "set_default_device"):
+            torch.set_default_device(args.device)
+        elif isinstance(args.device, str) and args.device.startswith("cuda"):
+            import re
+            m = re.search(r"cuda:(\d+)", args.device)
+            if m and torch.cuda.is_available():
+                torch.cuda.set_device(int(m.group(1)))

README.md (1)

188-188: Nit: “line arguments” → “command-line arguments”.

Small wording tweak for clarity.

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

newton/examples/selection/example_selection_materials.py (2)

72-81: Viewer-centric refactor looks good; drop redundant initial collide and keep FK note scoped.

• Good shift to a viewer-driven runtime with set_model() and capture().
• Minor: self.contacts = self.model.collide(self.state_0) (Line 106) is redundant since the solver is MuJoCo by default and simulate() resets contacts each step. Dropping it avoids a needless compute at startup.

-        self.contacts = self.model.collide(self.state_0)
+        self.contacts = None

• The FK evaluation note is helpful; consider gating it to non-MuJoCo solvers if you ever switch the default solver here to avoid extra work, but leaving it unconditional is acceptable.

Also applies to: 91-98, 101-107, 114-120, 151-159

---

270-273: Avoid double-parsing CLI args; let init() own parsing.

args = parser.parse_known_args()[0] (Line 270) is redundant since init(parser) parses again and returns args. Removing the first parse reduces confusion.

-    args = parser.parse_known_args()[0]
-
     viewer, args = newton.examples.init(parser)

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

README.md (1)

152-185: Selection Examples gallery verified
All example scripts, image assets, and registry entries are present and correctly referenced:

• Script files in newton/examples/selection/
  • example_selection_cartpole.py
  • example_selection_materials.py
  • example_selection_articulations.py

• Image assets in docs/images/examples/
  • example_selection_cartpole.jpg
  • example_selection_materials.jpg
  • example_selection_articulations.jpg

• Registry entries in newton/examples/__init__.py (lines 186–188)
  • "selection_articulations": "newton.examples.selection.example_selection_articulations"
  • "selection_cartpole": "newton.examples.selection.example_selection_cartpole"
  • "selection_materials": "newton.examples.selection.example_selection_materials"

All checks passed — ready to merge.

newton/examples/selection/example_selection_materials.py (4)

16-29: Helpful example header and usage hint.

The banner and command are clear and match the README section.

---

162-168: CUDA graph capture is set up correctly; verify invariants.

Capturing simulate() once at init is fine as long as launch shapes, solver, and sim_substeps remain constant. If any of these change at runtime (e.g., toggling solver or substeps), recapture will be required.

Would you like me to add a small guard that re-captures when sim_substeps changes or when a different solver is selected?

---

190-196: Graph launch fallback logic is clean.

The step() control flow between capture_launch and simulate is straightforward and correct.

---

253-255: Render path integrates properly with the new Viewer API.

begin_frame/log_state/end_frame usage is consistent with other updated examples.