Skip to content

Rename environment id to world id#945

Merged
eric-heiden merged 15 commits into
newton-physics:mainfrom
nvtw:dev/tw/rename_env_id_arrays
Oct 16, 2025
Merged

Rename environment id to world id#945
eric-heiden merged 15 commits into
newton-physics:mainfrom
nvtw:dev/tw/rename_env_id_arrays

Conversation

@nvtw

@nvtw nvtw commented Oct 15, 2025

Copy link
Copy Markdown
Member

Description

Should address #935
This PR refactors the codebase to use consistent "world" terminology throughout:

Changes:

  • Renamed: model.shape_groupmodel.shape_world
  • Renamed: add_builder(environment=...)add_builder(world=...)
  • Updated terminology everywhere:
    • All docstrings now use "world" instead of "environment"
    • Code comments updated to use "world terminology"
    • Updated particle_group, body_group, joint_group, articulation_group docstrings
    • ModelBuilder documentation section renamed from "Environment Grouping" to "World Grouping"

Newton Migration Guide

Please ensure the migration guide for warp.sim users is up-to-date with the changes made in this PR.

  • The migration guide in docs/migration.rst is up-to date

Before your PR is "Ready for review"

  • Necessary tests have been added and new examples are tested (see newton/tests/test_examples.py)
  • Documentation is up-to-date
  • Code passes formatting and linting checks with pre-commit run -a

Summary by CodeRabbit

  • Refactor
    • Replaced "environment"/"env" with "world"/"worlds" across public APIs, CLI flags, examples, benchmarks, viewer labels, docs, model/builder/solver fields and example interfaces (e.g., num_envs → num_worlds, --num-envs → --num-worlds).
  • Tests
    • Updated tests and helpers to use world-based names, indices, expectations, and invocation arguments.
  • Documentation
    • Updated help text, docstrings, example usage, and viewer labels to reflect world terminology.

@nvtw nvtw marked this pull request as draft October 15, 2025 13:44
@coderabbitai

coderabbitai Bot commented Oct 15, 2025

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Renames "environment"/"env" and "environment group" concepts to "world"/"worlds" across the codebase (APIs, kernels, examples, benchmarks, tests, docs, and viewer). Broadphase pre-filtering now tests world indices first (world_a/world_b) then defers to collision-group tests.

Changes

Cohort / File(s) Summary
Broadphase collision logic
newton/_src/geometry/broad_phase_common.py, newton/_src/geometry/broad_phase_nxn.py, newton/_src/geometry/broad_phase_sap.py
Renamed test_environment_and_group_pairtest_world_and_group_pair; per-geometry shape_groupshape_world / geom_shape_groupgeom_shape_world; pre-filter now checks world indices first, then collision groups via test_group_pair.
Contact kernels & collision launchers
newton/_src/geometry/kernels.py, newton/_src/sim/collide.py
Kernel signatures and call sites changed to use particle_world / shape_world; create_soft_contacts inputs updated and runtime checks compare world indices (with -1 global semantics).
Model builder & model public fields
newton/_src/sim/builder.py, newton/_src/sim/style3d/builder_style3d.py, newton/_src/sim/model.py
Builder API parameter environmentworld (e.g., add_builder(world=...)), current_env_groupcurrent_world, num_envsnum_worlds; finalized model arrays renamed (shape_groupshape_world, particle_groupparticle_world, body_groupbody_world, joint_groupjoint_world, articulation_grouparticulation_world).
MuJoCo solver & kernel params
newton/_src/solvers/mujoco/solver_mujoco.py
Per-env counts and parameters renamed to per-world equivalents (e.g., bodies_per_envbodies_per_world, joints_per_envjoints_per_world, ncon_per_envncon_per_world, separate_envs_to_worldsseparate_worlds); shape grouping arrays use shape_world; indexing and kernel launch dimensions updated.
Examples & helpers
newton/examples/**, newton/examples/__init__.py
Example APIs and internals renamed: num_envsnum_worlds, envworld, env_idworld_id; helper compute_env_offsetscompute_world_offsets; CLI flags updated (--num-envs--num-worlds).
Benchmarks
asv/benchmarks/setup/bench_model.py, asv/benchmarks/simulation/bench_mujoco.py, asv/benchmarks/simulation/bench_quadruped_xpbd.py, asv/benchmarks/simulation/bench_selection.py
Benchmark parameter names and method signatures changed from num_envsnum_worlds; labels/units updated (e.g., "ms/env-step"→"ms/world-step"); builder/example invocations adjusted.
Tests — broadphase, model, mujoco
newton/tests/test_broad_phase.py, newton/tests/test_environment_group_collision.py, newton/tests/test_model.py, newton/tests/test_mujoco_solver.py
Tests updated to use shape_world/body_world, world in builder calls, renamed test functions and expectations to world-centric naming; kernel invocation args updated.
Tests — miscellaneous
newton/tests/* (examples: test_anymal_reset.py, test_equality_constraints.py, test_examples.py, test_ik_fk_kernels.py, test_import_usd.py, test_joint_drive.py, test_kinematics.py)
Various renames in tests: num_envsnum_worlds, env_idworld_id, ncon_per_envncon_per_world, cloned_envcloned_world; small signature and assertion updates.
Utilities, USD import, selection, viewer, docs
newton/_src/utils/__init__.py, newton/_src/utils/import_usd.py, newton/_src/utils/selection.py, newton/_src/viewer/viewer_gl.py, newton/_src/viewer/viewer_null.py, docs/guide/installation.rst
parse_usd param cloned_envcloned_world; selection internals envs_*worlds_*; viewer display strings and model attribute references to num_worlds; installation example CLI shows --num-worlds.
Behavioral summary (broadphase path)
newton/_src/geometry/broad_phase_common.py
Control flow: evaluate world indices first — if both set and different, skip pair; otherwise call test_group_pair for collision-group decision.

Sequence Diagram(s)

sequenceDiagram
    participant BroadPhase as BroadPhase
    participant WorldTest as test_world_and_group_pair
    participant GroupTest as test_group_pair

    BroadPhase->>WorldTest: test_world_and_group_pair(world_a, world_b, coll_a, coll_b)
    alt both worlds set and world_a != world_b
        WorldTest-->>BroadPhase: False (skip pair)
    else
        WorldTest->>GroupTest: test_group_pair(coll_a, coll_b)
        GroupTest-->>WorldTest: True/False
        WorldTest-->>BroadPhase: return result
    end
    Note over BroadPhase,WorldTest: New ordering: world-index check → group collision test
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • mmacklin
  • eric-heiden
  • nvlukasz

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "Rename environment id to world id" is directly related to the primary change in the changeset. The raw summary confirms that across numerous files, environment-related terminology is systematically being replaced with world-related terminology, including renaming model attributes (shape_group → shape_world), function parameters (environment → world, num_envs → num_worlds), builder state (current_env_group → current_world), CLI flags (--num-envs → --num-worlds), and various identifiers throughout the codebase. The title captures the core conceptual change—migrating from "environment" to "world" terminology—though it could be slightly more comprehensive in scope since the changes extend beyond just identifiers to include parameters, docstrings, and architectural terminology.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 417fe2a and 1965ece.

📒 Files selected for processing (1)
  • newton/tests/test_joint_drive.py (15 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
newton/tests/test_joint_drive.py (1)
newton/_src/sim/builder.py (4)
  • ModelBuilder (67-4482)
  • add_body (1191-1253)
  • add_builder (912-1189)
  • finalize (4070-4428)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Run GPU Unit Tests on AWS EC2 (Pull Request)
  • GitHub Check: Run GPU Benchmarks (Pull Request)
🔇 Additional comments (3)
newton/tests/test_joint_drive.py (3)

25-47: LGTM! Clean parameter rename.

The function signature and parameter usage have been cleanly updated from env_id to world_id. All array indexing consistently uses the renamed parameter.


81-148: LGTM! Proper world-based builder setup.

The test correctly:

  • Updates terminology from nb_envs to nb_worlds and environment_builder to world_builder
  • Uses main_builder.add_builder(world_builder, world=i) to assign each sub-builder to a specific world
  • Directly indexes main_builder.joint_q[i] and main_builder.joint_qd[i], which works correctly because each world_builder has exactly one joint and they're added sequentially
  • Finalizes the main_builder rather than individual world builders

157-294: LGTM! Consistent world indexing throughout test execution.

The test execution properly:

  • Uses nb_worlds for all loop bounds
  • Passes world_id=i consistently to compute_expected_velocity_outcome
  • Indexes simulation results with state_out.joint_qd.numpy()[i] matching the world index
  • Validates multiple scenarios: initial state, updated parameters, velocity drive mode, and no-drive mode

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented Oct 15, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.76316% with 11 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
newton/_src/sim/builder.py 93.54% 4 Missing ⚠️
newton/_src/viewer/viewer_gl.py 0.00% 4 Missing ⚠️
newton/_src/utils/import_usd.py 93.54% 2 Missing ⚠️
newton/_src/solvers/mujoco/solver_mujoco.py 97.05% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@nvtw nvtw self-assigned this Oct 15, 2025

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
newton/_src/sim/builder.py (1)

4452-4471: Bug: collision-group filtering is incorrect; valid pairs are dropped

This prefilter only allows equal groups unless one is -1, which disagrees with test_group_pair semantics. It will wrongly exclude pairs like (+2, -5) that should collide.

Apply this change to mirror test_group_pair:

-        # Iterate over all shapes candidates
+        # Iterate over all shapes candidates
         for i1 in range(len(sorted_indices)):
             s1 = sorted_indices[i1]
-            env1 = self.shape_world[s1]
+            env1 = self.shape_world[s1]
             collision_group1 = self.shape_collision_group[s1]
             for i2 in range(i1 + 1, len(sorted_indices)):
                 s2 = sorted_indices[i2]
-                env2 = self.shape_world[s2]
-                # Skip shapes from different worlds (unless one is global). As the shapes are sorted,
-                # this means the shapes in this world have all been processed.
+                env2 = self.shape_world[s2]
+                # Skip shapes from different worlds (unless one is global). As the shapes are sorted,
+                # this means the shapes in this world have all been processed.
                 if env1 != -1 and env1 != env2:
                     break
 
-                # Skip shapes from different collision group (unless one is global).
-                collision_group2 = self.shape_collision_group[s2]
-                if collision_group1 != -1 and collision_group2 != -1 and collision_group1 != collision_group2:
-                    continue
+                # Skip shapes whose collision groups do not interact (mirror test_group_pair)
+                collision_group2 = self.shape_collision_group[s2]
+                def _groups_collide(a: int, b: int) -> bool:
+                    if a == 0 or b == 0:
+                        return False
+                    if a > 0:
+                        return a == b or b < 0
+                    # a < 0
+                    return a != b
+                if not _groups_collide(collision_group1, collision_group2):
+                    continue
🧹 Nitpick comments (5)
newton/_src/geometry/broad_phase_common.py (1)

94-119: Naming consistency: provide a world-named alias for clarity

Docstrings use “world” but function name/params are “environment”. Keep current function for compatibility, but add a world-named alias to avoid confusion in new code.

Example addition:

@wp.func
def test_world_and_group_pair(world_a: int, world_b: int, collision_group_a: int, collision_group_b: int) -> bool:
    return test_environment_and_group_pair(world_a, world_b, collision_group_a, collision_group_b)
newton/_src/sim/style3d/builder_style3d.py (1)

122-124: Doc: align update_num_env_count semantics with base

Mirror ModelBuilder.add_builder docs: when world is None increment, else ensure num_envs ≥ world+1; world=-1 doesn’t affect num_envs.

-            update_num_env_count (bool): if True, the number of worlds is incremented by 1.
-            world (int | None): world index to assign to ALL entities from this builder.
+            update_num_env_count (bool): if True, updates the number of worlds appropriately (see ModelBuilder.add_builder).
+            world (int | None): World index to assign to ALL entities from this builder.
newton/_src/sim/builder.py (2)

627-631: Doc: s/environment/world/

Replicate() description still says “environment”. Update to “world.”

-        specified spacing vector, and all entities from each copy are assigned to a new environment.
+        specified spacing vector, and all entities from each copy are assigned to a new world.

335-337: Minor: consider renaming num_envs in a follow-up

Field name still uses “envs”. Optional future cleanup to “num_worlds” for consistency.

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

689-689: Inconsistent parameter naming with world refactoring.

The parameter particle_group should be renamed to particle_world for consistency with the shape_world parameter on line 696. While the comment correctly says "World indices for particles", the parameter name hasn't been updated to match the world-centric terminology.

This inconsistency is visible in the code at lines 718-719:

particle_world = particle_group[particle_index]  # Reading from particle_group
shape_world_idx = shape_world[shape_index]       # Reading from shape_world

Apply this diff to make the naming consistent:

 @wp.kernel
 def create_soft_contacts(
     particle_q: wp.array(dtype=wp.vec3),
     particle_radius: wp.array(dtype=float),
     particle_flags: wp.array(dtype=wp.int32),
-    particle_group: wp.array(dtype=int),  # World indices for particles
+    particle_world: wp.array(dtype=int),  # World indices for particles
     body_q: wp.array(dtype=wp.transform),
     shape_transform: wp.array(dtype=wp.transform),

And update line 718:

-    particle_world = particle_group[particle_index]
+    particle_world = particle_world[particle_index]
📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 16472f6 and 344c290.

📒 Files selected for processing (13)
  • newton/_src/geometry/broad_phase_common.py (1 hunks)
  • newton/_src/geometry/broad_phase_nxn.py (5 hunks)
  • newton/_src/geometry/broad_phase_sap.py (6 hunks)
  • newton/_src/geometry/kernels.py (2 hunks)
  • newton/_src/sim/builder.py (17 hunks)
  • newton/_src/sim/collide.py (1 hunks)
  • newton/_src/sim/model.py (5 hunks)
  • newton/_src/sim/style3d/builder_style3d.py (1 hunks)
  • newton/_src/solvers/mujoco/solver_mujoco.py (5 hunks)
  • newton/tests/test_broad_phase.py (2 hunks)
  • newton/tests/test_environment_group_collision.py (6 hunks)
  • newton/tests/test_model.py (9 hunks)
  • newton/tests/test_mujoco_solver.py (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-12T17:51:37.474Z
Learnt from: nvlukasz
PR: newton-physics/newton#519
File: newton/geometry.py:16-22
Timestamp: 2025-08-12T17:51:37.474Z
Learning: In the Newton public API refactor, common geometry symbols like ParticleFlags and ShapeFlags are now exposed at the top level in newton/__init__.py rather than through newton.geometry. The newton/geometry.py module is intentionally focused only on broad-phase collision detection classes (BroadPhaseAllPairs, BroadPhaseExplicit, BroadPhaseSAP).

Applied to files:

  • newton/_src/geometry/broad_phase_nxn.py
🧬 Code graph analysis (8)
newton/_src/sim/style3d/builder_style3d.py (1)
newton/_src/sim/builder.py (1)
  • add_builder (912-1189)
newton/_src/geometry/broad_phase_nxn.py (1)
newton/_src/geometry/broad_phase_common.py (1)
  • test_environment_and_group_pair (91-119)
newton/tests/test_environment_group_collision.py (1)
newton/_src/sim/builder.py (4)
  • ModelBuilder (67-4483)
  • add_particle (3006-3040)
  • add_builder (912-1189)
  • finalize (4070-4428)
newton/_src/geometry/broad_phase_sap.py (1)
newton/_src/geometry/broad_phase_common.py (1)
  • test_environment_and_group_pair (91-119)
newton/tests/test_broad_phase.py (2)
newton/_src/geometry/broad_phase_nxn.py (2)
  • launch (145-201)
  • launch (218-264)
newton/_src/geometry/broad_phase_sap.py (1)
  • launch (341-481)
newton/_src/solvers/mujoco/solver_mujoco.py (1)
newton/_src/sim/builder.py (1)
  • shape_count (508-512)
newton/tests/test_mujoco_solver.py (1)
newton/_src/sim/builder.py (2)
  • add_builder (912-1189)
  • add_joint_free (1621-1672)
newton/tests/test_model.py (1)
newton/_src/sim/builder.py (2)
  • add_builder (912-1189)
  • ModelBuilder (67-4483)
🪛 Ruff (0.14.0)
newton/_src/sim/builder.py

961-961: Avoid specifying long messages outside the exception class

(TRY003)

⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Run GPU Benchmarks (Pull Request)
🔇 Additional comments (23)
newton/_src/geometry/broad_phase_sap.py (2)

269-277: World + group filtering in SAP kernel: LGTM

Using shape_world and test_environment_and_group_pair preserves semantics (-1 global, same-world or global). Good alignment with the new “world” model.


459-471: Correct kernel wiring for shape_world

Kernel input updated to pass geom_shape_world in the correct position. Looks correct.

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

1163-1190: add_builder world bookkeeping: LGTM

World index selection, preservation, and restoration logic are sound; num_envs update semantics are correct.


4163-4164: Finalize: shape_world export

Publishing m.shape_world is consistent with the API rename. Good.

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

237-238: Kernel wiring update to shape_world: LGTM

Passing model.shape_world aligns with the updated kernel signature and world semantics.

newton/tests/test_broad_phase.py (2)

129-141: LGTM! Consistent world-based API usage.

The test correctly creates a shape_world array with global world index (-1) and passes it to nxn_broadphase.launch, aligning with the world-centric API changes.


386-398: LGTM! Consistent world-based API usage.

The test correctly creates a shape_world array with global world index (-1) and passes it to sap_broadphase.launch, aligning with the world-centric API changes.

newton/_src/solvers/mujoco/solver_mujoco.py (3)

1024-1058: LGTM! Kernel parameter renamed consistently.

The update_incoming_shape_xform_kernel parameter shape_group has been correctly renamed to shape_world, and the logic for determining global vs per-world shapes (Line 1040: if shape_world[template_shape_idx] < 0) is preserved.


1957-1957: LGTM! Model attribute reference updated.

The code correctly references model.shape_world instead of the previous shape_group attribute.


2431-2447: LGTM! Kernel invocation updated.

The update_incoming_shape_xform_kernel is correctly invoked with self.model.shape_world, aligning with the renamed parameter and model attribute.

newton/tests/test_environment_group_collision.py (2)

41-64: LGTM! World-based grouping correctly used.

The test correctly uses builder.current_world to assign world indices (0, 1, -1 for global) and verifies collision filtering between worlds.


145-164: LGTM! World parameter in add_builder and assertions updated.

The test correctly:

  • Uses the world parameter in add_builder calls (Lines 145-146)
  • Asserts on model.shape_world instead of the previous shape_group (Lines 151-164)
  • Verifies world assignments: -1 for global, 0+ for world-specific
newton/_src/sim/model.py (3)

29-50: LGTM! Class documentation updated to world-centric terminology.

The class docstring has been consistently updated to use "world" terminology instead of "environment", clearly documenting the grouping semantics:

  • Index -1: global entities shared across all worlds
  • Indices 0, 1, 2, ...: world-specific entities

148-149: LGTM! Public API attribute renamed to shape_world.

The attribute has been correctly renamed from shape_group to shape_world with an updated docstring, maintaining the world-based indexing semantics (-1 for global, 0+ for world-specific).


95-95: LGTM! Group attribute docstrings consistently updated.

All entity group attribute docstrings (particle_group, body_group, joint_group, articulation_group) have been updated to use "world" terminology while preserving the indexing semantics.

Also applies to: 223-223, 284-284, 290-290

newton/tests/test_model.py (4)

269-289: LGTM! World-based particle grouping test updated.

The test correctly uses builder.current_world to assign world indices and verifies particle group assignments in the finalized model.


291-390: LGTM! Comprehensive world grouping test updated.

The test thoroughly verifies world-based entity grouping:

  • Uses builder.current_world for world assignments (Lines 296, 437, 466, 489)
  • Uses world parameter in add_builder calls (Lines 327, 331)
  • Asserts on model.shape_world instead of previous shape_group (Lines 353, 376-380)
  • Verifies all entity types (particles, bodies, shapes, joints, articulations)

392-431: LGTM! num_envs tracking test updated.

The test correctly verifies num_envs tracking with the world parameter in add_builder calls, including:

  • Global entities (world=-1) not incrementing num_envs
  • Auto-increment with world=None
  • Explicit world indices

563-563: LGTM! shape_world assertion updated.

The test correctly asserts on builder.shape_world instead of the previous shape_group attribute.

newton/tests/test_mujoco_solver.py (1)

1522-1530: LGTM! API updates align with world-centric refactoring.

The changes correctly update the test to use the new world-based API:

  • add_builder now uses world= parameter instead of environment=
  • current_env_group property renamed to current_world

The test logic remains functionally equivalent.

newton/_src/geometry/broad_phase_nxn.py (2)

85-106: LGTM! Consistent world-based refactoring.

The parameter rename from shape_group to shape_world is applied consistently:

  • Kernel parameter updated with clear comment
  • Logic correctly uses world indices for collision filtering
  • World compatibility check properly handles global entities (-1) and world-specific entities

The collision semantics are preserved: entities from different worlds don't collide unless at least one is global.


151-180: LGTM! Public API documentation clearly explains world semantics.

The public API changes are well-documented:

  • Parameter renamed from geom_shape_group to geom_shape_world
  • Docstring clearly explains world indices (-1 for global, 0+ for world-specific)
  • Collision rules are explicitly stated in the documentation
newton/_src/geometry/kernels.py (1)

689-723: Drop call-site rename check
The create_soft_contacts kernel signature still uses particle_group, so all existing call sites passing model.particle_group remain correct.

Likely an incorrect or invalid review comment.

Comment thread newton/_src/geometry/broad_phase_sap.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
newton/_src/geometry/kernels.py (1)

689-705: Optional naming consistency

Consider renaming particle_group → particle_world across comments/vars in future to fully align with the world terminology. Not blocking.

newton/tests/test_model.py (1)

279-281: Minor terminology nit

Tests still reference “environment” in names/docstrings while using current_world. Optional to rename to “world” for consistency.

Also applies to: 296-301

newton/tests/test_environment_group_collision.py (1)

327-361: Confirm helper exists and suggest renaming for consistency
test_environment_and_group_pair is defined in newton/_src/geometry/broad_phase_common.py and imported correctly in tests; consider renaming “environment” to “world” in the function name/parameters (e.g. at broad_phase_common.py:91) and in corresponding tests for consistent terminology.

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 16472f6 and 344c290.

📒 Files selected for processing (13)
  • newton/_src/geometry/broad_phase_common.py (1 hunks)
  • newton/_src/geometry/broad_phase_nxn.py (5 hunks)
  • newton/_src/geometry/broad_phase_sap.py (6 hunks)
  • newton/_src/geometry/kernels.py (2 hunks)
  • newton/_src/sim/builder.py (17 hunks)
  • newton/_src/sim/collide.py (1 hunks)
  • newton/_src/sim/model.py (5 hunks)
  • newton/_src/sim/style3d/builder_style3d.py (1 hunks)
  • newton/_src/solvers/mujoco/solver_mujoco.py (5 hunks)
  • newton/tests/test_broad_phase.py (2 hunks)
  • newton/tests/test_environment_group_collision.py (6 hunks)
  • newton/tests/test_model.py (9 hunks)
  • newton/tests/test_mujoco_solver.py (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-12T17:51:37.474Z
Learnt from: nvlukasz
PR: newton-physics/newton#519
File: newton/geometry.py:16-22
Timestamp: 2025-08-12T17:51:37.474Z
Learning: In the Newton public API refactor, common geometry symbols like ParticleFlags and ShapeFlags are now exposed at the top level in newton/__init__.py rather than through newton.geometry. The newton/geometry.py module is intentionally focused only on broad-phase collision detection classes (BroadPhaseAllPairs, BroadPhaseExplicit, BroadPhaseSAP).

Applied to files:

  • newton/_src/geometry/broad_phase_nxn.py
  • newton/_src/geometry/broad_phase_sap.py
🧬 Code graph analysis (8)
newton/tests/test_broad_phase.py (2)
newton/_src/geometry/broad_phase_nxn.py (2)
  • launch (145-201)
  • launch (218-264)
newton/_src/geometry/broad_phase_sap.py (1)
  • launch (341-481)
newton/_src/geometry/broad_phase_nxn.py (1)
newton/_src/geometry/broad_phase_common.py (1)
  • test_environment_and_group_pair (91-119)
newton/tests/test_mujoco_solver.py (2)
newton/_src/sim/builder.py (2)
  • add_builder (912-1189)
  • add_joint_free (1621-1672)
newton/_src/sim/style3d/builder_style3d.py (1)
  • add_builder (110-140)
newton/_src/geometry/broad_phase_sap.py (1)
newton/_src/geometry/broad_phase_common.py (1)
  • test_environment_and_group_pair (91-119)
newton/_src/sim/builder.py (1)
newton/_src/geometry/flags.py (1)
  • ShapeFlags (30-42)
newton/tests/test_model.py (1)
newton/_src/sim/builder.py (2)
  • add_builder (912-1189)
  • ModelBuilder (67-4483)
newton/tests/test_environment_group_collision.py (1)
newton/_src/sim/builder.py (3)
  • ModelBuilder (67-4483)
  • add_builder (912-1189)
  • finalize (4070-4428)
newton/_src/sim/style3d/builder_style3d.py (1)
newton/_src/sim/builder.py (1)
  • add_builder (912-1189)
🪛 Ruff (0.14.0)
newton/_src/sim/builder.py

961-961: Avoid specifying long messages outside the exception class

(TRY003)

🔇 Additional comments (27)
newton/tests/test_mujoco_solver.py (1)

1522-1531: World assignment in add_builder/current_world looks correct

Explicit worlds for sub-builders and per-world free joints align with ModelBuilder.add_builder(world=...) semantics. No issues.

newton/tests/test_broad_phase.py (2)

129-139: NXN broadphase: world indices wired correctly

Passing shape_world (all -1/global) matches the updated launch signature. LGTM.


386-396: SAP broadphase: world indices wired correctly

shape_world array and launch args are consistent with the new API. LGTM.

newton/tests/test_model.py (2)

326-338: World grouping tests align with new semantics

  • add_builder(world=...) and auto-assignment (None → next world) are exercised and validated via shape_world/body_group/joint_group checks. Looks good.

Also applies to: 353-381


563-564: shape_world assertion matches documented override behavior

Verifying [0, 1, 2] (including static shape via add_builder assigned to its own world) is consistent with the add_builder docs. LGTM.

newton/tests/test_environment_group_collision.py (1)

41-56: World indexing in tests is consistent and thorough

Setting current_world and using add_builder(world=...) correctly validates per-world collision filtering and global (-1) behavior. Looks good.

Also applies to: 92-104, 138-147, 151-164, 193-206, 211-234

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

689-705: All create_soft_contacts call sites updated with shape_world
The wp.launch in newton/_src/sim/collide.py now passes model.shape_world in the correct position, and no other usages remain.

newton/_src/geometry/broad_phase_sap.py (3)

234-234: LGTM! Terminology updated to world-centric semantics.

The parameter and variable renaming from environment group to world indices is consistent with the broader refactoring. The collision filtering logic correctly uses test_environment_and_group_pair with world indices.

Also applies to: 269-276


347-347: LGTM! Well-documented breaking API change.

The public API parameter rename and comprehensive docstring updates clearly communicate the world-centric semantics. The documentation of world index -1 for global entities and 0+ for world-specific entities provides clear guidance for users.

Also applies to: 357-377


470-470: LGTM! Kernel invocation updated consistently.

The kernel invocation correctly passes the renamed parameter.

newton/_src/geometry/broad_phase_nxn.py (3)

85-85: LGTM! Consistent world-centric terminology.

The kernel parameter and variable updates align with the broader refactoring. The collision filtering logic correctly uses world indices.

Also applies to: 98-105


151-151: LGTM! API consistently updated across broadphase implementations.

The public API parameter rename and docstring updates match the changes in broad_phase_sap.py, ensuring consistency across broadphase implementations.

Also applies to: 170-179


198-198: LGTM! Kernel invocation updated consistently.

newton/_src/solvers/mujoco/solver_mujoco.py (4)

1026-1026: LGTM! World-based shape mapping logic is correct.

The kernel parameter rename and the conditional logic correctly distinguish between global shapes (world index < 0) and world-specific shapes (world index >= 0). The index computation for world-specific shapes appropriately accounts for environment replication.

Also applies to: 1040-1045


1957-1957: LGTM! Attribute extraction updated consistently.


2011-2019: LGTM! Shape selection logic correctly implements world-based filtering.

The logic correctly:

  1. Identifies the first world by finding the minimum non-negative world index
  2. Counts shapes belonging to the first world (used for replication)
  3. Selects shapes from the first world plus global shapes (world < 0)

This appropriately handles the distinction between global shapes (shared across worlds) and world-specific shapes (replicated per world).


2436-2436: LGTM! Kernel invocation updated consistently.

The kernel invocation correctly passes the renamed model attribute.

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

106-138: Excellent documentation update for world grouping.

The updated documentation clearly explains the world grouping concept, including:

  • Global entities (index -1) vs. world-specific entities (index 0+)
  • Two approaches for assigning world indices
  • Concrete examples for both approaches

The examples effectively demonstrate the overriding behavior of add_builder().


912-957: API update correctly reflects world-based grouping.

The parameter rename from environment to world is consistent with the PR's terminology update. The documentation clearly explains the overriding behavior where ALL entities from the sub-builder are assigned to the specified world.


964-972: World assignment logic is correct.

The logic properly:

  • Determines the world index (either from parameter or auto-incremented)
  • Saves the previous world state
  • Sets the current world for the duration of entity addition

1071-1096: Entity world assignments are consistent.

All entity types (particles, bodies, shapes, joints, articulations) are correctly assigned to self.current_world, overriding any world assignments from the source builder.


1178-1189: Correct handling of num_envs and state restoration.

The logic correctly:

  • Excludes global entities (world=-1) from num_envs count
  • Handles explicit world index vs. auto-increment
  • Restores the previous world state after adding entities

656-656: Consistent world assignment across entity creation methods.

All entity creation methods (add_articulation, add_body, add_joint, add_shape, add_particle, add_particles) correctly assign self.current_world to new entities, maintaining consistent world grouping behavior.

Also applies to: 1252-1252, 1317-1317, 2446-2446, 3036-3036, 3072-3072


4451-4466: Collision filtering correctly updated for world-based grouping.

The logic properly:

  • Sorts shapes by shape_world for efficient processing
  • Skips shapes from different worlds (unless one is global with index -1)
  • Uses early break optimization when non-global shapes have different worlds
  • Maintains independent collision group filtering

The world-based filtering correctly replaces the previous environment-based approach.


4120-4120: Model arrays correctly created from world-based builder attributes.

The finalize method properly transfers world grouping data from builder lists to model arrays:

  • particle_groupmodel.particle_group
  • shape_worldmodel.shape_world
  • body_groupmodel.body_group
  • joint_groupmodel.joint_group
  • articulation_groupmodel.articulation_group

Also applies to: 4163-4163, 4317-4317, 4330-4330, 4377-4377


2220-2230: World group preservation is correct in collapse_fixed_joints.

The method properly:

  • Saves original world groups before clearing builder state
  • Restores world groups from original indices after body/joint remapping
  • Falls back to -1 (global) for entities without assigned groups

This ensures world assignments are maintained correctly when fixed joints are collapsed.

Also applies to: 2260-2264, 2284-2329

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

237-237: LGTM: Shape world parameter correctly updated.

The parameter rename from model.shape_group to model.shape_world is consistent with the model attribute rename and the broader world-based terminology refactoring across the codebase.

@nvtw nvtw marked this pull request as ready for review October 15, 2025 14:44
@nvtw nvtw requested review from eric-heiden and mmacklin October 15, 2025 14:45
@nvtw

nvtw commented Oct 15, 2025

Copy link
Copy Markdown
Member Author

Is there more documentation that needs update than just the in-code docstrings?

@nvtw

nvtw commented Oct 15, 2025

Copy link
Copy Markdown
Member Author

What about the command line argument:

parser.add_argument("--num-envs", type=int, default=4, help="Total number of simulated environments.")

Should it become --num-worlds?

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
newton/_src/geometry/broad_phase_common.py (1)

91-91: Consider renaming the function to match the new terminology.

The function name test_environment_and_group_pair still uses "environment" terminology, while the parameters (world_a, world_b) and docstring have been updated to use "world". For consistency with the rest of the PR, consider renaming this function to test_world_and_group_pair.

Apply this diff if you'd like to align the function name with the new terminology:

-def test_environment_and_group_pair(world_a: int, world_b: int, collision_group_a: int, collision_group_b: int) -> bool:
+def test_world_and_group_pair(world_a: int, world_b: int, collision_group_a: int, collision_group_b: int) -> bool:
newton/tests/test_model.py (1)

432-529: Consider updating comments to use "world" terminology consistently.

The function docstring and code correctly use "world" terminology (current_world, "world groups"), but the inline comments still refer to "Environment 0" and "Environment 1" (lines 436, 465, 488). For consistency, consider updating these to "World 0" and "World 1".

Apply these changes for consistency:

-        # Environment 0: Chain with fixed joints
+        # World 0: Chain with fixed joints
         builder.current_world = 0
-        # Environment 1: Another chain
+        # World 1: Another chain
         builder.current_world = 1
📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 344c290 and cb2e6fa.

📒 Files selected for processing (3)
  • docs/migration.rst (1 hunks)
  • newton/_src/geometry/broad_phase_common.py (1 hunks)
  • newton/tests/test_model.py (10 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
newton/tests/test_model.py (1)
newton/_src/sim/builder.py (2)
  • add_builder (912-1189)
  • ModelBuilder (67-4483)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Run GPU Benchmarks (Pull Request)
🔇 Additional comments (7)
docs/migration.rst (1)

151-166: LGTM! Documentation clearly explains the terminology migration.

The migration guide effectively documents the rename from "environment" to "world" with clear before/after mappings and helpful context about global entities.

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

92-117: LGTM! Docstring and implementation correctly updated.

The docstring clearly explains the world-based collision semantics, and the implementation correctly preserves the original logic while using the new "world" terminology.

newton/tests/test_model.py (5)

279-279: LGTM! Correct attribute rename.

The attribute current_env_group has been correctly renamed to current_world, consistent with the terminology change.


291-337: LGTM! Test correctly updated to use world terminology.

The test function test_environment_grouping has been correctly updated:

  • Attribute references changed from current_env_group to current_world
  • Comments updated to refer to "world" instead of "environment"
  • add_builder() calls now use the world parameter name

353-380: LGTM! Shape world attribute correctly renamed.

The attribute shape_group has been correctly renamed to shape_world, and all assertions properly verify world assignments for shapes across different worlds (including global entities with index -1).


392-430: LGTM! Test correctly validates world tracking behavior.

The test_num_envs_tracking function has been properly updated:

  • Docstring mentions "worlds" instead of environments
  • All add_builder() calls use the world parameter
  • Comments clearly describe world index behavior
  • Test logic correctly validates global entities (world=-1), auto-increment, and explicit world indices

563-563: LGTM! Correct attribute reference.

The assertion correctly uses shape_world instead of the old shape_group attribute name.

Comment thread docs/migration.rst Outdated
Comment thread newton/_src/sim/model.py Outdated

@eric-heiden eric-heiden left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good! Can you search for all occurrences of "env" and make sure they are renamed to world accordingly. So yes, that includes the documentation.

@eric-heiden

eric-heiden commented Oct 16, 2025

Copy link
Copy Markdown
Member

What about the command line argument:

parser.add_argument("--num-envs", type=int, default=4, help="Total number of simulated environments.")

Should it become --num-worlds?

Yes, let's rename those to --num-worlds. Similarly for the builder.replicate() function, should use a num_worlds argument.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (2)
newton/_src/sim/model.py (2)

222-223: Inconsistent attribute naming: body_group should be renamed to body_world.

Following the same pattern as shape_world (line 148), this attribute should be renamed from body_group to body_world for consistency.

Apply this diff:

-        self.body_group = None
-        """World index for each body, shape [body_count], int. Global entities have index -1."""
+        self.body_world = None
+        """World index for each body, shape [body_count], int. Global entities have index -1."""

Based on learnings from past review comments.


94-95: Inconsistent attribute naming: particle_group should be renamed to particle_world.

Based on the past review comment, all *_group attributes should be renamed to *_world for consistency. Currently, only shape_group was renamed to shape_world (line 148), but particle_group, body_group, joint_group, and articulation_group were not renamed—only their docstrings were updated.

Apply this diff to rename particle_group to particle_world:

-        self.particle_group = None
-        """World index for each particle, shape [particle_count], int. -1 for global."""
+        self.particle_world = None
+        """World index for each particle, shape [particle_count], int. -1 for global."""

Note: Similar changes are needed for body_group (line 222), joint_group (line 283), and articulation_group (line 289). Ensure all references to these attributes throughout the codebase are also updated.

Based on learnings from past review comments.

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cb2e6fa and 1c30c1b.

📒 Files selected for processing (4)
  • newton/_src/geometry/kernels.py (2 hunks)
  • newton/_src/sim/builder.py (17 hunks)
  • newton/_src/sim/collide.py (1 hunks)
  • newton/_src/sim/model.py (5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
newton/_src/sim/builder.py (1)
newton/_src/geometry/flags.py (1)
  • ShapeFlags (30-42)
🪛 Ruff (0.14.0)
newton/_src/sim/builder.py

961-961: Avoid specifying long messages outside the exception class

(TRY003)

⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Run GPU Unit Tests on AWS EC2 (Pull Request)
  • GitHub Check: Run GPU Benchmarks (Pull Request)
🔇 Additional comments (10)
newton/_src/sim/model.py (1)

148-149: LGTM! Attribute renamed correctly.

The shape_group attribute has been correctly renamed to shape_world with an appropriate docstring.

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

696-696: LGTM! Parameter renamed correctly.

The shape_group parameter has been correctly renamed to shape_world.


717-723: LGTM! World index logic updated correctly.

The collision filtering logic has been correctly updated to use world indices, with proper handling of the global world index (-1).

newton/_src/sim/collide.py (2)

239-239: LGTM! Correctly updated to use shape_world.

The reference has been correctly updated from model.shape_group to model.shape_world, consistent with the attribute renaming in newton/_src/sim/model.py.


232-232: The review comment is based on an incorrect assumption. The particle_group attribute has not been renamed to particle_world in newton/_src/sim/model.py.

The code at line 232 of collide.py is currently correct and consistent with the actual attribute name in model.py (line 94: self.particle_group = None). The reference to model.particle_group matches the existing codebase. No changes are needed at this location.

Likely an incorrect or invalid review comment.

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

106-138: LGTM! Clear and comprehensive documentation update.

The "World Grouping" documentation is well-written with clear examples showing both direct entity creation and add_builder() usage. The explanation of global entities (index -1) vs. world-specific entities is helpful.


336-449: Consistent renaming of world index arrays.

All entity types now have consistently named world index arrays:

  • particle_group → tracks world index for particles
  • shape_world → tracks world index for shapes
  • body_group → tracks world index for bodies
  • joint_group → tracks world index for joints
  • articulation_group → tracks world index for articulations

The naming is consistent and the functionality is preserved.


912-1190: Well-implemented add_builder refactoring with clear world grouping semantics.

The changes correctly implement the world grouping behavior:

  1. Parameter renamed from environment to world with clear documentation (lines 919-958)
  2. World index assignment logic handles both explicit and auto-increment cases (lines 964-972)
  3. All entities from the sub-builder are assigned to the same world, overriding any previous assignments (lines 1071-1096)
  4. World count updates correctly distinguish between global entities (-1) and world-specific entities (lines 1178-1186)
  5. Previous world state is properly saved and restored (lines 970-972, 1188-1189)

The implementation correctly ensures that:

  • Global entities (world=-1) don't increment num_envs
  • Explicit world indices expand num_envs as needed
  • Auto-assigned worlds use the current num_envs value

4450-4480: Collision filtering logic correctly handles world boundaries.

The shape contact pair generation correctly implements world-based collision filtering:

  1. Sorting optimization (line 4452): Shapes sorted by shape_world enables early termination
  2. Global entity handling (line 4464): Global shapes (world=-1) pair with all worlds because env1 != -1 is false
  3. World isolation (lines 4462-4465): Non-global shapes from different worlds don't collide; the break statement is safe because shapes are sorted
  4. Collision group filtering (lines 4467-4470): Preserves existing collision group logic within worlds

The logic is both correct and efficient.


457-457: No issue identified—num_envs attribute retention is intentional and correct.

The num_envs attribute is part of the public Model/ModelBuilder API. Evidence confirms it is used in user-facing example code (example_sensor_contact.py), passed to public methods like builder.replicate(), and accessed directly by users. The distinction is correct: internal state variables (current_env_groupcurrent_world) are renamed for conceptual clarity, while public API attributes (num_envs) are preserved for backward compatibility. No changes needed.

Comment thread newton/_src/geometry/kernels.py Outdated
Comment thread newton/_src/sim/model.py
Comment thread newton/_src/sim/model.py
@nvtw

nvtw commented Oct 16, 2025

Copy link
Copy Markdown
Member Author

I did searched for all "env" and renamed them. But the MR got a bit messy now, there are tons of references to env. Especially annoying are environment variables end eigenvectors which botch contain "env". I tried my best only renaming the right phrases.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
newton/_src/utils/import_usd.py (1)

47-58: Maintain backward compatibility for cloned_env alias
Accept the deprecated cloned_env parameter, map it to cloned_world with a DeprecationWarning, and update the docstring to reflect Optional[str].

Apply these diffs in newton/_src/utils/import_usd.py:

  1. Signature alias (after line 47):
-    cloned_world: str | None = None,
+    cloned_world: str | None = None,
+    cloned_env: str | None = None,
  1. Early mapping (immediately after ignore_paths is initialized):
     if ignore_paths is None:
         ignore_paths = []
+
+    # Back-compat alias: cloned_env → cloned_world
+    if cloned_world is None and cloned_env is not None:
+        warnings.warn(
+            "parse_usd(..., cloned_env=...) is deprecated; use cloned_world=...",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        cloned_world = cloned_env
  1. Update the docstring entry for cloned_world to cloned_world (Optional[str]) / cloned_env (Optional[str], deprecated) with a brief deprecation note.
newton/tests/test_joint_drive.py (1)

94-112: Builder variable shadowing and self-add lead to incorrect model composition

Inside the for-loop, world_builder is redefined and then added into itself via world_builder.add_builder(world_builder). This is unsafe (self-copy during mutation) and likely produces incorrect data and nondeterministic behavior. Maintain an outer scene builder and add per-world builders into it.

Apply this diff to fix:

-        world_builder = newton.ModelBuilder(gravity=g, up_axis=world_up_axis)
-        for i in range(0, nb_worlds):
+        scene_builder = newton.ModelBuilder(gravity=g, up_axis=world_up_axis)
+        for i in range(0, nb_worlds):
             body_mass = body_masses[i]
             ...
-            # Create a single body jointed to the world with a prismatic joint
-            # Make sure that we use the mass properties specified here by setting shape density to 0.0
-            world_builder = newton.ModelBuilder(gravity=g, up_axis=world_up_axis)
-            bodyIndex = world_builder.add_body(mass=body_mass, I_m=body_inertia, armature=0.0, com=body_com)
-            world_builder.add_shape_sphere(
+            # Build a single-world model, then add it to the scene
+            per_world = newton.ModelBuilder(gravity=g, up_axis=world_up_axis)
+            bodyIndex = per_world.add_body(mass=body_mass, I_m=body_inertia, armature=0.0, com=body_com)
+            per_world.add_shape_sphere(
                 radius=1.0, body=bodyIndex, cfg=newton.ModelBuilder.ShapeConfig(density=0.0, has_shape_collision=False)
             )
             if is_prismatic:
-                world_builder.add_joint_prismatic(
+                per_world.add_joint_prismatic(
                     mode=newton.JointMode.TARGET_POSITION,
                     axis=joint_motion_axis,
                     parent=-1,
                     child=bodyIndex,
                     target=joint_drive_target_position,
                     target_ke=drive_stiffness,
                     target_kd=joint_drive_damping,
                     armature=0.0,
                     effort_limit=1000000000000.0,
                     velocity_limit=100000000000000000.0,
                     friction=0.0,
                 )
             else:
-                world_builder.add_joint_revolute(
+                per_world.add_joint_revolute(
                     mode=newton.JointMode.TARGET_POSITION,
                     axis=joint_motion_axis,
                     parent=-1,
                     child=bodyIndex,
                     target=joint_drive_target_position,
                     target_ke=drive_stiffness,
                     target_kd=joint_drive_damping,
                     armature=0.0,
                     effort_limit=1000000000000.0,
                     velocity_limit=100000000000000000.0,
                     friction=0.0,
                 )
-
-            world_builder.add_builder(world_builder)
+            scene_builder.add_builder(per_world)
 
             # Set the start pos and vel of the dof.
-            world_builder.joint_q[i] = joint_start_position
-            world_builder.joint_qd[i] = joint_start_velocity
+            scene_builder.joint_q[i] = joint_start_position
+            scene_builder.joint_qd[i] = joint_start_velocity
@@
-        model = world_builder.finalize()
+        model = scene_builder.finalize()
newton/examples/example_mujoco.py (1)

261-292: Add backward-compat for deprecated num_envs

To avoid breaking external callers and fix the benchmark failure, accept num_envs as an alias to num_worlds.

Apply this diff:

-    def __init__(
+    def __init__(
         self,
         robot="humanoid",
-        world="None",
+        world="None",
         stage_path=None,
-        num_worlds=1,
+        num_worlds=1,
         use_cuda_graph=True,
         use_mujoco_cpu=False,
         randomize=False,
         headless=False,
         actuation="None",
         solver=None,
         integrator=None,
         solver_iteration=None,
         ls_iteration=None,
         njmax=None,
         nconmax=None,
         builder=None,
         ls_parallel=None,
-    ):
+        **kwargs,
+    ):
+        # Backward-compat: accept deprecated num_envs
+        if "num_envs" in kwargs and ("num_worlds" not in locals() or num_worlds == 1):
+            num_worlds = kwargs.pop("num_envs")
newton/_src/solvers/mujoco/solver_mujoco.py (1)

2452-2456: Replace incorrect body index mapping in solver_mujoco.py
The current code at lines 2452–2456 builds to_mjc_body_index from the dict keys (+1), ignoring the actual MuJoCo IDs stored in the values. This leads to wrong body indices. Replace with a value-based construction, e.g.:

# Build mapping from Newton body ID to MuJoCo body ID
max_newton_id = max(k for k in body_mapping.keys() if k >= 0)
to_mjc_body_index = np.full(max_newton_id + 1, -1, dtype=np.int32)
for newton_id, mj_id in body_mapping.items():
    if newton_id >= 0:
        to_mjc_body_index[newton_id] = mj_id
self.to_mjc_body_index = wp.array(to_mjc_body_index, dtype=wp.int32)

Add tests covering nontrivial Newton↔MuJoCo body order mappings.

♻️ Duplicate comments (1)
newton/_src/geometry/broad_phase_sap.py (1)

366-369: Doc/code mismatch: geom_shape_world None-handling will crash at kernel; add fallback or amend doc.

Doc claims None is allowed, but _sap_broadphase_kernel requires an array and will index into it.

Pick one:

  • Preferred: Support None by treating all geoms as global (-1):
@@ def launch(...):
-        wp.launch(
+        # Ensure world indices are available
+        if geom_shape_world is None:
+            with wp.ScopedDevice(geom_lower.device):
+                geom_shape_world = wp.array(np.full(geom_count, -1, dtype=np.int32), dtype=int)
+
+        wp.launch(
             kernel=_sap_project_kernel,

And update the doc:

-                Can be None if world indices are not used.
+                If None, all geometries are treated as global (-1).

Add missing import at the top of the file:

import numpy as np
  • Or, if you don’t want implicit behavior, remove the “Can be None” claim from the docstring.
🧹 Nitpick comments (15)
asv/benchmarks/setup/bench_model.py (2)

30-30: Annotate class attribute param_names with ClassVar (fixes RUF012).

param_names is a class attribute; annotate with typing.ClassVar to satisfy linting.

Apply this diff in each class:

-param_names = ["robot", "num_worlds"]
+param_names: ClassVar[list[str]] = ["robot", "num_worlds"]

Add import near the top of the file:

from typing import ClassVar

Also applies to: 52-52, 83-83, 118-118


38-38: Optional: silence ARG002 (unused args) in asv hooks.

These signatures are dictated by asv. If desired, prefix unused args with “_” or add a lint ignore to avoid noise.

Also applies to: 60-60, 72-72, 76-76, 97-97, 104-104, 125-125, 137-137, 141-141

newton/examples/__init__.py (2)

220-220: Remove unused noqa directive (RUF100).

The PLC0415 suppression isn’t active; drop the noqa.

-import warnings  # noqa: PLC0415
+import warnings

263-263: Nit: s/envs/worlds in comment.

Keep wording consistent with the new terminology.

-    # ensure the envs are not shifted below the ground plane
+    # ensure the worlds are not shifted below the ground plane
newton/_src/utils/import_usd.py (1)

74-75: Docstring type nit: cloned_world should be Optional[str].

Update the docstring entry to reflect Optional type.

-        cloned_world (str): The prim path of a world which is cloned within this USD file. Siblings of this world prim will not be parsed but instead be replicated via `ModelBuilder.add_builder(builder, xform)` to speed up the loading of many instantiated worlds.
+        cloned_world (str | None): The prim path of a world which is cloned within this USD file. Siblings of this world prim will not be parsed but instead be replicated via `ModelBuilder.add_builder(builder, xform)` to speed up the loading of many instantiated worlds.
newton/examples/robot/example_robot_cartpole.py (1)

163-165: Silence ARG005 by using underscore for unused lambda args.

No behavior change; reduces lint noise.

-            lambda q, qd: newton.utils.vec_allclose(qd, world0_cart_vel),
+            lambda _q, qd: newton.utils.vec_allclose(qd, world0_cart_vel),
@@
-            lambda q, qd: newton.utils.vec_allclose(qd, world0_pole1_vel),
+            lambda _q, qd: newton.utils.vec_allclose(qd, world0_pole1_vel),
@@
-            lambda q, qd: newton.utils.vec_allclose(qd, world0_pole2_vel),
+            lambda _q, qd: newton.utils.vec_allclose(qd, world0_pole2_vel),

Also applies to: 170-172, 177-179

newton/tests/test_environment_group_collision.py (1)

24-26: Class name still references “Environment”

Consider renaming the test class to TestWorldGroupCollision for consistency with the refactor.

asv/benchmarks/simulation/bench_mujoco.py (1)

34-44: Ruff RUF012: annotate class attributes as ClassVar (optional)

num_worlds and other class-level configs can be annotated as typing.ClassVar for clarity/static checks. Optional.

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

200-212: Unused lambda arg ‘qd’

Silence linter and clarify intent by ignoring the second arg.

Apply this diff:

-                lambda q, qd: newton.utils.vec_inside_limits(q.p, world_lower, world_upper),  # noqa: B023
+                lambda q, _qd: newton.utils.vec_inside_limits(q.p, world_lower, world_upper),

43-75: Kernel indexing by world_id: OK

Assumes 22 DOFs per hand instance; if this changes, root_joint_id computation must be updated. Consider deriving from model data instead of hard-coding 22.

Confirm 22 is stable across the asset variants used here.

newton/examples/example_mujoco.py (2)

373-409: create_model_builder signature: OK; add alias support (optional)

Optionally accept deprecated num_envs here too for callers using the factory directly.

Apply this diff:

-    def create_model_builder(robot, num_worlds, world="None", randomize=False, seed=123) -> newton.ModelBuilder:
+    def create_model_builder(robot, num_worlds=None, world="None", randomize=False, seed=123, **kwargs) -> newton.ModelBuilder:
+        # Backward-compat
+        if num_worlds is None and "num_envs" in kwargs:
+            num_worlds = kwargs.pop("num_envs")

453-497: CLI: consider adding deprecated flag alias

Provide --num-envs as hidden alias to ease scripts/users migration.

Suggested snippet (outside diff block):

  • parser.add_argument("--num-envs", type=int, default=None, help=argparse.SUPPRESS)
  • if args.num_envs is not None and args.num_worlds == 1: args.num_worlds = args.num_envs
newton/_src/solvers/mujoco/solver_mujoco.py (1)

197-237: Unused kernel arg bodies_per_world

bodies_per_world is not used in convert_newton_contacts_to_mjwarp_kernel. Remove to reduce confusion, or use it to compute worldid consistently.

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

265-271: Optional: define --num-worlds centrally

To avoid duplication, consider adding --num-worlds in the centralized examples parser so individual scripts don’t repeat it. Based on learnings

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

4328-4333: Verify Model field names: joint_group/articulation_group vs *_world

Builder arrays use joint_world/articulation_world, but finalize assigns m.joint_group/m.articulation_group. Ensure Model matches the new naming; otherwise, rename for consistency.

If Model expects the new names, apply:

-            m.joint_group = wp.array(self.joint_world, dtype=wp.int32)
+            m.joint_world = wp.array(self.joint_world, dtype=wp.int32)
...
-            m.articulation_group = wp.array(self.articulation_world, dtype=wp.int32)
+            m.articulation_world = wp.array(self.articulation_world, dtype=wp.int32)

Please confirm what Model currently exposes to avoid breaking dependents (e.g., ArticulationView).

Also applies to: 4374-4379

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1c30c1b and dbf0999.

📒 Files selected for processing (48)
  • asv/benchmarks/setup/bench_model.py (4 hunks)
  • asv/benchmarks/simulation/bench_mujoco.py (7 hunks)
  • asv/benchmarks/simulation/bench_quadruped_xpbd.py (1 hunks)
  • asv/benchmarks/simulation/bench_selection.py (1 hunks)
  • docs/guide/installation.rst (1 hunks)
  • newton/_src/geometry/broad_phase_common.py (1 hunks)
  • newton/_src/geometry/broad_phase_nxn.py (6 hunks)
  • newton/_src/geometry/broad_phase_sap.py (7 hunks)
  • newton/_src/geometry/kernels.py (2 hunks)
  • newton/_src/sim/builder.py (36 hunks)
  • newton/_src/sim/collide.py (1 hunks)
  • newton/_src/sim/model.py (6 hunks)
  • newton/_src/sim/style3d/builder_style3d.py (1 hunks)
  • newton/_src/solvers/mujoco/solver_mujoco.py (51 hunks)
  • newton/_src/utils/__init__.py (1 hunks)
  • newton/_src/utils/import_usd.py (5 hunks)
  • newton/_src/utils/selection.py (4 hunks)
  • newton/_src/viewer/viewer_gl.py (4 hunks)
  • newton/_src/viewer/viewer_null.py (1 hunks)
  • newton/examples/__init__.py (3 hunks)
  • newton/examples/basic/example_basic_urdf.py (4 hunks)
  • newton/examples/cloth/example_cloth_franka.py (3 hunks)
  • newton/examples/diffsim/example_diffsim_drone.py (8 hunks)
  • newton/examples/example_mujoco.py (16 hunks)
  • newton/examples/example_recording.py (2 hunks)
  • newton/examples/ik/example_ik_benchmark.py (1 hunks)
  • newton/examples/robot/example_robot_allegro_hand.py (6 hunks)
  • newton/examples/robot/example_robot_anymal_d.py (5 hunks)
  • newton/examples/robot/example_robot_cartpole.py (7 hunks)
  • newton/examples/robot/example_robot_g1.py (5 hunks)
  • newton/examples/robot/example_robot_h1.py (4 hunks)
  • newton/examples/robot/example_robot_humanoid.py (4 hunks)
  • newton/examples/robot/example_robot_policy.py (1 hunks)
  • newton/examples/robot/example_robot_ur10.py (7 hunks)
  • newton/examples/selection/example_selection_articulations.py (10 hunks)
  • newton/examples/selection/example_selection_cartpole.py (8 hunks)
  • newton/examples/selection/example_selection_materials.py (8 hunks)
  • newton/examples/sensors/example_sensor_contact.py (10 hunks)
  • newton/tests/test_anymal_reset.py (1 hunks)
  • newton/tests/test_environment_group_collision.py (10 hunks)
  • newton/tests/test_equality_constraints.py (1 hunks)
  • newton/tests/test_examples.py (1 hunks)
  • newton/tests/test_ik_fk_kernels.py (1 hunks)
  • newton/tests/test_import_usd.py (1 hunks)
  • newton/tests/test_joint_drive.py (15 hunks)
  • newton/tests/test_kinematics.py (5 hunks)
  • newton/tests/test_model.py (11 hunks)
  • newton/tests/test_mujoco_solver.py (22 hunks)
✅ Files skipped from review due to trivial changes (2)
  • docs/guide/installation.rst
  • newton/_src/viewer/viewer_null.py
🚧 Files skipped from review as they are similar to previous changes (3)
  • newton/_src/sim/collide.py
  • newton/_src/sim/model.py
  • newton/_src/geometry/broad_phase_common.py
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-08-25T21:41:45.795Z
Learnt from: dylanturpin
PR: newton-physics/newton#634
File: newton/tests/test_examples.py:329-333
Timestamp: 2025-08-25T21:41:45.795Z
Learning: Newton examples use a centralized CLI argument parsing system in newton/examples/__init__.py. The create_parser() function defines common arguments like --device, --viewer, --output-path, and --num-frames, while init(parser) creates viewers based on parsed arguments. Individual example scripts don't need to define these flags themselves - they inherit them from the centralized system via the example_map and runpy execution.

Applied to files:

  • newton/examples/basic/example_basic_urdf.py
  • newton/examples/sensors/example_sensor_contact.py
  • newton/examples/selection/example_selection_materials.py
  • newton/examples/robot/example_robot_g1.py
  • newton/examples/selection/example_selection_articulations.py
  • newton/examples/robot/example_robot_humanoid.py
  • newton/examples/robot/example_robot_ur10.py
  • newton/examples/robot/example_robot_h1.py
  • newton/examples/robot/example_robot_anymal_d.py
📚 Learning: 2025-08-27T19:05:44.697Z
Learnt from: Milad-Rakhsha-NV
PR: newton-physics/newton#535
File: newton/tests/test_examples.py:320-414
Timestamp: 2025-08-27T19:05:44.697Z
Learning: In newton/examples/__init__.py, the robot policy example is registered with the key "robot_policy" (not "robot.example_robot_policy"), so tests should reference it as name="robot_policy".

Applied to files:

  • newton/examples/robot/example_robot_policy.py
📚 Learning: 2025-08-12T17:51:37.474Z
Learnt from: nvlukasz
PR: newton-physics/newton#519
File: newton/geometry.py:16-22
Timestamp: 2025-08-12T17:51:37.474Z
Learning: In the Newton public API refactor, common geometry symbols like ParticleFlags and ShapeFlags are now exposed at the top level in newton/__init__.py rather than through newton.geometry. The newton/geometry.py module is intentionally focused only on broad-phase collision detection classes (BroadPhaseAllPairs, BroadPhaseExplicit, BroadPhaseSAP).

Applied to files:

  • newton/_src/geometry/broad_phase_sap.py
  • newton/_src/geometry/broad_phase_nxn.py
📚 Learning: 2025-08-19T12:27:30.629Z
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.

Applied to files:

  • newton/examples/example_mujoco.py
📚 Learning: 2025-08-18T15:56:26.587Z
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.587Z
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.

Applied to files:

  • newton/_src/solvers/mujoco/solver_mujoco.py
🧬 Code graph analysis (31)
newton/examples/basic/example_basic_urdf.py (2)
newton/_src/sim/builder.py (3)
  • ModelBuilder (67-4482)
  • replicate (618-642)
  • body_count (515-519)
newton/examples/__init__.py (2)
  • test_body_state (38-112)
  • init (309-352)
newton/_src/utils/selection.py (1)
newton/_src/sim/builder.py (1)
  • shape_count (508-512)
asv/benchmarks/simulation/bench_quadruped_xpbd.py (2)
newton/examples/basic/example_basic_urdf.py (1)
  • Example (36-150)
newton/_src/viewer/viewer_null.py (1)
  • ViewerNull (23-160)
newton/_src/sim/style3d/builder_style3d.py (1)
newton/_src/sim/builder.py (1)
  • add_builder (912-1189)
newton/tests/test_kinematics.py (2)
newton/_src/sim/builder.py (1)
  • ModelBuilder (67-4482)
newton/_src/sim/articulation.py (1)
  • eval_fk (407-463)
newton/examples/sensors/example_sensor_contact.py (2)
newton/examples/selection/example_selection_articulations.py (1)
  • random_forces_kernel (80-83)
newton/_src/sim/builder.py (2)
  • ModelBuilder (67-4482)
  • replicate (618-642)
newton/_src/geometry/broad_phase_sap.py (1)
newton/_src/geometry/broad_phase_common.py (1)
  • test_world_and_group_pair (91-117)
newton/tests/test_environment_group_collision.py (2)
newton/_src/geometry/broad_phase_common.py (1)
  • test_world_and_group_pair (91-117)
newton/_src/sim/builder.py (2)
  • add_builder (912-1189)
  • finalize (4070-4428)
newton/examples/selection/example_selection_materials.py (1)
newton/_src/sim/builder.py (3)
  • ModelBuilder (67-4482)
  • add_mjcf (824-908)
  • replicate (618-642)
newton/examples/selection/example_selection_cartpole.py (2)
newton/_src/sim/builder.py (2)
  • ModelBuilder (67-4482)
  • replicate (618-642)
newton/examples/__init__.py (1)
  • get_asset (34-35)
newton/examples/cloth/example_cloth_franka.py (2)
newton/_src/sim/builder.py (1)
  • body_count (515-519)
newton/_src/sim/state.py (3)
  • body_count (87-91)
  • joint_coord_count (101-105)
  • joint_dof_count (108-112)
newton/examples/__init__.py (1)
newton/_src/core/types.py (2)
  • Axis (63-160)
  • from_any (97-117)
newton/examples/example_mujoco.py (2)
newton/examples/basic/example_basic_urdf.py (1)
  • Example (36-150)
newton/_src/sim/builder.py (2)
  • ModelBuilder (67-4482)
  • replicate (618-642)
newton/examples/robot/example_robot_g1.py (3)
newton/_src/sim/builder.py (1)
  • replicate (618-642)
newton/examples/basic/example_basic_urdf.py (1)
  • Example (36-150)
newton/examples/example_mujoco.py (1)
  • Example (261-447)
newton/_src/utils/import_usd.py (1)
newton/_src/sim/builder.py (4)
  • shape_count (508-512)
  • body_count (515-519)
  • articulation_count (571-575)
  • add_builder (912-1189)
newton/examples/selection/example_selection_articulations.py (3)
newton/examples/sensors/example_sensor_contact.py (3)
  • random_forces_kernel (80-83)
  • Example (86-320)
  • init_masks (50-54)
newton/_src/sim/builder.py (4)
  • ModelBuilder (67-4482)
  • add_mjcf (824-908)
  • replicate (618-642)
  • finalize (4070-4428)
newton/_src/solvers/mujoco/solver_mujoco.py (1)
  • SolverMuJoCo (1136-2835)
newton/examples/robot/example_robot_humanoid.py (2)
newton/_src/sim/builder.py (1)
  • replicate (618-642)
newton/examples/basic/example_basic_urdf.py (1)
  • Example (36-150)
asv/benchmarks/simulation/bench_mujoco.py (2)
asv/benchmarks/setup/bench_model.py (3)
  • setup (38-39)
  • setup (60-69)
  • setup (125-134)
newton/examples/example_mujoco.py (2)
  • Example (261-447)
  • create_model_builder (374-409)
newton/_src/sim/builder.py (2)
newton/_src/core/types.py (2)
  • Axis (63-160)
  • from_any (97-117)
newton/_src/geometry/flags.py (1)
  • ShapeFlags (30-42)
asv/benchmarks/setup/bench_model.py (2)
asv/benchmarks/simulation/bench_mujoco.py (2)
  • setup (45-78)
  • setup (97-101)
newton/examples/example_mujoco.py (3)
  • Example (261-447)
  • create_model_builder (374-409)
  • create_solver (412-447)
newton/_src/geometry/broad_phase_nxn.py (1)
newton/_src/geometry/broad_phase_common.py (3)
  • check_aabb_overlap (22-38)
  • test_world_and_group_pair (91-117)
  • write_pair (54-65)
asv/benchmarks/simulation/bench_selection.py (2)
newton/examples/selection/example_selection_cartpole.py (1)
  • Example (57-218)
newton/_src/viewer/viewer_null.py (1)
  • ViewerNull (23-160)
newton/tests/test_joint_drive.py (1)
newton/_src/sim/builder.py (7)
  • ModelBuilder (67-4482)
  • add_body (1191-1253)
  • add_shape_sphere (2534-2565)
  • ShapeConfig (141-193)
  • add_joint_prismatic (1462-1544)
  • add_joint_revolute (1378-1460)
  • add_builder (912-1189)
newton/examples/robot/example_robot_ur10.py (2)
newton/_src/sim/builder.py (1)
  • replicate (618-642)
newton/examples/__init__.py (1)
  • init (309-352)
newton/examples/example_recording.py (1)
newton/_src/sim/builder.py (1)
  • ModelBuilder (67-4482)
newton/examples/robot/example_robot_cartpole.py (2)
newton/_src/sim/builder.py (2)
  • replicate (618-642)
  • body_count (515-519)
newton/examples/__init__.py (3)
  • test_body_state (38-112)
  • create_parser (269-306)
  • init (309-352)
newton/tests/test_model.py (1)
newton/_src/sim/builder.py (12)
  • ModelBuilder (67-4482)
  • add_builder (912-1189)
  • add_particles (3042-3072)
  • finalize (4070-4428)
  • add_body (1191-1253)
  • add_particle (3006-3040)
  • add_joint_revolute (1378-1460)
  • particle_count (529-533)
  • body_count (515-519)
  • shape_count (508-512)
  • joint_count (522-526)
  • articulation_count (571-575)
newton/_src/solvers/mujoco/solver_mujoco.py (4)
newton/_src/sim/builder.py (3)
  • body_count (515-519)
  • joint_count (522-526)
  • shape_count (508-512)
newton/_src/sim/state.py (2)
  • body_count (87-91)
  • joint_dof_count (108-112)
newton/_src/geometry/broad_phase_nxn.py (2)
  • launch (145-201)
  • launch (218-264)
newton/_src/geometry/broad_phase_sap.py (1)
  • launch (341-481)
newton/examples/robot/example_robot_h1.py (2)
newton/_src/sim/builder.py (1)
  • replicate (618-642)
newton/examples/basic/example_basic_urdf.py (1)
  • Example (36-150)
newton/tests/test_mujoco_solver.py (2)
newton/_src/sim/builder.py (5)
  • add_builder (912-1189)
  • finalize (4070-4428)
  • body_count (515-519)
  • ModelBuilder (67-4482)
  • add_joint_free (1621-1672)
newton/_src/solvers/mujoco/solver_mujoco.py (1)
  • SolverMuJoCo (1136-2835)
newton/examples/robot/example_robot_anymal_d.py (2)
newton/_src/sim/builder.py (1)
  • ModelBuilder (67-4482)
newton/examples/basic/example_basic_urdf.py (1)
  • Example (36-150)
🪛 GitHub Actions: GPU Benchmarks
asv/benchmarks/simulation/bench_quadruped_xpbd.py

[error] 31-31: TypeError: Example.init() got an unexpected keyword argument 'num_worlds' during benchmark setup.

asv/benchmarks/simulation/bench_mujoco.py

[error] 49-49: TypeError: Example.init() got an unexpected keyword argument 'num_worlds' during benchmark setup.

asv/benchmarks/simulation/bench_selection.py

[error] 32-32: TypeError: Example.init() got an unexpected keyword argument 'num_worlds' during benchmark setup.

🪛 GitHub Actions: GPU Unit Tests on AWS EC2
newton/examples/selection/example_selection_articulations.py

[error] 114-114: TypeError: ModelBuilder.replicate() got an unexpected keyword argument 'num_copies'. This caused the selection_articulations test to fail.

🪛 Ruff (0.14.0)
newton/examples/basic/example_basic_urdf.py

141-141: Unused lambda argument: qd

(ARG005)

newton/examples/selection/example_selection_cartpole.py

182-182: Unused lambda argument: qd

(ARG005)

newton/examples/__init__.py

220-220: Unused noqa directive (non-enabled: PLC0415)

Remove unused noqa directive

(RUF100)

newton/_src/utils/import_usd.py

250-250: Avoid specifying long messages outside the exception class

(TRY003)

newton/examples/robot/example_robot_allegro_hand.py

210-210: Unused lambda argument: qd

(ARG005)

asv/benchmarks/simulation/bench_mujoco.py

89-89: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)

newton/_src/sim/builder.py

961-961: Avoid specifying long messages outside the exception class

(TRY003)

asv/benchmarks/setup/bench_model.py

30-30: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)


38-38: Unused method argument: robot

(ARG002)


38-38: Unused method argument: num_worlds

(ARG002)


52-52: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)


72-72: Unused method argument: num_worlds

(ARG002)


76-76: Unused method argument: robot

(ARG002)


76-76: Unused method argument: num_worlds

(ARG002)


83-83: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)


118-118: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)


137-137: Unused method argument: num_worlds

(ARG002)


141-141: Unused method argument: robot

(ARG002)


141-141: Unused method argument: num_worlds

(ARG002)

newton/examples/robot/example_robot_cartpole.py

118-118: Unused lambda argument: qd

(ARG005)


163-163: Unused lambda argument: q

(ARG005)


170-170: Unused lambda argument: q

(ARG005)


177-177: Unused lambda argument: q

(ARG005)

newton/_src/solvers/mujoco/solver_mujoco.py

219-219: Unused function argument: bodies_per_world

(ARG001)

🔇 Additional comments (74)
newton/examples/ik/example_ik_benchmark.py (1)

115-115: LGTM! Attribute rename aligns with world terminology standardization.

The rename from num_rigid_contacts_per_env to num_rigid_contacts_per_world is consistent with the PR's objective to standardize on "world" nomenclature throughout the codebase.

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

19-21: LGTM! Terminology refactor is consistent and complete.

All references to "environment" and "env" have been correctly updated to "world" terminology throughout the file, including:

  • Documentation and command examples
  • Constructor signature and internal attributes
  • Grid calculations and loop ranges
  • Solver configuration parameters
  • CLI argument definitions and usage

The changes align with the PR objective to standardize on "world" terminology as per issue #935, and are consistent with the broader codebase refactor (e.g., builder.replicate() and other examples now use num_worlds).

Also applies to: 34-34, 41-41, 76-76, 79-79, 92-92, 158-158, 162-162

newton/examples/cloth/example_cloth_franka.py (3)

42-62: LGTM! Consistent renaming from environment to world terminology.

The parameter and variable renaming is consistent with the PR objectives. The kernel logic remains unchanged and correctly supports multiple worlds through thread indexing, even though it's currently launched with dim=1 for a single world.


177-179: LGTM! Instance variable renaming aligns with world terminology.

The renaming of bodies_per_env, dof_q_per_env, and dof_qd_per_env to their _per_world equivalents is consistent and correct. The initialization logic remains unchanged.


442-442: LGTM! Correct usage of renamed instance variable.

The update to use self.bodies_per_world is consistent with the renaming in the kernel signature and initialization.

newton/examples/sensors/example_sensor_contact.py (8)

20-21: LGTM: Docstring updated to reflect world terminology.

The documentation correctly uses "worlds" terminology, consistent with the PR's objective to standardize on this term.


80-83: LGTM: Kernel signature correctly updated.

The random_forces_kernel signature and seed calculation have been correctly updated to use num_worlds instead of num_envs, maintaining the same logic while aligning with the new terminology.


87-95: LGTM: Constructor parameter correctly renamed.

The constructor parameter and instance attribute have been consistently updated from num_envs to num_worlds, maintaining the same default value and semantics.


100-117: LGTM: Builder variable and usage correctly updated.

The builder variable has been consistently renamed from env_builder to world_builder throughout its usage, and the replicate() call correctly uses the updated terminology. Based on learnings, this aligns with the updated API signature in newton/_src/sim/builder.py.


139-139: LGTM: Solver parameter correctly updated.

The solver initialization correctly uses ncon_per_world instead of ncon_per_env, matching the updated API signature shown in newton/_src/solvers/mujoco/solver_mujoco.py.


178-208: LGTM: Mask initialization correctly updated.

Both the PyTorch and Warp code paths have been consistently updated to use num_worlds for array/tensor sizing and kernel launch dimensions, maintaining functional equivalence.


254-294: LGTM: Runtime logic correctly updated.

All references in the step() and reset() methods have been consistently updated to use self.num_worlds, maintaining correct tensor/array shapes and kernel launch dimensions.


345-351: LGTM: CLI argument correctly renamed.

The command-line argument has been updated from --num-envs to --num-worlds, and the constructor invocation correctly uses args.num_worlds. This aligns with the PR objectives where the maintainer confirmed that command-line flags should be renamed.

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

223-223: LGTM!

The docstring update from "environment" to "world" is consistent with the PR's terminology standardization objective.

newton/tests/test_anymal_reset.py (1)

34-34: LGTM!

The renaming from num_envs to num_worlds is consistent with the PR's terminology standardization.

newton/examples/example_recording.py (1)

49-49: LGTM!

The renaming from num_envs to num_worlds is applied consistently across storage (line 49), computation (line 72), and iteration (line 75).

Also applies to: 72-72, 75-75

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

1013-1013: LGTM!

The UI text updates from "Environment(s)" to "World(s)" across multiple panels (Model Information, Selection API, and Statistics) are consistent with the PR's terminology standardization.

Also applies to: 1413-1413, 1424-1424, 1460-1460

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

37-37: LGTM!

The renaming from num_envs to num_worlds in the constructor parameter (line 37), internal storage (line 45), and usage in replication (lines 74-78) is consistent with the PR's terminology standardization.

Also applies to: 45-45, 74-78


156-156: LGTM!

The CLI argument (line 156) and constructor call (line 162) have been updated to use --num-worlds and args.num_worlds, consistent with the PR's terminology standardization.

Also applies to: 162-162

newton/_src/sim/style3d/builder_style3d.py (1)

110-131: LGTM!

The renaming of parameters from update_num_env_count/environment to update_num_world_count/world (lines 114-115) and the corresponding docstring updates (lines 122-123) are consistent with the base class signature in newton/_src/sim/builder.py and align with the PR's terminology standardization.

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

22-22: LGTM!

The renaming from num_envs to num_worlds is applied consistently across the comment (line 22), constructor parameter (line 33), internal storage (line 40), replication call (line 65), CLI argument (line 135), and invocation (line 139).

Also applies to: 33-33, 40-40, 65-65, 135-135, 139-139

newton/examples/diffsim/example_diffsim_drone.py (1)

78-89: LGTM!

The comprehensive renaming from env_id to world_id and bodies_per_env to bodies_per_world across multiple kernel functions (sample_gaussian, replicate_states, drone_cost, collision_cost, enforce_control_limits, interpolate_control_linear) is consistent with the PR's terminology standardization. No behavioral changes are introduced.

Also applies to: 96-104, 118-163, 179-223, 231-233, 255-262

newton/tests/test_equality_constraints.py (1)

49-49: LGTM!

The parameter rename from ncon_per_env to ncon_per_world correctly aligns with the repository-wide terminology migration from environments to worlds.

newton/tests/test_ik_fk_kernels.py (1)

50-52: LGTM!

The comment and variable naming updates correctly reflect the world-centric indexing terminology. The residual writing logic remains unchanged and correct.

newton/tests/test_examples.py (1)

223-224: LGTM!

The parameter rename from num_envs to num_worlds correctly aligns with the repository-wide terminology migration and matches the expected interface of the example.

newton/tests/test_import_usd.py (1)

334-334: LGTM!

The parameter rename from cloned_env to cloned_world correctly aligns with the updated add_usd API signature and the repository-wide terminology migration.

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

275-275: LGTM!

The parameter rename from ncon_per_env to ncon_per_world correctly aligns with the updated SolverMuJoCo API signature and the repository-wide terminology migration.

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

180-181: LGTM!

The comments correctly reflect the world-centric assumptions about homogeneous worlds and global entities.


328-342: LGTM!

The internal variable renames from envs_shape_* to worlds_shape_* are consistent and correctly propagated throughout the shape trimming logic.


442-442: LGTM!

The slice calculation correctly uses the renamed worlds_shape_start variable.


507-507: LGTM!

The attribute trimming correctly uses the renamed self._worlds_shape_start and self._worlds_shape_end attributes.

newton/tests/test_kinematics.py (5)

30-32: LGTM!

The variable rename from num_envs to num_worlds and the loop comment update correctly align with the repository-wide terminology migration.


229-234: LGTM!

The docstring and comment updates correctly reflect the world-centric terminology for the Isaac Lab use case test.


256-256: LGTM!

The variable rename from env_indices_to_reset to world_indices_to_reset correctly aligns with the terminology migration.


270-271: LGTM!

The comment updates correctly reflect the world-centric terminology for resetting specific worlds.


281-288: LGTM!

The loop variable updates from env_idx to world_idx correctly align with the terminology migration and maintain test correctness.

asv/benchmarks/simulation/bench_selection.py (1)

32-34: No issues found—Example class correctly accepts num_worlds parameter.

The verification confirms that Example.__init__() in newton/examples/selection/example_selection_cartpole.py has the signature def __init__(self, viewer, num_worlds=16, verbose=True):, which accepts the num_worlds parameter passed in the benchmark code. The class has been updated correctly, and the code in bench_selection.py is compatible with it.

asv/benchmarks/setup/bench_model.py (1)

38-43: num_worlds propagation looks correct.

Builder and method signatures consistently use num_worlds. Bench behavior unchanged.

Also applies to: 66-67, 72-72, 76-77, 97-99, 104-109, 125-132, 137-140

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

22-22: World terminology and CLI flag changes LGTM.

Constructor, replicate(), and --num-worlds flag are consistent with the new API. Based on learnings.

Also applies to: 34-34, 42-42, 74-74, 141-141, 145-145

newton/examples/__init__.py (1)

77-80: Kernel thread-id rename to world_id LGTM.

Naming aligns with world semantics; no functional change.

Also applies to: 148-151

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

22-22: Terminology, solver arg, and CLI flag updates LGTM.

Consistent use of num_worlds and ncon_per_world; behavior unchanged.

Also applies to: 34-34, 41-41, 71-71, 82-82, 149-149, 153-153

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

21-22: World-based filtering integration LGTM.

Correctly switches to world indices with test_world_and_group_pair; launch signature updated accordingly.

Also applies to: 234-235, 269-277, 347-358

newton/tests/test_mujoco_solver.py (3)

156-169: World-based setup LGTM.

Multi-world replication via add_builder with per-world transforms is correct and aligns with num_worlds semantics.


1524-1560: Noncontiguous joint selection test is solid.

The world-separated selection and assertions correctly guard against the indexing bug.

Also applies to: 1562-1599


252-276: Mass mapping and bodies_per_world calculation are correct. bodies_per_world (4) matches the template builder’s body count per world.

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

32-41: World terminology + replication LGTM.

Constructor, replicate(), and CLI flag updates are consistent with the new API.

Also applies to: 57-59, 182-189

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

36-59: Kernel world indexing LGTM.

Per-world time, dof offset, and interpolation are correct; step wrapping avoids overflow.


97-99: World replication and per-world control LGTM.

Replicate(), articulation view assertions, and per-world launch dim are consistent.

Also applies to: 114-123, 151-158, 180-185


211-217: CLI rename LGTM.

--num-worlds flag matches repository-wide terminology.

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

689-723: World-based particle/shape filtering LGTM.

Renames and skip condition correctly preserve global (-1) semantics.

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

18-18: World-aware broadphase LGTM.

Passing shape_world and using test_world_and_group_pair before AABB checks is correct and efficient.

Also applies to: 85-106, 145-201

newton/tests/test_environment_group_collision.py (5)

40-56: World assignment via current_world: good

Explicitly pinning worlds (-1, 0, 1) is correct and matches builder semantics. No issues.


68-86: Collision pair expectations look correct

The assertions reflect world filtering: only (global, world0) and (global, world1) pairs. LGTM.

If shape indexing changes upstream, this test can become brittle. Please confirm shape indices [0,1,2] ordering remains stable in builder.add_shape_* for this setup.


123-164: add_builder(world=...) usage validated

Using explicit world indices and verifying shape_world/body_world mapping is correct and aligned with builder.add_builder behavior.


179-266: Mixed world/group filtering scenario: thorough and correct

Scenario and expected pairs align with test_world_and_group_pair semantics and the sorted-by-world broadphase path. LGTM.


324-361: Kernel-level test coverage for test_world_and_group_pair

Good direct coverage on CPU; covers global (-1), same/different worlds, and group semantics. LGTM.

newton/tests/test_joint_drive.py (3)

25-48: Parameter rename to world_id is consistent

Indexing into per-world arrays via world_id is correct. No issues here.


157-174: Expected velocity computation per-world is correct

Indexing with world_id across targets/ke/kd/q/qd/mass/inertia is consistent. LGTM.


175-179: Assertion indexing assumes contiguous per-world layout

Directly comparing vNew[i] to state_out.joint_qd[i] assumes single-DOF per world and contiguous layout; valid here. No change needed.

Confirm that the test remains valid if future robots contribute more than one DOF per world (would require adjusting offsets).

newton/tests/test_model.py (6)

197-221: World-based cloth replication: good

Using add_builder(update_num_world_count=True) without explicit world assigns consecutive world indices; open-edge counting logic is appropriate.


269-290: add_particles world grouping validated

Switching current_world and verifying particle_world after finalize is correct.


291-395: Comprehensive world grouping test

  • Proper use of current_world=-1 for globals.
  • add_builder with explicit world indices and auto-assignment.
  • Validates particle_world, body_world, shape_world, joint_group, articulation_group.
    LGTM.

396-435: num_worlds tracking behavior covered well

Global entities do not increment; auto and explicit world indices update counts correctly; update_num_world_count=False respected.


436-533: collapse_fixed_joints preserves world indices

Pre/post assertions for body_world and joint_group are consistent. Good coverage.


560-568: shape_world/body_world expectations after add_builder

Validation matches add_builder semantics. LGTM.

asv/benchmarks/simulation/bench_mujoco.py (3)

89-105: Param rename to num_worlds propagated correctly

_setup, builder caching, and track_simulate updated. LGTM.


125-128: Unit label updated to ms/world-step

Consistent with terminology change. LGTM.


45-58: ****

The review comment misidentifies a non-existent issue. The Example.__init__ in newton/examples/example_mujoco.py (line 267) does accept the num_worlds parameter with a default of 1. The benchmark code at lines 45-58 correctly passes this parameter. No CI error exists for this parameter, and the suggested compatibility shim is unnecessary—the code is already correct.

Likely an incorrect or invalid review comment.

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

76-87: Constructor param rename: consistent

num_worlds stored and used across replicate and kernel launch dims. LGTM.

newton/_src/solvers/mujoco/solver_mujoco.py (4)

964-1021: Indexing in update_joint_transforms_kernel: verify joint_in_world vs tid

Most arrays are indexed by global tid, but joint_child and joint_mjc_dof_start use joint_in_world. The latter is intentional for joint_mjc_dof_start (first-world mapping), but joint_child likely should use tid to refer to the actual Newton child index, then map to per-world index if needed. Verify correctness and adjust if required.


2613-2641: update_body_ kernels: world/index math looks correct*

Using // and % with bodies_per_world to derive worldid and index_in_world is correct. LGTM.


1417-1473: apply_mjc_control/apply_mjc_qfrc dims per world

Dim shapes and per-world counts computed from model/nworld look consistent. LGTM.


2417-2447: update_incoming_shape_xform_kernel launch grid

Launching over (num_worlds, ngeom) with shape_world and shape_range_len is consistent with per-world expansion logic. LGTM.

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

4451-4465: World-based collision filtering logic looks correct

Sorting by shape_world and early-breaking on world changes is efficient and consistent with global (-1) behavior.

Comment thread asv/benchmarks/simulation/bench_quadruped_xpbd.py Outdated
Comment thread newton/_src/utils/import_usd.py
Comment thread newton/examples/basic/example_basic_urdf.py
Comment thread newton/examples/selection/example_selection_articulations.py Outdated
Comment thread newton/examples/selection/example_selection_cartpole.py Outdated
Comment thread newton/examples/selection/example_selection_cartpole.py
Comment thread newton/examples/selection/example_selection_materials.py Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
newton/tests/test_joint_drive.py (1)

94-149: Broken multi-world assembly (self-add and variable shadowing).

Inside the loop a new ModelBuilder overwrites the outer one, then add_builder is called on itself; only the last world ends up in the model. Fix by using a separate scene aggregator.

Apply this diff to correct builder composition:

-        world_builder = newton.ModelBuilder(gravity=g, up_axis=world_up_axis)
-        for i in range(0, nb_worlds):
+        scene_builder = newton.ModelBuilder(gravity=g, up_axis=world_up_axis)
+        for i in range(0, nb_worlds):
             body_mass = body_masses[i]
@@
-            # Create a single body jointed to the world with a prismatic joint
-            # Make sure that we use the mass properties specified here by setting shape density to 0.0
-            world_builder = newton.ModelBuilder(gravity=g, up_axis=world_up_axis)
-            bodyIndex = world_builder.add_body(mass=body_mass, I_m=body_inertia, armature=0.0, com=body_com)
-            world_builder.add_shape_sphere(
+            # Create a single body jointed to the world with a prismatic/revolute joint
+            # Ensure mass properties are used by setting shape density to 0.0
+            world = newton.ModelBuilder(gravity=g, up_axis=world_up_axis)
+            bodyIndex = world.add_body(mass=body_mass, I_m=body_inertia, armature=0.0, com=body_com)
+            world.add_shape_sphere(
                 radius=1.0, body=bodyIndex, cfg=newton.ModelBuilder.ShapeConfig(density=0.0, has_shape_collision=False)
             )
             if is_prismatic:
-                world_builder.add_joint_prismatic(
+                world.add_joint_prismatic(
                     mode=newton.JointMode.TARGET_POSITION,
@@
                 )
             else:
-                world_builder.add_joint_revolute(
+                world.add_joint_revolute(
                     mode=newton.JointMode.TARGET_POSITION,
@@
                 )
 
-            world_builder.add_builder(world_builder)
+            scene_builder.add_builder(world)
 
             # Set the start pos and vel of the dof.
-            world_builder.joint_q[i] = joint_start_position
-            world_builder.joint_qd[i] = joint_start_velocity
+            scene_builder.joint_q[i] = joint_start_position
+            scene_builder.joint_qd[i] = joint_start_velocity
 
         # Create the MujocoSolver instance
-        model = world_builder.finalize()
+        model = scene_builder.finalize()
newton/_src/solvers/mujoco/solver_mujoco.py (1)

197-333: Address the unused parameter flagged by static analysis.

The bodies_per_world parameter on line 219 is not used within the convert_newton_contacts_to_mjwarp_kernel kernel body. While the comment on line 287-288 references a world-based indexing pattern, this kernel doesn't actually use bodies_per_world for any calculation.

Consider one of the following:

  1. Remove the parameter if it's truly unnecessary
  2. Keep it if it's required for API consistency with other kernels
  3. Use it if there's a missing calculation

Apply this diff if the parameter should be removed:

 @wp.kernel
 def convert_newton_contacts_to_mjwarp_kernel(
     body_q: wp.array(dtype=wp.transform),
     shape_body: wp.array(dtype=int),
     # Model:
     geom_condim: wp.array(dtype=int),
     geom_priority: wp.array(dtype=int),
     geom_solmix: wp.array2d(dtype=float),
     geom_solref: wp.array2d(dtype=wp.vec2),
     geom_solimp: wp.array2d(dtype=vec5),
     geom_friction: wp.array2d(dtype=wp.vec3),
     geom_margin: wp.array2d(dtype=float),
     geom_gap: wp.array2d(dtype=float),
     # Newton contacts
     rigid_contact_count: wp.array(dtype=wp.int32),
     rigid_contact_shape0: wp.array(dtype=wp.int32),
     rigid_contact_shape1: wp.array(dtype=wp.int32),
     rigid_contact_point0: wp.array(dtype=wp.vec3),
     rigid_contact_point1: wp.array(dtype=wp.vec3),
     rigid_contact_normal: wp.array(dtype=wp.vec3),
     rigid_contact_thickness0: wp.array(dtype=wp.float32),
     rigid_contact_thickness1: wp.array(dtype=wp.float32),
-    bodies_per_world: int,
     to_mjc_geom_index: wp.array(dtype=wp.vec2i),
     # Mujoco warp contacts
     ncon_out: wp.array(dtype=int),
     contact_dist_out: wp.array(dtype=float),

And update the call site on line 1382 accordingly.

🧹 Nitpick comments (11)
newton/examples/example_recording.py (1)

71-76: Prefer ModelBuilder.replicate for world instancing

Use builder.replicate(articulation_builder, num_worlds=self.num_worlds, spacing=(spacing, spacing, 0.0)) to simplify and ensure consistent world bookkeeping.

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

136-144: Address minor linter nit: unused lambda arg

qd is unused; rename to underscore to satisfy linters.

-            lambda q, qd: wp.abs(q[2] - 0.46) < 0.01,
+            lambda q, _qd: wp.abs(q[2] - 0.46) < 0.01,
newton/_src/sim/style3d/builder_style3d.py (1)

110-131: Docstring: clarify update_num_world_count semantics

Match ModelBuilder.add_builder semantics: when world is None, num_worlds increments; with explicit world, ensure num_worlds >= world+1; globals (-1) don’t affect it.

Implementation forwarding is correct.

newton/examples/__init__.py (2)

220-226: Remove unused noqa and keep deprecation warning.

Drop the # noqa: PLC0415 (RUF100). The deprecation warning text is fine.

-    import warnings  # noqa: PLC0415
+    import warnings

263-266: Nit: update comment to “worlds.”

Keep terminology consistent in comments.

-    # ensure the envs are not shifted below the ground plane
+    # ensure the worlds are not shifted below the ground plane
newton/examples/robot/example_robot_anymal_d.py (1)

75-83: Prefer Python math on host for grid sizing

Use math.sqrt/ceil on host code; Warp intrinsics are unnecessary here.

-        sqn = int(wp.ceil(wp.sqrt(float(self.num_worlds))))
+        import math
+        sqn = math.ceil(math.sqrt(self.num_worlds))
newton/tests/test_mujoco_solver.py (1)

230-241: Remove manual num_worlds override and warning; assert instead

add_builder(update_num_world_count=True) already maintains builder.num_worlds. The guard and print introduce noise in tests.

-        try:
-            if self.builder.num_worlds == 0 and num_worlds > 0:
-                self.builder.num_worlds = num_worlds
-            self.model = self.builder.finalize()
-            if self.model.num_worlds != num_worlds:
-                print(
-                    f"Warning: Model.num_worlds ({self.model.num_worlds}) does not match expected num_worlds ({num_worlds})."
-                )
+        try:
+            self.model = self.builder.finalize()
+            self.assertEqual(
+                self.model.num_worlds, num_worlds, "Model.num_worlds should match requested num_worlds"
+            )
newton/_src/geometry/broad_phase_nxn.py (1)

151-179: Launch signature/doc updated; consider class doc tweak

geom_shape_world wired through; docs explain -1/global vs per-world. Consider updating the class-level docstring (Lines 133-140) to also mention world filtering explicitly.

Also applies to: 198-201

newton/examples/sensors/example_sensor_contact.py (2)

176-183: Minor: prefer self.num_worlds for clarity

These spots still use the local num_worlds param. Using self.num_worlds everywhere avoids accidental divergence.

-            all_indices = torch.arange(num_worlds, dtype=torch.int32)
-            self.mask_0 = torch.zeros(num_worlds, dtype=bool)
+            all_indices = torch.arange(self.num_worlds, dtype=torch.int32)
+            self.mask_0 = torch.zeros(self.num_worlds, dtype=bool)
             self.mask_0[all_indices[::2]] = True
-            self.mask_1 = torch.zeros(num_worlds, dtype=bool)
+            self.mask_1 = torch.zeros(self.num_worlds, dtype=bool)
             self.mask_1[all_indices[1::2]] = True
-            self.mask_0 = wp.empty(num_worlds, dtype=bool)
-            self.mask_1 = wp.empty(num_worlds, dtype=bool)
-            wp.launch(init_masks, dim=num_worlds, inputs=[self.mask_0, self.mask_1])
+            self.mask_0 = wp.empty(self.num_worlds, dtype=bool)
+            self.mask_1 = wp.empty(self.num_worlds, dtype=bool)
+            wp.launch(init_masks, dim=self.num_worlds, inputs=[self.mask_0, self.mask_1])

Also applies to: 206-209


284-286: Align Torch and Warp reset semantics (spin vs. jump components)

Torch assigns spin to angular z (index 2) and jump to linear z (index 5). The Warp reset_kernel currently sets wz=jump and vz=spin. Align reset_kernel to match Torch for consistency.

Outside this hunk, in reset_kernel:

# Current:
ant_root_velocities[tid] = wp.spatial_vector(0.0, 0.0, jump_vel, 0.0, 0.0, spin_vel)
hum_root_velocities[tid] = wp.spatial_vector(0.0, 0.0, jump_vel, 0.0, 0.0, -spin_vel)

# Suggested:
ant_root_velocities[tid] = wp.spatial_vector(0.0, 0.0, spin_vel, 0.0, 0.0, jump_vel)
hum_root_velocities[tid] = wp.spatial_vector(0.0, 0.0, -spin_vel, 0.0, 0.0, jump_vel)
newton/_src/sim/builder.py (1)

910-958: Docstring is clear; one nit: emphasize that world=-1 never bumps num_worlds.

Small clarity tweak: right after listing args, explicitly note globals never affect num_worlds.

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1c30c1b and dbf0999.

📒 Files selected for processing (48)
  • asv/benchmarks/setup/bench_model.py (4 hunks)
  • asv/benchmarks/simulation/bench_mujoco.py (7 hunks)
  • asv/benchmarks/simulation/bench_quadruped_xpbd.py (1 hunks)
  • asv/benchmarks/simulation/bench_selection.py (1 hunks)
  • docs/guide/installation.rst (1 hunks)
  • newton/_src/geometry/broad_phase_common.py (1 hunks)
  • newton/_src/geometry/broad_phase_nxn.py (6 hunks)
  • newton/_src/geometry/broad_phase_sap.py (7 hunks)
  • newton/_src/geometry/kernels.py (2 hunks)
  • newton/_src/sim/builder.py (36 hunks)
  • newton/_src/sim/collide.py (1 hunks)
  • newton/_src/sim/model.py (6 hunks)
  • newton/_src/sim/style3d/builder_style3d.py (1 hunks)
  • newton/_src/solvers/mujoco/solver_mujoco.py (51 hunks)
  • newton/_src/utils/__init__.py (1 hunks)
  • newton/_src/utils/import_usd.py (5 hunks)
  • newton/_src/utils/selection.py (4 hunks)
  • newton/_src/viewer/viewer_gl.py (4 hunks)
  • newton/_src/viewer/viewer_null.py (1 hunks)
  • newton/examples/__init__.py (3 hunks)
  • newton/examples/basic/example_basic_urdf.py (4 hunks)
  • newton/examples/cloth/example_cloth_franka.py (3 hunks)
  • newton/examples/diffsim/example_diffsim_drone.py (8 hunks)
  • newton/examples/example_mujoco.py (16 hunks)
  • newton/examples/example_recording.py (2 hunks)
  • newton/examples/ik/example_ik_benchmark.py (1 hunks)
  • newton/examples/robot/example_robot_allegro_hand.py (6 hunks)
  • newton/examples/robot/example_robot_anymal_d.py (5 hunks)
  • newton/examples/robot/example_robot_cartpole.py (7 hunks)
  • newton/examples/robot/example_robot_g1.py (5 hunks)
  • newton/examples/robot/example_robot_h1.py (4 hunks)
  • newton/examples/robot/example_robot_humanoid.py (4 hunks)
  • newton/examples/robot/example_robot_policy.py (1 hunks)
  • newton/examples/robot/example_robot_ur10.py (7 hunks)
  • newton/examples/selection/example_selection_articulations.py (10 hunks)
  • newton/examples/selection/example_selection_cartpole.py (8 hunks)
  • newton/examples/selection/example_selection_materials.py (8 hunks)
  • newton/examples/sensors/example_sensor_contact.py (10 hunks)
  • newton/tests/test_anymal_reset.py (1 hunks)
  • newton/tests/test_environment_group_collision.py (10 hunks)
  • newton/tests/test_equality_constraints.py (1 hunks)
  • newton/tests/test_examples.py (1 hunks)
  • newton/tests/test_ik_fk_kernels.py (1 hunks)
  • newton/tests/test_import_usd.py (1 hunks)
  • newton/tests/test_joint_drive.py (15 hunks)
  • newton/tests/test_kinematics.py (5 hunks)
  • newton/tests/test_model.py (11 hunks)
  • newton/tests/test_mujoco_solver.py (22 hunks)
✅ Files skipped from review due to trivial changes (3)
  • newton/_src/utils/init.py
  • newton/tests/test_kinematics.py
  • newton/tests/test_anymal_reset.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • newton/_src/geometry/broad_phase_sap.py
  • newton/_src/geometry/kernels.py
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-08-25T21:41:45.795Z
Learnt from: dylanturpin
PR: newton-physics/newton#634
File: newton/tests/test_examples.py:329-333
Timestamp: 2025-08-25T21:41:45.795Z
Learning: Newton examples use a centralized CLI argument parsing system in newton/examples/__init__.py. The create_parser() function defines common arguments like --device, --viewer, --output-path, and --num-frames, while init(parser) creates viewers based on parsed arguments. Individual example scripts don't need to define these flags themselves - they inherit them from the centralized system via the example_map and runpy execution.

Applied to files:

  • newton/examples/selection/example_selection_materials.py
  • newton/examples/basic/example_basic_urdf.py
  • newton/examples/sensors/example_sensor_contact.py
  • newton/examples/selection/example_selection_articulations.py
  • newton/examples/robot/example_robot_h1.py
  • newton/examples/robot/example_robot_g1.py
  • newton/examples/robot/example_robot_humanoid.py
  • newton/examples/robot/example_robot_ur10.py
  • newton/examples/robot/example_robot_anymal_d.py
📚 Learning: 2025-08-12T17:51:37.474Z
Learnt from: nvlukasz
PR: newton-physics/newton#519
File: newton/geometry.py:16-22
Timestamp: 2025-08-12T17:51:37.474Z
Learning: In the Newton public API refactor, common geometry symbols like ParticleFlags and ShapeFlags are now exposed at the top level in newton/__init__.py rather than through newton.geometry. The newton/geometry.py module is intentionally focused only on broad-phase collision detection classes (BroadPhaseAllPairs, BroadPhaseExplicit, BroadPhaseSAP).

Applied to files:

  • newton/_src/geometry/broad_phase_nxn.py
📚 Learning: 2025-08-19T12:27:30.629Z
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.

Applied to files:

  • newton/examples/robot/example_robot_policy.py
  • newton/examples/example_mujoco.py
📚 Learning: 2025-08-19T12:27:30.629Z
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, Example.use_mujoco is a high-level attribute that controls whether to use MuJoCo solver vs other solvers (like XPBD), while SolverMuJoCo.use_mujoco_cpu controls the backend within MuJoCo (CPU vs Warp). These are separate concepts serving different purposes - the PR rename only applies to the solver parameter, not the Example class attributes.

Applied to files:

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

Applied to files:

  • newton/examples/robot/example_robot_policy.py
📚 Learning: 2025-08-18T15:56:26.587Z
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.587Z
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.

Applied to files:

  • newton/_src/solvers/mujoco/solver_mujoco.py
🧬 Code graph analysis (30)
newton/examples/selection/example_selection_materials.py (1)
newton/_src/sim/builder.py (2)
  • ModelBuilder (67-4482)
  • replicate (618-642)
asv/benchmarks/simulation/bench_quadruped_xpbd.py (2)
newton/examples/basic/example_basic_urdf.py (1)
  • Example (36-150)
newton/_src/viewer/viewer_null.py (1)
  • ViewerNull (23-160)
newton/examples/basic/example_basic_urdf.py (2)
newton/_src/sim/builder.py (3)
  • ModelBuilder (67-4482)
  • replicate (618-642)
  • body_count (515-519)
newton/examples/__init__.py (2)
  • test_body_state (38-112)
  • init (309-352)
newton/tests/test_joint_drive.py (1)
newton/_src/sim/builder.py (2)
  • ModelBuilder (67-4482)
  • add_builder (912-1189)
newton/examples/sensors/example_sensor_contact.py (3)
newton/examples/selection/example_selection_articulations.py (2)
  • random_forces_kernel (80-83)
  • Example (86-293)
newton/_src/sim/builder.py (2)
  • ModelBuilder (67-4482)
  • replicate (618-642)
newton/_src/solvers/mujoco/solver_mujoco.py (1)
  • SolverMuJoCo (1136-2835)
newton/examples/cloth/example_cloth_franka.py (2)
newton/_src/sim/builder.py (1)
  • body_count (515-519)
newton/_src/sim/state.py (3)
  • body_count (87-91)
  • joint_coord_count (101-105)
  • joint_dof_count (108-112)
newton/_src/utils/import_usd.py (1)
newton/_src/sim/builder.py (5)
  • shape_count (508-512)
  • body_count (515-519)
  • articulation_count (571-575)
  • collapse_fixed_joints (2055-2349)
  • add_builder (912-1189)
newton/examples/example_recording.py (1)
newton/_src/sim/builder.py (1)
  • ModelBuilder (67-4482)
newton/tests/test_model.py (1)
newton/_src/sim/builder.py (13)
  • ModelBuilder (67-4482)
  • add_builder (912-1189)
  • add_particles (3042-3072)
  • finalize (4070-4428)
  • add_body (1191-1253)
  • add_shape_box (2567-2604)
  • add_particle (3006-3040)
  • add_shape_sphere (2534-2565)
  • particle_count (529-533)
  • body_count (515-519)
  • shape_count (508-512)
  • joint_count (522-526)
  • articulation_count (571-575)
newton/_src/utils/selection.py (1)
newton/_src/sim/builder.py (1)
  • shape_count (508-512)
newton/_src/geometry/broad_phase_nxn.py (1)
newton/_src/geometry/broad_phase_common.py (3)
  • check_aabb_overlap (22-38)
  • test_world_and_group_pair (91-117)
  • write_pair (54-65)
newton/examples/selection/example_selection_cartpole.py (1)
newton/_src/sim/builder.py (2)
  • ModelBuilder (67-4482)
  • replicate (618-642)
newton/examples/__init__.py (1)
newton/_src/core/types.py (2)
  • Axis (63-160)
  • from_any (97-117)
newton/tests/test_environment_group_collision.py (2)
newton/_src/geometry/broad_phase_common.py (1)
  • test_world_and_group_pair (91-117)
newton/_src/sim/builder.py (3)
  • ModelBuilder (67-4482)
  • add_builder (912-1189)
  • finalize (4070-4428)
newton/examples/selection/example_selection_articulations.py (2)
newton/examples/sensors/example_sensor_contact.py (1)
  • random_forces_kernel (80-83)
newton/_src/sim/builder.py (2)
  • ModelBuilder (67-4482)
  • replicate (618-642)
newton/examples/robot/example_robot_h1.py (1)
newton/_src/sim/builder.py (1)
  • replicate (618-642)
asv/benchmarks/setup/bench_model.py (2)
asv/benchmarks/simulation/bench_mujoco.py (2)
  • setup (45-78)
  • setup (97-101)
newton/examples/example_mujoco.py (2)
  • Example (261-447)
  • create_model_builder (374-409)
newton/_src/sim/style3d/builder_style3d.py (1)
newton/_src/sim/builder.py (1)
  • add_builder (912-1189)
newton/examples/robot/example_robot_allegro_hand.py (2)
newton/_src/sim/builder.py (3)
  • replicate (618-642)
  • finalize (4070-4428)
  • copy (192-193)
newton/_src/solvers/mujoco/solver_mujoco.py (1)
  • SolverMuJoCo (1136-2835)
asv/benchmarks/simulation/bench_mujoco.py (4)
asv/benchmarks/setup/bench_model.py (3)
  • setup (38-39)
  • setup (60-69)
  • setup (125-134)
asv/benchmarks/simulation/bench_quadruped_xpbd.py (1)
  • setup (29-31)
asv/benchmarks/simulation/bench_selection.py (1)
  • setup (30-34)
newton/examples/example_mujoco.py (2)
  • Example (261-447)
  • create_model_builder (374-409)
newton/examples/robot/example_robot_cartpole.py (2)
newton/_src/sim/builder.py (2)
  • replicate (618-642)
  • body_count (515-519)
newton/examples/__init__.py (3)
  • test_body_state (38-112)
  • create_parser (269-306)
  • init (309-352)
newton/_src/sim/builder.py (2)
newton/_src/core/types.py (2)
  • Axis (63-160)
  • from_any (97-117)
newton/_src/geometry/flags.py (1)
  • ShapeFlags (30-42)
newton/examples/robot/example_robot_g1.py (2)
newton/_src/sim/builder.py (1)
  • replicate (618-642)
newton/examples/__init__.py (1)
  • init (309-352)
newton/examples/robot/example_robot_humanoid.py (2)
newton/_src/sim/builder.py (1)
  • replicate (618-642)
newton/examples/basic/example_basic_urdf.py (1)
  • Example (36-150)
newton/examples/robot/example_robot_ur10.py (3)
newton/examples/basic/example_basic_urdf.py (2)
  • step (120-126)
  • Example (36-150)
newton/_src/sim/builder.py (1)
  • replicate (618-642)
newton/examples/__init__.py (1)
  • init (309-352)
asv/benchmarks/simulation/bench_selection.py (2)
newton/examples/basic/example_basic_urdf.py (1)
  • Example (36-150)
newton/_src/viewer/viewer_null.py (1)
  • ViewerNull (23-160)
newton/_src/solvers/mujoco/solver_mujoco.py (6)
newton/_src/sim/joints.py (2)
  • JointMode (81-95)
  • JointType (20-44)
newton/_src/sim/builder.py (3)
  • body_count (515-519)
  • joint_count (522-526)
  • shape_count (508-512)
newton/_src/sim/state.py (2)
  • body_count (87-91)
  • joint_dof_count (108-112)
newton/_src/sim/model.py (2)
  • control (476-509)
  • state (440-474)
newton/_src/geometry/broad_phase_nxn.py (2)
  • launch (145-201)
  • launch (218-264)
newton/_src/geometry/broad_phase_sap.py (1)
  • launch (341-481)
newton/tests/test_mujoco_solver.py (1)
newton/_src/sim/builder.py (4)
  • add_builder (912-1189)
  • finalize (4070-4428)
  • body_count (515-519)
  • ModelBuilder (67-4482)
newton/examples/example_mujoco.py (2)
newton/examples/basic/example_basic_urdf.py (1)
  • Example (36-150)
newton/_src/sim/builder.py (2)
  • ModelBuilder (67-4482)
  • replicate (618-642)
newton/examples/robot/example_robot_anymal_d.py (1)
newton/_src/sim/builder.py (1)
  • ModelBuilder (67-4482)
🪛 GitHub Actions: GPU Benchmarks
asv/benchmarks/simulation/bench_quadruped_xpbd.py

[error] 31-31: TypeError: Example.init() got an unexpected keyword argument 'num_worlds'

asv/benchmarks/simulation/bench_mujoco.py

[error] 49-49: TypeError: Example.init() got an unexpected keyword argument 'num_worlds'

asv/benchmarks/simulation/bench_selection.py

[error] 32-32: TypeError: Example.init() got an unexpected keyword argument 'num_worlds'


[error] 32-32: TypeError: Example.init() got an unexpected keyword argument 'num_worlds'


[error] 32-32: asv benchmark failed (exit status 1)

🪛 GitHub Actions: GPU Unit Tests on AWS EC2
newton/examples/selection/example_selection_articulations.py

[error] 114-114: ModelBuilder.replicate() got an unexpected keyword argument 'num_copies'.

🪛 Ruff (0.14.0)
newton/examples/basic/example_basic_urdf.py

141-141: Unused lambda argument: qd

(ARG005)

newton/_src/utils/import_usd.py

250-250: Avoid specifying long messages outside the exception class

(TRY003)

newton/examples/selection/example_selection_cartpole.py

182-182: Unused lambda argument: qd

(ARG005)

newton/examples/__init__.py

220-220: Unused noqa directive (non-enabled: PLC0415)

Remove unused noqa directive

(RUF100)

asv/benchmarks/setup/bench_model.py

30-30: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)


38-38: Unused method argument: robot

(ARG002)


38-38: Unused method argument: num_worlds

(ARG002)


52-52: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)


72-72: Unused method argument: num_worlds

(ARG002)


76-76: Unused method argument: robot

(ARG002)


76-76: Unused method argument: num_worlds

(ARG002)


83-83: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)


118-118: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)


137-137: Unused method argument: num_worlds

(ARG002)


141-141: Unused method argument: robot

(ARG002)


141-141: Unused method argument: num_worlds

(ARG002)

newton/examples/robot/example_robot_allegro_hand.py

210-210: Unused lambda argument: qd

(ARG005)

asv/benchmarks/simulation/bench_mujoco.py

89-89: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)

newton/examples/robot/example_robot_cartpole.py

118-118: Unused lambda argument: qd

(ARG005)


163-163: Unused lambda argument: q

(ARG005)


170-170: Unused lambda argument: q

(ARG005)


177-177: Unused lambda argument: q

(ARG005)

newton/_src/sim/builder.py

961-961: Avoid specifying long messages outside the exception class

(TRY003)

newton/_src/solvers/mujoco/solver_mujoco.py

219-219: Unused function argument: bodies_per_world

(ARG001)

🔇 Additional comments (82)
newton/examples/ik/example_ik_benchmark.py (1)

115-115: LGTM! Attribute rename aligns with terminology standardization.

The rename from num_rigid_contacts_per_env to num_rigid_contacts_per_world is consistent with the PR's objective to standardize on "world" terminology throughout the codebase. The functionality remains unchanged.

newton/examples/diffsim/example_diffsim_drone.py (1)

78-262: LGTM! Consistent terminology migration.

All variable renames from env_* to world_* across the six Warp kernels are mechanically correct and consistent. The indexing logic, array accesses, and atomic operations remain equivalent. This aligns perfectly with the PR's objective to standardize on "world" terminology.

newton/examples/robot/example_robot_humanoid.py (5)

22-22: LGTM!

The command line example documentation is correctly updated to reflect the new --num-worlds flag, matching the implementation at line 135.


33-40: LGTM!

The constructor parameter and instance variable are correctly renamed from num_envs to num_worlds, maintaining the default value of 4. The changes are consistent with the updated API pattern shown in the relevant code snippets.


65-65: LGTM!

The replicate() call correctly uses self.num_worlds, which is consistent with both the instance variable rename and the updated builder.replicate() API signature that now expects a num_worlds parameter.


135-135: LGTM!

The command line argument is correctly renamed from --num-envs to --num-worlds, with appropriate help text update. The default value is preserved, and the change aligns with the PR objectives for standardizing on "world" terminology.


139-139: LGTM!

The example instantiation correctly uses args.num_worlds, completing the end-to-end consistency: CLI argument → parsed args → constructor parameter → instance variable → replicate() call. All renaming changes are properly aligned throughout the file.

asv/benchmarks/setup/bench_model.py (1)

28-144: LGTM! Consistent rename across all benchmark classes.

The systematic rename from num_envs to num_worlds is applied consistently across all four benchmark classes (KpiInitializeModel, KpiInitializeSolver, FastInitializeModel, FastInitializeSolver), including:

  • param_names lists
  • Method signatures (setup, time_*, teardown, peakmem_*)
  • All Example.create_model_builder() invocations

The changes align with the updated API in newton/examples/example_mujoco.py.

Note: The static analysis warnings about unused parameters are false positives—benchmark framework methods require specific signatures even when parameters aren't actively used within the method body.

asv/benchmarks/simulation/bench_selection.py (1)

32-34: No issues found—dependency is properly updated.

The Example class in newton/examples/selection/example_selection_cartpole.py has already been updated to accept the num_worlds parameter (line 58). The benchmark file is correctly calling the updated API.

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

34-42: LGTM! Constructor parameter and attribute renamed correctly.

The parameter name change from num_envs to num_worlds and the corresponding attribute assignment are consistent with the PR's goal of standardizing terminology across the codebase.


74-74: LGTM! replicate() call updated correctly.

The change from self.num_envs to self.num_worlds aligns with the updated builder.replicate() API signature shown in the relevant code snippets.


141-145: LGTM! CLI argument and usage updated correctly.

The command-line argument has been renamed from --num-envs to --num-worlds, with the help text and usage in the Example instantiation updated accordingly. This aligns with the PR objectives and the feedback from eric-heiden confirming that CLI flags should use the new --num-worlds naming.

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

271-277: ncon_per_world rename looks correct

Constructor args align with SolverMuJoCo world terminology. No issues seen.

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

28-30: Doc wording change is fine

Terminology update only; no behavior impact.

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

36-46: Constructor rename to num_worlds is consistent

Signature and internal usage align with new world terminology.


77-79: World replication usage looks good

Positional call matches replicate(builder, num_worlds, ...).

newton/_src/utils/selection.py (3)

327-340: World-based shape trimming variables are consistent

Renamed worlds_shape_* counters are correctly computed and stored on self.


441-443: Slice offset uses world-based start correctly

Indexing for shape-frequency slice subtracts worlds_shape_start as intended.


503-509: Attribute trimming aligns with new fields

Using self._worlds_shape_start/_end to trim shape-frequency arrays is correct.

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

37-58: API rename to cloned_world looks good; verify callers updated

Signature/doc switched to cloned_world; world replication uses add_builder correctly. Please ensure all call sites migrated from cloned_env.

Run to find stale references:

newton/tests/test_import_usd.py (1)

334-335: Rename looks correct (cloned_world).

Matches the API rename; no issues spotted.

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

47-54: World terminology and indexing are correct.

bodies_per_world and world_id usage are sound for single-world launch (dim=1).

Also applies to: 61-62

docs/guide/installation.rst (1)

109-109: LGTM!

The documentation correctly reflects the updated CLI flag from --num-envs to --num-worlds, consistent with the world-centric terminology shift.

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

225-255: LGTM!

The soft-contact generation kernel inputs correctly use the updated world-based attributes (particle_world and shape_world), aligning with the terminology shift from environment groups to worlds.

newton/tests/test_ik_fk_kernels.py (1)

49-52: LGTM!

The terminology update from "env" to "world" is consistent with the PR's objectives. The logic remains unchanged—writing a zero residual for the single world used in this test.

newton/tests/test_equality_constraints.py (1)

41-50: LGTM!

The SolverMuJoCo constructor parameter correctly updated from ncon_per_env to ncon_per_world, reflecting the world-centric API changes in the solver.

newton/tests/test_examples.py (1)

218-226: LGTM!

The test options correctly updated from num_envs to num_worlds, aligning with the CLI flag rename. The test will now pass --num-worlds flags to the example scripts.

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

1009-1018: LGTM!

The Model Information section correctly displays "Worlds:" using the updated num_worlds attribute, consistent with the world-centric terminology shift.


1410-1425: LGTM!

The batch selection UI correctly updated to "Batch/World Selection" and displays "World {idx}" in the descriptor text, maintaining consistency with the world-centric terminology.


1453-1470: LGTM!

The statistics section correctly uses "Statistics for World {idx}:" when displaying per-world attribute statistics, aligning with the world-centric terminology.

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

90-117: LGTM!

The function rename from test_environment_and_group_pair to test_world_and_group_pair correctly reflects the world-centric terminology shift. The logic remains sound:

  • Entities from different worlds (excluding global entities with index -1) do not collide
  • Collision group filtering is applied within the same world or for global entities
  • The comprehensive docstring clearly explains the collision rules
newton/examples/robot/example_robot_cartpole.py (3)

33-58: LGTM!

The constructor correctly updated to use num_worlds parameter, consistent with the world-centric terminology. The parameter is properly used in the builder.replicate() call.


112-179: LGTM!

The test method correctly updated to use world-centric terminology throughout:

  • Variable names (world0_cart_vel, etc.) clearly indicate world-based references
  • Test descriptions accurately reflect multi-world testing
  • Body indexing logic correctly uses per-world calculations

Note: The Ruff warnings about unused lambda arguments are false positives. The lambda signatures are constrained by the test framework's test_fn callback interface, which requires both q and qd parameters even when only one is used.


22-22: LGTM!

The CLI interface correctly updated to use --num-worlds:

  • Example command comment reflects the new flag (line 22)
  • Argument parser uses --num-worlds with appropriate help text (line 184)
  • The argument is properly passed to the Example constructor (line 187)

Also applies to: 183-189

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

19-22: Docs/CLI rename looks good

Switched narrative and command to “worlds” and --num-worlds. Consistent with PR goals.


34-41: Constructor param rename aligned

num_envs→num_worlds is applied consistently (param and attribute). No functional issues.


92-92: ncon_per_world: correct rename

Parameter matches SolverMuJoCo signature and intent.


156-163: CLI flag rename is consistent; verify example runner help text

--num-worlds added and plumbed to Example(...). Ensure docs that reference example CLI flags are updated.

newton/tests/test_mujoco_solver.py (4)

156-163: Setup docstring/num_worlds refactor looks good

Switched to worlds; using local num_worlds=2 is fine.


256-276: World-indexed loops/readbacks are consistent; confirm even partitioning

bodies_per_world and dofs_per_world use integer division. Ensure test builders produce counts divisible by model.num_worlds; otherwise, indexing will misalign.

Also applies to: 290-300, 306-336


524-569: Per-world DOF/property checks are correct under separate_worlds

Access patterns use first-world mappings for MuJoCo indices and per-world slices. Looks correct given solver mappings.

Also applies to: 570-581, 583-595, 602-619, 630-650, 653-663, 666-676


1524-1560: Noncontiguous joint selection test: good coverage

Switch to separate_worlds=True and asserting selected joints/joint types catches the prior bug. Nicely done.

newton/examples/robot/example_robot_g1.py (5)

22-22: Command line doc updated

--num-worlds shown; consistent with runner usage.


34-42: Constructor/attribute rename

num_worlds applied consistently. No issues.


71-72: Use of builder.replicate is preferred

Good adoption of replicate(num_worlds=...). Matches builder API.


82-83: ncon_per_world rename is correct

Aligned with SolverMuJoCo signature.


149-154: CLI flag and wiring OK

Adds --num-worlds and passes to Example(viewer, args.num_worlds).

newton/_src/geometry/broad_phase_nxn.py (2)

18-18: Import world/group test: correct dependency

Switch to test_world_and_group_pair is aligned with world semantics.


85-106: World gating before AABB overlap: correct logic

shape_world + test_world_and_group_pair correctly short-circuits inter-world collisions while keeping globals (-1) active.

newton/examples/sensors/example_sensor_contact.py (3)

86-96: World-centric builder changes look good

Constructor param, world_builder naming, and replicate(..., self.num_worlds, ...) all consistent.

Also applies to: 100-118, 116-118


139-139: ncon_per_world: correct rename

SolverMuJoCo param matches new API.


345-352: CLI flag rename wired correctly

Adds --num-worlds and passes through to Example. OK.

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

211-217: Centralize --num-worlds in the shared examples parser.

To avoid drift across examples, add --num-worlds to newton.examples.create_parser() and consume it here, instead of redefining per example. This mirrors other shared flags.

Use this to find remaining env-based flags/usages to update:

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

4329-4333: Align finalize with *_world naming; keep *_group as aliases for compatibility.

m.joint_group/m.articulation_group contradict the rename. Assign joint_world/articulation_world and set legacy aliases to ease migration.

-            m.joint_group = wp.array(self.joint_world, dtype=wp.int32)
+            # primary world-index fields
+            m.joint_world = wp.array(self.joint_world, dtype=wp.int32)
+            # temporary alias for backward compatibility
+            m.joint_group = m.joint_world
@@
-            m.articulation_group = wp.array(self.articulation_world, dtype=wp.int32)
+            # primary world-index fields
+            m.articulation_world = wp.array(self.articulation_world, dtype=wp.int32)
+            # temporary alias for backward compatibility
+            m.articulation_group = m.articulation_world

Find remaining usages to update:

Also applies to: 4373-4378

newton/examples/example_mujoco.py (7)

25-25: LGTM! Comment updated consistently.

The terminology has been updated from "environment" to "world" to match the broader refactoring effort.


261-289: LGTM! Constructor signature updated correctly.

The parameter renaming from env/num_envs to world/num_worlds is consistent and propagated to the instance variable on line 289.


302-311: LGTM! Static method calls updated correctly.

Both create_model_builder and create_solver calls use the new world and num_worlds parameters appropriately.


373-409: LGTM! Function signature and implementation updated consistently.

The create_model_builder method signature and body correctly use:

  • world parameter instead of env
  • num_worlds instead of num_envs
  • World-based configuration lookup on lines 395, 400, 403

411-447: LGTM! Function signature updated correctly.

The create_solver method signature and body correctly use:

  • world parameter instead of env
  • ncon_per_world instead of nconmax
  • World-based configuration lookup on lines 434-435

450-525: LGTM! CLI arguments updated consistently.

The command-line interface correctly reflects the new terminology:

  • --scene replaces the former environment/scene distinction (line 455)
  • --num-worlds replaces --num-envs (line 464)
  • Help text updated on lines 495-496
  • Argument passing updated on lines 510, 512

527-571: LGTM! Output labels updated consistently.

The simulation configuration output correctly uses world terminology:

  • "World Count" on line 533
  • "Max Constraints / world" on line 563
  • "Max Contacts / world" on line 564

The division logic on lines 554-556 is safe due to the conditional check on line 555.

newton/_src/solvers/mujoco/solver_mujoco.py (21)

335-409: LGTM! Kernel signature updated correctly.

The convert_mj_coords_to_warp_kernel correctly uses joints_per_world for world-based indexing:

  • Lines 354-355: Compute world-specific joint indices
  • The pattern joints_per_world * worldid + jntid is correct

411-480: LGTM! Kernel signature updated correctly.

The convert_warp_coords_to_mj_kernel correctly uses joints_per_world for world-based indexing on lines 430-431, maintaining symmetry with the corresponding conversion kernel.


527-543: LGTM! Kernel updated with world-based indexing.

The apply_mjc_control_kernel correctly computes and uses world-specific axis indices:

  • Line 540: worldid * axes_per_world + axisid pattern is correct

545-562: LGTM! Kernel updated with world-based body indexing.

The apply_mjc_body_f_kernel correctly computes world-specific body indices on line 558 using the pattern worldid * bodies_per_world + bodyid.


564-609: LGTM! Kernel updated with world-based indexing.

The apply_mjc_qfrc_kernel correctly uses world-based indexing throughout:

  • Lines 583-584: Joint DOF start indices
  • Line 587: Body transform index
  • The indexing patterns are consistent and correct

784-804: LGTM! Kernel updated with world-based body indexing.

The convert_body_xforms_to_warp_kernel correctly computes the world-specific body index on line 794 using bodies_per_world * worldid + bodyid.


806-832: LGTM! Kernel updated with world-based indexing.

The update_body_mass_ipos_kernel correctly:

  • Line 818: Computes worldid using tid // bodies_per_world
  • Line 819: Computes index_in_world using tid % bodies_per_world
  • This pattern is standard and correct for world-based indexing

834-882: LGTM! Kernel updated with world-based indexing.

The update_body_inertia_kernel uses the same correct world-based indexing pattern on lines 844-846 as the mass/ipos kernel.


895-940: LGTM! Kernel updated with world-based axis indexing.

The update_axis_properties_kernel correctly:

  • Line 910: Computes worldid from tid // axes_per_world
  • Line 911: Computes axis_in_world from tid % axes_per_world
  • Line 913: Uses axis_in_world to look up actuator index
  • This ensures actuators are correctly mapped per world

942-961: LGTM! Kernel updated with world-based DOF indexing.

The update_dof_properties_kernel correctly uses world-based indexing for DOF properties:

  • Lines 953-954: Standard world/dof computation
  • Lines 957, 960: Correctly index into 2D arrays with [worldid, dof_in_world]

963-1021: LGTM! Kernel updated with world-based joint indexing.

The update_joint_transforms_kernel correctly:

  • Lines 982-983: Computes world and joint indices
  • Line 995: Uses joint_in_world for MuJoCo DOF lookup
  • Lines 1016-1020: Uses joint_in_world for body mapping
  • All indexing patterns are correct

1023-1058: LGTM! Kernel updated with world-based shape indexing.

The update_incoming_shape_xform_kernel correctly handles world-based shape mapping:

  • Lines 1040-1046: Correctly distinguishes between global shapes (shape_world < 0) and world-specific shapes
  • Line 1044: Correctly computes global shape index as world_idx * shape_range_len + template_shape_idx
  • This logic properly supports both shared static shapes and per-world instantiated shapes

1208-1324: LGTM! Constructor signature updated correctly.

The __init__ method correctly:

  • Line 1214: Renamed separate_worlds parameter
  • Line 1216: Renamed ncon_per_world parameter
  • Lines 1240-1242: Updated docstrings to reflect world terminology
  • Lines 1304-1311: Updated initialization logic with new parameter names

1357-1401: LGTM! Contact conversion updated with world-based indexing.

The convert_contacts_to_mjwarp method correctly computes bodies_per_world on line 1358 and passes it to the kernel on line 1382.


1421-1495: LGTM! Control application updated with world-based dimensions.

The apply_mjc_control method correctly computes per-world dimensions:

  • Lines 1436-1438: Computes axes_per_world, joints_per_world, bodies_per_world
  • Lines 1442, 1456, 1478: Correctly uses these dimensions in kernel launches

1496-1534: LGTM! Data update method uses world-based joint dimension.

The update_mjc_data method correctly computes and uses joints_per_world on lines 1514, 1517, 1521.


1535-1617: LGTM! Newton state update uses world-based dimensions.

The update_newton_state method correctly:

  • Line 1559: Computes joints_per_world
  • Lines 1562, 1566: Uses it in kernel launch
  • Lines 1604, 1607, 1612: Computes and uses bodies_per_world

1757-2503: LGTM! Complex world selection logic is correct.

The convert_to_mjc method correctly handles world separation:

  • Line 1762: Parameter renamed to separate_worlds
  • Line 1766: Parameter renamed to ncon_per_world
  • Lines 2009-2027: Correctly determines which shapes/bodies/joints belong to the first world:
    • Uses shape_world >= 0 to find non-global shapes
    • Selects the minimum world index as first_group
    • Includes both first-world entities AND global entities (world < 0)
  • Lines 2458-2461: Correctly sets nworld based on separate_worlds
  • Lines 2474-2486: Correctly handles ncon_per_world with appropriate warnings

The logic properly supports both separate world mode and combined world mode.


2611-2683: LGTM! Update methods use world-based dimensions correctly.

The update_model_inertial_properties and update_joint_dof_properties methods:

  • Lines 2615, 2623, 2636: Use bodies_per_world correctly
  • Lines 2648, 2661, 2678: Use dofs_per_world correctly

All world-based calculations are correct.


2684-2751: LGTM! Joint and geom property updates use world-based dimensions.

The update_joint_properties and update_geom_properties methods:

  • Lines 2689, 2705: Use joints_per_world correctly
  • Line 2724: Uses num_worlds correctly

2771-2796: LGTM! Docstring updated correctly.

The render_mujoco_viewer method docstring correctly notes:

  • Line 2784: "single world" terminology
  • Line 2786: separate_worlds parameter reference

Comment thread asv/benchmarks/simulation/bench_mujoco.py Outdated
Comment thread asv/benchmarks/simulation/bench_quadruped_xpbd.py Outdated
Comment thread newton/examples/robot/example_robot_allegro_hand.py
Comment thread newton/examples/selection/example_selection_articulations.py
Comment thread newton/examples/selection/example_selection_cartpole.py
Comment thread newton/examples/selection/example_selection_materials.py
Comment thread newton/examples/sensors/example_sensor_contact.py
Comment thread newton/tests/test_model.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
newton/examples/selection/example_selection_cartpole.py (1)

155-159: Fix kernel launch dim: pass row count, not 2D shape

apply_forces_kernel uses a 1D tid, but you launch with a 2D dim (joint_f.shape). Launch with the number of rows instead.

Apply:

-            wp.launch(
-                apply_forces_kernel,
-                dim=joint_f.shape,
-                inputs=[joint_q, joint_f],
-            )
+            wp.launch(
+                apply_forces_kernel,
+                dim=joint_f.shape[0],  # one thread per world (row)
+                inputs=[joint_q, joint_f],
+            )
♻️ Duplicate comments (1)
newton/examples/selection/example_selection_cartpole.py (1)

182-182: Silence unused lambda arg

Rename qd to _qd to satisfy the linter.

-            "cart is at ground level and has correct orientation",
-            lambda q, qd: q[2] == 0.0 and newton.utils.vec_allclose(q.q, wp.quat_identity()),
+            "cart is at ground level and has correct orientation",
+            lambda q, _qd: q[2] == 0.0 and newton.utils.vec_allclose(q.q, wp.quat_identity()),
🧹 Nitpick comments (4)
newton/examples/selection/example_selection_materials.py (2)

63-66: Kernel flag: prefer runtime parameter over module constant

Using RANDOMIZE_PER_WORLD inside the kernel bakes the choice at compile time. If you ever toggle it at runtime, you’ll need a recompile. Consider passing a bool flag (or seed stride) into the kernel instead.


267-271: Optional: define --num-worlds in centralized examples parser

To avoid duplication across examples, consider moving this flag into newton/examples/init.py’s create_parser().

Based on learnings

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

224-228: Optional: centralize --num-worlds flag

Consider moving this CLI flag to the shared examples parser to keep flags consistent across examples.

Based on learnings

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

298-299: Optional: centralize --num-worlds flag

Consider adding this flag in the shared examples parser to avoid per-file duplication.

Based on learnings

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dbf0999 and e99fe0d.

📒 Files selected for processing (3)
  • newton/examples/selection/example_selection_articulations.py (10 hunks)
  • newton/examples/selection/example_selection_cartpole.py (8 hunks)
  • newton/examples/selection/example_selection_materials.py (8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-25T21:41:45.795Z
Learnt from: dylanturpin
PR: newton-physics/newton#634
File: newton/tests/test_examples.py:329-333
Timestamp: 2025-08-25T21:41:45.795Z
Learning: Newton examples use a centralized CLI argument parsing system in newton/examples/__init__.py. The create_parser() function defines common arguments like --device, --viewer, --output-path, and --num-frames, while init(parser) creates viewers based on parsed arguments. Individual example scripts don't need to define these flags themselves - they inherit them from the centralized system via the example_map and runpy execution.

Applied to files:

  • newton/examples/selection/example_selection_materials.py
  • newton/examples/selection/example_selection_articulations.py
🧬 Code graph analysis (3)
newton/examples/selection/example_selection_materials.py (1)
newton/_src/sim/builder.py (2)
  • ModelBuilder (67-4482)
  • replicate (618-642)
newton/examples/selection/example_selection_articulations.py (3)
newton/examples/sensors/example_sensor_contact.py (2)
  • random_forces_kernel (80-83)
  • Example (86-320)
newton/_src/sim/builder.py (2)
  • ModelBuilder (67-4482)
  • replicate (618-642)
newton/_src/solvers/mujoco/solver_mujoco.py (1)
  • SolverMuJoCo (1136-2835)
newton/examples/selection/example_selection_cartpole.py (2)
newton/_src/sim/builder.py (3)
  • ModelBuilder (67-4482)
  • replicate (618-642)
  • body_count (515-519)
newton/examples/__init__.py (2)
  • get_asset (34-35)
  • test_body_state (38-112)
🪛 Ruff (0.14.0)
newton/examples/selection/example_selection_cartpole.py

182-182: Unused lambda argument: qd

(ARG005)

⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Run GPU Unit Tests on AWS EC2 (Pull Request)
  • GitHub Check: Run GPU Benchmarks (Pull Request)
  • GitHub Check: run-newton-tests / newton-unittests (ubuntu-24.04-arm)
🔇 Additional comments (6)
newton/examples/selection/example_selection_materials.py (1)

92-92: Good: replicate() uses updated kwarg num_worlds

Matches ModelBuilder.replicate signature; avoids the previous num_copies mismatch.

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

76-76: Good: replicate() uses num_worlds kwarg

Conforms to the updated ModelBuilder.replicate API.


1-239: Repository-wide sanity check confirms no deprecated API references remain.

Verification complete: No occurrences of num_copies=, --num-envs, or num_envs found across the codebase. The file correctly uses the new API (--num-worlds and num_worlds=).

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

80-83: Good: 2D kernel uses (i, j) and num_worlds for seeding

Signature and indexing align with the 2D launch.


114-114: Good: replicate() now uses num_worlds kwarg

Matches the updated builder API.


119-119: Good: per-world solver contacts

Using ncon_per_world aligns with world-based contact budget configuration.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
newton/examples/selection/example_selection_materials.py (1)

275-279: Fix Torch device handling (torch.set_device is invalid; ensure correct tensor device)

torch.set_device does not exist. Use torch.set_default_device if available, else torch.cuda.set_device for CUDA devices. Also allocate material_mu on the same device.

Apply:

 if USE_TORCH:
     import torch
-    torch.set_device(args.device)
+    if hasattr(torch, "set_default_device"):
+        torch.set_default_device(args.device)
+    elif str(args.device).startswith("cuda"):
+        torch.cuda.set_device(args.device)

And in reset():

-            if RANDOMIZE_PER_WORLD:
-                material_mu = torch.rand(self.ants.count).unsqueeze(1).repeat(1, self.ants.shape_count)
-            else:
-                material_mu = torch.rand((self.ants.count, self.ants.shape_count))
+            if RANDOMIZE_PER_WORLD:
+                material_mu = torch.rand(
+                    self.ants.count,
+                    device=self.default_ant_root_transforms.device,
+                ).unsqueeze(1).repeat(1, self.ants.shape_count)
+            else:
+                material_mu = torch.rand(
+                    (self.ants.count, self.ants.shape_count),
+                    device=self.default_ant_root_transforms.device,
+                )

Also applies to: 209-213

🧹 Nitpick comments (3)
newton/examples/selection/example_selection_cartpole.py (1)

182-182: Silence unused lambda parameter warning.

The qd parameter in the lambda is unused. Rename it to _qd or _ to preserve the signature while silencing the linter warning.

Apply this diff:

-            lambda q, qd: q[2] == 0.0 and newton.utils.vec_allclose(q.q, wp.quat_identity()),
+            lambda q, _: q[2] == 0.0 and newton.utils.vec_allclose(q.q, wp.quat_identity()),
newton/examples/selection/example_selection_materials.py (2)

59-69: Avoid Python globals in Warp kernels; pass a flag parameter

Referencing RANDOMIZE_PER_WORLD inside the kernel couples compile-time codegen to a module global. Pass it as a parameter (or declare as wp.constant) for clarity and flexibility.

Apply:

 @wp.kernel
-def reset_materials_kernel(mu: wp.array2d(dtype=float), seed: int, shape_count: int):
+def reset_materials_kernel(
+    mu: wp.array2d(dtype=float),
+    seed: int,
+    shape_count: int,
+    randomize_per_world: int,  # bool-like
+):
     i, j = wp.tid()

-    if RANDOMIZE_PER_WORLD:
+    if randomize_per_world:
         rng = wp.rand_init(seed, i)
     else:
         rng = wp.rand_init(seed, i * shape_count + j)

     mu[i, j] = wp.randf(rng)

And pass the flag on launch:

 wp.launch(
     reset_materials_kernel,
     dim=material_mu.shape,
-    inputs=[material_mu, self.reset_count, self.ants.shape_count],
+    inputs=[material_mu, self.reset_count, self.ants.shape_count, int(RANDOMIZE_PER_WORLD)],
 )

Also applies to: 220-226


43-46: Optional: expose RANDOMIZE_PER_WORLD as a CLI flag

Let users toggle per-world vs per-shape randomization at runtime (and plumb it into the kernel arg above).

Example:

 parser = newton.examples.create_parser()
+parser.add_argument("--randomize-per-world", action="store_true", default=RANDOMIZE_PER_WORLD,
+                    help="If set, all shapes in a world share the same random material value.")

 # later:
-example = Example(viewer, num_worlds=args.num_worlds)
+RANDOMIZE_PER_WORLD = args.randomize_per_world
+example = Example(viewer, num_worlds=args.num_worlds)
📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dbf0999 and d82c712.

📒 Files selected for processing (4)
  • newton/examples/selection/example_selection_articulations.py (10 hunks)
  • newton/examples/selection/example_selection_cartpole.py (8 hunks)
  • newton/examples/selection/example_selection_materials.py (8 hunks)
  • newton/tests/test_model.py (11 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-25T21:41:45.795Z
Learnt from: dylanturpin
PR: newton-physics/newton#634
File: newton/tests/test_examples.py:329-333
Timestamp: 2025-08-25T21:41:45.795Z
Learning: Newton examples use a centralized CLI argument parsing system in newton/examples/__init__.py. The create_parser() function defines common arguments like --device, --viewer, --output-path, and --num-frames, while init(parser) creates viewers based on parsed arguments. Individual example scripts don't need to define these flags themselves - they inherit them from the centralized system via the example_map and runpy execution.

Applied to files:

  • newton/examples/selection/example_selection_materials.py
  • newton/examples/selection/example_selection_articulations.py
🧬 Code graph analysis (4)
newton/examples/selection/example_selection_materials.py (1)
newton/_src/sim/builder.py (2)
  • ModelBuilder (67-4482)
  • replicate (618-642)
newton/tests/test_model.py (1)
newton/_src/sim/builder.py (12)
  • ModelBuilder (67-4482)
  • add_builder (912-1189)
  • add_particles (3042-3072)
  • finalize (4070-4428)
  • add_body (1191-1253)
  • add_particle (3006-3040)
  • add_articulation (644-656)
  • particle_count (529-533)
  • body_count (515-519)
  • shape_count (508-512)
  • joint_count (522-526)
  • articulation_count (571-575)
newton/examples/selection/example_selection_articulations.py (2)
newton/examples/sensors/example_sensor_contact.py (2)
  • random_forces_kernel (80-83)
  • init_masks (50-54)
newton/_src/sim/builder.py (2)
  • ModelBuilder (67-4482)
  • replicate (618-642)
newton/examples/selection/example_selection_cartpole.py (2)
newton/_src/sim/builder.py (2)
  • ModelBuilder (67-4482)
  • replicate (618-642)
newton/examples/__init__.py (3)
  • get_asset (34-35)
  • test_body_state (38-112)
  • create_parser (269-306)
🪛 Ruff (0.14.0)
newton/examples/selection/example_selection_cartpole.py

182-182: Unused lambda argument: qd

(ARG005)

⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Run GPU Benchmarks (Pull Request)
  • GitHub Check: Run GPU Unit Tests on AWS EC2 (Pull Request)
  • GitHub Check: run-newton-tests / newton-unittests (ubuntu-24.04-arm)
🔇 Additional comments (24)
newton/examples/selection/example_selection_articulations.py (9)

21-22: LGTM! Documentation updated to world terminology.

The comments correctly reflect the rename from "environments" to "worlds" in line with the PR objectives.


80-83: LGTM! Kernel signature updated consistently.

The random_forces_kernel parameter has been correctly renamed from num_envs to num_worlds, and the implementation uses the updated parameter name throughout.


87-87: LGTM! Constructor signature updated to world terminology.

The constructor parameter and instance variable have been correctly renamed from num_envs to num_worlds, maintaining consistency with the PR objectives.

Also applies to: 95-95


97-109: LGTM! Model builder variable renamed consistently.

The local variable has been correctly renamed from env to world, and all references have been updated accordingly.


114-114: LGTM! Previous issue resolved.

The replicate() call now correctly uses the num_worlds keyword argument, matching the expected signature. The previous critical issue with num_copies has been successfully addressed.


119-119: LGTM! Solver configuration uses world terminology.

The SolverMuJoCo instantiation correctly uses the ncon_per_world parameter, consistent with the world-based terminology.


158-162: LGTM! Mask initialization updated consistently.

Both the Torch and Warp code paths correctly use num_worlds for mask allocation and kernel launch dimensions, maintaining consistency throughout the implementation.

Also applies to: 186-188


230-230: LGTM! Runtime operations use world-based dimensions.

All kernel launches, tensor allocations, and indexing operations correctly use self.num_worlds, ensuring consistent world-based dimensions throughout the simulation logic.

Also applies to: 233-233, 254-255, 264-264


298-298: LGTM! CLI and instantiation updated to world terminology.

The command-line argument has been correctly renamed to --num-worlds, and the example instantiation uses the updated parameter name, completing the end-to-end terminology update.

Also applies to: 307-307

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

58-66: LGTM: Constructor parameter renamed consistently.

The rename from num_envs to num_worlds is consistent with the PR objective and aligns with the updated replicate() API.


68-76: LGTM: World builder and replication updated correctly.

The variable rename and replicate() call now use the correct num_worlds parameter, matching the updated API signature.


100-106: LGTM: Randomization kernel launch updated.

The dimension parameter correctly uses num_worlds for both Torch and Warp code paths.


177-183: LGTM: Test indices updated for world-based partitioning.

The body index calculations correctly use self.num_worlds for world-based partitioning.


224-237: LGTM: CLI interface updated to world terminology.

The command-line argument and help text correctly reflect the world-centric API changes.

newton/tests/test_model.py (8)

197-227: LGTM: Open edges test updated with world terminology.

Variable names and parameter usage correctly reflect the world-centric rename.


269-289: LGTM: Particle grouping test updated.

The test correctly uses current_world and particle_world attributes, consistent with the renamed API.


291-347: LGTM: World grouping test structure updated.

The test setup correctly uses world-centric terminology for builders, variables, and helper functions.


360-394: LGTM: Group assignment assertions are correct.

The test logic correctly validates world assignments for all entity types, using the current API attribute names.


396-434: LGTM: World count tracking test updated.

The test correctly validates num_worlds tracking behavior with various world parameter values and update_num_world_count settings.


436-518: LGTM: Collapse fixed joints test preserves world assignments.

The test correctly validates that collapse_fixed_joints preserves world group assignments for bodies and joints, using the current API attribute names.


534-581: LGTM: Add builder test validates world assignments.

The test correctly validates that add_builder assigns proper world indices to all entity types.


355-359: Verify naming consistency for joint and articulation world indices
The ModelBuilder.finalize method exposes particle_world, body_world, and shape_world on Model but uses joint_group and articulation_group for the joint_world and articulation_world arrays. Confirm whether these should be renamed to joint_world and articulation_world (and update tests/docs) or if the current mixed naming is intentional.

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

72-92: World rename + replicate usage look correct

Constructor signature, num_worlds plumbing, and replicate call align with the updated API.


267-271: No stale or duplicate CLI flags found. There are no --num-envs flags, and each example defines --num-worlds independently.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
asv/benchmarks/simulation/bench_mujoco.py (1)

88-88: Consider adding ClassVar annotation.

The static analyzer suggests annotating param_names with typing.ClassVar to clarify it's a class-level attribute shared across instances.

Apply this diff:

+from typing import ClassVar
+
 class _KpiBenchmark:
     """Utility base class for KPI benchmarks."""
 
-    param_names = ["num_worlds"]
+    param_names: ClassVar = ["num_worlds"]
asv/benchmarks/simulation/bench_quadruped_xpbd.py (1)

31-31: Change resolves pipeline failure correctly.

The switch to positional arguments successfully addresses the TypeError from the pipeline. The Example constructor now receives the viewer and world count correctly.

For improved consistency and readability, consider using keyword arguments to match the style used in the ViewerNull construction:

-        self.example = Example(newton.viewer.ViewerNull(num_frames=self.num_frames), 200)
+        self.example = Example(viewer=newton.viewer.ViewerNull(num_frames=self.num_frames), num_worlds=200)

This makes the parameter names explicit and aligns with Python best practices for constructor calls.

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d82c712 and be1b435.

📒 Files selected for processing (2)
  • asv/benchmarks/simulation/bench_mujoco.py (7 hunks)
  • asv/benchmarks/simulation/bench_quadruped_xpbd.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
asv/benchmarks/simulation/bench_quadruped_xpbd.py (2)
newton/examples/basic/example_basic_urdf.py (1)
  • Example (36-150)
newton/_src/viewer/viewer_null.py (1)
  • ViewerNull (23-160)
asv/benchmarks/simulation/bench_mujoco.py (2)
asv/benchmarks/setup/bench_model.py (3)
  • setup (38-39)
  • setup (60-69)
  • setup (125-134)
newton/examples/example_mujoco.py (2)
  • Example (261-447)
  • create_model_builder (374-409)
🪛 Ruff (0.14.0)
asv/benchmarks/simulation/bench_mujoco.py

88-88: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)

⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Run GPU Unit Tests on AWS EC2 (Pull Request)
  • GitHub Check: Run GPU Benchmarks (Pull Request)
  • GitHub Check: run-newton-tests / newton-unittests (windows-latest)
  • GitHub Check: run-newton-tests / newton-unittests (macos-latest)
  • GitHub Check: run-newton-tests / newton-unittests (ubuntu-latest)
  • GitHub Check: run-newton-tests / newton-unittests (ubuntu-24.04-arm)
🔇 Additional comments (4)
asv/benchmarks/simulation/bench_mujoco.py (4)

49-57: Past review concern resolved.

The previous review flagged passing num_worlds to Example.__init__ when a pre-built builder is supplied. The current code correctly omits num_worlds here, since the builder is already replicated with the desired world count.


42-47: LGTM!

The attribute renaming from num_envs to num_worlds is consistent with the PR objective, and the builder is correctly created with the updated parameter.


88-123: LGTM!

The parameter name, method signatures, builder dictionary keys, calculation logic, and unit string are all correctly updated to use "world" terminology consistently.


128-195: LGTM!

All concrete benchmark classes consistently use the num_worlds attribute, aligning with the codebase-wide terminology standardization.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
newton/tests/test_model.py (2)

355-359: Use consistent variable naming with *_worlds suffix.

For clarity and consistency with the renamed API fields, the local variable names should use the _worlds suffix throughout. Currently, particle_groups, body_groups, and articulation_groups use the old _groups suffix, while joint_worlds and shape_worlds correctly use _worlds.

Apply this diff:

-        particle_groups = model.particle_world.numpy() if model.particle_world is not None else []
-        body_groups = model.body_world.numpy() if model.body_world is not None else []
+        particle_worlds = model.particle_world.numpy() if model.particle_world is not None else []
+        body_worlds = model.body_world.numpy() if model.body_world is not None else []
         shape_worlds = model.shape_world.numpy() if model.shape_world is not None else []
         joint_worlds = model.joint_world.numpy() if model.joint_world is not None else []
-        articulation_groups = model.articulation_world.numpy() if model.articulation_world is not None else []
+        articulation_worlds = model.articulation_world.numpy() if model.articulation_world is not None else []

Then update the usages throughout the test method (e.g., particle_groups[0:3]particle_worlds[0:3]).


519-520: Use consistent variable naming with *_worlds suffix.

The variable body_groups should be renamed to body_worlds for consistency with the API terminology and the adjacent joint_worlds variable.

Apply this diff:

-        body_groups = model.body_world.numpy()
+        body_worlds = model.body_world.numpy()
         joint_worlds = model.joint_world.numpy()

Then update usages: body_groups[0]body_worlds[0], etc.

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

2284-2284: Use a more descriptive variable name.

The variable name original_ is too generic. Since it stores the original joint world assignments, it should be named original_joint_world for clarity and consistency with original_body_group at line 2220.

Apply this diff:

-        original_ = self.joint_world[:] if self.joint_world else []
+        original_joint_world = self.joint_world[:] if self.joint_world else []

Then update usages at lines 2325-2326:

-            if original_ and joint["original_id"] < len(original_):
-                self.joint_world.append(original_[joint["original_id"]])
+            if original_joint_world and joint["original_id"] < len(original_joint_world):
+                self.joint_world.append(original_joint_world[joint["original_id"]])
📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between be1b435 and 417fe2a.

📒 Files selected for processing (4)
  • newton/_src/sim/builder.py (36 hunks)
  • newton/_src/solvers/mujoco/solver_mujoco.py (51 hunks)
  • newton/tests/test_model.py (10 hunks)
  • newton/tests/test_mujoco_solver.py (21 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-18T15:56:26.587Z
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.587Z
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.

Applied to files:

  • newton/_src/solvers/mujoco/solver_mujoco.py
🧬 Code graph analysis (4)
newton/tests/test_model.py (1)
newton/_src/sim/builder.py (7)
  • ModelBuilder (67-4482)
  • add_builder (912-1189)
  • add_particles (3042-3072)
  • finalize (4070-4428)
  • add_body (1191-1253)
  • add_particle (3006-3040)
  • add_articulation (644-656)
newton/_src/sim/builder.py (2)
newton/_src/core/types.py (2)
  • Axis (63-160)
  • from_any (97-117)
newton/_src/geometry/flags.py (1)
  • ShapeFlags (30-42)
newton/_src/solvers/mujoco/solver_mujoco.py (2)
newton/_src/geometry/broad_phase_nxn.py (2)
  • launch (145-201)
  • launch (218-264)
newton/_src/geometry/broad_phase_sap.py (1)
  • launch (341-481)
newton/tests/test_mujoco_solver.py (2)
newton/_src/sim/builder.py (4)
  • add_builder (912-1189)
  • finalize (4070-4428)
  • body_count (515-519)
  • ModelBuilder (67-4482)
newton/_src/solvers/mujoco/solver_mujoco.py (1)
  • SolverMuJoCo (1136-2835)
🪛 Ruff (0.14.0)
newton/_src/sim/builder.py

961-961: Avoid specifying long messages outside the exception class

(TRY003)

newton/_src/solvers/mujoco/solver_mujoco.py

219-219: Unused function argument: bodies_per_world

(ARG001)

⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Run GPU Unit Tests on AWS EC2 (Pull Request)
  • GitHub Check: Run GPU Benchmarks (Pull Request)
  • GitHub Check: run-newton-tests / newton-unittests (ubuntu-24.04-arm)
🔇 Additional comments (7)
newton/_src/solvers/mujoco/solver_mujoco.py (3)

1214-1216: LGTM! Public API parameter names updated consistently.

The parameter renames from separate_envs_to_worldsseparate_worlds and ncon_per_envncon_per_world align well with the PR's objective to standardize on "world" terminology. The docstrings correctly reflect these changes.


1273-1291: LGTM! Field docstrings updated to world terminology.

The docstrings for instance fields like shape_incoming_xform, joint_mjc_dof_start, to_mjc_body_index, and selected_* arrays have been updated to reference "world" instead of "environment". The explanations remain clear and accurate.


2783-2786: LGTM! Viewer documentation updated consistently.

The docstring for render_mujoco_viewer correctly reflects the world-based terminology, noting that the MuJoCo viewer only supports rendering models with a single world (or with separate_worlds=False). This is clear and accurate.

newton/tests/test_mujoco_solver.py (4)

156-242: LGTM! Test setup correctly uses world-based terminology.

The setUp method has been properly updated to use world-based terminology:

  • num_worlds = 2 instead of num_envs
  • add_builder(..., update_num_world_count=True) with world transforms
  • Verification checks model.num_worlds instead of model.num_envs

The test structure and logic remain sound while adopting the new naming convention.


264-276: LGTM! Test correctly uses world-based indexing.

The test properly calculates bodies_per_world using model.num_worlds and iterates over world_idx instead of env_idx. The indexing calculation newton_idx = world_idx * bodies_per_world + body_idx is correct for world-based partitioning.


513-537: LGTM! Test uses world-specific value patterns.

The test correctly uses different values for each world to catch indexing bugs:

  • Pattern: base_value + axis_idx * increment + world_idx * offset
  • Iterates properly over world_idx and calculates global indices
  • Comments clearly explain the value patterns per world

This is a thorough approach to validate world-based indexing.


1524-1554: Test correctly validates world-based joint selection.

The test properly verifies that separate_worlds=True selects joints belonging to world 0 (noncontiguous indices [0, 2]). The setup with explicit world= parameters and the verification of selected_joints demonstrates proper world-based filtering.

Comment thread newton/_src/solvers/mujoco/solver_mujoco.py
@nvtw nvtw requested a review from eric-heiden October 16, 2025 14:02
@nvtw

nvtw commented Oct 16, 2025

Copy link
Copy Markdown
Member Author

@eric-heiden do you know why the benchmarks can't work with the new num_worlds parameter in the init method of the Example class? Do they use an older version of newton on the benchmark machines that sill expects num_envs?

@eric-heiden

Copy link
Copy Markdown
Member

@eric-heiden do you know why the benchmarks can't work with the new num_worlds parameter in the init method of the Example class? Do they use an older version of newton on the benchmark machines that sill expects num_envs?

Yes the change of the parameter name causes the benchmark to fail when running the baseline on the current main branch, see https://github.com/newton-physics/newton/actions/runs/18563224514/job/52917236411?pr=945:

                 File "/var/snap/amazon-ssm-agent/12451/newton/asv/benchmarks/simulation/bench_selection.py", line 32, in setup
                   self.example = Example(
                                  ^^^^^^^^
               TypeError: Example.__init__() got an unexpected keyword argument 'num_worlds'

@eric-heiden eric-heiden left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I think this looks good now, let's merge it.

@eric-heiden eric-heiden merged commit 95c5d21 into newton-physics:main Oct 16, 2025
17 of 18 checks passed
eric-heiden pushed a commit to eric-heiden/newton that referenced this pull request Jan 28, 2026
mmacklin pushed a commit to mmacklin/newton that referenced this pull request Apr 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants