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

340-347: Clarify None-handling semantics for articulation_mask

The kernel signature doesn’t mark articulation_mask as optional, yet the implementation branches on its presence. Add a short note in the kernel doc (or a comment near the param) that None is supported and that the mask must reside on model.device.

Proposed comment near the param:

-    articulation_mask: wp.array(
-        dtype=bool
-    ),  # used to enable / disable FK for an articulation, if None then treat all as enabled
+    articulation_mask: wp.array(dtype=bool),  # Optional: pass None to enable all. Must be on the same device as model.

Also applies to: 365-371

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

21-45: Tighten wording for DISTANCE and D6 to avoid confusion

Minor clarity tweak: DISTANCE is represented with FREE-like coordinates (6 DoF velocities, 7 position coords) but semantically enforces a distance constraint. Adding that sentence helps users reconcile the representation with the name.

Suggested doc tweak:

-    """Distance constraint: maintains a fixed distance between two points (6 DoF, 7 coordinates)."""
+    """Distance constraint: maintains a fixed distance between two points.
+    Represented with FREE-like coordinates (6 DoF velocities, 7 position coordinates)."""

---

48-66: Validate num_axes for joint types or document expectations explicitly

get_joint_dof_count forwards num_axes for most types; callers passing num_axes != 1 for PRISMATIC/REVOLUTE (or unexpected values for others) will get surprising results. Either add guards or make the expectations explicit in the docstring.

Option A (doc-only):

-        num_axes (int): The number of axes for the joint (typically 1 for prismatic/revolute, 3 for D6, etc).
+        num_axes (int): The number of translational/rotational axes considered for this joint.
+            Expected values by type:
+            - PRISMATIC/REVOLUTE: 1
+            - D6: 3 (per-translation and per-rotation groups handled elsewhere)
+            - Others: ignored

Option B (light validation):

 def get_joint_dof_count(joint_type: int, num_axes: int) -> tuple[int, int]:
+    if joint_type in (JointType.PRISMATIC, JointType.REVOLUTE) and num_axes != 1:
+        raise ValueError(f"Expected num_axes=1 for {JointType(joint_type).name}, got {num_axes}")

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

122-122: Minor typo in docstring.

The docstring should maintain consistency by referring to "another Style3DModelBuilder" instead of starting with "another".

-        """Copies the data from another `Style3DModelBuilder` to this `Style3DModelBuilder`.
+        """Copies the data from another Style3DModelBuilder to this Style3DModelBuilder.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

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

You can enable these sources in your CodeRabbit configuration.

Learnt from: shi-eric
PR: newton-physics/newton#521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The Newton migration guide (docs/migration.rst) is specifically for documenting how to migrate existing warp.sim functionality to Newton equivalents. New Newton-only features that didn't exist in warp.sim do not need migration documentation.

Learnt from: nvlukasz
PR: newton-physics/newton#519
File: newton/_src/utils/import_usd.py:364-365
Timestamp: 2025-08-12T18:04:06.287Z
Learning: The quat_between_axes function in Newton accepts AxisType parameters, which includes both Axis enum values and strings. The function internally handles string-to-Axis conversion using Axis.from_any(), so passing strings directly to quat_between_axes is valid and correct.

Learnt from: nvlukasz
PR: newton-physics/newton#519
File: newton/_src/utils/import_usd.py:364-365
Timestamp: 2025-08-12T18:04:06.287Z
Learning: The quat_between_axes function in Newton accepts AxisType parameters, which includes strings like "X", "Y", "Z" as well as Axis enum values and integers. The function internally handles conversion using Axis.from_any(), so passing strings directly to quat_between_axes is valid and correct behavior.

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

Learnt from: eric-heiden
PR: newton-physics/newton#587
File: newton/_src/sim/builder.py:656-661
Timestamp: 2025-08-20T18:02:36.042Z
Learning: In Newton physics, collisions between shapes with body index -1 (world-attached/global shapes) are automatically skipped by the collision system, so no manual collision filter pairs need to be added between global shapes from different builders.

Learnt from: dylanturpin
PR: newton-physics/newton#450
File: newton/sim/ik.py:1286-1296
Timestamp: 2025-07-21T19:11:04.077Z
Learning: In Newton's IK system, the JointLimitObjective class intentionally only handles joints where DOF count equals coordinate count (1-1 mapping). This is appropriate because joint limits are typically only meaningful for simple joint types like revolute and prismatic joints, not complex joints like ball or free joints where DOF count != coordinate count.

Learnt from: shi-eric
PR: newton-physics/newton#521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The Newton migration guide (docs/migration.rst) is specifically for documenting how to migrate existing warp.sim functionality to Newton equivalents. New Newton-only features that didn't exist in warp.sim do not need migration documentation.

docs/api/newton_viewer.rst (1)

16-19: Verified: public exports for Viewer classes are correct

The four classes—ViewerGL, ViewerNull, ViewerRerun, and ViewerUSD—are indeed imported from the internal module and re-exported in newton/viewer.py (see the from ._src.viewer import … at line 21 and the matching entries in __all__ on lines 28–31). They therefore exist as public attributes of newton.viewer, and the Sphinx autosummary targets will resolve successfully. No changes needed.

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

21-23: Good incremental doc improvements on public enums

Enum-level docstrings read clearly and match existing member-level docs. No API or behavior change introduced.

Also applies to: 31-33

newton/_src/core/types.py (4)

65-72: LGTM! Clear and consistent documentation improvements.

The enhanced class docstring and per-member docstrings provide better clarity for API users. The documentation follows a consistent pattern across all axis members.

---

76-87: LGTM! Comprehensive docstring with proper error documentation.

The docstring clearly explains the function's behavior, including case-insensitive handling and the specific ValueError that can be raised.

---

99-111: LGTM! Well-documented type conversion method.

The docstring properly documents all accepted input types and potential exceptions, making the API contract clear for users.

---

141-161: LGTM! Clear documentation for vector conversion methods.

Both to_vector() and to_vec3() methods now have clear docstrings explaining their return types and purpose.

newton/_src/sim/ik.py (4)

26-41: LGTM! Enhanced enum documentation for IK Jacobian modes.

The expanded docstrings clearly explain the different Jacobian computation strategies, helping users understand when to use each mode.

---

1262-1279: LGTM! New target position setter method is well-implemented.

The set_target_position method provides a clean API for updating individual problem targets. The implementation correctly uses a Warp kernel to update the target array on the specified device.

---

1281-1296: LGTM! Batch target setter complements the single-problem setter.

The set_target_positions method efficiently updates all targets in batch using the appropriate Warp kernel dimension.

---

919-957: LGTM! Comprehensive IKObjective base class documentation.

The enhanced docstring clearly describes the purpose and interface of IK objectives, with detailed method descriptions that will help developers implement custom objectives.

newton/_src/sim/state.py (3)

22-43: LGTM! Excellent State class documentation with clear attribute descriptions.

The enhanced docstring provides a comprehensive overview of the State class and its role in simulations. The Attributes section clearly documents each array's shape and dtype, making the API contract explicit.

---

77-82: LGTM! Clear documentation for force clearing behavior.

The expanded docstring explicitly states the method's behavior, making it clear which arrays are affected.

---

63-69: Documentation verification complete: no inconsistencies found.

I’ve confirmed in the code that:

• The Euler and XPBD solvers apply torques using the body’s center of mass (they compute moments with cross(r_com, f) in their kernels).
• The Featherstone solver treats input wrenches at the world origin (it uses cross(r_world, f) when assembling body forces).
• The state attribute docstring in newton/_src/sim/state.py accurately describes this distinction.

No updates are needed to the existing documentation.

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

29-47: LGTM! Excellent technical documentation for transform_inertia.

The enhanced docstring provides both the mathematical formulation and a proper reference to Frank & Park's textbook, making it valuable for users who need to understand the underlying theory.

---

66-84: LGTM! Clear documentation for mathematical utility functions.

The Boltzmann function documentation clearly explains the interpolation behavior and the role of the alpha parameter.

---

88-159: LGTM! Consistent documentation pattern for smooth approximation functions.

All smooth/leaky min/max functions now have clear docstrings explaining their differentiable nature and the role of smoothing parameters. The relationship between leaky and smooth variants (with eps=1e-5) is properly documented.

---

162-236: LGTM! Complete documentation for vector utility functions.

All vector operations now have clear docstrings following a consistent pattern, making the API intuitive to use.

newton/_src/geometry/types.py (6)

26-64: LGTM! Comprehensive GeoType enum documentation.

Each geometry type now has a clear description, making it easier for users to understand the available shape primitives.

---

72-106: LGTM! Improved SDF class with proper initialization and documentation.

The SDF class now properly initializes the com attribute with a default zero vector when not provided, and the documentation clearly describes all attributes and their defaults.

---

122-156: LGTM! Excellent Mesh class documentation with practical example.

The enhanced docstring provides a clear description of the Mesh class purpose and includes a practical example using OpenMesh. The Attributes section comprehensively documents all mesh properties.

---

158-205: LGTM! Well-documented Mesh constructor with flexible data handling.

The constructor now supports optional per-vertex normals and UVs, with clear documentation of all parameters and their defaults. The inertia computation behavior is properly controlled and documented.

---

206-222: LGTM! Useful copy method with clear documentation.

The copy method provides flexibility for creating mesh variants, with optional vertex/index replacement and inertia recomputation.

---

259-268: LGTM! Clear documentation for finalize method.

The updated docstring clearly explains that the method creates a Warp Mesh and returns its ID.

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

95-106: LGTM! Clear and comprehensive documentation added.

The expanded docstring properly documents the initialization parameters and public-facing attributes that are specific to Style3D models. This improves the API documentation clarity.

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

49-67: Well-documented factory method implementation.

The from_model method now has clear documentation describing its purpose and behavior. The implementation correctly uses cls.__new__ to avoid calling __init__, copies all attributes from the base Model, and initializes Style3D-specific attributes to None.

newton/_src/sim/contacts.py (3)

24-60: Excellent comprehensive documentation for the Contacts class.

The expanded docstring provides clear descriptions of all buffer attributes, their shapes, and data types. The documentation properly explains the purpose of each field and notes that this is a temporary solution whose interface may change.

---

100-103: LGTM! Clear method documentation added.

The docstring clearly describes the clearing behavior.

---

115-120: LGTM! Device property properly documented.

The docstring clearly explains what the property returns.

newton/_src/sim/model.py (6)

30-53: Excellent Model class documentation overhaul.

The expanded docstring provides a comprehensive overview of the Model class, its purpose, key features, and the recommended construction workflow via ModelBuilder. The documentation clearly explains the environment grouping concept which is crucial for understanding the simulation architecture.

---

443-455: Clear documentation for state creation method.

The docstring properly documents the method's purpose, the optional requires_grad parameter, and clarifies that the returned state is initialized from the model description.

---

479-492: Well-documented control object creation.

The docstring clearly explains the method's purpose, parameters including the clone_variables option, and return value.

---

526-548: Comprehensive collision documentation.

The docstring thoroughly documents all parameters and clearly explains that this method creates a Contacts object for use in contact-dynamics kernels. The collision pipeline creation behavior is well explained.

---

576-605: Robust input validation added to add_attribute method.

The method now properly validates that:

1. The attribute doesn't already exist
2. The attribute is a wp.array
3. The attribute is on the same device as the Model

This prevents common errors and ensures consistency. The documentation clearly lists the supported frequency values.

---

607-630: Clear documentation for attribute frequency retrieval.

The docstring properly documents the supported frequency values and the error condition when the frequency is unknown.

newton/_src/utils/contact_sensor.py (6)

113-126: Clear documentation for populate_contacts function.

The docstring properly describes the function's purpose, parameters, and in-place update behavior.

---

131-149: Comprehensive ContactSensor class documentation.

The class docstring clearly explains the sensor's purpose, configuration options, and key attributes. This provides users with a good understanding of how to use the sensor.

---

151-181: Improved parameter naming and documentation in ContactSensor.init.

The parameter names have been updated to be more explicit and clear:

• sensor_body → sensing_obj_bodies
• contact_partners_shape → counterpart_shapes
• contact_partners_body → counterpart_bodies

The documentation clearly explains the mutual exclusivity constraints and the purpose of each parameter.

---

236-246: Clear documentation for eval method.

The docstring properly describes the method's purpose and behavior.

---

248-255: Well-documented get_total_force method.

The docstring clearly describes the return value and its type.

---

183-227: Backward-compatibility check completed

I searched the entire codebase—including all Python files, tests, and examples—and found no internal references to the old ContactSensor parameters (sensor_body, contact_partners_shape, or contact_partners_body). This means the rename is self-contained and won’t break anything in our own examples or tests. External users will still need to update their instantiations to the new parameter names; please be sure to document this change in the changelog or an upgrade guide before the next release.

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

33-45: LGTM! Well-documented function signature with return type annotation.

The addition of the return type annotation and comprehensive docstring improves API clarity.

---

65-90: Excellent documentation for the CollisionPipeline class!

The comprehensive docstring clearly describes the class purpose, responsibilities, and all attributes. This will greatly help developers understand and use the collision system.

---

168-184: Clear documentation for the from_model class method.

The docstring properly explains all parameters and their defaults, making the API intuitive to use.

---

347-354: Good addition of the device property with clear documentation.

The property provides convenient access to the device information with a well-documented return value.

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

126-163: Comprehensive docstring for ArticulationView class!

The detailed docstring with clear descriptions of arguments and attributes will significantly improve the usability of this complex class.

---

693-703: Consistent and clear method documentation.

All the new method docstrings follow a consistent pattern with clear descriptions of parameters and return values. The use of forward-referenced types in signatures is appropriate for avoiding circular imports.

---

705-715: Well-documented get_link_velocities method.

Clear description of the world-space spatial velocities return value.

---

717-738: Clear documentation for DoF position methods.

Both getter and setter methods have consistent documentation explaining their purpose and parameters.

---

740-761: Consistent documentation for DoF velocity methods.

Maintains the same documentation pattern as the position methods.

---

763-784: Complete documentation for DoF force methods.

The force-related methods are properly documented with appropriate forward-referenced Control type.

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

268-433: Exceptional documentation for ModelBuilder.init!

This comprehensive docstring documents all 150+ attributes, providing invaluable reference for developers. The organization by category (particles, shapes, joints, etc.) makes it easy to navigate.

---

647-658: Clear documentation for the up_vector property.

The read-only property is well-documented with its behavior clearly explained.

---

668-736: Concise property documentation for counts.

Each count property has a brief but clear docstring describing what it counts.

---

778-811: Comprehensive documentation for the replicate method.

The detailed docstring with example usage and notes about environment grouping and spatial arrangement helps users understand this complex functionality.

---

1787-1787: Clear specification of allowed joint types for equality constraints.

The explicit mention of "prismatic and revolute" as the only allowed scalar joint types prevents user confusion.

---

3921-3939: Good documentation for add_free_joints_to_floating_bodies.

The expanded docstring clearly explains the method's behavior and the expected use of the new_bodies parameter.

---

3622-3646: Well-documented add_particle_grid method.

Comprehensive parameter descriptions with clear explanations of optional parameters and their defaults.

---

4004-4023: Excellent documentation for the finalize method.

The expanded docstring clearly describes the finalization process, validation steps, and the purpose of the returned Model object.

---

4335-4336: Integration of collision pair computation during finalization.

The addition of find_shape_contact_pairs and count_rigid_contact_points properly integrates the new collision system into the model finalization process.

---

4348-4395: Well-implemented and documented find_shape_contact_pairs method.

The implementation correctly:

1. Handles collision groups and filters
2. Ensures canonical ordering of pairs
3. Handles the special -1 group for global collisions
4. Documents side effects clearly

The logic for computing collision pairs based on groups and filters is sound.

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

228-237: Viewer process tracking is ineffective; simplify or capture a real handle

self._viewer_process is never assigned (rr.serve_web_viewer doesn’t return a Popen-like handle), so is_running() always falls back to self._running and close()’s terminate/kill block is dead code. The Rerun API exposes rr.spawn() for launching a native viewer process or rr.disconnect() to close connections/servers; if you want process tracking, use rr.spawn() and store the returned handle. Otherwise, simplify the code to rely on _running only and rr.disconnect(). (ref.rerun.io)

If you don’t plan to use rr.spawn(), apply this simplification:

@@
-        # Check if viewer process is still alive
-        if self._viewer_process is not None:
-            return self._viewer_process.poll() is None
-        return self._running
+        return self._running
@@
-        # Close viewer process if we spawned one
-        if self._viewer_process is not None:
-            try:
-                self._viewer_process.terminate()
-                self._viewer_process.wait(timeout=5)
-            except subprocess.TimeoutExpired:
-                self._viewer_process.kill()
-            except Exception:
-                pass
-            self._viewer_process = None
+        # If we ever start managing an external process via rr.spawn(), handle it here.

If you want to support an external viewer, capture and manage the handle returned by rr.spawn(). (ref.rerun.io)

Also applies to: 247-257

---

104-108: Prefer explicit exceptions over asserts for argument validation

assert can be disabled with Python -O, removing your runtime checks. Raise TypeError/ValueError explicitly to validate inputs.

-        assert isinstance(points, wp.array)
-        assert isinstance(indices, wp.array)
-        assert normals is None or isinstance(normals, wp.array)
-        assert uvs is None or isinstance(uvs, wp.array)
+        if not isinstance(points, wp.array):
+            raise TypeError("points must be a wp.array")
+        if not isinstance(indices, wp.array):
+            raise TypeError("indices must be a wp.array")
+        if normals is not None and not isinstance(normals, wp.array):
+            raise TypeError("normals must be a wp.array or None")
+        if uvs is not None and not isinstance(uvs, wp.array):
+            raise TypeError("uvs must be a wp.array or None")

---

113-116: Validate flattened index buffers are triangles

If indices is 1D, ensure its length is divisible by 3 before reshaping to (-1, 3) to avoid silent shape errors.

         # Rerun expects indices as (N, 3) for triangles
         if indices_np.ndim == 1:
+            if (indices_np.size % 3) != 0:
+                raise ValueError("indices length must be divisible by 3")
             indices_np = indices_np.reshape(-1, 3)

---

92-103: Docstring is clear; note unused params or wire them through

Doc is solid. Consider either removing hidden/backface_culling from the signature (if unsupported by Rerun) or documenting that they are currently ignored to avoid confusion.

---

240-244: Cleanup path is sound; consider flushing before disconnect

rr.disconnect() will close connections and servers. Optionally call rr.flush(blocking=True) first to ensure pending batches are sent before teardown (micro-batching is enabled by default). (ref.rerun.io)

         # Disconnect from rerun
         try:
-            rr.disconnect()
+            try:
+                rr.flush(blocking=True)
+            except Exception:
+                pass
+            rr.disconnect()
         except Exception:
             pass

Also applies to: 258-263

---

135-145: Clarify color range and materials behavior in the docstring

• Colors: specify accepted ranges (float 0–1 or uint 0–255) to align with Rerun’s expectations. (ref.rerun.io)
• materials is currently unused; call it out in the docstring to avoid implying support.

-            colors (wp.array): Instance colors (wp.vec3).
-            materials (wp.array): Instance materials (wp.vec4).
+            colors (wp.array): Instance colors (wp.vec3). Floats in [0,1] or uints in [0,255]; first color is applied to all vertices.
+            materials (wp.array): Instance materials (wp.vec4). Currently unused.

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

1163-1193: Minor documentation enhancement suggestion.

The class docstring could be slightly more explicit about the coordinate system conventions.

Consider adding a note about coordinate system conventions:

 class IKPositionObjective(IKObjective):
     """
     Inverse kinematics objective for enforcing a positional target on a single end-effector link.

     This objective constrains the world-space position of a specified link (optionally offset in the link's local frame)
     to match a target position for each problem in a batch. The objective supports both analytic and autodiff Jacobian
-    computation.
+    computation.
+    
+    All positions are specified in world coordinates, with the link_offset applied in the link's local frame
+    before transformation to world space.

---

1803-1837: Minor documentation enhancement suggestion.

The rotation objective documentation could clarify the quaternion representation and error computation.

Consider adding clarity about quaternion conventions:

     This objective constrains the world-space orientation of a specified link (optionally offset by a local rotation)
     to match a target quaternion for each problem in a batch. The objective supports both analytic and autodiff Jacobian
-    computation.
+    computation.
+    
+    Quaternions are represented as vec4 (x, y, z, w) and rotation errors are computed in axis-angle form.
+    The canonicalize_quat_err parameter controls whether to use the shortest rotation path.

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

220-221: Consider clarifying the gradient-related allocation behavior

The comment mentions allocating new contact memory "for gradients", but the condition also checks if contacts is None. This might be clearer if separated.

Consider clarifying the logic:

-        # Allocate new contact memory for contacts if needed (e.g., for gradients)
-        if self.contacts is None or self.requires_grad:
+        # Allocate new contact memory if not yet allocated or if gradients are required
+        # (gradient computation needs fresh buffers each time)
+        if self.contacts is None or self.requires_grad:

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

212-235: Mesh.copy should preserve optional attributes (normals/uvs/color) by default

Current copy drops normals, uvs, and color. That’s surprising given the expanded API/docs. Preserve them unless explicitly overridden.

Apply this diff:

-        m = Mesh(
-            vertices, indices, compute_inertia=recompute_inertia, is_solid=self.is_solid, maxhullvert=self.maxhullvert
-        )
+        m = Mesh(
+            vertices,
+            indices,
+            normals=self._normals,
+            uvs=self._uvs,
+            compute_inertia=recompute_inertia,
+            is_solid=self.is_solid,
+            maxhullvert=self.maxhullvert,
+            color=self._color,
+        )

---

309-321: Hash stability and cost on large meshes

The hash flattens arrays into Python tuples, which is O(N) memory and time. For large meshes this is heavy and may spike memory.

Consider hashing on immutable bytes with dtype enforced:

-            self._cached_hash = hash(
-                (tuple(np.array(self.vertices).flatten()), tuple(np.array(self.indices).flatten()), self.is_solid)
-            )
+            v = np.asarray(self.vertices, dtype=np.float32).tobytes()
+            i = np.asarray(self.indices, dtype=np.int32).tobytes()
+            self._cached_hash = hash((v, i, bool(self.is_solid)))

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

1786-1786: Clarify “scalar joint types” phrasing

“Only scalar joint types (prismatic and revolute) can be used.” is correct; consider linking to these types or referring to JointType.PRISMATIC / JointType.REVOLUTE for consistency across docs.

---

266-432: Suppress duplicate Sphinx index entries for ModelBuilder attributes

CI is reporting duplicate-object warnings for the per-attribute docs in ModelBuilder. To avoid this, update the generated stub at docs/api/_generated/newton.ModelBuilder.rst by either:

• Excluding those attributes from the auto-member list:

 .. autoclass:: newton.ModelBuilder
    :members:
    :undoc-members:
    :show-inheritance:
    :member-order: groupwise
+   :exclude-members: balance_inertia, bound_mass, bound_inertia, validate_inertia_detailed

• Or disabling index entries altogether on this directive:

 .. autoclass:: newton.ModelBuilder
    :members:
    :undoc-members:
    :show-inheritance:
    :member-order: groupwise
+   :noindex:

Either approach prevents Sphinx from generating duplicate index entries for those attributes. Let me know if you’d like a PR to apply one of these patches.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

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

You can enable these sources in your CodeRabbit configuration.

Learnt from: shi-eric
PR: newton-physics/newton#521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The Newton migration guide (docs/migration.rst) is specifically for documenting how to migrate existing warp.sim functionality to Newton equivalents. New Newton-only features that didn't exist in warp.sim do not need migration documentation.

Learnt from: dylanturpin
PR: newton-physics/newton#450
File: newton/sim/ik.py:1286-1296
Timestamp: 2025-07-21T19:11:04.077Z
Learning: In Newton's IK system, the JointLimitObjective class intentionally only handles joints where DOF count equals coordinate count (1-1 mapping). This is appropriate because joint limits are typically only meaningful for simple joint types like revolute and prismatic joints, not complex joints like ball or free joints where DOF count != coordinate count.

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

206-216: Good: rr.set_time usage matches the 0.23+ API

Using rr.set_time("time", timestamp=time) is the recommended unified API after 0.23 (replacing set_time_* variants). This is consistent with current docs. (rerun.io)

---

31-37: Nice: clear, high-level class docstring

The overview reads well and matches the intended capabilities. No action needed.

newton/_src/sim/ik.py (10)

25-41: LGTM - Clear documentation for new MIXED mode.

The addition of the MIXED mode to the IKJacobianMode enum is well-documented and provides a sensible hybrid approach combining analytic Jacobians where available with autodiff fallback. The docstrings clearly explain the behavior of each mode.

---

107-150: LGTM - Comprehensive parameter documentation.

The init method documentation has been significantly improved with detailed parameter descriptions and clear notes about the batch structure. The documentation style is consistent and informative.

---

201-222: LGTM - Proper objective initialization with mode-aware buffer allocation.

The _init_objectives method correctly handles the new MIXED mode by determining the appropriate Jacobian mode for each objective based on whether it supports analytic computation. The logic is sound and the implementation is clean.

---

276-313: LGTM - Well-documented compute_residuals method.

The enhanced documentation clearly explains the parameters, return values, and behavior of the method. The implementation correctly handles both autodiff/mixed and analytic modes with appropriate forward kinematics computation.

---

315-388: LGTM - Robust MIXED mode implementation in compute_jacobian.

The MIXED mode implementation is well-structured:

• Efficiently checks which objectives need each type of computation
• Properly sets up autodiff tape only when needed
• Correctly computes motion subspace only when needed
• Iterates through objectives using the appropriate method for each

The logic handles all three modes (AUTODIFF, ANALYTIC, MIXED) correctly.

---

919-1063: LGTM - Comprehensive IKObjective base class documentation.

The extensive documentation for the IKObjective base class is excellent:

• Clearly describes the purpose and role of IK objectives
• Documents all required and optional methods
• Provides detailed parameter descriptions
• Explains the contract that subclasses must implement

This greatly improves the API usability for developers creating custom objectives.

---

1262-1296: LGTM - Valuable per-problem target update APIs.

The addition of set_target_position and set_target_positions methods provides essential runtime flexibility for IK problems. The implementation is efficient using GPU kernels and the documentation clearly explains the usage.

---

1918-1952: LGTM - Consistent rotation target update APIs.

The set_target_rotation and set_target_rotations methods mirror the position objective implementation perfectly, providing consistent API design across objective types.

---

1209-1254: LGTM - Efficient buffer initialization with kinematic analysis.

The init_buffers method in IKPositionObjective demonstrates sophisticated implementation:

• For analytic mode: performs kinematic tree traversal to determine which DoFs affect the end-effector
• For autodiff mode: pre-allocates one-hot vectors for efficient gradient extraction
• Uses numpy for CPU computations then transfers to GPU arrays

The kinematic analysis logic correctly traces ancestors in the joint hierarchy.

---

1545-1573: LGTM - Proper DoF-to-coordinate mapping for joint limits.

The init_buffers method in IKJointLimitObjective correctly handles the mapping between DoF indices and coordinate indices, which is essential for joint types where these don't align 1:1 (as noted in the retrieved learnings about the Newton IK system design).

newton/_src/utils/recorder_gui.py (5)

22-29: Documentation looks good and comprehensive

The expanded class docstring clearly describes the purpose and functionality of the RecorderImGuiManager, including its role in visualization and control of simulation playback.

---

31-41: Detailed initialization documentation

The docstring now provides clear parameter descriptions with proper type hints and defaults, making the API easy to understand.

---

53-59: Clear documentation for internal method

The docstring appropriately documents this private method's purpose and behavior for future maintainers.

---

65-85: Functional improvements with proper documentation

The expanded logic for handling paused state with transforms and point clouds is well-documented. The method now properly consumes recorder playback results when paused, applying transforms and rendering contact points as needed.

---

86-91: Well-documented public interface method

The docstring clearly describes the UI drawing functionality, making it easy for users to understand what this method does.

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

22-34: Comprehensive class documentation

The new docstring clearly explains the purpose of ViewerNull as a headless, non-rendering viewer for automated environments, with well-documented attributes.

---

36-42: Clear initialization documentation

The docstring properly documents the parameter and its purpose.

---

46-46: Frame tracking implementation looks correct

The addition of frame_count initialization enables proper frame-limited operation as described in the documentation.

---

95-99: Proper frame counting implementation

The end_frame method correctly increments the frame counter, enabling the frame-limited behavior.

---

101-108: Correct frame limit check

The is_running method properly checks against the frame limit, consistent with other viewer implementations.

---

48-161: Comprehensive documentation for all no-op methods

All logging and lifecycle methods now have clear docstrings explaining their no-op behavior and parameter meanings, which is helpful for API consistency.

newton/_src/sim/state.py (3)

21-43: Excellent high-level class documentation

The expanded docstring provides a clear overview of the State class's role, proper usage via Model.state(), and comprehensive attribute documentation with shapes and dtypes.

---

46-75: Consistent and precise attribute documentation

All attribute docstrings now follow a consistent format with shape and dtype information. The descriptions are more precise, especially for body transforms (7-DOF) and spatial vectors with explicit angular/linear component ordering.

---

77-82: Clear documentation for clear_forces method

The expanded docstring explicitly states the behavior of clearing particle_f and body_f arrays.

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

14-21: Well-structured class documentation

The docstring clearly explains the USD viewer's role in creating USD stages with instanced rendering and time-sampled transforms.

---

23-35: Comprehensive initialization documentation

The docstring provides clear parameter descriptions and properly documents the ImportError that may be raised.

---

67-91: Improved type hints and documentation for log_mesh

The addition of type hints for arrays and the comprehensive docstring with parameter descriptions and return value documentation improves API clarity.

---

260-275: Proper frame counting implementation

The begin_frame method now correctly increments _frame_count and updates the USD stage end time as needed, enabling frame-limited recording.

---

284-293: Correct frame limit check for USD viewer

The is_running method properly checks the frame count against the limit, consistent with other viewer implementations.

---

130-144: Clear error handling documentation

The docstring properly documents the RuntimeError that may be raised if the mesh prototype is not found.

---

305-335: Consistent documentation for unimplemented methods

All placeholder methods have clear docstrings indicating they are not implemented for the USD backend.

---

338-349: Well-documented utility method

The _ensure_scopes_for_path static method has comprehensive documentation explaining its purpose and behavior.

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

33-45: Clear function signature and documentation

The addition of explicit return type annotation and comprehensive docstring improves API clarity.

---

65-90: Comprehensive CollisionPipeline class documentation

The new class docstring clearly describes the responsibilities and attributes of the CollisionPipeline, making it easy to understand its role in the simulation.

---

92-125: Detailed initialization documentation

The docstring provides comprehensive parameter descriptions with defaults and types, making the API clear and easy to use.

---

139-139: Comment improvement

The updated comment "Allocate buffers for broadphase collision handling" is more descriptive than the previous version.

---

156-184: Well-documented factory method

The from_model classmethod has clear documentation explaining how it constructs a CollisionPipeline from a Model with appropriate parameter descriptions.

---

206-219: Clear collide method documentation

The docstring explains the method's purpose, behavior regarding buffer allocation, and return value.

---

346-354: Clear property documentation

The device property has appropriate documentation explaining what it returns.

---

357-360: Proper module exports

The all list correctly exports the new CollisionPipeline class alongside the existing function.

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

777-810: Nice addition: clear usage docs for replicate()

Documentation upgrades are thorough and actionable. No code issues here.

---

648-656: LGTM on up_vector docs and read-only enforcement

Clearer docs and explicit setter error improve UX.

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

24-60: Docstring expansion looks good; keeping Sphinx notes is appropriate

The richer attribute documentation and the explicit “.. note::” block read well and are consistent with our docs style.

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

3621-3645: Jitter distribution is biased; radius can go negative under Gaussian noise

• rng.random(3) ∈ [0,1) yields one-sided jitter; positions are biased.
• radius_mean + N(0,σ) can become negative.

If you want to keep this PR doc-only, at least document that jitter is one-sided and radii are unclamped. Otherwise, consider this minimal fix:

-        rng = np.random.default_rng(42)
+        rng = np.random.default_rng(42)
@@
-                    p = wp.quat_rotate(rot, v) + pos + wp.vec3(rng.random(3) * jitter)
+                    # symmetric jitter in [-jitter, +jitter]
+                    jitter_vec = (rng.random(3) * 2.0 - 1.0) * jitter
+                    p = wp.quat_rotate(rot, v) + pos + wp.vec3(*jitter_vec)
@@
-                    if radius_std > 0.0:
-                        r = radius_mean + rng.standard_normal() * radius_std
-                    else:
-                        r = radius_mean
+                    if radius_std > 0.0:
+                        r = float(radius_mean + rng.standard_normal() * radius_std)
+                        r = max(0.0, r)  # clamp
+                    else:
+                        r = float(radius_mean)

Also applies to: 3648-3661

---

4341-4387: Empty contact-pairs edge case: wp.vec2i requires Nx2 input

When no pairs exist, np.array(contact_pairs) has shape (0,), which doesn’t map to vec2i. Create a shape-(0,2) array before wrapping.

-        model.shape_contact_pairs = wp.array(np.array(contact_pairs), dtype=wp.vec2i, device=model.device)
-        model.shape_contact_pair_count = len(contact_pairs)
+        if contact_pairs:
+            arr = np.asarray(contact_pairs, dtype=np.int32).reshape(-1, 2)
+        else:
+            arr = np.empty((0, 2), dtype=np.int32)
+        model.shape_contact_pairs = wp.array(arr, dtype=wp.vec2i, device=model.device)
+        model.shape_contact_pair_count = int(arr.shape[0])

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

120-150: Type in constructor docs: use newton.Model consistently

Some places say “Articulation” and others “newton.Model”. The code expects a Model with articulation data. Please unify to newton.Model here to match the class-level docs.

-        model : Articulation
-            The shared articulation model for all IK problems in the batch.
+        model : newton.Model
+            The shared model/articulation for all IK problems in the batch.

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

100-114: clear() doc/code mismatch; some indices aren’t reset to -1

Doc says “resetting counts and filling indices with -1,” but rigid_contact_point_id isn’t filled, and soft/rigid position/normal buffers aren’t zeroed consistently. Either narrow the docstring or reset additional fields.

Suggested minimal code change to align with the doc (safe and cheap):

 def clear(self):
     """
     Clear all contact data, resetting counts and filling indices with -1.
     """
     self.rigid_contact_count.zero_()
+    self.rigid_contact_point_id.fill_(-1)
     self.rigid_contact_shape0.fill_(-1)
     self.rigid_contact_shape1.fill_(-1)
     self.rigid_contact_tids.fill_(-1)
     self.rigid_contact_force.zero_()

     self.soft_contact_count.zero_()
     self.soft_contact_particle.fill_(-1)
     self.soft_contact_shape.fill_(-1)
     self.soft_contact_tids.fill_(-1)

---

86-94: Nit: dtype consistency for soft-contact index arrays

soft_contact_particle/shape/tids use dtype=int, while rigid equivalents use wp.int32. Consider standardizing on wp.int32 for cross-device consistency and to match the docs style used elsewhere.

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

3398-3412: Parameter name typo in docs: dim_x_ → dim_x

The docstring lists “dim_x_” but the signature uses “dim_x”. This can confuse users and Sphinx/autodoc cross-refs.

-            dim_x_: The number of rectangular cells along the x-axis
+            dim_x: The number of rectangular cells along the x-axis

---

3434-3438: Bug: total_mass formula uses dim_x twice; should include dim_y

The cloth grid total particle count is (dim_x + 1) * (dim_y + 1). Using dim_x twice underestimates/overestimates density.

-        total_mass = mass * (dim_x + 1) * (dim_x + 1)
+        total_mass = mass * (dim_x + 1) * (dim_y + 1)

---

2416-2433: Plane-from-equation sign bug: position should be -d·n

For plane a·x + b·y + c·z + d = 0, a point on the plane is x0 = -d·n̂, not d·n̂.

-            pos = plane[3] * normal
+            pos = -plane[3] * normal

---

3818-3827: Density unit in soft mesh docs should be per-volume, not per-area

Tetrahedral mass accumulation uses volume. Please correct the parameter description:

-            density: The density per-area of the mesh
+            density: The density per-volume of the mesh

---

3962-3973: Parameter name mismatches in color() doc

Doc lists include_bending_energy, target_max_min_color_ratio “the color balancing algorithm…”, and “algorithm”, while the signature is include_bending, target_max_min_color_ratio, coloring_algorithm. Align names to signature to avoid confusion.

-            include_bending_energy: Whether to consider bending energy ...
+            include_bending: Whether to consider bending energy ...
@@
-            algorithm: Value should be an enum type of ColoringAlgorithm, ...
+            coloring_algorithm: Value should be an enum type of ColoringAlgorithm, ...

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

94-103: _cache type annotation doesn’t match the actual key

Key includes (n_dofs, n_residuals, arch), but the type is dict[tuple[int, int], type]. Update the annotation to avoid static check noise.

-    _cache: ClassVar[dict[tuple[int, int], type]] = {}
+    _cache: ClassVar[dict[tuple[int, int, object], type]] = {}

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

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

You can enable these sources in your CodeRabbit configuration.

Learnt from: shi-eric
PR: newton-physics/newton#521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The Newton migration guide (docs/migration.rst) is specifically for documenting how to migrate existing warp.sim functionality to Newton equivalents. New Newton-only features that didn't exist in warp.sim do not need migration documentation.

Learnt from: dylanturpin
PR: newton-physics/newton#450
File: newton/sim/ik.py:1286-1296
Timestamp: 2025-07-21T19:11:04.077Z
Learning: In Newton's IK system, the JointLimitObjective class intentionally only handles joints where DOF count equals coordinate count (1-1 mapping). This is appropriate because joint limits are typically only meaningful for simple joint types like revolute and prismatic joints, not complex joints like ball or free joints where DOF count != coordinate count.

Learnt from: shi-eric
PR: newton-physics/newton#521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The Newton migration guide (docs/migration.rst) is specifically for documenting how to migrate existing warp.sim functionality to Newton equivalents. New Newton-only features that didn't exist in warp.sim do not need migration documentation.

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

777-810: Replicate() doc improvements read well

The updated description, example, and notes are clear and match the implementation.

---

1785-1797: Clarify allowed “scalar” joint types in equality constraint docs

Updating this to “prismatic and revolute” improves accuracy for scalar joints. Looks good.

newton/_src/sim/ik.py (3)

315-333: Compute Jacobian docs are clear and match the three backends

No issues spotted with the description of AUTODIFF/ANALYTIC/MIXED behavior.

---

1162-1223: IKPositionObjective buffer-init docs and logic look solid

Analytic path precomputes affects_dof; autodiff path initializes per-component e-arrays. This aligns with the solver’s usage.

---

1963-1995: Rotation residuals and canonicalization docs match the implementation

Axis-angle residual behavior and canonicalization flag are clearly documented and implemented as described.

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

232-233: Fix potential bug with or operator on arrays.

Using or with Warp/NumPy arrays will raise an ambiguous truth value error. Use explicit None checks instead.

-        joint_q = joint_q or self.joint_q
-        output_residuals = output_residuals or self.residuals
+        if joint_q is None:
+            joint_q = self.joint_q
+        if output_residuals is None:
+            output_residuals = self.residuals

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

134-135: Consider validating jacobian_mode parameter.

The code assumes jacobian_mode is one of the valid enum values. Consider adding validation to provide a clearer error message if an invalid mode is passed.

 grad = jacobian_mode in (IKJacobianMode.AUTODIFF, IKJacobianMode.MIXED)
+
+# Validate jacobian_mode
+if not isinstance(jacobian_mode, IKJacobianMode):
+    raise ValueError(f"jacobian_mode must be an IKJacobianMode enum value, got {type(jacobian_mode)}")

---

1057-1091: Complex buffer initialization could benefit from helper methods.

The buffer initialization logic for computing affected DOFs is duplicated between IKPositionObjective and IKRotationObjective. Consider extracting this into a shared helper method.

def _compute_affected_dofs(model, link_index, device):
    """Helper to compute which DOFs affect a given link."""
    joint_qd_start_np = model.joint_qd_start.numpy()
    dof_to_joint_np = np.empty(joint_qd_start_np[-1], dtype=np.int32)
    for j in range(len(joint_qd_start_np) - 1):
        dof_to_joint_np[joint_qd_start_np[j] : joint_qd_start_np[j + 1]] = j
    
    # ... rest of the logic ...
    
    return wp.array(affects_dof_np.astype(np.uint8), device=device)

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

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

You can enable these sources in your CodeRabbit configuration.

Learnt from: shi-eric
PR: newton-physics/newton#521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The Newton migration guide (docs/migration.rst) is specifically for documenting how to migrate existing warp.sim functionality to Newton equivalents. New Newton-only features that didn't exist in warp.sim do not need migration documentation.

newton/_src/sim/ik.py (6)

25-38: LGTM! Well-structured enum with clear documentation.

The IKJacobianMode enum is properly designed with descriptive docstrings for each mode. The MIXED mode provides a nice hybrid approach that can optimize performance while maintaining flexibility.

---

51-74: Documentation is comprehensive and well-formatted.

The Args section clearly documents all parameters with types and default values. The batch structure explanation is particularly helpful for understanding the solver's parallelization approach.

---

836-930: Excellent abstract base class design for IK objectives.

The IKObjective class provides a well-thought-out interface with clear separation between required and optional methods. The documentation thoroughly explains the purpose of each method and the batching paradigm.

---

1095-1112: Well-designed per-problem target update API.

The new methods set_target_position and set_target_positions provide a clean interface for updating targets either individually or in batch, which is essential for dynamic IK scenarios.

---

1554-1571: Consistent API design for rotation target updates.

The set_target_rotation and set_target_rotations methods mirror the position objective's API, maintaining consistency across objective types.

---

280-304: MIXED mode implementation is elegant and efficient.

The MIXED mode cleverly combines both autodiff and analytic approaches, only computing what's needed for each objective type. This provides optimal performance while maintaining flexibility.

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

3612-3636: Clarify jitter distribution and radius sampling behavior in docs (no code change).

• Jitter is sampled per-axis as uniform in [0, jitter) and added in world space; it’s not symmetric.
• Radii sampled with radius_std > 0 are not clamped; negative values are possible with large std.

Doc-only diff:

-            jitter (float): Maximum random offset to apply to each particle position.
+            jitter (float): Per-axis uniform random offset added in world space; sampled in [0, jitter).
+                Set to 0.0 for a perfect lattice.
@@
-            radius_std (float, optional): Standard deviation for particle radii. If > 0, radii are sampled from a normal distribution.
+            radius_std (float, optional): Standard deviation for particle radii. If > 0, radii are sampled
+                from a normal distribution without clamping; ensure chosen mean/std produce positive radii.

---

4376-4378: Handle empty contact-pair list to avoid shape mismatch with wp.vec2i.

When no pairs exist, np.array(contact_pairs) has shape (0,), which won’t map cleanly to wp.vec2i. Create an empty (0, 2) array.

Apply this code diff:

-        model.shape_contact_pairs = wp.array(np.array(contact_pairs), dtype=wp.vec2i, device=model.device)
-        model.shape_contact_pair_count = len(contact_pairs)
+        if contact_pairs:
+            arr = np.asarray(contact_pairs, dtype=np.int32).reshape(-1, 2)
+        else:
+            arr = np.empty((0, 2), dtype=np.int32)
+        model.shape_contact_pairs = wp.array(arr, dtype=wp.vec2i, device=model.device)
+        model.shape_contact_pair_count = int(arr.shape[0])

---

3913-3921: Doc/code mismatch for add_free_joints_to_floating_bodies(new_bodies).

Doc advertises Iterable[int] | None, but passing None will raise a TypeError at set(new_bodies). Since this PR is docs-only, tighten the doc to require an iterable, or (in a follow-up PR) explicitly handle/raise on None.

Doc-only fix now:

-            new_bodies (Iterable[int] or None, optional): The set of body indices to consider for adding free joints.
+            new_bodies (Iterable[int]): The set of body indices to consider for adding free joints.

Option for a future code change (if desired later):

 def add_free_joints_to_floating_bodies(self, new_bodies: Iterable[int] | None = None):
@@
-        floating_bodies = set(new_bodies) - set(self.joint_child)
+        if new_bodies is None:
+            raise ValueError("new_bodies must be provided; pass the set of newly added body indices.")
+        floating_bodies = set(new_bodies) - set(self.joint_child)

newton/_src/geometry/types.py (7)

89-97: Resolve potentially unresolved type name in docs (“Mat33”).

The docstring refers to “Mat33”, which isn’t imported here. Use a resolvable symbol (e.g., wp.mat33) so intersphinx/autodoc can link it.

-            I (Mat33, optional): 3x3 inertia matrix. Defaults to identity.
+            I (wp.mat33, optional): 3x3 inertia matrix. Defaults to identity.

---

108-113: Document the precondition for finalize() when volume is None.

Right now this will error if volume is None. Make that explicit so users aren’t surprised. (Implementation guard can come in a separate PR.)

         Returns:
             wp.uint64: The unique identifier of the SDF volume.
+
+        Note:
+            Requires a non-None ``volume``. If ``volume`` is None, this method will error.

---

122-156: Tighten Mesh class docs: “Examples” section name, attribute shapes, and linkable types.

• Use “Examples:” header for Napoleon.
• Clarify index array shape as (T*3,) rather than “(3 per triangle)”.
• Prefer wp.mat33 for linkable type.

-    Example:
+    Examples:
@@
-    Attributes:
+    Attributes:
         vertices (np.ndarray): Mesh 3D vertex positions, shape (N, 3).
-        indices (np.ndarray): Flattened triangle indices, shape (3 per triangle).
+        indices (np.ndarray): Flattened triangle indices, shape (T*3,) where T is the number of triangles.
         normals (np.ndarray or None): Optional per-vertex normals, shape (N, 3).
         uvs (np.ndarray or None): Optional per-vertex UV coordinates, shape (N, 2).
-        I (Mat33): 3x3 inertia matrix of the mesh (about the center of mass, assuming density 1.0).
+        I (wp.mat33): 3x3 inertia matrix of the mesh (about the center of mass, assuming density 1.0).

---

169-185: Make default for maxhullvert reference the constant to avoid drift.

Docs currently hard-code “64”. Referencing the constant is more robust if the value changes.

-            maxhullvert (int, optional): Max vertices for convex hull approximation (default: 64).
+            maxhullvert (int, optional): Max vertices for convex hull approximation (default: MESH_MAXHULLVERT).

---

258-266: Clarify finalize() side effects and defaults.

• Mention that self.mesh is populated.
• Call out default device behavior when device is None.
• Clarify that requires_grad applies to points/velocities only.

-        Construct a simulation-ready Warp Mesh object from the mesh data and return its ID.
+        Construct a simulation-ready Warp Mesh object from the mesh data, store it in ``self.mesh``, and return its ID.
@@
-            device (Devicelike, optional): Device on which to allocate mesh buffers.
-            requires_grad (bool, optional): If True, mesh points and velocities are allocated with gradient tracking.
+            device (Devicelike, optional): Device on which to allocate mesh buffers. If ``None``, uses the current Warp device.
+            requires_grad (bool, optional): If True, mesh points and velocities are allocated with gradient tracking (indices remain integer).

---

277-285: Reference maxhullvert in convex hull docs.

Readers will appreciate knowing how the hull complexity is controlled.

-        Compute and return the convex hull of this mesh.
+        Compute and return the convex hull of this mesh. The number of hull vertices is capped by ``self.maxhullvert``.

---

26-31: Consolidate GeoType member docs into the class docstring or enable enum-member autodoc

Checked the current Sphinx setup and discovered that:

• No enum-aware extension (e.g. autoenum, enum_tools, sphinxcontrib.enum) is enabled in docs/conf.py
• The generated RST (docs/api/_generated/newton.GeoType.rst) uses

  .. autoclass:: newton.GeoType
  

  without a :members: option, so bare string literals after each enum member will never be rendered

To ensure your per-member documentation isn’t lost, you can choose one of:

• Fold descriptions into the class docstring (simplest) and remove all no-op literals:

 class GeoType(enum.IntEnum):
-    """
-    Enumeration of geometric shape types supported in Newton.
-
-    Each member represents a different primitive or mesh-based geometry
-    that can be used for collision, rendering, or simulation.
-    """
+    """
+    Enumeration of geometric shape types supported in Newton.
+
+    Each member represents a different primitive or mesh-based geometry
+    that can be used for collision, rendering, or simulation.
+
+    Members:
+      - PLANE: Infinite plane
+      - HFIELD: Height field (terrain)
+      - SPHERE: Sphere
+      - CAPSULE: Capsule (cylinder with hemispherical ends)
+      - ELLIPSOID: Ellipsoid
+      - CYLINDER: Cylinder
+      - BOX: Axis-aligned box
+      - MESH: Triangle mesh
+      - SDF: Signed distance field
+      - CONE: Cone
+      - NONE: No geometry (placeholder)
+    """
 
     PLANE = 0
-    """Infinite plane."""
 
     HFIELD = 1
-    """Height field (terrain)."""
 
     SPHERE = 2
-    """Sphere."""
 
     CAPSULE = 3
-    """Capsule (cylinder with hemispherical ends)."""
 
     ELLIPSOID = 4
-    """Ellipsoid."""
 
     CYLINDER = 5
-    """Cylinder."""
 
     BOX = 6
-    """Axis-aligned box."""
 
     MESH = 7
-    """Triangle mesh."""
 
     SDF = 8
-    """Signed distance field."""
 
     CONE = 9
-    """Cone."""
 
     NONE = 10
-    """No geometry (placeholder)."""

• Enable enum-member autodoc by adding an enum extension and/or members flags:

– In docs/conf.py, add to extensions:
python extensions += [ "sphinxcontrib.enum", # or another enum-autodoc extension ]
– Either set in conf.py:
python autodoc_default_options = { 'members': True, 'undoc-members': True, }
or update the generated RST:
diff .. autoclass:: newton.GeoType + :members: + :undoc-members:

Choose the approach that fits your doc-build workflow.

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

313-315: Fix doc mismatch: shape_material_ka is adhesion distance, not “area stiffness”.

The ShapeConfig docstring correctly defines ka as “contact adhesion distance”, but the init Attributes section lists “area stiffness,” which is confusing.

Apply this doc-only diff:

-            shape_material_ka (list): Shape material area stiffness.
+            shape_material_ka (list): Contact adhesion distance for shape contacts.

---

363-364: Clarify joint_parents mapping semantics.

This maps from “child body -> list of parent bodies,” not “joint -> parent bodies.”

Doc-only fix:

-            joint_parents (dict): Mapping from joint to parent bodies.
+            joint_parents (dict): Mapping from child body index to its immediate parent body indices (used for collision-filter inheritance).

---

1776-1789: Document enforcement of “scalar joints only” or relax the claim.

The doc says only revolute/prismatic are allowed, but the function does not validate this. If enforcement happens downstream, say so; otherwise, consider softening the claim.

Doc-only tweak (if enforcement is downstream):

-        Constrains the position or angle of one joint to be a quartic polynomial of another joint. Only scalar joint types (prismatic and revolute) can be used.
+        Constrains the position or angle of one joint to be a quartic polynomial of another joint.
+        This constraint is intended for scalar joints (revolute or prismatic). The builder does not validate types;
+        solvers may raise or ignore invalid combinations.

---

4332-4347: Document environment-group semantics of contact pairs.

If cross-environment collisions are intentionally included (group -1 vs. per-env), spell this out here. If not, consider filtering by shape_group to avoid cross-env pairs. For this docs-only PR, at least clarify intent.

Doc-only addition example:

-        The resulting contact pairs are stored in the model as a 2D array of shape indices.
+        The resulting contact pairs are stored in the model as a 2D array of shape indices.
+        Note: Contact pairs are generated per collision group and include interactions with the global group (-1).
+        Environment groups are not filtered here; downstream collision stages may apply per-environment filtering.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

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

You can enable these sources in your CodeRabbit configuration.

Learnt from: shi-eric
PR: newton-physics/newton#521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The Newton migration guide (docs/migration.rst) is specifically for documenting how to migrate existing warp.sim functionality to Newton equivalents. New Newton-only features that didn't exist in warp.sim do not need migration documentation.

Learnt from: shi-eric
PR: newton-physics/newton#521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The Newton migration guide (docs/migration.rst) is specifically for documenting how to migrate existing warp.sim functionality to Newton equivalents. New Newton-only features that didn't exist in warp.sim do not need migration documentation.

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

72-86: LGTM: SDF class docs are clear and aligned with current behavior.

---

212-222: LGTM: copy() docstring reads well and matches behavior.

---

307-314: LGTM: Hashing docs accurately describe the cache and inputs.

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

3898-3908: Doc/code mismatch: new_bodies cannot be None as implemented

Docstring says “Iterable[int] or None”, but set(new_bodies) will raise when new_bodies is None. Either:

• Update the doc to require a non-None iterable (quick fix for this docs-only PR), or
• Add a small guard and raise a clear ValueError (preferred long-term).

Doc-only fix (recommended for this PR):

-            new_bodies (Iterable[int] or None, optional): The set of body indices to consider for adding free joints.
+            new_bodies (Iterable[int]): The set of body indices to consider for adding free joints. Must not be None.

Optional future code tweak for clarity:

     def add_free_joints_to_floating_bodies(self, new_bodies: Iterable[int] | None = None):
@@
-        # set(self.joint_child) is connected_bodies
-        floating_bodies = set(new_bodies) - set(self.joint_child)
+        if new_bodies is None:
+            raise ValueError("new_bodies must be provided; pass the set of newly added body indices.")
+        # set(self.joint_child) is connected_bodies
+        floating_bodies = set(new_bodies) - set(self.joint_child)

---

4318-4333: Empty-contact-pairs edge case: ensure (0, 2) shape for wp.vec2i array

When no shape pairs are found, np.array(contact_pairs) yields shape (0,), which does not map cleanly to wp.vec2i. Create an empty (0, 2) array to avoid runtime issues. The docs mention storing a 2D array; this change aligns code with that contract.

Apply this diff:

-        model.shape_contact_pairs = wp.array(np.array(contact_pairs), dtype=wp.vec2i, device=model.device)
-        model.shape_contact_pair_count = len(contact_pairs)
+        if contact_pairs:
+            arr = np.asarray(contact_pairs, dtype=np.int32).reshape(-1, 2)
+        else:
+            arr = np.empty((0, 2), dtype=np.int32)
+        model.shape_contact_pairs = wp.array(arr, dtype=wp.vec2i, device=model.device)
+        model.shape_contact_pair_count = int(arr.shape[0])

Also applies to: 4362-4363

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

271-275: Clarify gravity semantics (it’s a signed scalar, not a magnitude)

As written, “magnitude … Defaults to -9.81” is contradictory. Suggest tightening wording to reflect the signed scalar applied along up_vector.

Apply this diff:

-            up_axis (AxisType, optional): The axis to use as the "up" direction in the simulation.
-                Defaults to Axis.Z.
-            gravity (float, optional): The magnitude of gravity to apply along the up axis.
-                Defaults to -9.81.
+            up_axis (AxisType, optional): Axis used as the "up" direction in the simulation. Defaults to Axis.Z.
+            gravity (float, optional): Scalar acceleration applied along the up axis; use a negative value
+                for downward gravity relative to `up_axis`. Defaults to -9.81.

---

768-789: Replicate docs: note env/collision grouping and centering behavior

Two small clarifications will save reader surprises:

• Each copy goes into a distinct environment group (because add_builder(..., environment=None) advances num_envs).
• Copies are centered around the non-up axes (ground-plane preserved along the up-axis).

Apply this diff:

         """
         Replicates the given builder multiple times, offsetting each copy according to the supplied spacing.

         This method is useful for creating multiple instances of a sub-model (e.g., robots, environments)
         arranged in a regular grid or along a line. Each copy is offset in space by a multiple of the
-        specified spacing vector, and all entities from each copy are assigned to a new environment group.
+        specified spacing vector.
+
+        Notes
+        -----
+        - Environment grouping: each replicated copy is assigned to a new environment group by default.
+        - Collision grouping: shapes from the replicated builder will also be placed in a new collision
+          group (mirrors `add_builder(..., separate_collision_group=True)` defaults).
+        - Centering: offsets are recentred so the set of copies is centred in non-up directions; the
+          up-axis component is preserved to avoid shifting below the ground plane.

---

1763-1763: Equality-constraint docs: make intent and support explicit

Document that only scalar joints are intended here, and that other joint types are unsupported.

Apply this diff:

-        Constrains the position or angle of one joint to be a quartic polynomial of another joint. Only scalar joint types (prismatic and revolute) can be used.
+        Constrains the position or angle of one joint to be a quartic polynomial of another joint.
+        Only scalar joint types (prismatic, revolute) are intended. Passing non‑scalar joints is
+        unsupported and may be ignored by solvers.

---

3598-3622: add_particle_grid docs: specify jitter distribution and radius sampling behavior

Current implementation samples:

• Jitter per-axis uniformly in [0, jitter) (positive-only bias).
• Radii from a normal distribution with no lower bound clamp.

Make that explicit to prevent incorrect assumptions.

Apply this diff:

-            jitter (float): Maximum random offset to apply to each particle position.
-            radius_mean (float, optional): Mean radius for particles. If None, uses the builder's default.
-            radius_std (float, optional): Standard deviation for particle radii. If > 0, radii are sampled from a normal distribution.
+            jitter (float): Per-axis random offset applied uniformly in the range [0, jitter). Set to 0 to disable.
+            radius_mean (float, optional): Mean radius for particles. If None, uses the builder's default.
+            radius_std (float, optional): Standard deviation for particle radii. If > 0, radii are sampled
+                from a normal distribution around `radius_mean` (no lower clamp is applied in code).
+                Choose values to keep radii positive.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

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

You can enable these sources in your CodeRabbit configuration.

Learnt from: shi-eric
PR: newton-physics/newton#521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The Newton migration guide (docs/migration.rst) is specifically for documenting how to migrate existing warp.sim functionality to Newton equivalents. New Newton-only features that didn't exist in warp.sim do not need migration documentation.

Learnt from: shi-eric
PR: newton-physics/newton#521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The Newton migration guide (docs/migration.rst) is specifically for documenting how to migrate existing warp.sim functionality to Newton equivalents. New Newton-only features that didn't exist in warp.sim do not need migration documentation.

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

71-74: Nice clarity upgrade on the class overview and finalize call path

The added narrative around “lists unsuitable for simulation” and explicitly calling finalize() is helpful for users scanning the API.

---

639-647: Read-only up_vector doc is crisp

Good, the getter explains behavior and the property is explicitly documented as read-only.

---

659-725: Count properties: concise and consistent

The short docstrings on the various *_count properties improve discoverability without noise.

---

3973-3992: Finalize docs: improved end-to-end description

The expanded “Args/Returns/Notes” explains the lifecycle and what finalize() prepares on device memory. This is a solid step for API readers.

docs/generate_api.py (2)

99-101: Broaden “constant” detection to include common Python scalars and None.

Right now plain Python ints/floats/bools/bytes and None may still fall through to Functions depending on wp.types behavior. Consider folding them into Constants for more predictable docs.

Apply this minimal change to the predicate:

-        if wp.types.type_is_value(type(attr)) or isinstance(attr, str):
+        if (
+            wp.types.type_is_value(type(attr))
+            or isinstance(attr, (str, int, float, bool, complex, bytes, bytearray))
+            or attr is None
+        ):
             constants.append(name)
             continue

Optional (if you want Enum values to show under Constants too), add this small extension which requires an import outside this hunk:

# at top-level imports
import enum  # stdlib

-        if (
+        if (
             wp.types.type_is_value(type(attr))
-            or isinstance(attr, (str, int, float, bool, complex, bytes, bytearray))
+            or isinstance(attr, (str, int, float, bool, complex, bytes, bytearray, enum.Enum))
             or attr is None
         ):

---

185-196: Escape/quote constant values to avoid reST markup surprises and multiline table breaks.

Rendering raw strings (and other objects) can trigger unintended reST formatting (e.g., trailing underscores, backticks) or break list-tables on newlines. Wrap strings as literals and use repr() for non-scalar objects.

         for const in constants:
             value = getattr(module, const, "?")

             # unpack the warp scalar value, we can remove this
             # when the warp.types.scalar_base supports __str__()
             if type(value) in wp.types.scalar_types:
                 value = value.value

-            lines.extend(
-                [
-                    f"   * - {const}",
-                    f"     - {value}",
-                ]
-            )
+            # Format the cell safely for reST list-table:
+            # - Strings as literals, escape backticks/newlines
+            # - Plain Python scalars as-is
+            # - Everything else via repr() to avoid unintended markup
+            if isinstance(value, str):
+                safe = value.replace("`", "\\`").replace("\n", "\\n")
+                display_value = f"``{safe}``"
+            elif isinstance(value, (int, float, bool, complex)):
+                display_value = value
+            else:
+                display_value = repr(value)
+
+            lines.extend(
+                [
+                    f"   * - {const}",
+                    f"     - {display_value}",
+                ]
+            )

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

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

You can enable these sources in your CodeRabbit configuration.

Learnt from: shi-eric
PR: newton-physics/newton#521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The Newton migration guide (docs/migration.rst) is specifically for documenting how to migrate existing warp.sim functionality to Newton equivalents. New Newton-only features that didn't exist in warp.sim do not need migration documentation.

docs/generate_api.py (2)

99-101: Good call: treating strings as constants fixes misclassification (e.g., version).

This aligns the generated docs with expectations by moving string literals out of Functions. Looks good.

---

211-217: Ensure docs generation prerequisites are in place

Before running the API generator, make sure your environment has the warp module (and any other developer dependencies) installed. For example:

# In your project’s virtualenv:
pip install -e .
pip install -r requirements-dev.txt

Then regenerate the API pages and build the docs with warnings-as-errors to confirm Constants render correctly and no links break:

# Regenerate API pages
python docs/generate_api.py

# Verify that string constants (e.g. __version__) appear under “Constants”
rg -n '__version__' docs/api/newton.rst || { echo "Missing __version__ row"; exit 1; }

# Build the HTML docs, treating warnings as errors
sphinx-build -nW -b html docs docs/_build/html

Once you’ve verified the above steps complete without errors, you can be confident that the Constants section renders cleanly and that no broken references have been introduced.

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

31-33: Keeping Sphinx note directive is aligned with prior feedback

Thanks for retaining the .. note:: admonition; this matches the earlier review preference to have such notes stand out in the docs.

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

225-228: Bug: using or with Warp/NumPy arrays raises “ambiguous truth value”

If callers pass arrays for joint_q/output_residuals, these lines will error because arrays cannot be used in boolean contexts. Use explicit None checks.

Apply this change:

-        joint_q = joint_q or self.joint_q
-        output_residuals = output_residuals or self.residuals
+        if joint_q is None:
+            joint_q = self.joint_q
+        if output_residuals is None:
+            output_residuals = self.residuals

Note: A prior review already flagged this; queuing the fix for a follow-up PR is fine if this one is doc-only.

docs/_templates/class.rst (1)

8-8: De-duplicate :member-order: overrides in class templates
Centralize member ordering in docs/conf.py by removing the :member-order: groupwise lines from all class templates.

Affected files:

• docs/_templates/class.rst (line 8)
• docs/_templates/autosummary/class.rst (line 7)

Apply these diffs:

--- docs/_templates/class.rst
+++ docs/_templates/class.rst
@@ -6,7 +6,6 @@
 .. autoclass:: {{ objname }}
    :members:
    :inherited-members:
-   :member-order: groupwise

--- docs/_templates/autosummary/class.rst
+++ docs/_templates/autosummary/class.rst
@@ -5,7 +5,6 @@
 .. autoclass:: {{ objname }}
    :members:
    :inherited-members:
-   :member-order: groupwise

With those overrides removed, the single member-order setting in docs/conf.py will govern all classes, avoiding drift and making future global changes trivial.

docs/conf.py (2)

80-80: Nit: fix PyTorch intersphinx URL

The canonical URL is under pytorch.org. Using the non-canonical host can intermittently break cross-refs.

Apply:

-    "pytorch": ("https://docs.pytorch.org/docs/stable", None),
+    "pytorch": ("https://pytorch.org/docs/stable", None),

---

73-75: Consolidate duplicate exclude_patterns definitions

exclude_patterns is assigned twice; the latter overrides the former. Merge them to a single list to avoid confusion.

Apply:

 templates_path = ["_templates"]
-exclude_patterns = ["_build", "Thumbs.db", ".DS_Store"]
+exclude_patterns = [
+    "_build",
+    "Thumbs.db",
+    ".DS_Store",
+    "sphinx-env/**",
+    "sphinx-env",
+    "**/site-packages/**",
+    "**/lib/**",
+]
@@
-exclude_patterns = [
-    "sphinx-env/**",
-    "sphinx-env",
-    "**/site-packages/**",
-    "**/lib/**",
-]
+# (moved into the single exclude_patterns definition above)

Also applies to: 167-173

newton/_src/sim/contacts.py (4)

35-41: Validate maxima and surface clearer errors on construction

Guard against negative maxima early to prevent silent allocation of zero-length buffers or downstream index errors.

     def __init__(
         self,
         rigid_contact_max: int,
         soft_contact_max: int,
         requires_grad: bool = False,
         device: Devicelike = None,
     ):
+        if rigid_contact_max < 0 or soft_contact_max < 0:
+            raise ValueError(
+                f"Contact maxima must be non-negative, got rigid_contact_max={rigid_contact_max}, soft_contact_max={soft_contact_max}"
+            )
         with wp.ScopedDevice(device):

---

77-87: Consider resetting all per-contact vectors when clearing, or document why stale values are OK

clear() resets counts and index-like arrays, but leaves vectors/scalars (points, offsets, normals, thicknesses, body_pos/vel) unchanged. If any consumer assumes zeroed vectors after clear, stale values may leak into computations.

If intentional, add a brief note to the docstring clarifying that only indices are reset and that consumers must gate by contact_count. Otherwise, extend clear() to zero vectors:

         self.rigid_contact_force.zero_()
+        self.rigid_contact_point0.zero_()
+        self.rigid_contact_point1.zero_()
+        self.rigid_contact_offset0.zero_()
+        self.rigid_contact_offset1.zero_()
+        self.rigid_contact_normal.zero_()
+        self.rigid_contact_thickness0.zero_()
+        self.rigid_contact_thickness1.zero_()
@@
         self.soft_contact_tids.fill_(-1)
+        self.soft_contact_body_pos.zero_()
+        self.soft_contact_body_vel.zero_()
+        self.soft_contact_normal.zero_()

---

19-21: Unify Devicelike import to project alias for consistency

Model uses Devicelike from ..core.types; Contacts imports from warp.context. Recommend using the same alias source across the codebase.

-import warp as wp
-from warp.context import Devicelike
+import warp as wp
+from ..core.types import Devicelike

---

88-93: Device property is fine; optional: store device once for readability

Returning device from a representative array works. For readability and future-proofing, consider caching wp.get_device(device) into self._device during init and returning that here.

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

511-523: Collision pipeline lifecycle: verify requires_grad and device assumptions

Two potential gotchas:

• If an existing self._collision_pipeline was built with a different requires_grad than the current call/model, the code reuses it without adjustment.
• If an external collision_pipeline is passed on a different device than the model/state, this could misallocate Contacts buffers.

Consider rebuilding the pipeline (or error) when requires_grad or device mismatches occur.

Example approach:

         if collision_pipeline is not None:
-            self._collision_pipeline = collision_pipeline
+            self._collision_pipeline = collision_pipeline
+            # Optional guard:
+            if getattr(self._collision_pipeline, "device", None) not in (None, self.device):
+                raise RuntimeError("CollisionPipeline device mismatch with Model.device")
         elif not hasattr(self, "_collision_pipeline"):
             self._collision_pipeline = CollisionPipeline.from_model(
                 self,
                 rigid_contact_max_per_pair,
                 rigid_contact_margin,
                 soft_contact_max,
                 soft_contact_margin,
                 edge_sdf_iter,
                 iterate_mesh_vertices,
                 requires_grad,
             )
+        # If pipeline exists but requires_grad changed, rebuild it (or add a setter if supported).
+        if getattr(self._collision_pipeline, "requires_grad", requires_grad) != requires_grad:
+            self._collision_pipeline = CollisionPipeline.from_model(
+                self,
+                rigid_contact_max_per_pair,
+                rigid_contact_margin,
+                soft_contact_max,
+                soft_contact_margin,
+                edge_sdf_iter,
+                iterate_mesh_vertices,
+                requires_grad,
+            )

Also applies to: 524-545, 548-570

---

573-589: Validate frequency argument and error types in add_attribute

Docs state the allowed set for frequency, but it’s not enforced. Add a check to prevent invalid values from entering attribute_frequency. Also consider ValueError for invalid inputs (AttributeError is fine for “already exists”).

     def add_attribute(self, name: str, attrib: wp.array, frequency: str):
@@
-        if not isinstance(attrib, wp.array):
-            raise AttributeError(f"Attribute '{name}' must be an array, got {type(attrib)}")
+        if not isinstance(attrib, wp.array):
+            raise ValueError(f"Attribute '{name}' must be a wp.array, got {type(attrib)}")
         if attrib.device != self.device:
-            raise AttributeError(
+            raise ValueError(
                 f"Attribute '{name}' must be on the same device as the Model, expected {self.device}, got {attrib.device}"
             )
+        allowed = {"body", "joint", "joint_coord", "joint_dof", "shape"}
+        if frequency not in allowed:
+            raise ValueError(f"Invalid frequency '{frequency}'; expected one of {sorted(allowed)}")
@@
         setattr(self, name, attrib)
 
         self.attribute_frequency[name] = frequency

Also applies to: 593-599, 600-603

newton/_src/sim/ik.py (6)

47-68: Docstrings: great rewrite; minor dtype/shape notation nits for consistency

To match conventions used elsewhere in the file and the rest of Newton, suggest using explicit Warp dtypes and dimensionality in the Args section.

Apply this doc-only diff within the class docstring:

-        joint_q (wp.array2d(dtype=float)): Shape (n_problems, model.joint_coord_count). Initial joint coordinates, one row per problem. Modified in place.
-        objectives (Sequence[IKObjective]): Ordered list of objectives shared by all problems. Each `IKObjective` instance can carry arrays of per-problem parameters (e.g., an array of target positions of length `n_problems`).
+        joint_q (wp.array2d(dtype=wp.float32)): Shape (n_problems, model.joint_coord_count). Initial joint coordinates, one row per problem. Modified in place.
+        objectives (Sequence[IKObjective]): Ordered list of objectives shared by all problems. Each `IKObjective` instance can carry arrays of per-problem parameters (e.g., a wp.array1d of target positions of length `n_problems`).

---

864-869: Doc nit: use explicit Warp dtypes

Use wp.float32 and explicit dimensionality in the interface docs for consistency.

-            body_q (wp.array2d(dtype=wp.transform)): Array of body transforms for each problem.
-            joint_q (wp.array2d(dtype=wp.float)): Array of joint coordinates for each problem.
+            body_q (wp.array2d(dtype=wp.transform)): Array of body transforms for each problem.
+            joint_q (wp.array2d(dtype=wp.float32)): Array of joint coordinates for each problem.
-            residuals (wp.array2d(dtype=wp.float)): Output array for residuals.
+            residuals (wp.array2d(dtype=wp.float32)): Output array for residuals.

---

879-882: Doc nit: clarify Jacobian and dq_dof array types

Align with the rest of the file’s dtype notation.

-            jacobian (wp.array3d(dtype=wp.float)): Output array for the Jacobian.
+            jacobian (wp.array3d(dtype=wp.float32)): Output array for the Jacobian.
-            dq_dof (wp.array2d(dtype=wp.float)): Derivative of q with respect to DoF.
+            dq_dof (wp.array2d(dtype=wp.float32)): Derivative of q with respect to DoF.

---

916-922: Doc nit: analytic path dtype/shape

Tiny consistency fix.

-            joint_q (wp.array2d(dtype=wp.float)): Array of joint coordinates for each problem.
-            jacobian (wp.array3d(dtype=wp.float)): Output array for the Jacobian.
+            joint_q (wp.array2d(dtype=wp.float32)): Array of joint coordinates for each problem.
+            jacobian (wp.array3d(dtype=wp.float32)): Output array for the Jacobian.

---

1027-1035: Doc nit: target_positions should be 1D

Reflect that targets are one per problem.

-        target_positions (wp.array(dtype=wp.vec3)): One target position per problem.
+        target_positions (wp.array1d(dtype=wp.vec3)): One target position per problem.

---

1474-1485: Doc nit: clarify target_rotations dimensionality

Keep array dimensionality explicit and consistent with position objective.

-        target_rotations (wp.array(dtype=wp.vec4)): One target quaternion per problem (stored as vec4).
+        target_rotations (wp.array1d(dtype=wp.vec4)): One target quaternion per problem (stored as vec4).

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

318-354: Avoid per-frame allocations in _render_picking_line by reusing tiny buffers.

Allocating three new wp.arrays every frame while picking is active adds avoidable GPU/host churn. Cache 1-element arrays and just write values.

Apply this localized diff:

-        # Create line data
-        line_begins = wp.array([com_position], dtype=wp.vec3, device=self.device)
-        line_ends = wp.array([pick_target], dtype=wp.vec3, device=self.device)
-        line_colors = wp.array([wp.vec3(0.0, 1.0, 1.0)], dtype=wp.vec3, device=self.device)
+        # Create or reuse 1-element buffers
+        if not hasattr(self, "_picking_line_buffers"):
+            self._picking_line_buffers = {
+                "beg": wp.empty(1, dtype=wp.vec3, device=self.device),
+                "end": wp.empty(1, dtype=wp.vec3, device=self.device),
+                "col": wp.empty(1, dtype=wp.vec3, device=self.device),
+            }
+            self._picking_line_buffers["col"].fill_(wp.vec3(0.0, 1.0, 1.0))
+        line_begins = self._picking_line_buffers["beg"]
+        line_ends = self._picking_line_buffers["end"]
+        line_begins[0] = com_position
+        line_ends[0] = pick_target

---

272-291: Normalize log_points inputs to wp.arrays and validate shapes.

log_points assumes points/widths/colors are already wp.arrays with compatible lengths. Add light input normalization to reduce caller footguns.

Apply this diff:

         if self._point_mesh is None:
             self._create_point_mesh()
 
         if name not in self.objects:
             self.objects[name] = MeshInstancerGL(len(points), self._point_mesh)
 
-        self.objects[name].update_from_points(points, widths, colors)
+        # Normalize inputs
+        if not isinstance(points, wp.array):
+            points = wp.array(points, dtype=wp.vec3, device=self.device)
+        if not isinstance(widths, wp.array):
+            widths = wp.array(widths, dtype=float, device=self.device)
+        if not isinstance(colors, wp.array):
+            colors = wp.array(colors, dtype=wp.vec3, device=self.device)
+        assert len(points) == len(widths) == len(colors), "points/widths/colors must have same length"
+
+        self.objects[name].update_from_points(points, widths, colors)
         self.objects[name].hidden = hidden

---

473-529: Key mapping covers left modifiers only; consider RSHIFT/RCTRL/RALT for completeness.

Optional for parity with common input schemes; not blocking.

---

142-156: Remove redundant device init and normalize up-axis

• The device attribute is already initialized in ViewerBase.__init__ (newton/_src/viewer/viewer.py line 36), so reassigning it in set_model() is unnecessary and may introduce confusion.
• Ensure that the up_axis passed to Camera is always one of the strings "X", "Y", or "Z", even if model.up_axis is stored as an integer.

Apply this diff:

     def set_model(self, model):
         """
         Set the Newton model to visualize.

         Args:
             model: The Newton model instance.
         """
-        super().set_model(model)
+        super().set_model(model)

         self.picking = Picking(model, pick_stiffness=10000.0, pick_damping=1000.0)
         self.wind = Wind(model)

         fb_w, fb_h = self.renderer.window.get_framebuffer_size()
-        self.camera = Camera(width=fb_w, height=fb_h, up_axis=model.up_axis if model else "Z")
+        # Normalize up_axis to ensure Camera always receives "X"/"Y"/"Z"
+        if model and isinstance(model.up_axis, int):
+            up_str = ["X", "Y", "Z"][model.up_axis]
+        else:
+            up_str = model.up_axis if model and isinstance(model.up_axis, str) else "Z"
+        self.camera = Camera(width=fb_w, height=fb_h, up_axis=up_str)

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

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

You can enable these sources in your CodeRabbit configuration.

Learnt from: shi-eric
PR: newton-physics/newton#521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The Newton migration guide (docs/migration.rst) is specifically for documenting how to migrate existing warp.sim functionality to Newton equivalents. New Newton-only features that didn't exist in warp.sim do not need migration documentation.

Learnt from: shi-eric
PR: newton-physics/newton#521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The Newton migration guide (docs/migration.rst) is specifically for documenting how to migrate existing warp.sim functionality to Newton equivalents. New Newton-only features that didn't exist in warp.sim do not need migration documentation.

docs/conf.py (1)

119-119: LGTM: Global autodoc member-order switched to groupwise

This aligns the docs to grouped member ordering. Once the template override is removed, conf.py becomes the single source of truth.

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

30-50: Docstring overhaul reads well and clarifies construction/usage

Clear guidance on static model semantics, grouping, and ModelBuilder recommendation. Good improvement in discoverability.

---

76-96: API docs polish across core attributes looks consistent

The added “used by …” hints and group-index explanations will help users navigate solver expectations. No functional issues noted.

Also applies to: 145-152, 184-185, 208-212, 227-233, 279-287, 291-296, 297-304, 313-316, 362-366, 371-374

---

440-474: state(): cloning semantics are appropriate and coherent with requires_grad

Cloning only when counts are non-zero and propagating requires_grad from the model by default looks correct.

---

476-509: control(): clone_variables behavior is well-documented and implemented

Using counts to gate cloning and allowing reference sharing is sensible. No issues.

---

605-627: get_attribute_frequency(): interface and errors are clear

Good, explicit error when unknown; aligns with add_attribute behavior.

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

26-38: Nice: clear IKJacobianMode docs and explicit MIXED semantics

The added enum and per-member docstrings are crisp and remove ambiguity about back-end selection. No issues.

---

128-128: Correct requires_grad gating for AD/MIXED

Using grad = jacobian_mode in (IKJacobianMode.AUTODIFF, IKJacobianMode.MIXED) correctly scopes tape-enabled allocations to only when autodiff is needed. Looks good.

---

154-157: Good: allocate motion subspace only when analytically needed

Conditionally creating joint_S_s when not AUTODIFF and at least one objective supports analytic keeps memory use in check. LGTM.

---

167-176: Per-objective Jacobian mode selection is appropriate

In MIXED mode, passing ANALYTIC vs AUTODIFF to each objective’s init_buffers() based on supports_analytic() is the right call and avoids over-allocating buffers. Nice.

---

212-220: Helpful: solve() doc clarifies side effects

The Side Effects section calling out in-place updates of self.joint_q improves usability. No issues.

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

36-53: Solid class-level docstring — clear feature inventory and intent.

Good high-level overview that aligns with the public viewer docs and helps new users understand capabilities.

---

128-141: create_sphere_mesh always returns 8-column vertices; no guard needed

The create_sphere_mesh helper is defined to emit a float32 array of shape (N, 8) ― [x,y,z, nx,ny,nz, u,v] ― in both its docstring and implementation, so indexing into vertices[:,3:6] and vertices[:,6:8] is safe as-is. Since there’s no code path that omits normals or UVs, the suggested defensive branches aren’t required at this time. If we later introduce variants that drop normals/UVs, we can revisit and add guards then.

---

744-816: No change needed for up-axis display
The viewer’s use of self.model.up_axis safely handles only integer indices because:

• In ModelBuilder.__init__, self.up_axis is always converted via Axis.from_any(up_axis) into an Axis IntEnum, even if the user passed in a string.
• In finalize(), that enum value is assigned directly to the Model.up_axis field.
• An Axis IntEnum is a subclass of int, so indexing ["X","Y","Z"][self.model.up_axis] will never raise an IndexError.

No refactoring is required.

Likely an incorrect or invalid review comment.