newton/tests/test_recorder.py (1)

107-121: Add playback round-trip assertions in test_model_and_state_recorder

After you compare the serialized state blobs, instantiate a fresh state and call new_recorder.playback for each frame to verify that Warp arrays are restored with the correct structured dtypes and on the intended device.

• File: newton/tests/test_recorder.py
• Location: inside test_model_and_state_recorder, immediately after the existing history comparisons

Suggested diff:

         for original_state_data, loaded_state_data in zip(recorder.history, new_recorder.history):
             _compare_serialized_data(test, original_state_data, loaded_state_data)
+
+        # Round-trip playback: verify dtype & device restoration
+        for i in range(len(states)):
+            restored_state = model.state()
+            new_recorder.playback(restored_state, i)
+            # body_q should be a wp.transform array on the correct device
+            test.assertIsInstance(restored_state.body_q, wp.array)
+            test.assertEqual(restored_state.body_q.dtype, wp.transform.dtype)
+            test.assertEqual(restored_state.body_q.device, device)
+            # body_qd should be a wp.spatial_vector array on the correct device
+            test.assertEqual(restored_state.body_qd.dtype, wp.spatial_vector.dtype)
+            test.assertEqual(restored_state.body_qd.device, device)

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

176-205: Playback cannot reliably restore wp dtypes; nested collections aren’t serialized; empty arrays dropped.

• record() stores raw numpy arrays, losing dtype info (e.g., wp.transform, wp.spatial_vector).
• playback() reconstructs wp arrays without dtype, risking incorrect types.
• _serialize_value() returns collections as-is (no recursion), so nested wp arrays aren’t handled.
• Zero-length wp arrays are discarded (returned None).

Recommend serializing wp arrays with dtype metadata and recursively handling collections; mirror that in deserialization and use it in record()/playback().

Here’s a concrete refactor:

@@
 class ModelAndStateRecorder:
@@
-    def _serialize_value(self, value):
+    def _serialize_value(self, value):
@@
-        if value is None:
-            return None
-
-        if isinstance(value, (int, float, bool, str, list, dict, set, tuple)):
-            return value
-        elif isinstance(value, np.ndarray):
-            return value
-        elif isinstance(value, wp.array):
-            if value.size > 0:
-                return {
-                    "__type__": "wp.array",
-                    "data": value.numpy(),
-                }
-        return None
+        if value is None:
+            return None
+        # Primitives
+        if isinstance(value, (int, float, bool, str, bytes, bytearray, complex)):
+            return value
+        # Numpy arrays are pickle-able as-is
+        if isinstance(value, np.ndarray):
+            return value
+        # Warp arrays: always serialize (even if size==0) with dtype metadata
+        if isinstance(value, wp.array):
+            dtype_name = getattr(value.dtype, "__name__", str(value.dtype))
+            return {"__type__": "wp.array", "dtype": dtype_name, "data": value.numpy()}
+        # Collections (recursive)
+        if isinstance(value, dict):
+            return {k: self._serialize_value(v) for k, v in value.items()}
+        if isinstance(value, list):
+            return [self._serialize_value(v) for v in value]
+        if isinstance(value, tuple):
+            return tuple(self._serialize_value(v) for v in value)
+        if isinstance(value, set):
+            return {self._serialize_value(v) for v in value}
+        # Fallback: not serializable
+        return None
@@
-    def _deserialize_and_restore_value(self, value, device):
+    def _deserialize_and_restore_value(self, value, device):
@@
-        if isinstance(value, dict) and "__type__" in value:
-            type_name = value["__type__"]
-            obj_data = value["data"]
-
-            if type_name == "wp.array":
-                return wp.array(obj_data, device=device)
-        if isinstance(value, np.ndarray):
-            return value
-        return value
+        # Rehydrate serialized wp arrays
+        if isinstance(value, dict) and "__type__" in value:
+            type_name = value.get("__type__")
+            if type_name == "wp.array":
+                obj_data = value.get("data")
+                dtype_name = value.get("dtype")
+                # Map dtype name back to wp dtype if possible
+                dtype = getattr(wp, dtype_name, None) if isinstance(dtype_name, str) else None
+                if dtype is not None:
+                    return wp.array(obj_data, dtype=dtype, device=device)
+                return wp.array(obj_data, device=device)
+        # Collections (recursive)
+        if isinstance(value, dict):
+            return {k: self._deserialize_and_restore_value(v, device) for k, v in value.items()}
+        if isinstance(value, list):
+            return [self._deserialize_and_restore_value(v, device) for v in value]
+        if isinstance(value, tuple):
+            return tuple(self._deserialize_and_restore_value(v, device) for v in value)
+        if isinstance(value, set):
+            return {self._deserialize_and_restore_value(v, device) for v in value}
+        # Numpy arrays and primitives
+        return value
@@
-    def record(self, state: State):
+    def record(self, state: State):
@@
-        state_data = {}
-        for name, value in state.__dict__.items():
-            if isinstance(value, wp.array):
-                state_data[name] = value.numpy()
-        self.history.append(state_data)
+        state_data = {}
+        for name, value in state.__dict__.items():
+            if isinstance(value, wp.array) and value is not None:
+                state_data[name] = self._serialize_value(value)
+        self.history.append(state_data)
@@
-    def playback(self, state: State, frame_id: int):
+    def playback(self, state: State, frame_id: int):
@@
-        for name, value_np in state_data.items():
-            if hasattr(state, name):
-                value_wp = wp.array(value_np, device=device)
-                setattr(state, name, value_wp)
+        for name, serialized in state_data.items():
+            if hasattr(state, name):
+                value_wp = self._deserialize_and_restore_value(serialized, device)
+                if isinstance(value_wp, wp.array):
+                    setattr(state, name, value_wp)

This preserves dtype during round-trips, handles empty arrays, and supports nested structures.

Also applies to: 206-229, 230-242, 253-276

newton/tests/test_equality_constraints.py (1)

43-56: Add MuJoCo availability guard in TestEqualityConstraints

The new test instantiates newton.solvers.SolverMuJoCo and calls newton.eval_fk, which will error in environments without MuJoCo. To keep CI green, skip this entire test class when MuJoCo isn’t installed.

• File: newton/tests/test_equality_constraints.py
Wrap the class with an availability check:

@@
 import newton
+try:
+    import mujoco  # noqa: F401
+    _HAS_MUJOCO = True
+except ImportError:
+    _HAS_MUJOCO = False
@@
-class TestEqualityConstraints(unittest.TestCase):
+@unittest.skipUnless(_HAS_MUJOCO, "MuJoCo not available")
+class TestEqualityConstraints(unittest.TestCase):
     def setUp(self):
         …

docs/concepts/articulations.rst (1)

21-66: Mismatch between documentation and JointType enum: missing COMPOUND/UNIVERSAL members

The docs still list legacy JOINT_COMPOUND and JOINT_UNIVERSAL, but the JointType enum in newton/_src/sim/joints.py only defines:

• PRISMATIC
• REVOLUTE
• BALL
• FIXED
• FREE
• DISTANCE
• D6

You must choose one of the following fixes:

• Add COMPOUND and UNIVERSAL to the JointType enum (with the correct integer values) so that JointType.COMPOUND and JointType.UNIVERSAL are valid, then update the docs to use those enum members.
• Remove or update the two entries in docs/concepts/articulations.rst for compound and universal joints to match the supported joint types.

Files to update:

• docs/concepts/articulations.rst (remove or rename lines 46–51)
• newton/_src/sim/joints.py (if adding new enum members)

newton/_src/solvers/style3d/kernels.py (1)

206-217: Potential Warp-kernel mis-compile around flag test

if not particle_flags[tid] & ParticleFlags.ACTIVE:

1. not converts the bitwise result to Python bool; Warp’s Python-to-CUDA translation has historically been brittle with Python boolean operators inside kernels.
2. The array was switched to wp.int32, but many call-sites still pass wp.uint32. A silent dtype mismatch will raise at launch time.

Safer, explicit form:

flag = particle_flags[tid] & ParticleFlags.ACTIVE
if flag == 0:
    ...

and keep particle_flags as the same unsigned type used elsewhere (or update all call-sites).

newton/_src/solvers/euler/particles.py (1)

88-96: Guard against division by zero when particles coincide

If two active particles occupy the same position, d can be 0 and n/d will NaN. Add a small epsilon check before normalizing.

-            err = d - radius - particle_radius[index]
+            err = d - radius - particle_radius[index]
@@
-            if err <= k_cohesion:
-                n = n / d
+            if err <= k_cohesion:
+                # avoid normalization when particles coincide
+                if d <= 1.0e-9:
+                    continue
+                n = n / d

newton/_src/solvers/featherstone/solver_featherstone.py (1)

500-503: Hard-coded DOF=18 assertions break general models

These asserts should use self.dof_count, not 18. As-is, any non-18-DOF articulation will assert.

-                            assert H_tiled.shape == (model.articulation_count, 18, 18)
-                            assert L_tiled.shape == (model.articulation_count, 18, 18)
-                            assert R_tiled.shape == (model.articulation_count, 18)
+                            assert H_tiled.shape == (model.articulation_count, self.dof_count, self.dof_count)
+                            assert L_tiled.shape == (model.articulation_count, self.dof_count, self.dof_count)
+                            assert R_tiled.shape == (model.articulation_count, self.dof_count)

If articulations can differ in structure, also validate/guard the earlier assumption “all articulations have the same structure”.

newton/_src/solvers/solver.py (1)

49-49: Gravity term masked incorrectly for particles

Using wp.step(-inv_mass) applies gravity when inv_mass <= 0 (immobile), which is inverted. Match the rigid-body path and gate with nonzero(inv_mass).

-    v1 = v0 + (f0 * inv_mass + gravity * wp.step(-inv_mass)) * dt
+    v1 = v0 + (f0 * inv_mass + gravity * wp.nonzero(inv_mass)) * dt

Optionally: wp.step(inv_mass) also works if step(0)=0 on your Warp version; nonzero mirrors integrate_rigid_body.

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

132-138: Case mapping bug: 'meshSimplification' will never match after .lower()

approximation.lower() yields 'meshsimplification', but the mapping key is 'meshSimplification'. This silently disables the intended remeshing path.

     approximation_to_remeshing_method = {
         "convexdecomposition": "coacd",
         "convexhull": "convex_hull",
         "boundingsphere": "bounding_sphere",
         "boundingcube": "bounding_box",
-        "meshSimplification": "quadratic",
+        "meshsimplification": "quadratic",
     }

newton/examples/example_selection_articulations.py (1)

117-125: Fix RendererOpenGL up_axis type (should be Axis/None, not str)

RendererOpenGL expects up_axis as an Axis-like enum (or None), not a string. Passing str(up_axis) can lead to misconfiguration.

Apply this diff:

-            self.renderer = newton.viewer.RendererOpenGL(
-                path=stage_path,
-                model=self.model,
-                scaling=2.0,
-                up_axis=str(up_axis),
-                screen_width=1280,
-                screen_height=720,
-                camera_pos=(0, 4, 30),
-            )
+            self.renderer = newton.viewer.RendererOpenGL(
+                path=stage_path,
+                model=self.model,
+                scaling=2.0,
+                up_axis=up_axis,
+                screen_width=1280,
+                screen_height=720,
+                camera_pos=(0, 4, 30),
+            )

newton/examples/example_selection_materials.py (1)

88-96: Pass Axis enum, not string, to RendererOpenGL

RendererOpenGL expects an Axis enum (or None), not str(up_axis). Passing a string can break axis handling and camera orientation.

Apply:

-                up_axis=str(up_axis),
+                up_axis=up_axis,

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

941-959: Possible illegal in-place element write on vec10f

actuator_bias[worldid, actuator_idx][1] = -kp (and similar writes) mutates a single element of a vector stored inside a Warp array.
Warp only supports arr[idx].x = … style field writes; indexed writes ([...]) are not currently allowed and will raise a code-gen error. Safest pattern:

bias   = actuator_bias[worldid, actuator_idx]
bias[1] = -kp
bias[2] = -kv
actuator_bias[worldid, actuator_idx] = bias

Apply the same pattern for actuator_gain.

---

1871-1877: Graph-colour allocation limited to 32 collision groups

if color < 32: adds every new colour bit into a 32-bit mask.
When more than 32 colour classes are produced, shapes silently fall into the “collide with everything” bucket, defeating the filter. Consider:

• upgrading to a 64-bit mask, or
• detecting overflow and raising a clear error.

This avoids extremely hard-to-trace contact explosions on big scenes.

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

569-640: Repeat of the if-chain / shadowing pattern in IK kernel

The same comments about type shadowing and the missing elifs apply here. Consider the same refactor for consistency.

newton/tests/test_recorder.py (1)

24-24: Using internal _src import in tests is acceptable here; consider planning a public re-export.

Per team practice, testing private APIs is fine. Longer term, expose BasicRecorder/ModelAndStateRecorder via the public API to avoid test fragility on internal paths.

I can draft the public re-export (e.g., newton.utils.recorder or newton.recorder) and update tests. Want me to open an issue/PR?

newton/tests/test_broad_phase.py (1)

22-22: Private-API import in tests is acceptable.

Swap to _src path is fine and keeps the test compiling with the refactor.

Consider adding a TODO to switch back to the public façade once the constant is re-exported there, to avoid future private-API coupling.

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

40-41: Resolve outstanding FIXME before merge

The new comment highlights uncertainty about exposing MESH_MAXHULLVERT. Since the constructor already accepts maxhullvert, keeping a module-level mutable constant can be confusing. Consider:

1. Removing the constant entirely and hard-coding the default in the Mesh signature, or
2. Promoting the constant to the public API and documenting how users should adjust it.

Clarifying this before the public refactor freezes will avoid future breaking changes.

newton/tests/test_raycast.py (1)

20-27: GeoType re-export confirmed; consider public exports for raycast

• Verified that GeoType is listed in newton/__init__.py (lines 28 & 64), so importing it from newton is correct.
• All five ray_intersect_* functions live in newton/_src/geometry/raycast.py (defs at lines 41, 85, 133, 224, 294). Testing them directly is fine, but you may want to add public re-exports (for example under newton.geometry.raycast or newton.raycast) to avoid private _src imports in the long term.

resolve_review_comment

docs/conventions.rst (1)

84-90: Use consistent “Solver” naming and add cross-refs*

Align with the new naming scheme in this list and link the MuJoCo solver:

-* **Standard Newton solvers** (XPBD, Euler, SolverMuJoCo etc.)  
+* **Standard Newton solvers** (SolverXPBD, SolverSemiImplicit, :class:`newton.solvers.SolverMuJoCo`, etc.)  
@@
-  convert from this convention to MuJoCo's mixed-frame format when using the SolverMuJoCo.
+  convert from this convention to MuJoCo's mixed-frame format when using :class:`newton.solvers.SolverMuJoCo`.

newton/_src/solvers/featherstone/__init__.py (1)

16-20: Re-export rename LGTM; consider a temporary compat alias

The rename to SolverFeatherstone matches the new convention. If you want a smoother migration for internal imports, add a soft alias (optional):

 from .solver_featherstone import SolverFeatherstone
 
 __all__ = [
     "SolverFeatherstone",
 ]
+
+# Optional: backward-compat alias (remove in a future release)
+# from warnings import warn as _warn
+# _warn("FeatherstoneSolver is deprecated; use SolverFeatherstone", DeprecationWarning, stacklevel=2)
+FeatherstoneSolver = SolverFeatherstone
+# __all__.append("FeatherstoneSolver")  # only if you want it in star-imports

newton/_src/solvers/euler/__init__.py (1)

16-20: Rename looks good; consider adding a temporary alias for smoother migration.

To reduce churn for downstreams, optionally provide a deprecated alias:

 from .solver_euler import SolverSemiImplicit
 
 __all__ = [
     "SolverSemiImplicit",
 ]
+
+# Optional backwards-compat alias (deprecate in a future release)
+SemiImplicitSolver = SolverSemiImplicit
+# If you want to expose it, also:
+# __all__.append("SemiImplicitSolver")

newton/_src/solvers/vbd/__init__.py (1)

16-20: Rename to SolverVBD looks good; consider a temporary alias.

Optionally provide an alias for downstream compatibility:

 from .solver_vbd import SolverVBD
 
 __all__ = [
     "SolverVBD",
 ]
+
+# Optional backwards-compat alias (deprecate in a future release)
+VBDSolver = SolverVBD
+# If you want to expose it, also:
+# __all__.append("VBDSolver")

newton/_src/solvers/euler/solver_euler.py (2)

84-92: Type-hint control as optional

control may be None at call sites and is handled as such inside. Reflect this in the signature for clarity and typing tools.

-    def step(
+    def step(
         self,
         state_in: State,
         state_out: State,
-        control: Control,
+        control: Control | None,
         contacts: Contacts,
         dt: float,
     ):

---

138-139: Remove or gate dead code for muscles

if False: is dead code. Either delete it or guard with a feature flag to keep intent clear.

-            # muscles
-            if False:
-                eval_muscle_forces(model, state_in, control, body_f)
+            # muscles (enable when muscle dynamics are supported)
+            # if self.enable_muscles:
+            #     eval_muscle_forces(model, state_in, control, body_f)

docs/api/newton_geometry.rst (1)

63-70: Prefer documenting enum classes over duplicating numeric values

Using ParticleFlags.ACTIVE and ShapeFlags.* is clearer. To avoid numeric drift, consider documenting the enum classes directly (e.g., with .. autoclass:: ParticleFlags and .. autoclass:: ShapeFlags plus :members:) instead of hardcoding numeric values in a list-table.

newton/selection.py (1)

16-20: Add a brief module docstring for Sphinx discoverability

Consider adding a top-level docstring describing this as the public interface for selection utilities and briefly summarizing ArticulationView usage.

Example:

"""Public selection utilities.

Exposes:
- ArticulationView: Select and manipulate subsets of articulations, joints, and links in a Model.
"""

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

27-31: Relative imports align with the new private layout; consider simplifying to a single module import for consistency.

Right now both the module and a symbol are imported from geometry:

• from .. import geometry
• from ..geometry import Mesh

To reduce ambiguity and ease refactors, prefer importing the module only and referring to geometry.Mesh at use sites.

Apply this diff:

-from .. import geometry
-from ..core import Axis, AxisType, quat_between_axes
-from ..core.types import Transform
-from ..geometry import Mesh
-from ..sim import ModelBuilder
+from .. import geometry
+from ..core import Axis, AxisType, quat_between_axes
+from ..core.types import Transform
+from ..sim import ModelBuilder

And update Mesh constructions below (also in Lines 220 and 232):

-                        m_mesh = Mesh(m_vertices, m_faces, maxhullvert=geometry.MESH_MAXHULLVERT)
+                        m_mesh = geometry.Mesh(m_vertices, m_faces, maxhullvert=geometry.MESH_MAXHULLVERT)

newton/tests/test_ik.py (2)

25-25: Using the private IK module in tests is acceptable here; consider adding a public-API smoke test.

Per team practice, testing private APIs is OK. To guard the public surface, add a tiny smoke test that imports from newton.ik and instantiates the renamed classes.

I can draft an additional test to import newton.ik and construct IKPositionObjective, IKRotationObjective, and IKJointLimitObjective to ensure they’re re-exported.

---

278-280: Prefer keyword arguments for readability and future-proofing.

These two calls use positional args where there are many parameters. Using keywords reduces ordering mistakes and helps future refactors.

Apply this diff as an example:

-        pos_obj = ik.IKPositionObjective(0, wp.vec3(0.0, 0.0, 0.0), pos_targets, n_problems, 6, 0)
-        rot_obj = ik.IKRotationObjective(0, wp.quat_identity(), rot_targets, n_problems, 6, 3)
+        pos_obj = ik.IKPositionObjective(
+            link_index=0,
+            link_offset=wp.vec3(0.0, 0.0, 0.0),
+            target_positions=pos_targets,
+            n_problems=n_problems,
+            total_residuals=6,
+            residual_offset=0,
+        )
+        rot_obj = ik.IKRotationObjective(
+            link_index=0,
+            link_offset_rotation=wp.quat_identity(),
+            target_rotations=rot_targets,
+            n_problems=n_problems,
+            total_residuals=6,
+            residual_offset=3,
+        )

Also applies to: 419-421

docs/guide/quickstart.rst (1)

36-37: Keep terminology consistent

The sentence still mentions “OpenGLRenderer”. Everywhere else the class is now called RendererOpenGL; consider updating the prose to avoid reader confusion.

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

23-24: Circular alias can confuse public API

SimRenderer is now just an alias of RendererUsd, but exporting all three names (RendererOpenGL, RendererUsd, and SimRenderer) may mislead newcomers into thinking they are three distinct back-ends.
Consider deprecating SimRenderer (e.g., with warnings.warn("SimRenderer is deprecated …")) or dropping it from __all__ in the next major release.

---

114-116: __all__ order nit

Purely stylistic: keep list alphabetically sorted to avoid merge conflicts (RendererUsd before RendererOpenGL).

newton/examples/example_mujoco.py (3)

247-256: Variable shadowing can obscure intent

Parameter solver is reused as a local variable passed into SolverMuJoCo(solver=solver, …).
Because it masks the earlier imported module namespace (newton.solvers), future edits may accidentally shadow more attributes.
Renaming the arg to solver_backend (or similar) mitigates this risk.

-        solver = solver if solver is not None else ROBOT_CONFIGS[robot]["solver"]
+        solver_backend = solver if solver is not None else ROBOT_CONFIGS[robot]["solver"]
 ...
-        self.solver = newton.solvers.SolverMuJoCo(
+        self.solver = newton.solvers.SolverMuJoCo(
             self.model,
             use_mujoco=use_mujoco,
-            solver=solver_backend,
+            solver=solver_backend,

---

259-260: Missing explicit import for newton.viewer

Unlike newton.utils, the viewer sub-module is not imported at the file top.
If newton/__init__.py ever stops auto-exposing it, this line will break. A one-liner import keeps the script robust:

import newton.viewer

---

360-362: Help string stale after rename

The CLI help still references MuJoCo viewer “when SolverMuJoCo is active.” Fine, but you renamed the flag earlier; just sanity-check that other docs/examples spell the class the same way (SolverMuJoCo).

newton/_src/solvers/xpbd/solver_xpbd.py (1)

18-20: Relative import depth fragile

from ...core.types import override relies on the package remaining exactly three levels deep.
If xpbd gets nested further (xpbd.v2 etc.) the import will break.
Using absolute import (from newton._src.core.types import override) avoids this class of errors.

newton/_src/solvers/featherstone/kernels.py (1)

128-175: Consider renaming the type parameter and typing it as JointType

type shadows the Python builtin and is annotated as plain int, yet throughout the file it is compared against newton.JointType.* (an IntEnum).
Changing the signature to something like

def jcalc_transform(
    joint_type: newton.JointType,
    ...
):

(and similarly for the other helpers) removes the shadowing, documents the expected domain, and lets static checkers catch mismatched callers. Warp kernels accept IntEnum values transparently (IntEnum inherits int), so this is a zero-cost refactor.

Also applies to: 239-299, 420-469, 523-536

newton/tests/test_import_urdf.py (1)

176-182: Mock path couples the test to a private module location

patch("newton._src.utils.import_urdf._download_file", …) reaches into the private _src tree.
If the implementation is later relocated, the test will break while public behaviour is unchanged. Patching via the public symbol exposed through newton.utils.import_urdf (after importing it) would decouple the test from internal layout.

with patch(f"{parse_urdf.__module__}._download_file", side_effect=mock_mesh_download):
    ...

newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (1)

1164-1169: Avoid shadowing the collider variable inside the list-comprehension

collider is first bound to the Collider() struct and then immediately re-used as the loop variable:

collider.meshes = wp.array([collider.id for collider in colliders], dtype=wp.uint64)

Using the same identifier makes the code harder to read and can easily lead to bugs when the outer binding is referenced later. Rename the comprehension variable, e.g.:

- collider.meshes = wp.array([collider.id for collider in colliders], dtype=wp.uint64)
+ collider.meshes = wp.array([mesh.id for mesh in colliders], dtype=wp.uint64)

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

205-249: Use elif and avoid shadowing built-ins

Inside the loop type shadows Python’s built-in type() and every branch is written as an independent if, so each subsequent comparison is evaluated even after a match.

type = joint_type[i]
...
if type == JointType.PRISMATIC:
    ...
if type == JointType.REVOLUTE:
    ...

Rename the variable (e.g., jt) and chain with elif to improve clarity and save cycles:

jt = joint_type[i]
if jt == JointType.PRISMATIC:
    ...
elif jt == JointType.REVOLUTE:
    ...

newton/_src/solvers/euler/particles.py (1)

82-84: Remove unused 'count' to reduce register pressure

count is incremented but never used.

-    count = int(0)
+    # count removed (unused)
@@
-            count += 1
+            # increment removed (unused)

Also applies to: 91-92

newton/examples/example_ik_benchmark.py (2)

225-231: Avoid private API in example: replace _fk_two_pass with a public helper

This example calls solver._fk_two_pass, which is private and may change without notice. Consider exposing a public batched FK in newton (e.g., eval_fk_batched) or adding a public method on IKSolver that wraps this call.

I can draft a minimal public wrapper (e.g., ik.fk_two_pass) to maintain performance and stability. Want a patch?

---

215-216: Reduce side-effects in _fk_targets

Instead of mutating model.joint_q each loop, pass a temporary q array directly to eval_fk.

-            wp.copy(self.model.joint_q, wp.array(q, dtype=wp.float32))
-            newton.eval_fk(self.model, self.model.joint_q, self.model.joint_qd, state)
+            q_d = wp.array(q, dtype=wp.float32)
+            newton.eval_fk(self.model, q_d, self.model.joint_qd, state)

If allocation overhead matters, preallocate q_d once outside the loop and reuse.

newton/_src/solvers/solver.py (1)

263-263: Docstring type: Contact → Contacts

Parameter type name is inconsistent with the import and usage.

-            contacts (Contact): The contact information.
+            contacts (Contacts): The contact information.

newton/examples/example_selection_cartpole.py (1)

109-111: Type-check can be simplified

self.solver is always an instance of SolverMuJoCo (created on Line 82). The isinstance guard will therefore be false for all other solver types but true here, making the FK bypass unconditional. Consider flipping the logic or comparing against an abstract base (SolverBase) to future-proof this path.

newton/tests/test_model.py (1)

169-170: Make joint-type assertion explicit

builder.joint_type stores raw ints. Relying on IntEnum == int coercion is implicit and brittle. Cast for clarity:

assert list(map(newton.JointType, builder.joint_type)) == [
    newton.JointType.REVOLUTE,
    newton.JointType.FREE,
]

newton/ik.py (1)

16-24: Consider lazy re-export to keep import cost minimal

Importing the heavy _src.sim.ik module unconditionally adds overhead for users who never touch IK. A __getattr__-based lazy loader (PEP 562) would defer that cost until first use.

Not critical for correctness but worth considering.

newton/tests/test_ik_fk_kernels.py (2)

24-26: Prefer public newton.ik import over private path

import newton.ik as ik

keeps the test aligned with the new public surface while still allowing access to private helpers when necessary.

---

185-200: Consistent enum reference style

Within the same function both JointType.* and newton.JointType.* are used. Pick one style (ideally the imported alias) for readability.

elif jt == JointType.D6:
    ...
elif jt == JointType.FREE:
    ...

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

23-23: Avoid name shadowing with the class method eval_fk.

Importing eval_fk into the module namespace and then defining an ArticulationView.eval_fk() method with the same identifier is legal but confusing.
Consider importing the function under an alias (e.g., from ..sim import eval_fk as _eval_fk) or renaming the method to make the call site intention crystal-clear.
This prevents readers from having to double-check Python’s LEGB lookup rules.

---

355-360: Prefer explicit casting to JointType for clarity.

root_joint_type is stored as a plain int, so the comparison against JointType.FIXED / FREE / DISTANCE relies on implicit IntEnum–int equality.
Wrapping once:

root_joint_type = JointType(root_joint_type)

makes the intent obvious and future-proofs the checks if additional enum members appear.

newton/tests/test_rigid_contact.py (1)

30-36: Minor: reuse the public collide() behaviour check instead of explicit type test.

The special-casing of SolverMuJoCo by isinstance() ties the test to one concrete class.
If additional MuJoCo-derived solvers appear the branch will silently mis-classify them.

Consider feature detection instead, e.g.:

if getattr(solver, "uses_mujoco_backend", False):
    contacts = None
else:
    contacts = model.collide(state_0, rigid_contact_margin=100.0)

Makes the test resilient to future solver variants.

newton/examples/example_cartpole.py (1)

70-73: Remove stale, commented-out solver lines.

Leaving three alternative solver instantiations in the example clutters the tutorial and confuses new users.
Pick one (or expose a CLI flag) and delete the others.

newton/examples/example_cloth_self_contact.py (1)

183-187: Explicitly cast ParticleFlags when masking NumPy arrays.

-        flags[fixed_vertex_id] = flags[fixed_vertex_id] & ~ParticleFlags.ACTIVE
+        flags[fixed_vertex_id] &= ~int(ParticleFlags.ACTIVE)

Makes the intent crystal-clear and avoids relying on IntFlag’s subclass-of-int behaviour.

newton/examples/example_anymal_c_walk.py (1)

34-38: Unnecessary import – can drop newton.utils.

After the refactor this file only uses newton.utils.download_asset, which can be imported directly:

-import newton.utils
+from newton.utils import download_asset

Keeps the namespace clean.

newton/examples/example_rigid_force.py (1)

55-59: Consider importing newton.viewer explicitly for clarity

RendererOpenGL/RendererUsd are accessed through newton.viewer.*, but the sub-module is never imported.
While attribute access works after the earlier from newton.viewer import … in other files, being explicit here avoids relying on side-effects.

-import newton
+import newton
+import newton.viewer

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

23-26: Consider adding a NONE = 0 member to each IntFlag

Having an explicit zero-value member improves clarity and avoids magic 0s when clearing/initializing flags.

 class ParticleFlags(IntFlag):
+    NONE = 0
     ACTIVE = 1 << 0
@@
 class ShapeFlags(IntFlag):
+    NONE = 0
     VISIBLE = 1 << 0
     COLLIDE_SHAPES = 1 << 1
     COLLIDE_PARTICLES = 1 << 2

Also applies to: 39-47

---

18-20: Remove deprecated commented constants to reduce noise

These legacy wp.constant lines add clutter and can confuse readers. Consider deleting them; the git history retains the context.

-# ParticleFlags.ACTIVE = wp.constant(wp.uint32(1 << 0))
-# """Indicates that the particle is active."""
@@
-# ShapeFlags.VISIBLE = wp.constant(wp.uint32(1 << 0))
-# """Indicates that the shape is visible."""
-
-# ShapeFlags.COLLIDE_SHAPES = wp.constant(wp.uint32(1 << 1))
-# """Indicates that the shape collides with other shapes."""
-
-# ShapeFlags.COLLIDE_PARTICLES = wp.constant(wp.uint32(1 << 2))
-# """Indicates that the shape collides with particles."""

Also applies to: 28-36

newton/examples/example_cloth_style3d.py (1)

116-116: Bitmask with IntFlag: cast to int to avoid dtype surprises

~ParticleFlags.ACTIVE evaluates to a negative IntFlag (two’s complement). While numpy int32 handles it fine, being explicit avoids static/type check noise.

-            flags[fixed_vertex_id] = flags[fixed_vertex_id] & ~ParticleFlags.ACTIVE
+            flags[fixed_vertex_id] = flags[fixed_vertex_id] & ~int(ParticleFlags.ACTIVE)

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

1114-1115: Bitmask with IntFlag: cast to int for clarity and dtype consistency

This avoids potential mypy/linters complaints and keeps operations purely on ints.

-                    builder.shape_flags[shape_id] &= ~newton.ShapeFlags.COLLIDE_SHAPES
+                    builder.shape_flags[shape_id] &= ~int(newton.ShapeFlags.COLLIDE_SHAPES)

newton/examples/example_quadruped.py (2)

35-37: Avoid module-level side-effects during import

RecorderImGuiManager pulls in an OpenGL context at import-time. Importing it unconditionally makes the script crash on headless machines even when stage_path is None. Delay the import until you actually create a renderer (or guard it with the same if stage_path: check).

---

82-86: Provide run-time selection instead of commented options

Switching solvers by commenting / uncommenting code is error-prone. Accept a CLI flag (e.g. --solver xpbd|featherstone|mujoco) and instantiate accordingly.

newton/_src/solvers/xpbd/kernels.py (3)

20-23: Relative import reaches across package layers

from ...core import velocity_at_point jumps three levels up from newton._src.solvers.xpbd. That path will break if solvers is ever moved. Prefer absolute import from newton._src.core import velocity_at_point to decouple the solver package layout.

---

70-71: Cast enum to kernel integer once

Warp captures the whole IntFlag object each time you write

if (particle_flags[i] & ParticleFlags.ACTIVE) == 0:

Introduce a local constant:

ACTIVE = wp.int32(int(ParticleFlags.ACTIVE))

and use ACTIVE in the bit-tests. This avoids repeated Python-side enum resolution during kernel compilation and makes the intent clearer.

Also applies to: 157-158, 252-253, 267-268

---

999-1004: Minor: built-in name shadowing

Functions such as update_joint_dof_mode use the parameter name mode, but elsewhere the variable mode is overloaded for bit-flags. Consider renaming to axis_mode to reduce cognitive load.

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

16-18: Misleading module docstring

The docstring says “Implementation of the Newton model class.” but the file only defines SolverNotifyFlags. Update the description to match the content.

newton/examples/example_contact_sensor.py (2)

129-130: Explicit import of sub-package

newton.solvers is accessed without an explicit import. Relying on newton/__init__.py to import sub-packages increases start-up time and may break if lazy import machinery is introduced later. Add

import newton.solvers

near the top.

---

231-236: Avoid negative isinstance checks

Instead of not isinstance(self.solver, newton.solvers.SolverMuJoCo) create a boolean flag once:

self.uses_explicit_collision = not isinstance(self.solver, SolverMuJoCo)

This prevents the tight inner loop from performing Python isinstance every sub-step.

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

16-24: Consider lazy loading to cut import time

Eagerly importing every solver (XPBD, MuJoCo, VBD, …) adds ~200 ms to import newton. A lightweight pattern:

import importlib, types, sys

def __getattr__(name):
    if name in _LAZY:
        mod, attr = _LAZY[name]
        value = getattr(importlib.import_module(mod), attr)
        globals()[name] = value
        return value
    raise AttributeError(name)

_LAZY = {
    "SolverXPBD": ("newton._src.solvers.xpbd", "SolverXPBD"),
    ...
}

delays heavy dependencies until first use.

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

282-283: Sphinx link: add module qualifier for SolverXPBD

Use fully-qualified path for consistent cross-referencing.

-        """Restitution coefficient of soft contacts (used by :class:`SolverXPBD`). Default is 0.0."""
+        """Restitution coefficient of soft contacts (used by :class:`~newton.solvers.SolverXPBD`). Default is 0.0."""

---

287-290: Sphinx link: add module qualifier for SolverXPBD (friction coefficients)

Same nit: ensure consistent dotted path for Sphinx.

-        """Torsional friction coefficient for rigid body contacts (used by :class:`SolverXPBD`)."""
+        """Torsional friction coefficient for rigid body contacts (used by :class:`~newton.solvers.SolverXPBD`)."""

-        """Rolling friction coefficient for rigid body contacts (used by :class:`SolverXPBD`)."""
+        """Rolling friction coefficient for rigid body contacts (used by :class:`~newton.solvers.SolverXPBD`)."""

newton/tests/test_joint_drive.py (1)

75-80: Nit: simplify joint_type selection

Minor readability improvement.

-        joint_type = newton.JointType.PRISMATIC
-        if is_prismatic:
-            joint_type = newton.JointType.PRISMATIC
-        else:
-            joint_type = newton.JointType.REVOLUTE
+        joint_type = newton.JointType.PRISMATIC if is_prismatic else newton.JointType.REVOLUTE

newton/examples/example_selection_materials.py (1)

158-161: Type checks are fine; consider capability checks (optional)

isinstance(..., SolverMuJoCo) works, but a capability-based check (e.g., solver.requires_explicit_collisions) would decouple the example from class names. Optional.

Also applies to: 226-228

newton/tests/test_joint_controllers.py (1)

84-86: eval_ik gating by solver type (optional improvement)

The check excludes MuJoCo and Featherstone before calling eval_ik. Consider a capability flag (e.g., solver.owns_kinematics or similar) to avoid class-name checks. Optional.

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

18-26: Remove stale commented constant definitions

These commented wp.constant lines are legacy and now superseded by enums. Removing them will reduce confusion and noise.

-# # Types of joints linking rigid bodies
-# JointType.PRISMATIC = wp.constant(0)
-# JointType.REVOLUTE = wp.constant(1)
-# JointType.BALL = wp.constant(2)
-# JointType.FIXED = wp.constant(3)
-# JointType.FREE = wp.constant(4)
-# JointType.DISTANCE = wp.constant(5)
-# JointType.D6 = wp.constant(6)

---

29-36: Enums introduced correctly; consider docstrings

JointType values preserve the prior mapping. Consider adding a brief docstring for clarity; otherwise LGTM.

---

39-52: Tighten type annotations and comparisons

Since JointType is an IntEnum, update the signature to reflect that and keep comparisons to enum members. Behavior unchanged.

-def get_joint_dof_count(joint_type: int, num_axes: int) -> tuple[int, int]:
+def get_joint_dof_count(joint_type: JointType, num_axes: int) -> tuple[int, int]:

newton/tests/test_import_usd.py (1)

56-57: Consistently treat ShapeFlags as an IntFlag.

Line 56 casts the enum to int, while Line 84 relies on the raw IntFlag. Pick one style (prefer the enum directly) to avoid needless casts and keep the code uniform.

Also applies to: 84-85

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

532-537: Why exclude prismatic joints from visual arrows?

JointType.PRISMATIC was previously rendered; it’s now commented out.
If that’s intentional, leave a comment explaining the rationale; otherwise restore it:

-                newton.JointType.REVOLUTE,
-                # newton.JointType.PRISMATIC,
+                newton.JointType.REVOLUTE,
+                newton.JointType.PRISMATIC,

docs/migration.rst (1)

158-163: Update table entry for SimRenderer.

newton.utils.SimRenderer no longer exists; the public alias lives in newton.viewer.
Replace the cell with ``:class:`newton.viewer.SimRenderer``` for accuracy.

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

1470-1483: Consider using a dictionary for joint type string conversion.

While the current implementation is correct, consider using a dictionary mapping for cleaner code:

JOINT_TYPE_NAMES = {
    JointType.FREE: "free",
    JointType.BALL: "ball",
    JointType.PRISMATIC: "prismatic",
    JointType.REVOLUTE: "revolute",
    JointType.D6: "D6",
    JointType.FIXED: "fixed",
    JointType.DISTANCE: "distance",
}

def joint_type_str(type):
    return JOINT_TYPE_NAMES.get(type, "unknown")

newton/__init__.py (2)

48-54: Remove stray “TODO: HMMM” and tidy import section

Left-over TODOs in the public package entry-point look unprofessional and may trigger import-time warnings in downstream projects. Either address the task or delete the comment before merge.

---

84-90: __all__ list is missing new top-level symbols added later

Sub-modules like solvers.flags.SolverNotifyFlags (now public) are not exported via __all__. Exporting them avoids from newton import * surprises.

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

226-229: Fix runtime crash: wrong argument type passed to wp.vec3.

The existing review comment remains valid - wp.vec3 expects three scalars, but euler is a NumPy array. This will raise a type error at runtime.

Apply this diff to fix the issue:

-            return quat_from_euler(wp.vec3(euler), *euler_seq)
+            return quat_from_euler(
+                wp.vec3(float(euler[0]), float(euler[1]), float(euler[2])),
+                *euler_seq,
+            )

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

364-365: Fix runtime error: quat_between_axes expects Axis enum, not string.

The existing review comment remains valid - axis_str is a string but quat_between_axes expects an Axis enum value.

Apply this diff to fix the issue:

-                if axis_str != "Z":
-                    axis_q = quat_between_axes(Axis.Z, axis_str)
+                if axis_str != "Z":
+                    axis = Axis.from_string(axis_str)
+                    axis_q = quat_between_axes(Axis.Z, axis)

newton/_src/solvers/featherstone/kernels.py (1)

174-179: Nit: prefer wp.vec3() over wp.vec3(0.0) for clarity

Both construct a zero vector, but wp.vec3() is clearer and consistent with usage elsewhere in this file.

Apply:

-        pos = wp.vec3(0.0)
+        pos = wp.vec3()

newton/_src/solvers/xpbd/kernels.py (3)

1418-1428: Clean up commented code or document its purpose.

This large block of commented code appears to be related to joint mode handling. If this code is no longer needed, it should be removed. If it's kept for reference, consider adding a comment explaining why it's preserved.

-    # if (type == JointType.PRISMATIC):
-    #     if mode == JointMode.TARGET_POSITION:
-    #         target = wp.clamp(target, limit_lower, limit_upper)
-    #         if target_ke > 0.0:
-    #             err = dx - target * axis
-    #             compliance = 1.0 / target_ke
-    #         damping = axis_damping[dim]
-    #     elif mode == JointMode.TARGET_VELOCITY:
-    #         if target_ke > 0.0:
-    #             err = (derr - target) * dt
-    #             compliance = 1.0 / target_ke
-    #         damping = axis_damping[dim]

Or add a documentation comment if this code serves as a reference:

+    # TODO: The following commented code shows the original joint mode handling pattern
+    # for PRISMATIC joints. This may be re-enabled in future versions.
     # if (type == JointType.PRISMATIC):
     #     if mode == JointMode.TARGET_POSITION:

---

1486-1495: Clean up commented joint type checks.

Similar to the previous comment, these commented conditional checks should either be removed if no longer needed or documented if they serve as reference for future development.

-    # if type == JointType.FIXED:
-    #     return
-    # if type == JointType.REVOLUTE:
-    #     return
-    # if type == JointType.PRISMATIC:
-    #     return
-    # if type == JointType.BALL:
-    #     return

---

1873-1880: Remove or uncomment debugging printf statements.

The commented debug printf statements should either be removed if they're no longer needed or uncommented if they're still useful for debugging. Large blocks of commented debug code can clutter the codebase.

-        # if type == JointType.D6:
-        #     wp.printf("axis_target: %f %f %f\t axis_stiffness: %f %f %f\t axis_damping: %f %f %f\t axis_limits_lower: %f %f %f \t axis_limits_upper: %f %f %f\n",
-        #               axis_target[0], axis_target[1], axis_target[2],
-        #               axis_stiffness[0], axis_stiffness[1], axis_stiffness[2],
-        #               axis_damping[0], axis_damping[1], axis_damping[2],
-        #               axis_limits_lower[0], axis_limits_lower[1], axis_limits_lower[2],
-        #               axis_limits_upper[0], axis_limits_upper[1], axis_limits_upper[2])
-        #     # wp.printf("wp.sqrt(1.0-qtwist[0]*qtwist[0]) = %f\n", wp.sqrt(1.0-qtwist[0]*qtwist[0]))

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

Learnt from: shi-eric
PR: newton-physics/newton#521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.404Z
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#394
File: newton/examples/example_ik_benchmark.py:132-146
Timestamp: 2025-07-15T19:00:39.991Z
Learning: In the Newton library, `newton.utils.download_asset()` already handles internal caching and can work with both local and remote assets, so conditional logic to check for local paths is unnecessary. The standard practice is to use `download_asset` directly rather than implementing custom path handling logic.

newton/_src/solvers/euler/kernels.py (3)

659-659: Use Warp intrinsics inside kernels: replace Python min() with wp.min().

Python’s min() may not be supported in Warp kernels and can cause codegen errors. Use wp.min for device-compatibility.

-    jd = min(vn, 0.0) * kd
+    jd = wp.min(vn, 0.0) * kd

---

786-786: Use Warp abs() in kernels: replace abs() with wp.abs().

Python’s abs() may not be supported in Warp kernels and can break codegen. Use wp.abs for correctness.

-    ft = wp.normalize(vt) * wp.min(kf * wp.length(vt), abs(mu * c * ke))
+    ft = wp.normalize(vt) * wp.min(kf * wp.length(vt), wp.abs(mu * c * ke))

---

633-644: Cast ParticleFlags.ACTIVE to int in kernel bit tests

All particle_flags arrays on host and device use wp.int32, so the dtype is consistent. To avoid any JIT surprises when using an IntFlag in bitwise operations, explicitly cast the enum to int in the kernel.

Files and locations to update:

• newton/_src/solvers/euler/kernels.py:643

Apply this diff:

--- a/newton/_src/solvers/euler/kernels.py
+++ b/newton/_src/solvers/euler/kernels.py
@@ -640,7 +640,7 @@ def eval_particle_ground_contacts(
     tid = wp.tid()
-    if (particle_flags[tid] & ParticleFlags.ACTIVE) == 0:
+    if (particle_flags[tid] & int(ParticleFlags.ACTIVE)) == 0:
         return

     x = particle_x[tid]

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

229-229: Fix runtime crash: wrong argument type passed to wp.vec3 in Euler path

wp.vec3 expects three scalars; passing the NumPy array raises a type error when an MJCF has euler=.

-            return quat_from_euler(wp.vec3(euler), *euler_seq)
+            return quat_from_euler(
+                wp.vec3(float(euler[0]), float(euler[1]), float(euler[2])),
+                *euler_seq,
+            )

newton/_src/solvers/euler/kernels.py (1)

685-721: Duplicate: IntFlag mask in kernel — cast to int to be explicit.

Same concern as in eval_particle_ground_contacts: cast the enum to int to be robust with Warp’s JIT.

-    if (particle_flags[particle_index] & ParticleFlags.ACTIVE) == 0:
+    if (particle_flags[particle_index] & int(ParticleFlags.ACTIVE)) == 0:

newton/_src/solvers/xpbd/kernels.py (1)

906-907: Addressed previous feedback: Local enum imports used correctly.

The joint type comparisons now use the locally imported JointType enum constants (e.g., JointType.FIXED, JointType.FREE) instead of referencing the public namespace, which addresses the previous concern about compile-time coupling between private kernels and the public API.

Also applies to: 945-955

newton/examples/example_selection_articulations.py (1)

284-286: Forward kinematics evaluation complete for both articulations

The FK evaluation correctly updates both ants and humanoids when not using MuJoCo solver, ensuring proper transforms for rendering.

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

364-365: Bug: quat_between_axes expects Axis enum, not a string

The axis_str variable is a string ("X"/"Y"/"Z"), but quat_between_axes expects an Axis enum value. This will cause a runtime error when the axis is not "Z".

Apply this fix to convert the string to an Axis enum:

-                    axis_q = quat_between_axes(Axis.Z, axis_str)
+                    axis = Axis.from_string(axis_str)
+                    axis_q = quat_between_axes(Axis.Z, axis)

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

151-152: Nit: avoid unnecessary str() cast for default; parse more defensively

No need to cast the default to str before int(). Also guard against malformed values.

-                maxhullvert = int(mesh.attrib.get("maxhullvert", str(geometry.MESH_MAXHULLVERT)))
+                maxhullvert_attr = mesh.attrib.get("maxhullvert")
+                maxhullvert = (
+                    int(maxhullvert_attr)
+                    if maxhullvert_attr is not None
+                    else int(geometry.MESH_MAXHULLVERT)
+                )

newton/_src/solvers/euler/kernels.py (1)

1040-1052: Minor readability: avoid shadowing builtins; prefer elif for mutually exclusive joint types.

• Variable name type shadows Python’s builtin type. Rename to jt or joint_type_id for clarity.
• Since joint types are mutually exclusive, consider elif for the subsequent branches for clarity (functional behavior is the same due to exclusivity and the early return on FREE/DISTANCE).

Example:

-    type = joint_type[tid]
+    jt = joint_type[tid]
...
-    if type == JointType.FREE or type == JointType.DISTANCE:
+    if jt == JointType.FREE or jt == JointType.DISTANCE:
       ...
-    if type == JointType.FIXED:
+    elif jt == JointType.FIXED:
       ...
-    if type == JointType.PRISMATIC:
+    elif jt == JointType.PRISMATIC:
       ...

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

1891-1891: Consider caching topological sort results

The topological_sort function is called during model conversion. For models with many joints, consider caching the sorted order if the joint topology doesn't change frequently.

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

Learnt from: shi-eric
PR: newton-physics/newton#521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.404Z
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.404Z
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#394
File: newton/examples/example_ik_benchmark.py:132-146
Timestamp: 2025-07-15T19:00:39.991Z
Learning: In the Newton library, `newton.utils.download_asset()` already handles internal caching and can work with both local and remote assets, so conditional logic to check for local paths is unnecessary. The standard practice is to use `download_asset` directly rather than implementing custom path handling logic.

newton/tests/test_ik_fk_kernels.py (1)

120-136: Clarify D6 Joint Keyword Args and Angular Axis Types

The add_joint_d6 method signature is:

def add_joint_d6(
    self,
    parent: int,
    child: int,
    linear_axes: Sequence[JointDofConfig] | None = None,
    angular_axes: Sequence[JointDofConfig] | None = None,
    …
) -> int

and JointDofConfig.axis accepts either an Axis enum or a 3-vector. To improve readability and consistency:

• Name the parent/child arguments explicitly.
• Use newton.Axis enums for both linear and angular axes.

Affected file:
• newton/tests/test_ik_fk_kernels.py (around lines 120–136)

Proposed diff:

         builder.add_joint_d6(
-            -1,
-            child,
+            parent=-1,
+            child=child,
             linear_axes=[
                 cfg(axis=newton.Axis.X),
                 cfg(axis=newton.Axis.Y),
                 cfg(axis=newton.Axis.Z),
             ],
             angular_axes=[
-                cfg(axis=[1, 0, 0]),
-                cfg(axis=[0, 1, 0]),
-                cfg(axis=[0, 0, 1]),
+                cfg(axis=newton.Axis.X),
+                cfg(axis=newton.Axis.Y),
+                cfg(axis=newton.Axis.Z),
             ],
             parent_xform=parent_xf,
             child_xform=child_xf,
         )

newton/examples/example_contact_sensor.py (3)

323-338: Bug: torch.device is not a context manager — use torch.cuda.device or a nullcontext

torch.device(...) doesn’t implement enter/exit, so using it as a context manager will raise at runtime whenever USE_TORCH is True.

Robust fix that supports both CUDA and CPU:

 class ScopedDevice:
     def __init__(self, device):
         self.warp_scoped_device = wp.ScopedDevice(device)
         if USE_TORCH:
-            import torch  # noqa: PLC0415
-
-            self.torch_scoped_device = torch.device(wp.device_to_torch(device))
+            import torch  # noqa: PLC0415
+            from contextlib import nullcontext
+            torch_dev = wp.device_to_torch(device) if device is not None else None
+            # torch.cuda.device is a context manager, torch.device is not.
+            if torch_dev and str(torch_dev).startswith("cuda"):
+                self.torch_scoped_device = torch.cuda.device(torch_dev)
+            else:
+                self.torch_scoped_device = nullcontext()

     def __enter__(self):
         self.warp_scoped_device.__enter__()
         if USE_TORCH:
             self.torch_scoped_device.__enter__()

     def __exit__(self, exc_type, exc_val, exc_tb):
         self.warp_scoped_device.__exit__(exc_type, exc_val, exc_tb)
         if USE_TORCH:
             self.torch_scoped_device.__exit__(exc_type, exc_val, exc_tb)

---

112-113: Cannot locate populate_contacts in Contacts. The class only preallocates fixed-size arrays—it does not auto-resize. Thus, calling Contacts(0, 0) creates zero-length buffers and will break as soon as any contact is recorded.

Action Required: Allocate reasonable upper bounds instead of zeros.

• File newton/examples/example_contact_sensor.py, around line 112:
Replace
python self.contacts = Contacts(0, 0)
with something like
python # Preallocate for expected max contacts per environment max_rigid = 128 max_soft = 64 self.contacts = Contacts(max_rigid, max_soft)

Adjust max_rigid/max_soft based on your use case.

---

134-142: Fix up_axis parameter in RendererOpenGL call

The up_axis argument expects a newton.AxisType (e.g. an Axis enum or simple "X"|"Y"|"Z"), not the string returned by str(up_axis) (which produces "Axis.Z"). Update the call to pass the enum directly:

• File: newton/examples/example_contact_sensor.py
• Lines: 134–142

             self.renderer = newton.viewer.RendererOpenGL(
                 path=stage_path,
                 model=self.model,
                 scaling=2.0,
-                up_axis=str(up_axis),
+                up_axis=up_axis,
                 screen_width=1280,
                 screen_height=720,
                 camera_pos=(0, 4, 30),
             )

newton/examples/example_robot_manipulating_cloth.py (3)

50-67: Fix return type and docstring of vec_rotation (returns a quaternion, not a transform)

The function returns a wp.quat, and usage unpacks four components. The return annotation and docstring are misleading.

Apply:

-def vec_rotation(x: float, y: float, z: float) -> wp.transform:
-    """Convert plane coordinates given by the plane normal and its offset along the normal to a transform."""
+def vec_rotation(x: float, y: float, z: float) -> wp.quat:
+    """Return a quaternion that rotates +Z onto the normalized vector (x, y, z)."""

---

150-151: Use NumPy dtype when allocating with NumPy

np.empty(..., dtype=wp.float32) is invalid; NumPy expects a NumPy dtype.

Apply:

-    J = np.empty((out_dim, in_dim), dtype=wp.float32)
+    J = np.empty((out_dim, in_dim), dtype=np.float32)

---

472-475: Prevent out-of-bounds on searchsorted boundary conditions

When t_mod equals the last time point, np.searchsorted can return len(...), causing OOB on self.targets[current_interval].

Apply:

-        include_rotation = True
-        current_interval = np.searchsorted(self.robot_key_poses_time, t_mod)
-        self.target = self.targets[current_interval]
+        include_rotation = True
+        idx = np.searchsorted(self.robot_key_poses_time, t_mod, side="right")
+        current_interval = min(int(idx), self.targets.shape[0] - 1)
+        self.target = self.targets[current_interval]

newton/examples/example_robot_manipulating_cloth.py (1)

37-39: Import transform_twist from public newton.core (not newton.utils)

transform_twist is part of the public spatial API exposed via newton.core (see newton/_src/core/spatial.py). Importing it from newton.utils is incorrect and may break at runtime.

Apply this diff:

-from newton.utils import transform_twist
+from newton.core import transform_twist

newton/utils.py (1)

30-33: Add a safety note where exposing pickle-based recorder.

ModelAndStateRecorder uses pickle; it’s good that the class docstring warns, but since this module is the public façade, consider adding a brief module-level note about only loading recordings from trusted sources.

I can add a short module docstring summarizing utilities and warning about pickle upon request.

newton/tests/test_ik.py (3)

184-184: Centralize FK calls behind a local wrapper to ease future API swaps

Multiple direct invocations of the private _eval_fk_batched(...) make the test brittle. Wrap the call once (e.g., _run_fk(...)) and use that everywhere. This also makes switching to a future public helper trivial.

Apply these diffs at the call sites:

-        _eval_fk_batched(model, joint_q_2d, joint_qd_2d, body_q_2d, body_qd_2d)
+        _run_fk(model, joint_q_2d, joint_qd_2d, body_q_2d, body_qd_2d)

And add this helper near your existing FK utility (outside these ranges):

def _run_fk(model, joint_q_2d, joint_qd_2d, body_q_2d, body_qd_2d):
    # Funnel FK through one place to decouple tests from private/internal symbol changes.
    _eval_fk_batched(model, joint_q_2d, joint_qd_2d, body_q_2d, body_qd_2d)

Also applies to: 190-190, 238-238, 243-243, 284-284, 289-289

---

279-281: Prefer keyword arguments for clarity and to resist signature churn

The D6 objective creation mixes positional args, which is error-prone in refactors. Use keywords to match the other call sites and improve readability.

-        pos_obj = ik.IKPositionObjective(0, wp.vec3(0.0, 0.0, 0.0), pos_targets, n_problems, 6, 0)
-        rot_obj = ik.IKRotationObjective(0, wp.quat_identity(), rot_targets, n_problems, 6, 3)
+        pos_obj = ik.IKPositionObjective(
+            link_index=0,
+            link_offset=wp.vec3(0.0, 0.0, 0.0),
+            target_positions=pos_targets,
+            n_problems=n_problems,
+            total_residuals=6,
+            residual_offset=0,
+        )
+        rot_obj = ik.IKRotationObjective(
+            link_index=0,
+            link_offset_rotation=wp.quat_identity(),
+            target_rotations=rot_targets,
+            n_problems=n_problems,
+            total_residuals=6,
+            residual_offset=3,
+        )

---

420-421: Use keyword arguments for D6 Jacobian objective builder (consistency with other builders)

Minor style consistency to avoid positional arg pitfalls.

-    pos_obj = ik.IKPositionObjective(0, wp.vec3(0.0, 0.0, 0.0), pos_targets, n_problems, 6, 0)
-    rot_obj = ik.IKRotationObjective(0, wp.quat_identity(), rot_targets, n_problems, 6, 3)
+    pos_obj = ik.IKPositionObjective(
+        link_index=0,
+        link_offset=wp.vec3(0.0, 0.0, 0.0),
+        target_positions=pos_targets,
+        n_problems=n_problems,
+        total_residuals=6,
+        residual_offset=0,
+    )
+    rot_obj = ik.IKRotationObjective(
+        link_index=0,
+        link_offset_rotation=wp.quat_identity(),
+        target_rotations=rot_targets,
+        n_problems=n_problems,
+        total_residuals=6,
+        residual_offset=3,
+    )

newton/tests/test_ik_fk_kernels.py (2)

31-37: Use precise typing for JOINT_KINDS

The list is annotated as list[int] but populated with JointType (IntEnum). Typing it as list[JointType] improves clarity and keeps the tests aligned with the new public API.

Apply this diff:

-JOINT_KINDS: list[int] = [
+JOINT_KINDS: list[JointType] = [
     JointType.REVOLUTE,
     JointType.PRISMATIC,
     JointType.BALL,
     JointType.D6,
     JointType.FREE,
 ]

As a follow-up (outside this hunk), consider updating function signatures for precision:

# Outside this diff hunk
def _add_single_joint(builder: newton.ModelBuilder, jt: JointType) -> None: ...
def _build_model_for_joint(jt: JointType, device): ...

---

185-201: Unify enum usage style within this function

Elsewhere the file uses JointType.* via the import; here newton.JointType.* is used. For consistency, prefer the imported JointType.

Apply this diff:

-        elif jt == newton.JointType.D6:
+        elif jt == JointType.D6:
             # lin X,Y,Z
             q_np[q0 + 0 : q0 + 3] = rng.uniform(-0.1, 0.1, size=3)
             # rot XYZ
             q_np[q0 + 3 : q0 + 6] = rng.uniform(-np.pi / 8, np.pi / 8, size=3)

-        elif jt == newton.JointType.FREE:
+        elif jt == JointType.FREE:
             # translation
             q_np[q0 + 0 : q0 + 3] = rng.uniform(-0.3, 0.3, size=3)
             # quaternion
             axis = rng.normal(size=3)
             axis /= np.linalg.norm(axis) + 1e-8
             angle = rng.uniform(-np.pi / 6, np.pi / 6)
             qw = np.cos(angle / 2.0)
             qv = axis * np.sin(angle / 2.0)
             q_np[q0 + 3 : q0 + 7] = (*qv, qw)

newton/sensors.py (2)

17-20: Optional: make imports lazy to keep import-time light

If newton/_src/utils/contact_sensor pulls in heavy deps (e.g., Warp), consider lazy-loading the symbols to avoid unnecessary startup cost when users import newton but don’t use sensors.

Example refactor within this module:

-from ._src.utils.contact_sensor import (
-    ContactSensor,
-    populate_contacts,
-)
+def __getattr__(name: str):
+    if name in __all__:
+        from ._src.utils.contact_sensor import ContactSensor, populate_contacts
+        return {"ContactSensor": ContactSensor, "populate_contacts": populate_contacts}[name]
+    raise AttributeError(f"module {__name__!r} has no attribute {name!r}")

---

16-25: Consider a deprecation shim for old import paths

External code likely used newton.utils.contact_sensor.{ContactSensor,populate_contacts}. To smooth the migration, add a shim that re-exports and emits a DeprecationWarning.

Proposed new file: newton/utils/contact_sensor.py

# Deprecated shim: newton.utils.contact_sensor
import warnings
from newton.sensors import ContactSensor, populate_contacts  # public façade

warnings.warn(
    "newton.utils.contact_sensor is deprecated; import from newton.sensors instead.",
    DeprecationWarning,
    stacklevel=2,
)

__all__ = ["ContactSensor", "populate_contacts"]

I can open a follow-up PR to add the shim and wire it into the test suite if desired.

newton/_src/solvers/featherstone/kernels.py (1)

91-102: Minor: prefer elif for mutually exclusive joint-type branches

Since each branch returns immediately, behavior is correct; using elif clarifies mutual exclusivity and avoids redundant type comparisons.

Apply within this block:

- if type == JT_PRISMATIC:
+ if type == JT_PRISMATIC:
    ...
- if type == JT_REVOLUTE:
+ elif type == JT_REVOLUTE:
    ...
- if type == JT_BALL:
+ elif type == JT_BALL:
    ...

Repeat pattern for the subsequent branches in this function.

newton/examples/example_robot_manipulating_cloth.py (2)

330-333: Remove unused allocations to reduce clutter

self.delta_q, self.joint_q_des, and self.J_shape are not used. Removing them reduces mental overhead.

Apply:

-        # for robot control
-        self.delta_q = wp.empty(self.model.joint_count, dtype=float)
-        self.joint_q_des = wp.array(self.model.joint_q.numpy(), dtype=float)
+        # for robot control
+        # (removed unused: delta_q, joint_q_des)
@@
-        self.J_shape = wp.array((out_dim, in_dim), dtype=int)
+        # (removed unused: J_shape)

Also applies to: 345-347

---

86-89: Orientation error uses quaternion vector part; consider log-map/axis-angle for better scaling

Taking ang_diff[0:3] as angular error couples error magnitude to quaternion parameterization. Consider using a scaled-axis (log map) or axis-angle representation for more consistent IK behavior.

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

Learnt from: shi-eric
PR: newton-physics/newton#521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.404Z
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/utils/import_usd.py (3)

132-139: Bug: mapping key won’t match due to lower() usage.

approximation.lower() is used, but the dict contains "meshSimplification" with capital S. That key will never match.

Fix by lowercasing the key:

-    approximation_to_remeshing_method = {
+    approximation_to_remeshing_method = {
         "convexdecomposition": "coacd",
         "convexhull": "convex_hull",
         "boundingsphere": "bounding_sphere",
         "boundingcube": "bounding_box",
-        "meshSimplification": "quadratic",
+        "meshsimplification": "quadratic",
     }

---

421-436: Indices should be int32, not float32.

Face indices are read as float32 and only later cast; use int32 at the source to avoid unnecessary conversions and potential precision pitfalls.

Apply:

-                indices = np.array(mesh.GetFaceVertexIndicesAttr().Get(), dtype=np.float32)
+                indices = np.array(mesh.GetFaceVertexIndicesAttr().Get(), dtype=np.int32)
...
-                m = Mesh(points, np.array(faces, dtype=np.int32).flatten())
+                m = Mesh(points, np.array(faces, dtype=np.int32).flatten(), maxhullvert=mesh_maxhullvert)

Note: The second line also applies the optional consistency tweak for maxhullvert.

---

1048-1067: Indices should be int32 here as well.

Same dtype issue for mesh collision shapes: read indices as int32 upstream.

Apply:

-                    indices = np.array(mesh.GetFaceVertexIndicesAttr().Get(), dtype=np.float32)
+                    indices = np.array(mesh.GetFaceVertexIndicesAttr().Get(), dtype=np.int32)

newton/tests/test_import_usd.py (1)

224-227: Missing USD guard will break environments without usd-core.

Other tests are guarded with @unittest.skipUnless(USD_AVAILABLE, ...). This one imports pxr inside the test but lacks the decorator.

Add the skip decorator:

-    def test_mesh_approximation(self):
+    @unittest.skipUnless(USD_AVAILABLE, "Requires usd-core")
+    def test_mesh_approximation(self):

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

398-407: Bug: cylinders are added as capsules.

The cylinder branch calls add_shape_capsule instead of add_shape_cylinder, leading to incorrect geometry.

Fix the call:

-                if geom_type == "cylinder":
-                    s = builder.add_shape_capsule(
+                if geom_type == "cylinder":
+                    s = builder.add_shape_cylinder(
                         xform=tf,
                         radius=geom_radius,
                         half_height=geom_height,
                         axis=geom_up_axis,
                         **shape_kwargs,
                     )

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

365-366: Passing strings to quat_between_axes is correct.

This was previously flagged; however, quat_between_axes accepts AxisType and handles string inputs internally via Axis.from_any(). No change needed.

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

69-91: New parameter mesh_maxhullvert is well-threaded and documented.

Signature and docstring addition look good and match usage below. Minor nit: consider documenting the default explicitly in the docstring for discoverability.

Apply this docstring tweak:

-        mesh_maxhullvert (int): Maximum vertices for convex hull approximation of meshes.
+        mesh_maxhullvert (int): Maximum vertices for convex hull approximation of meshes (default: MESH_MAXHULLVERT).

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

53-54: Expose mesh_maxhullvert in parse_usd API — consider also applying to visual mesh shapes.

Adding mesh_maxhullvert is great. Currently it’s applied to Mesh for collision shapes; visual meshes created in load_visual_shapes don’t pass it (they rely on the default). Optional: pass mesh_maxhullvert there too for consistency across all Mesh instances.

Apply this change where visual meshes are created:

-                m = Mesh(points, np.array(faces, dtype=np.int32).flatten())
+                m = Mesh(points, np.array(faces, dtype=np.int32).flatten(), maxhullvert=mesh_maxhullvert)

Also applies to: 80-81

---

1112-1113: Type robustness when clearing COLLIDE_SHAPES bit.

If builder.shape_flags is an array of ints, mixing IntFlag in-place ops can yield IntFlag objects. Cast to int for predictability.

Apply:

-                    builder.shape_flags[shape_id] &= ~ShapeFlags.COLLIDE_SHAPES
+                    builder.shape_flags[shape_id] = int(builder.shape_flags[shape_id]) & ~int(ShapeFlags.COLLIDE_SHAPES)

If builder.shape_flags is already IntFlag-typed, this is a no-op semantically; the change ensures consistency across backends.

newton/tests/test_import_usd.py (1)

55-56: Nit: unify flag checks style for readability.

You use both int(ShapeFlags...) and direct IntFlag bit checks in this file. Consider consistently using the IntFlag style as below (matches usage later in this test).

Apply:

-        collision_shapes = [
-            i for i in range(builder.shape_count) if builder.shape_flags[i] & int(newton.ShapeFlags.COLLIDE_SHAPES)
-        ]
+        collision_shapes = [
+            i for i in range(builder.shape_count) if builder.shape_flags[i] & newton.ShapeFlags.COLLIDE_SHAPES
+        ]

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

230-230: Prefer unpacked construction for wp.vec3 for consistency.

While Warp may accept an array, the codebase consistently uses wp.vec3(*array). Align for readability and consistency.

Apply:

-            return quat_from_euler(wp.vec3(euler), *euler_seq)
+            return quat_from_euler(wp.vec3(*euler), *euler_seq)

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

Learnt from: shi-eric
PR: newton-physics/newton#521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.404Z
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/solvers/featherstone/kernels.py:75-75
Timestamp: 2025-08-12T18:04:06.551Z
Learning: The Newton physics framework requires nightly Warp builds, which means compatibility concerns with older stable Warp versions (like missing functions such as wp.spatial_adjoint) are not relevant for this project.

Learnt from: shi-eric
PR: newton-physics/newton#521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.404Z
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/utils.py:30-33
Timestamp: 2025-08-12T18:07:17.216Z
Learning: In newton/_src/utils/import_usd.py, the PXR/USD modules are lazily loaded using try-except blocks within the functions rather than at module import time, making it safe to import parse_usd at the module level without causing import-time failures for users without USD dependencies.

Learnt from: nvlukasz
PR: newton-physics/newton#519
File: newton/utils.py:30-33
Timestamp: 2025-08-12T18:07:17.216Z
Learning: In newton/_src/utils/import_usd.py, the PXR/USD modules are lazily loaded using try-except blocks within the functions (lines 109-111 and 1289-1291) rather than at module import time, making it safe to import parse_usd at the module level without causing import-time failures for users without USD dependencies.

Learnt from: nvlukasz
PR: newton-physics/newton#519
File: newton/geometry.py:16-22
Timestamp: 2025-08-12T17:51:37.434Z
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: nvlukasz
PR: newton-physics/newton#519
File: newton/_src/utils/import_usd.py:364-365
Timestamp: 2025-08-12T18:04:06.254Z
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/_src/utils/import_usd.py:364-365
Timestamp: 2025-08-12T18:04:06.254Z
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/tests/test_ik.py:25-26
Timestamp: 2025-08-12T17:58:16.858Z
Learning: In the Newton physics engine codebase, it's acceptable for tests to import and use private `_src` internal modules and functions when needed for testing purposes, even though this breaks the public API boundary. This is an intentional project decision for test code.

Learnt from: nvlukasz
PR: newton-physics/newton#519
File: newton/_src/utils/import_mjcf.py:226-231
Timestamp: 2025-08-12T17:52:14.970Z
Learning: In the Newton codebase, when passing array-like objects (numpy arrays, lists, tuples) to wp.vec3(), the consistent pattern is to use the unpacking operator: wp.vec3(*array) rather than wp.vec3(array). This pattern is used throughout newton/_src/utils/import_mjcf.py and newton/_src/core/types.py.

Learnt from: shi-eric
PR: newton-physics/newton#521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.404Z
Learning: The STYLE3D solver in Newton is a new solver that did not exist in warp.sim, so it does not require any migration documentation.

newton/examples/example_mpm_granular.py (1)

225-226: Update example to import load_mesh from its defining module
The public newton/utils.py does not re-export load_mesh, so calling newton.utils.load_mesh will fail. You should import it directly from the internal geometry utils where it’s defined.

• In newton/examples/example_mpm_granular.py (around line 225), replace:

    mesh_points, mesh_indices = newton.utils.load_mesh(collider)

with:

from newton._src.geometry.utils import load_mesh

    mesh_points, mesh_indices = load_mesh(collider)

newton/_src/solvers/style3d/kernels.py (1)

216-225: Avoid using not on bitwise expressions in Warp kernels; compare explicitly to zero

Using not on an int-typed bitwise result can be brittle in Warp’s type system and relies on implicit truthiness. Compare the mask against zero to produce a clear boolean.

Apply this diff:

-    if not particle_flags[tid] & ParticleFlags.ACTIVE:
+    # Explicit boolean check avoids relying on truthiness of an int in Warp kernels
+    if (particle_flags[tid] & ParticleFlags.ACTIVE) == 0:
         x_inertia[tid] = x_prev[tid]
         static_A_diags[tid] = 0.0
         dx[tid] = wp.vec3(0.0)
     else:
         v_prev = v_curr[tid]
         mass = particle_masses[tid]
         static_A_diags[tid] = pd_diags[tid] + mass / (dt * dt)
         x_inertia[tid] = x_last + v_prev * dt + (gravity + f_ext[tid] / mass) * (dt * dt)
         dx[tid] = v_prev * dt

Note: Keeping ParticleFlags as an IntEnum is fine; Warp handles IntEnum constants in kernels without explicit casts.

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

64-67: Guard SDF.finalize/hash when volume is None

If SDF.volume is not provided (default None), both finalize() and __hash__() will raise AttributeError by accessing .id. Add a guard to fail fast in finalize() and a safe fallback in __hash__().

 def finalize(self) -> wp.uint64:
     """Returns the volume pointer of the SDF volume"""
-    return self.volume.id
+    if self.volume is None:
+        raise ValueError("SDF.volume is None; cannot finalize without a valid wp.Volume.")
+    return self.volume.id
@@
 @override
 def __hash__(self) -> int:
-    return hash(self.volume.id)
+    # Guard against uninitialized volume; fall back to object identity hash
+    if self.volume is None or not hasattr(self.volume, "id"):
+        return object.__hash__(self)
+    return hash(self.volume.id)

Also applies to: 68-71

newton/examples/example_anymal_c_walk_on_sand.py (1)

223-229: Recompute contacts each substep to avoid stale contact manifolds

Contacts are computed once before the substeps loop, which can produce stale contacts as the robot moves during the substeps. Recomputing per substep improves stability and collision accuracy. This also makes the CUDA graph capture reflect the intended contact update cadence.

-        self.contacts = self.model.collide(self.state_0, rigid_contact_margin=0.1)
-        for _ in range(self.sim_substeps):
+        for _ in range(self.sim_substeps):
+            self.contacts = self.model.collide(self.state_0, rigid_contact_margin=0.1)
             self.state_0.clear_forces()
             self.controller.assign_control(self.control, self.state_0)
             self.solver.step(self.state_0, self.state_1, self.control, self.contacts, self.sim_dt)
             self.state_0, self.state_1 = self.state_1, self.state_0

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

191-205: Warp array serialization drops empty arrays and loses dtype information.

Current logic:

• Skips empty wp.array values (data loss).
• Does not capture dtype (e.g., wp.vec3, wp.transform), so deserialization may reconstruct with incorrect types, breaking downstream kernels.

Serialize all warp arrays and include dtype; also handle basic containers to support nested warp arrays.

Apply this diff:

-        if isinstance(value, (int, float, bool, str, list, dict, set, tuple)):
-            return value
-        elif isinstance(value, np.ndarray):
-            return value
-        elif isinstance(value, wp.array):
-            if value.size > 0:
-                return {
-                    "__type__": "wp.array",
-                    "data": value.numpy(),
-                }
-        return None
+        # Primitives
+        if isinstance(value, (int, float, bool, str)):
+            return value
+        # Numpy arrays are pickleable
+        if isinstance(value, np.ndarray):
+            return value
+        # Warp arrays: always serialize (including empty) and include dtype hint
+        if isinstance(value, wp.array):
+            return {
+                "__type__": "wp.array",
+                "dtype": getattr(value.dtype, "__name__", str(value.dtype)),
+                "data": value.numpy(),
+            }
+        # Containers: recurse so nested warp arrays are serialized too
+        if isinstance(value, (list, tuple)):
+            return [self._serialize_value(v) for v in value]
+        if isinstance(value, dict):
+            return {k: self._serialize_value(v) for k, v in value.items()}
+        if isinstance(value, set):
+            return {"__type__": "set", "data": [self._serialize_value(v) for v in value]}
+        # Unsupported types
+        return None

---

220-228: Deserialization must restore warp dtype and handle nested containers.

Without restoring dtype, arrays like wp.transform may come back as plain numeric arrays. Also, nested containers serialized above must be walked recursively.

Apply this diff:

-        if isinstance(value, dict) and "__type__" in value:
-            type_name = value["__type__"]
-            obj_data = value["data"]
-
-            if type_name == "wp.array":
-                return wp.array(obj_data, device=device)
-        if isinstance(value, np.ndarray):
-            return value
-        return value
+        # Handle typed records
+        if isinstance(value, dict) and "__type__" in value:
+            type_name = value["__type__"]
+            if type_name == "wp.array":
+                obj_data = value["data"]
+                dtype_name = value.get("dtype")
+                wp_dtype = getattr(wp, dtype_name, None) if isinstance(dtype_name, str) else None
+                if wp_dtype is not None:
+                    return wp.array(obj_data, dtype=wp_dtype, device=device)
+                return wp.array(obj_data, device=device)
+            if type_name == "set":
+                return set(self._deserialize_and_restore_value(v, device) for v in value.get("data", []))
+        # Recurse containers
+        if isinstance(value, list):
+            return [self._deserialize_and_restore_value(v, device) for v in value]
+        if isinstance(value, dict):
+            return {k: self._deserialize_and_restore_value(v, device) for k, v in value.items()}
+        # Numpy arrays are already fine
+        if isinstance(value, np.ndarray):
+            return value
+        # Primitives and everything else
+        return value

---

230-242: Preserve warp array dtype in state recording and playback

The current record() (lines 230–242) only saves value.numpy(), and playback() (lines 253–271) always calls

wp.array(value_np, device=device)

without passing the original dtype. This drops structured types like wp.transform, wp.vec3, and wp.spatial_vector, which will break kernels expecting those dtypes.

Please update newton/_src/utils/recorder.py so that:

• record() captures both the raw data and the array’s dtype.
• playback() checks for this metadata and reconstructs via wp.array(data, dtype=dtype, device=device) when present.

Locations to change:

• record(): around line 230
• playback(): around line 253

Example diff:

--- a/newton/_src/utils/recorder.py
+++ b/newton/_src/utils/recorder.py
@@   def record(self, state: State):
-        for name, value in state.__dict__.items():
-            if isinstance(value, wp.array):
-                state_data[name] = value.numpy()
+        for name, value in state.__dict__.items():
+            if isinstance(value, wp.array):
+                state_data[name] = {
+                    "__type__": "wp.array",
+                    "data": value.numpy(),
+                    "dtype": value.dtype.name
+                }
@@   def playback(self, state: State, frame_id: int):
-        for name, value_np in state_data.items():
-            if hasattr(state, name):
-                value_wp = wp.array(value_np, device=device)
-                setattr(state, name, value_wp)
+        for name, entry in state_data.items():
+            if not hasattr(state, name):
+                continue
+            if isinstance(entry, dict) and entry.get("__type__") == "wp.array":
+                dtype = getattr(wp, entry["dtype"], None)
+                value_wp = wp.array(entry["data"], dtype=dtype, device=device)
+            else:
+                value_wp = wp.array(entry, device=device)
+            setattr(state, name, value_wp)

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

420-444: Use integer dtype for mesh indices and carry mesh_maxhullvert for consistency

Face indices should be integers to avoid precision pitfalls; also consider passing maxhullvert for visual meshes for consistency (harmless even if unused).

Apply this diff:

-                indices = np.array(mesh.GetFaceVertexIndicesAttr().Get(), dtype=np.float32)
+                indices = np.array(mesh.GetFaceVertexIndicesAttr().Get(), dtype=np.int32)
@@
-                m = Mesh(points, np.array(faces, dtype=np.int32).flatten())
+                m = Mesh(points, np.array(faces, dtype=np.int32).flatten(), maxhullvert=mesh_maxhullvert)

---

131-139: Fix mapping key case for approximation flag

approximation.lower() is used later, but the dict contains "meshSimplification" mixed-case key; it won’t match. Use lowercase key.

Apply this diff:

-    approximation_to_remeshing_method = {
+    approximation_to_remeshing_method = {
         "convexdecomposition": "coacd",
         "convexhull": "convex_hull",
         "boundingsphere": "bounding_sphere",
         "boundingcube": "bounding_box",
-        "meshSimplification": "quadratic",
+        "meshsimplification": "quadratic",
     }

Also applies to: 1072-1085

---

1048-1067: Use integer dtype for mesh indices when building collision meshes

Same as for visual meshes: face indices should be int, not float.

Apply this diff:

-                    indices = np.array(mesh.GetFaceVertexIndicesAttr().Get(), dtype=np.float32)
+                    indices = np.array(mesh.GetFaceVertexIndicesAttr().Get(), dtype=np.int32)

newton/examples/example_selection_articulations.py (2)

205-211: Guard CUDA graph capture with mempool and non-MuJoCo solvers

The simulate() call in this example may allocate (e.g., through collision clones), so we should only enable CUDA graph capture when a mempool is available and we’re not using the pure MuJoCo backend. Update the assignment in newton/examples/example_selection_articulations.py (around lines 205–206):

• File: newton/examples/example_selection_articulations.py
Location: inside the method where self.use_cuda_graph is set

-        self.use_cuda_graph = wp.get_device().is_cuda and use_cuda_graph
+        dev = wp.get_device()
+        self.use_cuda_graph = (
+            dev.is_cuda
+            and wp.is_mempool_enabled(dev)
+            and not getattr(self.solver, "use_mujoco", False)
+            and use_cuda_graph
+        )

Please verify that the example still runs correctly on both CPU and CUDA targets, with and without MuJoCo enabled.

---

117-125: Prefer passing the Axis enum (or omit) for RendererOpenGL.up_axis
RendererOpenGL.init is declared as

def __init__(…, up_axis: AxisType | None = None, …)

and internally calls Axis.from_any(up_axis). Converting your enum to a string at the call site is redundant and undermines static type checks.

• Location to update:

• newton/examples/example_selection_articulations.py: around lines 117–125

Apply one of the following diffs:

Option 1 – Pass the enum directly

-            self.renderer = newton.viewer.RendererOpenGL(
-                …,
-                up_axis=str(up_axis),
+            self.renderer = newton.viewer.RendererOpenGL(
+                …,
+                up_axis=up_axis,
                …,
            )

Option 2 – Omit the argument entirely (defaults to model’s up axis)

-                up_axis=str(up_axis),
+                # up_axis omitted; defaults to model’s up axis

docs/migration.rst (3)

119-134: Incorrect claim: add_joint_*(..., target=...) → action=...

In code, the add_joint_* APIs still use the argument name target (e.g., add_joint_revolute(..., target=...)). There is no action parameter in the public API.

Please update the table to keep target as the correct parameter name to avoid breaking user migrations.

---

56-62: Remove incorrect Model.joint_axis removal note

The migration guide claims that Model.joint_axis was removed, but the code still populates and uses this attribute throughout (e.g. in builder.finalize, render utilities, IK routines, articulations, and all solvers). Please update the docs or the implementation:

• In docs/migration.rst (around line 61), remove or revise the sentence:
  “The Model.joint_axis attribute has been removed since it now equals :attr:newton.Model.joint_qd_start.”

If the intent is to keep joint_axis, document its continued presence or aliasing behavior. If you intend to drop it, ensure the code no longer assigns or uses model.joint_axis.

---

79-91: Update migration docs for eval_ik signature

The documentation currently shows an optional mask=None for eval_ik, but the implementation does not accept a mask. Please remove the mask from the Newton column in the migration table (or implement mask support in eval_ik).

• File: docs/migration.rst
• Section: “Forward and Inverse Kinematics” table

Suggested change:

--- a/docs/migration.rst
+++ b/docs/migration.rst
@@
-| ``eval_ik(model, state, joint_q, joint_qd)``           | ``eval_ik(model, state, joint_q, joint_qd, mask=None)``                |
+| ``eval_ik(model, state, joint_q, joint_qd)``           | ``eval_ik(model, state, joint_q, joint_qd)``                           |

newton/examples/example_anymal_c_walk_on_sand.py (1)

34-34: Renderer public-API migration looks correct

Switching to newton.viewer.RendererOpenGL aligns with the refactor goals to avoid utils exposure for rendering.

Resolved the prior note about utils/renderer import paths. The explicit newton.utils import is present (Line 33), and rendering moved to newton.viewer as intended.

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

361-367: Correctness note: passing strings to quat_between_axes is valid

The call quat_between_axes(Axis.Z, axis_str) is correct; quat_between_axes accepts AxisType and internally converts strings via Axis.from_any().

newton/tests/test_ik.py (1)

25-27: Private _src import in tests acknowledged; acceptable per project conventions

Importing _eval_fk_batched from newton._src.sim.ik is fine for tests in this repo (as per maintainers’ guidance). The public renames (newton.ik module and IK*Objective classes) are correctly adopted.

newton/examples/example_selection_articulations.py (1)

284-286: Humanoid FK on reset added — thank you

Including self.hums.eval_fk(...) fixes stale transforms for humanoids when not using MuJoCo. Matches the prior review request.

newton/examples/example_robot_manipulating_cloth.py (1)

37-39: Import transform_twist from the public API (newton.core), not utils

transform_twist is re-exported publicly from newton.core; using newton.utils ties the example to an internal-ish path. This was discussed earlier and addressed; aligning the example keeps it purely on public APIs.

-from newton.utils import transform_twist
+from newton.core import transform_twist

newton/examples/example_mpm_granular.py (1)

73-73: Minor: pass fps and up_axis explicitly to RendererOpenGL for clarity.

RendererOpenGL supports fps and up_axis; passing them avoids relying on defaults and keeps the example consistent with the model’s up axis and CLI arg.

Apply this diff:

-            self.renderer = newton.viewer.RendererOpenGL(self.model, "MPM Granular")
+            self.renderer = newton.viewer.RendererOpenGL(
+                self.model, "MPM Granular", fps=int(options.fps), up_axis=newton.Axis.Y
+            )

newton/examples/example_cloth_hanging.py (2)

132-141: Optional: simplify solver selection via a small dispatch table.

Removes branching and centralizes per-solver constructor differences.

-        if self.solver_type == SolverType.EULER:
-            self.solver = newton.solvers.SolverSemiImplicit(model=self.model)
-        elif self.solver_type == SolverType.XPBD:
-            self.solver = newton.solvers.SolverXPBD(
-                model=self.model,
-                iterations=self.iterations,
-            )
-        else:  # self.solver_type == SolverType.VBD
-            self.solver = newton.solvers.SolverVBD(model=self.model, iterations=self.iterations)
+        solver_factories = {
+            SolverType.EULER: lambda: newton.solvers.SolverSemiImplicit(model=self.model),
+            SolverType.XPBD: lambda: newton.solvers.SolverXPBD(model=self.model, iterations=self.iterations),
+            SolverType.VBD: lambda: newton.solvers.SolverVBD(model=self.model, iterations=self.iterations),
+        }
+        self.solver = solver_factories[self.solver_type]()

---

145-151: Optional: guard renderer creation for headless environments.

In CI or servers without an OpenGL context, consider a try/except around RendererOpenGL and fall back to no viewer with a warning to avoid example crashes.

newton/_src/solvers/style3d/kernels.py (1)

19-19: Prefer explicit import path for private symbol to reduce coupling on re-exports

ParticleFlags is defined in geometry/flags.py. Importing from the package root assumes init.py re-exports it, which can break silently if that re-export changes. Import directly from the defining module.

Apply this diff:

-from ...geometry import ParticleFlags
+from ...geometry.flags import ParticleFlags

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

127-128: Normalize vertex dtype to float32 in init for consistency with Warp and hashing

vertices are set without an explicit dtype here but are coerced to float32 in the setter, and Warp typically expects float32. Normalizing upfront avoids subtle dtype-dependent behavior and ensures stable hashes.

-        self._vertices = np.array(vertices).reshape(-1, 3)
+        self._vertices = np.array(vertices, dtype=np.float32).reshape(-1, 3)

---

224-235: Reduce hashing overhead and ensure dtype-stable hashing for Mesh

Converting large arrays to Python tuples is slow and memory-heavy. Hashing bytes of normalized dtypes is faster and produces stable results across runs and platforms (given consistent endianness and dtype).

-        if self._cached_hash is None:
-            self._cached_hash = hash(
-                (tuple(np.array(self.vertices).flatten()), tuple(np.array(self.indices).flatten()), self.is_solid)
-            )
+        if self._cached_hash is None:
+            verts = np.asarray(self.vertices, dtype=np.float32)
+            inds = np.asarray(self.indices, dtype=np.int32)
+            self._cached_hash = hash((verts.tobytes(), inds.tobytes(), bool(self.is_solid)))

If is_solid can change after construction, consider clearing _cached_hash when it changes (optional):

# Add to class Mesh (optional)
@property
def is_solid(self) -> bool:
    return getattr(self, "_is_solid", False)

@is_solid.setter
def is_solid(self, value: bool):
    self._is_solid = bool(value)
    self._cached_hash = None

newton/examples/example_anymal_c_walk_on_sand.py (3)

183-183: Be consistent with solver imports to avoid relying on package attribute side-effects

You import SolverImplicitMPM directly but access SolverFeatherstone via newton.solvers.SolverFeatherstone. Prefer importing both explicitly for clarity and to help static analysis.

Apply:

-from newton.solvers import SolverImplicitMPM
+from newton.solvers import SolverImplicitMPM, SolverFeatherstone
...
-        self.solver = newton.solvers.SolverFeatherstone(self.model)
+        self.solver = SolverFeatherstone(self.model)

---

196-198: Consider setting collider friction to match the sand friction

MPM’s setup_collider supports per-collider friction. Using the sand_friction value for the shank colliders can improve robot-sand interaction fidelity.

-        self.mpm_solver = SolverImplicitMPM(self.model, options)
-        self.mpm_solver.setup_collider(self.model, [self.collider_mesh])
+        self.mpm_solver = SolverImplicitMPM(self.model, options)
+        self.mpm_solver.setup_collider(self.model, [self.collider_mesh], collider_friction=[sand_friction])

---

199-199: Optional: pass fps to the renderer to reflect simulation rate

RendererOpenGL accepts fps; setting it to match your 60 Hz sim can make playback pacing more predictable.

-        self.renderer = None if headless else newton.viewer.RendererOpenGL(self.model, stage_path)
+        self.renderer = None if headless else newton.viewer.RendererOpenGL(self.model, stage_path, fps=fps)

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

16-24: Guard type-only imports to avoid circular-imports at runtime.

Model and State are only used in annotations. You can avoid importing ..sim at runtime (and eliminate potential import cycles) by gating the import behind TYPE_CHECKING.

Apply this diff:

 from __future__ import annotations

 import pickle
+import warnings

 import numpy as np
 import warp as wp

-from ..sim import Model, State
+from typing import TYPE_CHECKING
+
+if TYPE_CHECKING:
+    from ..sim import Model, State

---

62-68: Use warnings.warn instead of print and keep return types predictable.

Using warnings.warn is preferable for library code. Also, consider consistently returning an empty list instead of None for point clouds to simplify call-sites.

Apply this diff:

-        if not (0 <= frame_id < len(self.transforms_history)):
-            print(f"Warning: frame_id {frame_id} is out of bounds. Playback skipped.")
-            return None, None
+        if not (0 <= frame_id < len(self.transforms_history)):
+            warnings.warn(f"frame_id {frame_id} is out of bounds. Playback skipped.", RuntimeWarning)
+            return None, None

Optional (behavioral change; consider for DX consistency):

-        point_clouds = self.point_clouds_history[frame_id] if frame_id < len(self.point_clouds_history) else None
+        point_clouds = self.point_clouds_history[frame_id] if frame_id < len(self.point_clouds_history) else []

---

77-83: Add a simple version tag to NPZ for forward compatibility.

Embedding a small metadata version helps with future format changes.

Apply this diff:

-        np.savez_compressed(file_path, **history_np)
+        history_np["__format_version__"] = 1
+        np.savez_compressed(file_path, **history_np)

---

265-271: Device detection never raises ValueError; handle None explicitly and warn.

_get_device_from_state returns None when no wp.array is found; the try/except ValueError is dead code. Make the behavior explicit and warn via warnings.

Apply this diff:

-        try:
-            device = self._get_device_from_state(state)
-        except ValueError:
-            print("Warning: Unable to determine device from state. Playback skipped.")
-            return
+        device = self._get_device_from_state(state)
+        if device is None:
+            warnings.warn("Unable to determine device from state. Playback skipped.", RuntimeWarning)
+            return

---

292-301: Use highest pickle protocol and document security in code.

You already warn in the docstring. Also set protocol=pickle.HIGHEST_PROTOCOL for efficiency and to be explicit.

Apply this diff:

-        with open(file_path, "wb") as f:
-            data_to_save = {"model": self.model_data, "states": self.history}
-            pickle.dump(data_to_save, f)
+        with open(file_path, "wb") as f:
+            data_to_save = {"model": self.model_data, "states": self.history}
+            pickle.dump(data_to_save, f, protocol=pickle.HIGHEST_PROTOCOL)

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

39-52: Improve type hints and comparisons in get_joint_dof_count

Use the enum type in the signature and simplify comparisons with membership.

Apply this diff:

-def get_joint_dof_count(joint_type: int, num_axes: int) -> tuple[int, int]:
+def get_joint_dof_count(joint_type: JointType | int, num_axes: int) -> tuple[int, int]:
@@
-    if joint_type == JointType.BALL:
+    if joint_type == JointType.BALL:
@@
-    elif joint_type == JointType.FREE or joint_type == JointType.DISTANCE:
+    elif joint_type in (JointType.FREE, JointType.DISTANCE):

---

18-26: Remove legacy constant comments to reduce noise

The commented legacy JOINT_* and JOINT_MODE_/EQ_ constants add clutter and can confuse readers. They’re superseded by the enums.

Apply this diff:

-# # Types of joints linking rigid bodies
-# JointType.PRISMATIC = wp.constant(0)
-# JointType.REVOLUTE = wp.constant(1)
-# JointType.BALL = wp.constant(2)
-# JointType.FIXED = wp.constant(3)
-# JointType.FREE = wp.constant(4)
-# JointType.DISTANCE = wp.constant(5)
-# JointType.D6 = wp.constant(6)
@@
-# # Joint axis control mode types
-# JointMode.NONE = wp.constant(0)
-# JointMode.TARGET_POSITION = wp.constant(1)
-# JointMode.TARGET_VELOCITY = wp.constant(2)
@@
-# # (temporary) equality constraint types
-# EqType.CONNECT = wp.constant(0)
-# EqType.WELD = wp.constant(1)
-# EqType.JOINT = wp.constant(2)

Also applies to: 55-59, 67-71

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

34-54: Docstring drift: update parameters and remove stale entry

The docstring still mentions default_density and lacks the new mesh_maxhullvert parameter detail.

• Remove default_density from Args.
• Add a brief explanation for mesh_maxhullvert in Args (you already added a short one-line at Line 80; keep it and remove default_density).

Also applies to: 80-81

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

1800-1805: Unsupported JointType.DISTANCE in MuJoCo conversion

Distance joints are not included in supported_joint_types and will trigger NotImplementedError below (Line 2170) if present. Given parse_usd can add DistanceJoint, this conversion path will fail at runtime for such models.

• If intended, document this limitation in the SolverMuJoCo docstring.
• Otherwise, consider mapping Distance to an equivalent constraint in MuJoCo (e.g., equality constraint) during conversion.

Would you like a follow-up patch mapping Distance joints to weld/connect equality constraints for parity?

---

1878-1883: Boolean mask: make the intention explicit

The current expression relies on non-zero ints being truthy. Make it explicit for readability and to avoid dtype gotchas.

Apply this diff:

-            selected_shapes = np.where(shape_flags & ShapeFlags.COLLIDE_SHAPES)[0]
+            selected_shapes = np.where((shape_flags & ShapeFlags.COLLIDE_SHAPES) != 0)[0]

---

1902-1916: Consistent collision-flag checks

Same readability point: make the bit check explicit in both places.

Apply this diff:

-                if skip_visual_only_geoms and not (shape_flags[shape] & ShapeFlags.COLLIDE_SHAPES):
+                if skip_visual_only_geoms and (shape_flags[shape] & ShapeFlags.COLLIDE_SHAPES) == 0:
                     continue

docs/api/newton_solvers.rst (1)

12-18: Docs list updated to new names — minor ordering nit

The renamed solver classes are correctly listed. For readability, consider listing SolverBase first, then the concrete solvers:

-   SolverFeatherstone
-   SolverMuJoCo
-   SolverSemiImplicit
-   SolverBase
-   SolverStyle3D
-   SolverVBD
-   SolverXPBD
+   SolverBase
+   SolverFeatherstone
+   SolverMuJoCo
+   SolverSemiImplicit
+   SolverStyle3D
+   SolverVBD
+   SolverXPBD

newton/examples/example_cloth_style3d.py (1)

116-117: Set device explicitly when reassigning particle_flags

Without specifying device, wp.array(flags) may go to the default device, which can differ from the model’s device in multi-device contexts. Use the model’s device for safety.

Apply this diff:

-        self.model.particle_flags = wp.array(flags)
+        self.model.particle_flags = wp.array(flags, device=self.model.device)

newton/examples/example_cloth_self_contact.py (2)

183-188: Set device explicitly for particle_flags reassignment; also consider dtype safety

To avoid device mismatches, pass device=self.model.device. Optionally cast the mask to the array’s dtype when clearing bits to avoid Python’s negative ~int behavior on different widths.

Apply this diff for device:

-            self.model.particle_flags = wp.array(flags)
+            self.model.particle_flags = wp.array(flags, device=self.model.device)

Outside this hunk (no diff): consider

import numpy as np
flags[fixed_vertex_id] = flags[fixed_vertex_id] & ~np.int32(ParticleFlags.ACTIVE)

---

183-188: Avoid hard-coded cloth_size when selecting edge vertices

The indices for left_side/right_side assume cloth_size = 50. If the asset changes, this will break. Compute cloth_size from mesh topology instead (e.g., infer grid size from vertex count) before clearing flags.

Would you like a helper to infer grid dimensions from the USD mesh to robustly build rot_point_indices?

newton/examples/example_anymal_c_walk_physx_policy.py (1)

145-147: Solver/renderer renames applied correctly; verify optional deps and consider a small perf tweak.

• SolverMuJoCo and viewer.RendererOpenGL usages match the new API.
• Request: ensure the example’s README/usage mentions the extras needed for MuJoCo/OpenGL on the target environment.
• Optional perf: avoid host round-trips in compute_obs by converting Warp arrays to Torch via wp.to_torch once and slicing tensors, instead of wrapping slices with torch.tensor(...). This keeps data on device and reduces allocations.

Example pattern (outside this hunk, for compute_obs):

# cache once somewhere (e.g., in __init__)
self.joint_q_torch = wp.to_torch(self.state_0.joint_q)
self.joint_qd_torch = wp.to_torch(self.state_0.joint_qd)

# then in compute_obs, prefer slicing tensors you've already materialized on device
root_quat_w = self.joint_q_torch[3:7].unsqueeze(0)
root_lin_vel_w = self.joint_qd_torch[3:6].unsqueeze(0)
root_ang_vel_w = self.joint_qd_torch[:3].unsqueeze(0)
joint_pos_current = self.joint_q_torch[7:].unsqueeze(0)
joint_vel_current = self.joint_qd_torch[6:].unsqueeze(0)

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

477-487: Clear ACTIVE flag without bitwise NOT on IntEnum to avoid negative intermediates.

Given ParticleFlags is an IntEnum (and only ACTIVE is used here), directly assigning 0 is simpler and avoids the -2 intermediate produced by ~ on IntEnum. It also clarifies intent.

-                particle_flag = ParticleFlags.ACTIVE
+                particle_flag = ParticleFlags.ACTIVE

                 if (
                     (x == 0 and fix_left)
                     or (x == dim_x and fix_right)
                     or (y == 0 and fix_bottom)
                     or (y == dim_y and fix_top)
                 ):
-                    particle_flag = particle_flag & ~ParticleFlags.ACTIVE
+                    particle_flag = 0  # deactivate
                     particle_mass = 0.0

Note: Storing ints in self.particle_flags is OK and consistent with the model’s int32 flag arrays.

newton/solvers.py (1)

28-38: Consider temporary deprecation aliases to smooth migration.

If you want to reduce breakage for downstream users, alias the old names to the new ones for one release cycle and optionally emit a DeprecationWarning. Example:

 __all__ = [
     "SolverBase",
     "SolverFeatherstone",
     "SolverImplicitMPM",
     "SolverMuJoCo",
     "SolverNotifyFlags",
     "SolverSemiImplicit",
     "SolverStyle3D",
     "SolverVBD",
     "SolverXPBD",
 ]
+
+# Optional backward-compat aliases (deprecated; remove in a future release)
+# from newton.solvers import MuJoCoSolver, SemiImplicitSolver, etc. will continue to work.
+MuJoCoSolver = SolverMuJoCo
+SemiImplicitSolver = SolverSemiImplicit
+VBDSolver = SolverVBD
+XPBDSolver = SolverXPBD
+FeatherstoneSolver = SolverFeatherstone
+Style3DSolver = SolverStyle3D

newton/tests/test_ik_fk_kernels.py (2)

31-37: Tighten type annotations and keep enum usage consistent.

Minor nit: JOINT_KINDS is a list of JointType values; annotate accordingly.

-JOINT_KINDS: list[int] = [
+JOINT_KINDS: list[JointType] = [
     JointType.REVOLUTE,
     JointType.PRISMATIC,
     JointType.BALL,
     JointType.D6,
     JointType.FREE,
 ]

---

170-201: Unify enum references (JointType vs newton.JointType).

Mixing styles can confuse readers. Prefer the imported JointType consistently.

-        elif jt == newton.JointType.D6:
+        elif jt == JointType.D6:
             # lin X,Y,Z
             q_np[q0 + 0 : q0 + 3] = rng.uniform(-0.1, 0.1, size=3)
             # rot XYZ
             q_np[q0 + 3 : q0 + 6] = rng.uniform(-np.pi / 8, np.pi / 8, size=3)

-        elif jt == newton.JointType.FREE:
+        elif jt == JointType.FREE:
             # translation
             q_np[q0 + 0 : q0 + 3] = rng.uniform(-0.3, 0.3, size=3)
             # quaternion

newton/_src/solvers/vbd/solver_vbd.py (1)

1215-1223: ParticleFlags IntEnum with int32 buffers and bit-tests — correct usage

• particle_flags: wp.int32 + ParticleFlags.ACTIVE checks are consistent with the refactor.
• Using IntEnum in Warp kernels is supported; the bitmask tests are fine. For readability, consider adding parentheses:

Apply this minor clarity tweak (optional):

-    if not particle_flags[particle] & ParticleFlags.ACTIVE:
+    if not (particle_flags[particle] & ParticleFlags.ACTIVE):

Repeat similarly for the other kernels’ ACTIVE checks.

Also applies to: 1237-1247, 1356-1361, 1491-1514, 1984-2009, 2104-2130

newton/tests/test_joint_controllers.py (3)

59-61: Remove no-op Model.ground assignment

The Model.ground attribute was removed in the API refactor. Setting it here does nothing and may mislead readers.

-    model.ground = False

---

84-86: IK fallback guard looks correct; keep an eye on future solver additions

Skipping eval_ik for reduced-coordinate solvers (SolverMuJoCo, SolverFeatherstone) is appropriate. If new reduced-coordinate solvers are added, extend this tuple accordingly.

---

98-103: Re-enable Featherstone and SemiImplicit test entries when ready

Once SolverFeatherstone/SolverSemiImplicit are fixed per TODOs, please re-enable their entries here to keep parity across solvers.

Would you like me to open a tracking issue and prepare parametrized tests for those solvers?

newton/examples/example_robot_manipulating_cloth.py (1)

276-276: Featherstone mass matrix update interval tied to substeps

Using update_mass_matrix_interval=self.num_substeps is fine, but it can under/over-update depending on contact/nonlinearities. Consider making this configurable via a constructor arg or CLI flag if users need tuning.

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

149-156: ShapeConfig.flags getter/setter: ensure consistent int typing

Bitwise ops with IntEnum yield ints. You already normalize at finalize time (wp.int32), so this is fine. For clarity, you could explicitly cast to int in getter/setter to avoid mixed IntEnum/int values in self.shape_flags.

-            shape_flags = ShapeFlags.VISIBLE if self.is_visible else 0
+            shape_flags = int(ShapeFlags.VISIBLE) if self.is_visible else 0
-            shape_flags |= ShapeFlags.COLLIDE_SHAPES if self.has_shape_collision else 0
+            shape_flags |= int(ShapeFlags.COLLIDE_SHAPES) if self.has_shape_collision else 0
-            shape_flags |= ShapeFlags.COLLIDE_PARTICLES if self.has_particle_collision else 0
+            shape_flags |= int(ShapeFlags.COLLIDE_PARTICLES) if self.has_particle_collision else 0
             return shape_flags

Also applies to: 162-165

---

879-883: Quaternion normalization for multi-DOF joints

Setting the last coord of q to 1.0 for FREE/DISTANCE/BALL establishes a valid quaternion. Consider normalizing the full quat in case prior assignments modify other components.

-            self.joint_q[-1] = 1.0
+            # ensure a valid unit quaternion
+            qi = self.joint_q_start[-1]
+            qx, qy, qz, qw = 0.0, 0.0, 0.0, 1.0
+            self.joint_q[qi + (coord_count - 4) : qi + coord_count] = [qx, qy, qz, qw]

Note: coord_count is available above (computed from get_joint_dof_count). If it’s not in scope here, compute indices accordingly.

---

3051-3064: Bit-twiddling ParticleFlags with IntEnum works but returns int

particle_flag & ~ParticleFlags.ACTIVE yields int. Since finalize() normalizes flags, this is OK. If you want to keep internal lists uniform, consider wrapping in int(...) explicitly.

docs/migration.rst (1)

98-99: Control: recommend clarifying JointMode.NONE as the way to disable targets

The guidance is correct. Consider adding a short code snippet to show setting JointMode.NONE via ModelBuilder.JointDofConfig for discoverability.