newton/tests/test_import_usd.py (8)

21-21: Move math import to maintain import order consistency.

The math import should be grouped with the standard library imports at the top of the file.

 import os
+import math
 import unittest

 import numpy as np
 import warp as wp
-import math
 import newton

---

437-437: Remove unused import and fix import placement.

The UsdGeom import is unused and the USD imports should be at the top of the file.

-        from pxr import Sdf, Usd, UsdGeom, UsdPhysics
+        from pxr import Sdf, Usd, UsdPhysics

---

526-526: Clean up whitespace issues throughout the method.

Multiple lines contain trailing whitespace or blank lines with whitespace that should be cleaned up.

Apply formatting fixes to remove trailing whitespace and blank lines with whitespace throughout the verify_usdphysics_parser method. Use your pre-commit hooks with pre-commit run -a as mentioned in the PR objectives to automatically fix these issues.

Also applies to: 537-537, 559-559, 561-561, 566-566, 573-573, 580-580, 583-583, 588-588, 599-599, 608-608, 614-614, 633-633, 638-638, 641-641, 646-646, 648-648, 658-658, 661-661, 671-671, 683-683, 686-686, 690-690, 698-698, 704-704, 708-708, 712-712, 716-716, 726-726, 731-731

---

653-653: Remove unused variable assignment.

shape_body_array is assigned but never used in the function.

-        shape_body_array = model.shape_body.numpy()

---

718-718: Fix import placement inside function.

The quat_between_axes import should be moved to the top of the file or the beginning of the method.

Move the import to the beginning of the verify_usdphysics_parser method:

 def verify_usdphysics_parser(self, file, model):
     """Verify model based on the UsdPhysics Parsing Utils"""
     # [1] https://openusd.org/release/api/usd_physics_page_front.html
     from pxr import Sdf, Usd, UsdPhysics
+    from newton.core import quat_between_axes

And remove it from line 718:

-                from newton.core import quat_between_axes

---

467-467: Rename unused loop variables to indicate they're intentionally unused.

Loop control variables that aren't used should be prefixed with underscore to indicate intentional non-use.

-        for body_path, body_desc in parsed_bodies:
+        for body_path, _ in parsed_bodies:

-            for joint_path, joint_desc in list(zip(*parsed.get(joint_objtype, ()))):
+            for joint_path, _ in list(zip(*parsed.get(joint_objtype, ()))):

Also applies to: 506-506

---

434-744: Consider breaking down the large verification method for better maintainability.

The verify_usdphysics_parser method is 310+ lines long with complex nested logic. Consider extracting helper methods for better readability and testability.

The method could be broken down into smaller, focused helper methods:

• _verify_bodies(stage, parsed, model, body_key_to_idx)
• _verify_mass_and_inertia(stage, parsed_bodies, model, body_key_to_idx)
• _verify_joints(stage, parsed, model, joint_key_to_idx, body_key_to_idx)
• _verify_joint_frames(model, body_positions, body_quaternions)
• _verify_shapes(stage, parsed, model, shape_key_to_idx)

Would you like me to help refactor this into smaller, more maintainable methods?

---

461-490: Add error handling for USD operations that might fail.

The USD parsing operations could fail with various exceptions. Consider adding try-catch blocks to provide better error messages.

For example, wrap USD operations in error handling:

try:
    stage = Usd.Stage.Open(file)
    parsed = UsdPhysics.LoadUsdPhysicsFromRange(stage, ["/"])
except Exception as e:
    self.fail(f"Failed to parse USD file {file}: {e}")

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

💡 Knowledge Base configuration:

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

You can enable these sources in your CodeRabbit configuration.

Learnt from: nvlukasz
PR: newton-physics/newton#519
File: newton/utils.py:30-33
Timestamp: 2025-08-12T18:07:17.267Z
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.267Z
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.267Z
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/utils.py:30-33
Timestamp: 2025-08-12T18:07:17.267Z
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.267Z
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.

newton/tests/test_import_usd.py (2)

609-611: Remove debug print and compute free-joint count robustly.

Drop the debug print and avoid hard-coding “+1” for free joints. Compute the number of FREE joints present in the model.

-        print(model.joint_key)
-        self.assertEqual(len(joints_found) + 1, model.joint_count)
+        free_count = int((model.joint_type.numpy() == newton.JointType.FREE).sum())
+        self.assertEqual(len(joints_found), model.joint_count - free_count)

---

845-856: Replace hardcoded asset path with dynamic asset resolution (CI-friendly).

Hardcoding a local path will break in CI and on other machines. Use the same pattern as test_anymal to fetch and locate the asset.

-        asset_path = "/home/chris/Documents/usd-test-assets/isaaclab/g1.usd"
+        # Resolve G1 asset dynamically; skip if not available
+        asset_root = newton.utils.download_asset("g1_usd")
+        asset_path = None
+        for root, _, files in os.walk(asset_root):
+            if "g1.usd" in files:
+                asset_path = os.path.join(root, "g1.usd")
+                break
+        if not asset_path or not os.path.exists(asset_path):
+            raise unittest.SkipTest("G1 asset file not found")

newton/tests/test_import_usd.py (11)

27-27: Avoid mixing JointType imports; use newton.JointType consistently.

Currently both newton.JointType and a direct JointType import are used. Prefer a single style to avoid confusion and improve readability.

Apply this diff:

-from newton import JointType
+# Use newton.JointType directly for consistency

-        joint_mapping = {
-            JointType.PRISMATIC: UsdPhysics.ObjectType.PrismaticJoint,
-            JointType.REVOLUTE: UsdPhysics.ObjectType.RevoluteJoint,
-            JointType.BALL: UsdPhysics.ObjectType.SphericalJoint,
-            JointType.FIXED: UsdPhysics.ObjectType.FixedJoint,
-            # JointType.FREE: None,
-            JointType.DISTANCE: UsdPhysics.ObjectType.DistanceJoint,
-            JointType.D6: UsdPhysics.ObjectType.D6Joint,
-        }
+        joint_mapping = {
+            newton.JointType.PRISMATIC: UsdPhysics.ObjectType.PrismaticJoint,
+            newton.JointType.REVOLUTE: UsdPhysics.ObjectType.RevoluteJoint,
+            newton.JointType.BALL: UsdPhysics.ObjectType.SphericalJoint,
+            newton.JointType.FIXED: UsdPhysics.ObjectType.FixedJoint,
+            # newton.JointType.FREE: None,
+            newton.JointType.DISTANCE: UsdPhysics.ObjectType.DistanceJoint,
+            newton.JointType.D6: UsdPhysics.ObjectType.D6Joint,
+        }

-            if jt == JointType.REVOLUTE:
+            if jt == newton.JointType.REVOLUTE:
                 self.assertEqual((lin, ang), (0, 1), f"{model.joint_key[j]} DOF dim mismatch")
-            elif jt == JointType.FIXED:
+            elif jt == newton.JointType.FIXED:
                 self.assertEqual((lin, ang), (0, 0), f"{model.joint_key[j]} DOF dim mismatch")
-            elif jt == JointType.FREE:
+            elif jt == newton.JointType.FREE:
                 self.assertGreater(lin + ang, 0, f"{model.joint_key[j]} expected nonzero DOFs for free joint")
-            elif jt == JointType.PRISMATIC:
+            elif jt == newton.JointType.PRISMATIC:
                 self.assertEqual((lin, ang), (1, 0), f"{model.joint_key[j]} DOF dim mismatch")
-            elif jt == JointType.BALL:
+            elif jt == newton.JointType.BALL:
                 self.assertEqual((lin, ang), (0, 3), f"{model.joint_key[j]} DOF dim mismatch")

Also applies to: 588-596, 624-633

---

534-537: Keep the in-function USD import (to avoid optional-dep import-time failures), but drop the unused symbol and silence the linter.

UsdGeom is unused. Keeping the import inside the function is appropriate for optional dependencies (aligned with the repo’s lazy-import pattern).

-        from pxr import Sdf, Usd, UsdGeom, UsdPhysics
+        from pxr import Sdf, Usd, UsdPhysics  # noqa: PLC0415

---

544-547: Use unittest assertions instead of bare assert.

Replace bare assert with self.assertIn for clearer failures and consistency with the rest of the test suite.

-            assert body_key_to_idx.get(str(body_path), None) is not None
+            self.assertIn(str(body_path), body_key_to_idx)

---

548-557: Collider equality may be brittle when bodies have descendant shapes.

Comment notes a TODO, but the test currently enforces strict equality. For USD assets where shapes are author-authored on descendants (common), this will fail. Consider either:

• comparing only shapes directly attached to the body, or
• aggregating descendant shapes for both USD and Newton before compare, or
• relaxing to subset checks.

Would you like me to propose a helper that gathers colliders attached to the prim and descendants on both sides and compares normalized sets?

---

564-566: Rename unused loop variable to underscore.

body_desc is unused in this loop; rename to _ to satisfy linters and improve clarity.

-        for body_path, body_desc in parsed_bodies:
+        for body_path, _ in parsed_bodies:

---

603-607: Rename unused joint_desc to underscore and guard for empty parsed lists.

Minor cleanups for readability and to satisfy the linter.

-        for joint_type, joint_objtype in joint_mapping.items():
-            for joint_path, joint_desc in list(zip(*parsed.get(joint_objtype, ()))):
+        for joint_type, joint_objtype in joint_mapping.items():
+            for joint_path, _ in list(zip(*parsed.get(joint_objtype, ()))):
                 joint_idx = joint_key_to_idx.get(str(joint_path), None)
                 joints_found.append(joint_idx)
-                assert joint_key_to_idx.get(str(joint_path), None) is not None
+                self.assertIn(str(joint_path), joint_key_to_idx)
                 assert model_joint_type[joint_idx] == joint_type

---

612-617: Use joint_qd_start for DOF indexing instead of summing sizes.

Rely on the model’s precomputed DOF start indices to avoid off-by-ones and improve clarity.

-        body_q_array = model.body_q.numpy()
-        joint_dof_dim_array = model.joint_dof_dim.numpy()
+        body_q_array = model.body_q.numpy()
+        joint_dof_dim_array = model.joint_dof_dim.numpy()
+        joint_qd_start = model.joint_qd_start.numpy()

-            dof_index = 0 if j <= 0 else sum(int(joint_dof_dim_array[i][0] + joint_dof_dim_array[i][1]) for i in range(j))
+            dof_index = int(joint_qd_start[j])

Also applies to: 657-657

---

678-679: Fix trailing whitespace and make axis assertion clearer.

Eliminate trailing whitespace and format the condition for readability.

-                    self.assertTrue(all(abs(actual_axis[i] - expected_axis[i]) < 1e-6 for i in range(3)) or 
-                                  all(abs(actual_axis[i] - (-expected_axis[i])) < 1e-6 for i in range(3)))
+                    self.assertTrue(
+                        all(abs(actual_axis[i] - expected_axis[i]) < 1e-6 for i in range(3))
+                        or all(abs(actual_axis[i] - (-expected_axis[i])) < 1e-6 for i in range(3))
+                    )

---

750-750: Remove unused variable.

shape_body_array is assigned but never used.

-        shape_body_array = model.shape_body.numpy()

---

814-821: Avoid magic numbers for shape type checks; key off USD spec instead.

Using [3, 5] is brittle. Prefer checking if the USD shape has an axis attribute, which is what drives the rotation correction. Also silence the function-level import linter.

-            if newton_type in [3, 5]:
-                from newton.core import quat_between_axes
-                usd_axis = int(shape_spec.axis) if hasattr(shape_spec, 'axis') else 2
-                axis_quat = (quat_between_axes(newton.Axis.Z, newton.Axis.X) if usd_axis == 0 else
-                           quat_between_axes(newton.Axis.Z, newton.Axis.Y) if usd_axis == 1 else
-                           wp.quat_identity())
+            if hasattr(shape_spec, "axis"):
+                from newton.core import quat_between_axes  # noqa: PLC0415
+                usd_axis = int(shape_spec.axis)
+                axis_quat = (
+                    quat_between_axes(newton.Axis.Z, newton.Axis.X)
+                    if usd_axis == 0
+                    else quat_between_axes(newton.Axis.Z, newton.Axis.Y)
+                    if usd_axis == 1
+                    else wp.quat_identity()
+                )
                 expected_quat = wp.mul(usd_quat, axis_quat)
             else:
                 expected_quat = usd_quat

---

623-623: Trailing whitespace and whitespace-only lines.

There are multiple instances of trailing whitespace (W291) and whitespace-only blank lines (W293). Please run the project’s pre-commit hooks (ruff/black) to auto-fix.

Run:

• pre-commit run -a
• or: ruff --fix . && black .

Also applies to: 634-634, 656-656, 658-658, 663-663, 670-670, 677-677, 680-680, 685-685, 696-696, 705-705, 711-711, 730-730, 735-735, 738-738, 743-743, 745-745, 755-755, 758-758, 768-768, 780-780, 783-783, 787-787, 795-795, 801-801, 805-805, 809-809, 813-813, 823-823, 828-828

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

💡 Knowledge Base configuration:

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

You can enable these sources in your CodeRabbit configuration.

Learnt from: nvlukasz
PR: newton-physics/newton#519
File: newton/utils.py:30-33
Timestamp: 2025-08-12T18:07:17.267Z
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.267Z
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.267Z
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.

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

784-792: Make warn_invalid_desc more robust and classify the warning.

Two tweaks improve resilience and debuggability:

• Guard against descriptors that might not expose isValid (future pxr changes) using getattr.
• Set the warning category explicitly to UserWarning so it can be filtered independently.

Apply this diff:

-    def warn_invalid_desc(path, descriptor) -> bool:
-        if not descriptor.isValid:
-            warnings.warn(
-                f'Warning: Invalid {type(descriptor).__name__} descriptor for prim at path "{path}".',
-                stacklevel=2,
-            )
+    def warn_invalid_desc(path, descriptor) -> bool:
+        # Be defensive in case pxr changes the API and .isValid is absent
+        if not getattr(descriptor, "isValid", True):
+            warnings.warn(
+                f'Invalid {type(descriptor).__name__} descriptor for prim at path "{str(path)}".',
+                category=UserWarning,
+                stacklevel=2,
+            )
             return True
         return False

---

133-139: Case mismatch: meshSimplification key won’t match .lower() lookup.

You lower the input before the dict lookup (approximation.lower()), but one dict key is mixed-case. It will never match and will silently disable that remeshing path.

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",
     }

newton/tests/test_import_usd.py (5)

28-29: Dual imports of parse_usd (module and qualified) — consider consistent usage.

You import parse_usd directly and also call newton.utils.parse_usd later. It’s harmless but a bit inconsistent. Prefer one style throughout the file.

- from newton.utils import parse_usd, quat_between_axes
+ from newton.utils import parse_usd, quat_between_axes
...
-        newton.utils.parse_usd(
+        parse_usd(
             str(asset_path),
             builder,

---

554-557: Collider set equality should ignore visual-only shapes.

model_collisions is built from all shapes attached to the body; if visual shapes are present (COLLIDE_SHAPES off), this will over-count and fail equality against USD’s collisions. Filter by the collide flag.

Apply this diff:

-            model_collisions = {model.shape_key[sk] for sk in model.body_shapes[body_idx]}
+            # Only compare physics collision shapes, not visuals
+            flags = model.shape_flags.numpy()
+            model_collisions = {
+                model.shape_key[sk]
+                for sk in model.body_shapes[body_idx]
+                if (int(flags[sk]) & int(newton.ShapeFlags.COLLIDE_SHAPES)) != 0
+            }

---

603-604: Free-joint accounting is brittle; compute it instead of assuming +1.

Assets may have zero or multiple automatically-added free joints (e.g., multiple floating subgraphs). Derive the count from the model.

Apply this diff:

-        self.assertEqual(len(joints_found) + 1, model.joint_count)
+        model_joint_types = model.joint_type.numpy()
+        num_free = int(np.sum(model_joint_types == JointType.FREE))
+        self.assertEqual(len(joints_found) + num_free, model.joint_count)

---

832-845: Account for prim-local axis scale when validating shape dimensions.

The parser multiplies radii/half-heights by the prim’s local scale components (and reorients by axis). The assertions compare against unscaled USD values, which can false-fail when assets use authored scales.

Apply this diff to incorporate per-axis scale:

-            if newton_type in [3, 5]:
+            if newton_type in [3, 5]:
                 usd_axis = int(shape_spec.axis) if hasattr(shape_spec, "axis") else 2
                 axis_quat = (
                     quat_between_axes(newton.Axis.Z, newton.Axis.X)
                     if usd_axis == 0
                     else quat_between_axes(newton.Axis.Z, newton.Axis.Y)
                     if usd_axis == 1
                     else wp.quat_identity()
                 )
                 expected_quat = wp.mul(usd_quat, axis_quat)
             else:
                 expected_quat = usd_quat
@@
-            if newton_type == newton.GeoType.CAPSULE:
-                self.assertAlmostEqual(newton_scale[0], shape_spec.radius, places=5)
-                self.assertAlmostEqual(newton_scale[1], shape_spec.halfHeight, places=5)
+            # Extract authored local scale (only last authored scale op is used in parser)
+            from pxr import UsdGeom  # noqa: PLC0415
+            xf = UsdGeom.Xform(prim)
+            local_scale = np.ones(3, dtype=np.float32)
+            for op in xf.GetOrderedXformOps():
+                if op.GetOpType() == UsdGeom.XformOp.TypeScale:
+                    local_scale = np.array(op.Get(), dtype=np.float32)
+            if newton_type == newton.GeoType.CAPSULE:
+                usd_axis = int(getattr(shape_spec, "axis", 2))
+                self.assertAlmostEqual(newton_scale[0], shape_spec.radius * local_scale[(usd_axis + 1) % 3], places=5)
+                self.assertAlmostEqual(newton_scale[1], shape_spec.halfHeight * local_scale[usd_axis], places=5)
             elif newton_type == newton.GeoType.BOX:
                 for i, (n_scale, u_extent) in enumerate(zip(newton_scale, shape_spec.halfExtents)):
                     self.assertAlmostEqual(
                         n_scale, u_extent, places=5, msg=f"Box {sid} extent[{i}]: USD={u_extent}, Newton={n_scale}"
                     )
             elif newton_type == newton.GeoType.SPHERE:
-                self.assertAlmostEqual(newton_scale[0], shape_spec.radius, places=5)
+                self.assertAlmostEqual(newton_scale[0], shape_spec.radius * local_scale[0], places=5)
             elif newton_type == newton.GeoType.CYLINDER:
-                self.assertAlmostEqual(newton_scale[0], shape_spec.radius, places=5)
-                self.assertAlmostEqual(newton_scale[1], shape_spec.halfHeight, places=5)
+                usd_axis = int(getattr(shape_spec, "axis", 2))
+                self.assertAlmostEqual(newton_scale[0], shape_spec.radius * local_scale[(usd_axis + 1) % 3], places=5)
+                self.assertAlmostEqual(newton_scale[1], shape_spec.halfHeight * local_scale[usd_axis], places=5)

Also applies to: 852-865

---

867-880: Asset tests (g1, anymal) are well-scoped and CI-friendly.

• Uses newton.utils.download_asset to avoid hardcoded local paths.
• Skips gracefully when USD is unavailable.
• Validates via the shared verification harness.

One nit: unify the parse_usd import style (see earlier comment).

Also applies to: 882-902

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

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

You can enable these sources in your CodeRabbit configuration.

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

Learnt from: nvlukasz
PR: newton-physics/newton#519
File: newton/utils.py:30-33
Timestamp: 2025-08-12T18:07:17.267Z
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.267Z
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.267Z
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/utils.py:30-33
Timestamp: 2025-08-12T18:07:17.267Z
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.267Z
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.

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

21-21: Importing warnings is appropriate for parser-level soft failures.

Using the warnings subsystem (vs print) is the right call for invalid USD descriptors. No issues here.

---

795-797: Early-return skip points on invalid descriptors look good.

Placing the skip directly at the top of each loop is clear and prevents partial state mutation. This should reduce hard-to-reproduce errors when datasets contain malformed prims.

Also applies to: 808-810, 850-852, 990-992

newton/tests/test_import_usd.py (3)

16-16: Import math for angle/DOF checks — OK.

Needed for radians/degrees conversions below.

---

25-25: Importing JointType directly simplifies readability.

This reduces newton.JointType verbosity in assertions.

---

531-866: Great end-to-end validation utility for USD assets.

The helper cross-validates bodies, shapes, joints, transforms, and properties against pxr’s parsed descriptors. This will catch regressions in both parser and model-building logic. Nice coverage.

newton/tests/test_import_usd.py (1)

714-728: Correct drive gains scaling: use per-joint-type factor (angular vs. linear)

Using math.degrees(drive_gain_scale) treats the scale as an angle and applies the same factor to linear joints. Angular drives should be scaled by drive_gain_scale * (180/pi), linear drives by drive_gain_scale only.

Apply this diff:

-            if ke_attr:
+            if ke_attr:
                 ke_val = ke_attr.Get() if ke_attr.HasAuthoredValue() else None
                 if ke_val is not None:
                     ke = float(ke_val)
-                    self.assertAlmostEqual(
-                        float(model.joint_target_ke.numpy()[dof_index]), ke * math.degrees(drive_gain_scale), places=2
-                    )
+                    gain_scale = (math.degrees(1.0) * drive_gain_scale) if prim.IsA(UsdPhysics.RevoluteJoint) else drive_gain_scale
+                    self.assertAlmostEqual(
+                        float(model.joint_target_ke.numpy()[dof_index]),
+                        ke * gain_scale,
+                        places=2,
+                    )
 
-            if kd_attr:
+            if kd_attr:
                 kd_val = kd_attr.Get() if kd_attr.HasAuthoredValue() else None
                 if kd_val is not None:
                     kd = float(kd_val)
-                    self.assertAlmostEqual(
-                        float(model.joint_target_kd.numpy()[dof_index]), kd * math.degrees(drive_gain_scale), places=2
-                    )
+                    gain_scale = (math.degrees(1.0) * drive_gain_scale) if prim.IsA(UsdPhysics.RevoluteJoint) else drive_gain_scale
+                    self.assertAlmostEqual(
+                        float(model.joint_target_kd.numpy()[dof_index]),
+                        kd * gain_scale,
+                        places=2,
+                    )

newton/tests/test_import_usd.py (6)

605-607: Compute expected joint count from model free joints rather than a boolean

Assets can have multiple floating bodies; counting free joints directly is more robust than assuming exactly one when floating=True.

-        expected_model_joints = len(joints_found) + 1 if floating else len(joints_found)
-        self.assertEqual(model.joint_count, expected_model_joints)
+        # Count free joints as imported to avoid assumptions about # of floating bodies.
+        free_joints = int(np.count_nonzero(model.joint_type.numpy() == JointType.FREE))
+        expected_model_joints = len(joints_found) + free_joints
+        self.assertEqual(model.joint_count, expected_model_joints)

---

835-846: Prefer enums over magic numbers for shape types

Use newton.GeoType.CAPSULE and newton.GeoType.CYLINDER instead of [3, 5] to improve readability and reduce risk of mismatch if enum values change.

-            if newton_type in [3, 5]:
+            if newton_type in (newton.GeoType.CAPSULE, newton.GeoType.CYLINDER):

---

763-763: Remove no-op call

model.shape_body.numpy() is not used; drop it to reduce noise.

-        model.shape_body.numpy()

---

84-87: Normalize bitmask checks for collision flag

Bitwise checks mix int(enum) and raw enum usages across the file. For consistency and to avoid surprises with IntFlag vs IntEnum, wrap the enum in int(...) here, as done earlier in Line 57.

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

---

27-31: Drop unused get_test_devices and devices

devices = get_test_devices() is never used in this file. Removing the import and assignment avoids lint warnings.

-from newton.tests.unittest_utils import USD_AVAILABLE, assert_np_equal, get_test_devices
+from newton.tests.unittest_utils import USD_AVAILABLE, assert_np_equal
-
-devices = get_test_devices()

---

874-880: Unify parse_usd usage style

You import parse_usd at Line 28 but still call newton.utils.parse_usd in several places. Pick one style (prefer the imported name) for consistency.

Would you like me to submit a follow-up patch to switch all calls in this file to the imported parse_usd?

Also applies to: 896-902, 911-917, 926-932, 941-947

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

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

You can enable these sources in your CodeRabbit configuration.

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

Learnt from: nvlukasz
PR: newton-physics/newton#519
File: newton/utils.py:30-33
Timestamp: 2025-08-12T18:07:17.267Z
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.267Z
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.267Z
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/utils.py:30-33
Timestamp: 2025-08-12T18:07:17.267Z
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.267Z
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.

newton/tests/test_import_usd.py (2)

701-716: Fix KE/KD scaling: use drive_gain_scale / radians(1) (not degrees(drive_gain_scale)).

Current assertions treat the scale as an angle. Expect scale * (180/π) for angular drives.

-                    self.assertAlmostEqual(
-                        float(model.joint_target_ke.numpy()[dof_index]), ke * math.degrees(drive_gain_scale), places=2
-                    )
+                    self.assertAlmostEqual(
+                        float(model.joint_target_ke.numpy()[dof_index]),
+                        ke * drive_gain_scale * math.degrees(1.0),
+                        places=2,
+                    )
...
-                    self.assertAlmostEqual(
-                        float(model.joint_target_kd.numpy()[dof_index]), kd * math.degrees(drive_gain_scale), places=2
-                    )
+                    self.assertAlmostEqual(
+                        float(model.joint_target_kd.numpy()[dof_index]),
+                        kd * drive_gain_scale * math.degrees(1.0),
+                        places=2,
+                    )

---

559-566: Fix principal-axes quaternion misuse (Gf.Quat.Normalize returns magnitude, not a quaternion).

This assigns a float to principal_axes and produces a wrong inertia rotation. Build a normalized Warp quaternion from the Gf.Quat components (or use GetNormalized) before forming the rotation matrix.

-                if mass_api.GetDiagonalInertiaAttr().HasAuthoredValue():
-                    diag_inertia = mass_api.GetDiagonalInertiaAttr().Get()
-                    principal_axes = mass_api.GetPrincipalAxesAttr().Get().Normalize()
-                    p = np.array(wp.quat_to_matrix(wp.quat(*principal_axes.imaginary, principal_axes.real))).reshape(
-                        (3, 3)
-                    )
+                if mass_api.GetDiagonalInertiaAttr().HasAuthoredValue():
+                    diag_inertia = mass_api.GetDiagonalInertiaAttr().Get()
+                    gfq = mass_api.GetPrincipalAxesAttr().Get()
+                    if gfq is not None:
+                        ix, iy, iz = gfq.GetImaginary()
+                        w = gfq.GetReal()
+                        q_wp = wp.normalize(wp.quat(ix, iy, iz, w))
+                    else:
+                        q_wp = wp.quat_identity()
+                    p = np.array(wp.quat_to_matrix(q_wp)).reshape(3, 3)

newton/tests/test_import_usd.py (4)

822-833: Avoid magic numbers for GeoType; use enum members.

Using raw ints [3, 5] is brittle; prefer newton.GeoType.CAPSULE and newton.GeoType.CYLINDER.

-            if newton_type in [3, 5]:
+            if newton_type in [newton.GeoType.CAPSULE, newton.GeoType.CYLINDER]:

---

640-643: Use precomputed DOF start to avoid O(n²) summation and off-by-one risks.

model.joint_qd_start is available and clearer.

-            dof_index = (
-                0 if j <= 0 else sum(int(joint_dof_dim_array[i][0] + joint_dof_dim_array[i][1]) for i in range(j))
-            )
+            dof_index = int(model.joint_qd_start.numpy()[j])

---

539-545: Collider set equivalence may fail for bodies with child shapes.

Equality assumes all collisions are authored directly on the body prim. USD scenes often attach colliders to children; consider normalizing to the set of all descendant collider paths for comparison or limit the check to bodies without collider children.

---

30-31: Remove unused variable.

devices = get_test_devices() is never used.

-devices = get_test_devices()
+# devices = get_test_devices()  # Unused; remove if not needed.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

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

You can enable these sources in your CodeRabbit configuration.

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

Learnt from: nvlukasz
PR: newton-physics/newton#519
File: newton/utils.py:30-33
Timestamp: 2025-08-12T18:07:17.267Z
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.267Z
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.267Z
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/utils.py:30-33
Timestamp: 2025-08-12T18:07:17.267Z
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.267Z
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: adenzler-nvidia
PR: newton-physics/newton#552
File: newton/_src/solvers/mujoco/solver_mujoco.py:0-0
Timestamp: 2025-08-18T15:56:26.587Z
Learning: In Newton's MuJoCo solver, when transforming joint axes from Newton's internal frame to MuJoCo's expected frame, use wp.quat_rotate(joint_rot, axis) not wp.quat_rotate_inv(joint_rot, axis). The joint_rot represents rotation from joint-local to body frame, so forward rotation is correct.

newton/tests/test_import_usd.py (2)

559-566: Fix principal-axes quaternion misuse: Normalize() returns magnitude, not a quaternion.

This assigns a float to principal_axes and produces wrong inertia rotation.

-                    principal_axes = mass_api.GetPrincipalAxesAttr().Get().Normalize()
-                    p = np.array(wp.quat_to_matrix(wp.quat(*principal_axes.imaginary, principal_axes.real))).reshape(
-                        (3, 3)
-                    )
+                    gfq = mass_api.GetPrincipalAxesAttr().Get()
+                    if gfq is not None:
+                        ix, iy, iz = gfq.GetImaginary()
+                        w = gfq.GetReal()
+                        q_wp = wp.normalize(wp.quat(ix, iy, iz, w))
+                    else:
+                        q_wp = wp.quat_identity()
+                    p = np.array(wp.quat_to_matrix(q_wp)).reshape(3, 3)

---

701-715: Correct angular drive gain scaling: don’t use degrees(drive_gain_scale).

Importer scales angular gains by drive_gain_scale / radians(1) = drive_gain_scale * degrees(1). Current code treats the scale as an angle.

-                    self.assertAlmostEqual(
-                        float(model.joint_target_ke.numpy()[dof_index]), ke * math.degrees(drive_gain_scale), places=2
-                    )
+                    self.assertAlmostEqual(
+                        float(model.joint_target_ke.numpy()[dof_index]),
+                        ke * math.degrees(1.0) * drive_gain_scale,
+                        places=2,
+                    )
...
-                    self.assertAlmostEqual(
-                        float(model.joint_target_kd.numpy()[dof_index]), kd * math.degrees(drive_gain_scale), places=2
-                    )
+                    self.assertAlmostEqual(
+                        float(model.joint_target_kd.numpy()[dof_index]),
+                        kd * math.degrees(1.0) * drive_gain_scale,
+                        places=2,
+                    )

newton/tests/test_import_usd.py (2)

640-643: Use joint_qd_start for O(1) DOF start index.

Avoid recomputing prefix sums each loop.

-            dof_index = (
-                0 if j <= 0 else sum(int(joint_dof_dim_array[i][0] + joint_dof_dim_array[i][1]) for i in range(j))
-            )
+            dof_index = int(model.joint_qd_start.numpy()[j])

---

750-750: Remove unused call.

model.shape_body.numpy() return value isn’t used.

-        model.shape_body.numpy()

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

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

You can enable these sources in your CodeRabbit configuration.

Learnt from: nvlukasz
PR: newton-physics/newton#519
File: newton/utils.py:30-33
Timestamp: 2025-08-12T18:07:17.267Z
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.267Z
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.267Z
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: adenzler-nvidia
PR: newton-physics/newton#552
File: newton/_src/solvers/mujoco/solver_mujoco.py:0-0
Timestamp: 2025-08-18T15:56:26.587Z
Learning: In Newton's MuJoCo solver, when transforming joint axes from Newton's internal frame to MuJoCo's expected frame, use wp.quat_rotate(joint_rot, axis) not wp.quat_rotate_inv(joint_rot, axis). The joint_rot represents rotation from joint-local to body frame, so forward rotation is correct.