newton/examples/example_mujoco.py (1)

16-23: Update module documentation to reflect all supported robot types.

The module documentation mentions only "G1" and "humanoid articulation from xml", but the script actually supports multiple robot types (humanoid, g1, cartpole, quadruped) and both XML and URDF formats.

-# Example Sim G1
+# Example MuJoCo-style Robot Simulations
 #
-# Shows how to set up a simulation of a rigid-body humanoid articulation
-# from a xml using the newton.ModelBuilder().
+# Shows how to set up simulations of various rigid-body articulated robots
+# (humanoid, g1, cartpole, quadruped) from XML or URDF files using the newton.ModelBuilder().
 # Note this example does not include a trained policy.

Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro

newton/examples/example_mujoco.py (3)

182-183: Raise an exception instead of just printing an error.

When an unrecognized robot name is provided, the method should raise an exception to prevent undefined behavior in the rest of the code.

-            print("Name of the provided robot not recognized: ", robot)
-            return
+            raise ValueError(f"Name of the provided robot not recognized: {robot}")

---

329-329: Fix AttributeError: use_mujoco attribute doesn't exist.

The Example class doesn't define self.use_mujoco, so referencing example.use_mujoco causes an AttributeError. The solver is created with use_mujoco=False but this parameter isn't stored as an accessible attribute.

Store the use_mujoco value in the Example class or access it from the solver if available:

-        show_mujoco_viewer = args.show_mujoco_viewer and example.use_mujoco
+        # Since solver is created with use_mujoco=False, we can check the args directly
+        show_mujoco_viewer = args.show_mujoco_viewer and False

Or store the value during initialization:

# In __init__ method, add:
+        self.use_mujoco = False  # or pass as parameter

---

344-345: Fix AttributeError: solver.use_mujoco attribute access.

The code references example.solver.use_mujoco but based on the MuJoCoSolver initialization on line 199, the use_mujoco parameter is passed to the constructor but may not be stored as an accessible attribute.

Since the solver is created with use_mujoco=False, you can either:

1. Check if the MuJoCoSolver exposes this attribute
2. Use the known value directly since it's hardcoded to False

-                if not example.solver.use_mujoco:
+                # Since use_mujoco is hardcoded to False in solver creation
+                if True:  # Always true since use_mujoco=False

newton/examples/example_mujoco.py (1)

17-22: Update the header comment to reflect the generic nature of the script.

The comment describes this as "Example Sim G1" but the script actually supports multiple robot types (humanoid, g1, cartpole, quadruped). Update the comment to reflect its generic, unified nature.

-# Example Sim G1
+# Example MuJoCo Unified Script
#
-# Shows how to set up a simulation of a rigid-body humanoid articulation
-# from a xml using the newton.ModelBuilder().
-# Note this example does not include a trained policy.
+# Shows how to set up simulations for various articulated robots
+# (humanoid, g1, cartpole, quadruped) using newton.ModelBuilder().
+# Supports multiple robots to avoid code duplication and enable scaling.

Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro

newton/examples/example_mujoco.py (1)

360-361: Fix AttributeError: solver.use_mujoco attribute access.

The code references example.solver.use_mujoco but based on the MuJoCoSolver initialization on lines 199-208, the use_mujoco parameter is passed to the constructor, not stored as an attribute.

Either store the use_mujoco value in the Example class during initialization or check if the MuJoCoSolver exposes this attribute.

-                if not example.solver.use_mujoco:
+                if not example.use_mujoco:

newton/examples/example_mujoco.py (2)

104-104: Remove unused noqa directive.

The noqa directive is unnecessary since the import error handling makes the PLC0415 rule irrelevant here.

-                import tqdm  # noqa: PLC0415
+                import tqdm

---

36-229: Consider refactoring the constructor for better maintainability.

The __init__ method is quite long (~194 lines) and handles multiple robot configurations. Consider extracting robot-specific setup into separate methods (e.g., _setup_humanoid(), _setup_g1(), etc.) to improve readability and maintainability.

This would also make it easier to add new robot types in the future and reduce the complexity of the main constructor.

Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro

newton/examples/example_mujoco.py (1)

367-367: Fix AttributeError: solver.use_mujoco attribute access.

Based on the MuJoCoSolver initialization, the use_mujoco parameter is passed to the constructor but not stored as an attribute. Use the value stored in the Example class instead.

-                if not example.solver.use_mujoco:
+                if not example.use_mujoco:

newton/examples/example_mujoco.py (2)

111-111: Remove unused noqa directive.

The noqa: PLC0415 directive is not needed here as PLC0415 is not enabled in your linting configuration.

-                import tqdm  # noqa: PLC0415
+                import tqdm

---

93-188: Consider refactoring repetitive parameter initialization.

The parameter initialization code is repeated for each robot type with different default values. This could be simplified using a configuration dictionary to improve maintainability and reduce code duplication.

Define a configuration dictionary at the class level:

ROBOT_CONFIGS = {
    "humanoid": {
        "solver": "newton",
        "integrator": "euler", 
        "solver_iteration": 10,
        "ls_iteration": 5,
        "njmax": 100,
        "nconmax": 50,
    },
    "g1": {
        "solver": "newton",
        "integrator": "euler",
        "solver_iteration": 5,
        "ls_iteration": 5,
        "njmax": 300,
        "nconmax": 150,
    },
    # ... other robots
}

Then replace the repetitive code with:

-            # Setting mujoco_warp parameters
-            solver = solver if solver is not None else "newton"
-            integrator = integrator if integrator is not None else "euler"
-            solver_iteration = solver_iteration if solver_iteration is not None else 10
-            ls_iteration = ls_iteration if ls_iteration is not None else 5
-            njmax = njmax if njmax is not None else 100
-            nconmax = nconmax if nconmax is not None else 50
+            # Setting mujoco_warp parameters
+            config = self.ROBOT_CONFIGS[robot]
+            solver = solver if solver is not None else config["solver"]
+            integrator = integrator if integrator is not None else config["integrator"]
+            solver_iteration = solver_iteration if solver_iteration is not None else config["solver_iteration"]
+            ls_iteration = ls_iteration if ls_iteration is not None else config["ls_iteration"]
+            njmax = njmax if njmax is not None else config["njmax"]
+            nconmax = nconmax if nconmax is not None else config["nconmax"]

Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro

newton/examples/example_mujoco.py (1)

97-97: Remove unused noqa directive.

The noqa directive is not needed since PLC0415 is not enabled in the linting configuration.

-        import tqdm  # noqa: PLC0415
+        import tqdm

Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro

Learnt from: Kenny-Vilella
PR: newton-physics/newton#398
File: newton/examples/example_mujoco.py:352-352
Timestamp: 2025-07-14T03:57:29.659Z
Learning: The use_mujoco option in newton/examples/example_mujoco.py is currently unsupported and causes crashes. The code automatically disables this option with a warning message when users attempt to enable it. This is intentionally kept as a placeholder for future implementation.

Learnt from: Kenny-Vilella
PR: newton-physics/newton#398
File: newton/examples/example_mujoco.py:352-352
Timestamp: 2025-07-14T03:57:29.659Z
Learning: The use_mujoco option in newton/examples/example_mujoco.py is currently unsupported and causes crashes. The code automatically disables this option with a warning message when users attempt to enable it. This is intentionally kept as a placeholder for future implementation.

newton/examples/example_mujoco.py (3)

96-96: Remove unused noqa directive.

The noqa directive for PLC0415 is unused and should be removed to keep the code clean.

-        import tqdm  # noqa: PLC0415
+        import tqdm

---

41-66: Consider increasing default solver iteration counts.

The current default solver iteration counts (e.g., 100 for humanoid) are relatively low. Based on the past review discussions, these could be set higher by default since conditional termination is available, and users ideally shouldn't need to worry about these details.

Consider increasing the default iteration counts across all robot configurations to ensure better convergence while maintaining the conditional termination safety net.

---

185-186: Consider making iteration defaults more explicit.

The solver and line search iteration defaults are set inline. Consider moving these to the robot configuration constants or making them named constants for better maintainability.

+DEFAULT_SOLVER_ITERATION = 100
+DEFAULT_LS_ITERATION = 50
+
# In __init__ method:
-        solver_iteration = solver_iteration if solver_iteration is not None else 100
-        ls_iteration = ls_iteration if ls_iteration is not None else 50
+        solver_iteration = solver_iteration if solver_iteration is not None else DEFAULT_SOLVER_ITERATION
+        ls_iteration = ls_iteration if ls_iteration is not None else DEFAULT_LS_ITERATION

Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro

Learnt from: Kenny-Vilella
PR: newton-physics/newton#398
File: newton/examples/example_mujoco.py:352-352
Timestamp: 2025-07-14T03:57:29.659Z
Learning: The use_mujoco option in newton/examples/example_mujoco.py is currently unsupported and causes crashes. The code automatically disables this option with a warning message when users attempt to enable it. This is intentionally kept as a placeholder for future implementation.

Learnt from: Kenny-Vilella
PR: newton-physics/newton#398
File: newton/examples/example_mujoco.py:352-352
Timestamp: 2025-07-14T03:57:29.659Z
Learning: The use_mujoco option in newton/examples/example_mujoco.py is currently unsupported and causes crashes. The code automatically disables this option with a warning message when users attempt to enable it. This is intentionally kept as a placeholder for future implementation.