Margin and Gap rename#1732
Merged
Merged
Conversation
Contributor
|
Caution Review failedAn error occurred during the review process. Please try again later. 📝 WalkthroughWalkthroughSystematic rename of collision parameters: Changes
Sequence Diagram(s)sequenceDiagram
participant Builder as rgba(52, 152, 219, 0.5)
participant USD as rgba(46, 204, 113, 0.5)
participant Broad as rgba(155, 89, 182, 0.5)
participant Narrow as rgba(241, 196, 15, 0.5)
participant Reduction as rgba(230, 126, 34, 0.5)
participant Solver as rgba(231, 76, 60, 0.5)
Builder->>USD: read shape attrs (gap, margin)
USD-->>Builder: provide ShapeConfig(gap, margin)
Builder->>Broad: prepare geom data (shape_margin, shape_gap)
Broad->>Broad: AABB overlap check: cutoff = gap_a + gap_b
Broad-->>Narrow: candidate pairs (with gaps/margins)
Narrow->>Narrow: compute contacts using margin_a/margin_b + rigid_gap
Narrow-->>Reduction: emit contacts (contact.margin_a/margin_b, contact.margin)
Reduction->>Solver: reduced contacts (use margin fields in solver inputs)
Solver->>Solver: resolve constraints using per-contact margin params
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
newton/examples/contacts/example_nut_bolt_hydro.py (1)
106-106:⚠️ Potential issue | 🟡 MinorStale docstring term after rename.
The parameter description still uses the old "contact margin" terminology after
contact_marginwas renamed togap.📝 Proposed fix
- shape_cfg: Optional shape configuration used for contact margin [m]. + shape_cfg: Optional shape configuration used to derive the SDF margin from the shape gap [m].🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/examples/contacts/example_nut_bolt_hydro.py` at line 106, The docstring for the parameter that documents shape_cfg still uses the old term "contact margin" after the parameter rename; update the docstring in example_nut_bolt_hydro.py so it refers to the renamed parameter "gap" (e.g., "Optional shape configuration used for gap [m]") and search for any other occurrences of "contact margin" in that file or in comments near shape_cfg, contact_margin, or gap to ensure all text reflects the new name.newton/tests/test_broad_phase.py (1)
2054-2149:⚠️ Potential issue | 🟡 MinorAddress gap semantics mismatch between kernel and test reference helpers.
The broad-phase kernel uses additive gap combination (
cutoff_combined = box1_cutoff + box2_cutoff), but the test reference helpers (check_aabb_overlap_hostandfind_overlapping_pairs_np) use max combination (max(cutoff[i], cutoff[j])). For this specific test case, both approaches yield correct results (Sphere B overlaps, Sphere A does not), so the test is not weakened. However, the mismatch should be reconciled to ensure consistency and prevent issues with future tests using different gap value patterns. The regression test requirement is satisfied: the test fails without the kernel-side gap expansion logic, as verified.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/tests/test_broad_phase.py` around lines 2054 - 2149, The test and broad-phase kernel disagree on gap combination semantics: the kernel uses additive combination (cutoff_combined = box1_cutoff + box2_cutoff) while the test reference helpers check_aabb_overlap_host and find_overlapping_pairs_np use max(cutoff[i], cutoff[j]); update the reference helper logic to mirror the kernel by replacing the max-based combination with additive combination (use cutoff_combined = cutoff[i] + cutoff[j]) so the host-side overlap checks and pair-finding match BroadPhaseAllPairs and BroadPhaseSAP behavior.newton/_src/solvers/xpbd/solver_xpbd.py (1)
523-554:⚠️ Potential issue | 🟡 MinorUpdate kernel parameter names to match call-site terminology.
The
solve_body_contact_positionsandapply_rigid_restitutionkernels innewton/_src/solvers/xpbd/kernels.pystill usecontact_thickness0/1as parameter names, but the call-site now passescontacts.rigid_contact_margin0/1. While Warp's positional dispatch makes this functionally correct, the parameter names should be renamed tocontact_margin0/1for consistency and clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/solvers/xpbd/solver_xpbd.py` around lines 523 - 554, The kernels solve_body_contact_positions and apply_rigid_restitution use parameter names contact_thickness0 and contact_thickness1 but the call-site passes contacts.rigid_contact_margin0 and contacts.rigid_contact_margin1; rename the kernel parameters to contact_margin0 and contact_margin1 (and update any internal references) so the parameter names match the call-site terminology and improve clarity—update definitions in newton/_src/solvers/xpbd/kernels.py for both solve_body_contact_positions and apply_rigid_restitution accordingly.newton/tests/test_import_mjcf.py (1)
2235-2266:⚠️ Potential issue | 🟡 MinorUpdate the test docstring to match “margin” terminology and include units.
The test now validates
shape_margin, but the docstring still references “shape thickness” and omits units for margin/scale. Updating the docstring avoids confusion and keeps docs aligned with the rename.✏️ Suggested docstring update
- def test_mjcf_geom_margin_parsing(self): - """Test MJCF geom margin is parsed to shape thickness. - - Verifies that MJCF geom margin values are mapped to shape thickness and - that geoms without an explicit margin use the default thickness. - Also checks that the model scale is applied to the margin value. - """ + def test_mjcf_geom_margin_parsing(self): + """Test MJCF geom margin is parsed to shape margin. + + Verifies that MJCF geom margin values [m] are mapped to shape margin [m], + that geoms without an explicit margin use the default margin [m], and that + the model scale [unitless] is applied to the margin value. + """As per coding guidelines, "Keep the documentation up-to-date" and "State SI units for all physical quantities in docstrings using inline [unit] notation."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/tests/test_import_mjcf.py` around lines 2235 - 2266, Update the test_mjcf_geom_margin_parsing docstring to use the term "margin" instead of "shape thickness" and state units for margin and scale (meters) using inline [m] notation; e.g., describe that MJCF geom margin values are parsed to shape_margin (in meters) and that the model scale multiplies the margin value, referencing the test functions/variables builder, builder_scaled, shape_margin, and scale so readers know which symbols the docstring describes.newton/tests/test_import_usd.py (2)
4494-4539:⚠️ Potential issue | 🟡 MinorAlign USD attribute name with renamed gap API (and document units).
This test still authors
newton:contactMargin. If the rename intendsnewton:gapfor rigid bodies, the test should reflect that (and optionally add a separate backward‑compat test forcontactMargin). While touching this docstring, please include the gap unit in[m]to follow the docstring/unit guideline.Suggested update
- def test_gap_parsing(self): - """Test that gap is parsed correctly from USD.""" + def test_gap_parsing(self): + """Tests that USD gap [m] is parsed into shape_gap.""" @@ - # Create a collider with newton:contactMargin (mapped to internal "gap") + # Create a collider with newton:gap @@ - collider1_prim.GetAttribute("newton:contactMargin").Set(0.05) + collider1_prim.GetAttribute("newton:gap").Set(0.05)As per coding guidelines: “Use Google-style docstrings… State SI units for all physical quantities in docstrings using inline [unit] notation (e.g., [m], [N], [N·m]).”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/tests/test_import_usd.py` around lines 4494 - 4539, Update the test_gap_parsing docstring and authored USD attribute to use the renamed gap API: change any authored attribute "newton:contactMargin" to "newton:gap" on collider1_prim and note the unit in the test docstring as "[m]"; keep the rest of the logic that sets builder.default_shape_cfg.gap and asserts model.shape_gap values, and optionally add a small separate test verifying backward compatibility that still reads "newton:contactMargin" if you want to ensure legacy support (reference symbols: test_gap_parsing, newton.ModelBuilder, builder.default_shape_cfg.gap, model.shape_gap, collider1_prim.GetAttribute).
4494-4539:⚠️ Potential issue | 🟡 MinorVerify this regression test fails when the internal schema key is reverted from
gaptocontact_margin.Temporarily revert the schema change in
newton/_src/usd/schemas.py(changing"gap"back to"contact_margin"for thenewton:contactMarginattribute mapping), run the test to confirm it fails, then reapply the change and verify the test passes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/tests/test_import_usd.py` around lines 4494 - 4539, The regression shows the USD attribute "newton:contactMargin" is being mapped to the wrong internal key; fix the mapping in the USD schema so "newton:contactMargin" maps to the internal "gap" key (not "contact_margin") so that ModelBuilder.add_usd and test_gap_parsing (which asserts model.shape_gap values and uses builder.default_shape_cfg.gap) work correctly; locate the mapping in the USD schema module (the dict/constant that maps USD attribute names to internal schema keys in schemas.py) and change the value for "newton:contactMargin" to "gap", then run the test to confirm it passes.newton/_src/geometry/sdf_contact.py (1)
686-874:⚠️ Potential issue | 🟡 MinorUpdate kernel docstring to reflect shape_gap/margin terminology (with units).
The docstring still mentions
geom_cutoffandthickness, but the signature now usesshape_gapand the renamed margin semantics. Please update the Args block to match parameter names and add[m]units.Suggested docstring tweak
- geom_types: Array of geometry types for all shapes - geom_data: Array of vec4 containing scale (xyz) and thickness (w) for each shape - geom_transform: Array of world-space transforms for each shape - geom_source: Array of source pointers (mesh IDs) for each shape + shape_data: Array of vec4 containing scale xyz and margin [m] in w for each shape + shape_transform: Array of world-space transforms for each shape + shape_source: Array of source pointers (mesh IDs) for each shape @@ - geom_cutoff: Array of cutoff distances for each shape + shape_gap: Per-shape broad-phase gap [m]As per coding guidelines: “Use Google-style docstrings… State SI units for all physical quantities in docstrings using inline [unit] notation (e.g., [m], [N], [N·m]).”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/geometry/sdf_contact.py` around lines 686 - 874, The kernel docstring in mesh_sdf_collision_kernel is outdated: replace references to geom_cutoff and thickness with the actual parameter names (shape_gap and shape_data[...,3] for per-shape thickness) and document their units as SI (e.g., shape_gap [m], triangle_mesh_thickness [m], sdf_mesh_thickness [m], contact_threshold [m]); update the Args block to list shape_data, shape_transform, shape_source, sdf_table, shape_sdf_index, shape_gap, shape_pairs_mesh_mesh, shape_pairs_mesh_mesh_count, writer_data, total_num_blocks and briefly describe each (include that shape_gap is the per-shape margin in meters and that thickness values are read from shape_data[...,3] in meters) while keeping Google-style formatting.newton/_src/sim/builder.py (1)
5374-5381:⚠️ Potential issue | 🟠 MajorPreserve per‑shape
gapwhen generating remeshed collision shapesDuring remeshing/convex decomposition, new shapes inherit
marginbut notgap, so custom broad‑phase spacing is lost. Copyingshape_gapkeeps collision behavior consistent.✅ Suggested fix
cfg = ModelBuilder.ShapeConfig( density=0.0, # do not add extra mass / inertia margin=self.shape_margin[shape], + gap=self.shape_gap[shape], is_solid=self.shape_is_solid[shape], has_shape_collision=False, has_particle_collision=False, is_visible=True, )cfg = ModelBuilder.ShapeConfig( density=0.0, # do not add extra mass / inertia ke=self.shape_material_ke[shape], kd=self.shape_material_kd[shape], kf=self.shape_material_kf[shape], ka=self.shape_material_ka[shape], mu=self.shape_material_mu[shape], restitution=self.shape_material_restitution[shape], mu_torsional=self.shape_material_mu_torsional[shape], mu_rolling=self.shape_material_mu_rolling[shape], kh=self.shape_material_kh[shape], margin=self.shape_margin[shape], + gap=self.shape_gap[shape], is_solid=self.shape_is_solid[shape], collision_group=self.shape_collision_group[shape], collision_filter_parent=self.default_shape_cfg.collision_filter_parent, )Also applies to: 5447-5463
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/sim/builder.py` around lines 5374 - 5381, Remeshed/convex-decomposed shapes currently copy margin but omit per-shape gap, losing custom broad-phase spacing; update the ModelBuilder.ShapeConfig construction(s) (the block creating cfg = ModelBuilder.ShapeConfig(...)) to include gap=self.shape_gap[shape] so new shapes preserve per-shape gap, and apply the same change in the other analogous block that builds ShapeConfig during remeshing/convex decomposition (the second occurrence around the convex-decomposition remesh code).
🧹 Nitpick comments (14)
newton/_src/solvers/semi_implicit/kernels_contact.py (1)
410-411: Incomplete rename: kernel parameterscontact_thickness0/1still use the oldthicknessterminology.The local variable rename to
margin_a/margin_b(lines 433–434) and the call-site update torigid_contact_margin0/1(lines 642–643) are correct, but the kernel's formal parameter names at lines 410–411 were not updated. The comment on line 426 also still references "thickness".♻️ Proposed fix
- contact_thickness0: wp.array(dtype=float), - contact_thickness1: wp.array(dtype=float), + contact_margin0: wp.array(dtype=float), + contact_margin1: wp.array(dtype=float),- # retrieve contact thickness, compute average contact material properties + # retrieve contact margin, compute average contact material properties- margin_a = contact_thickness0[tid] - margin_b = contact_thickness1[tid] + margin_a = contact_margin0[tid] + margin_b = contact_margin1[tid]Also applies to: 426-426, 433-434
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/solvers/semi_implicit/kernels_contact.py` around lines 410 - 411, The kernel's formal parameter names contact_thickness0 and contact_thickness1 should be renamed to contact_margin0 and contact_margin1 to match the local variables margin_a/margin_b and the call-site arguments rigid_contact_margin0/1; update any in-kernel references and the nearby comment that still mentions "thickness" to use "margin" instead so naming is consistent (look for the kernel signature parameters contact_thickness0/contact_thickness1, the local variables margin_a/margin_b, and the call sites using rigid_contact_margin0/1).newton/examples/contacts/example_nut_bolt_sdf.py (1)
108-108: Consider a short clarifying comment for thegap→ SDFmarginmapping.With the new naming,
ShapeConfignow has bothmargin(surface collision margin) andgap(broad-phase detection offset). Passingshape_cfg.gap— notshape_cfg.margin— tobuild_sdf(margin=…)is correct (the SDF narrow band must cover at least the detection gap), but it can surprise readers who might expectshape_cfg.marginhere. A brief inline comment makes the intent self-documenting:📝 Suggested clarification
mesh.build_sdf( max_resolution=MESH_SDF_MAX_RESOLUTION, narrow_band_range=MESH_SDF_NARROW_BAND_RANGE, - margin=shape_cfg.gap if shape_cfg and shape_cfg.gap is not None else 0.05, + # Use gap (broad-phase detection offset) so the SDF band covers the contact detection range. + margin=shape_cfg.gap if shape_cfg and shape_cfg.gap is not None else 0.05, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/examples/contacts/example_nut_bolt_sdf.py` at line 108, The code passes shape_cfg.gap into build_sdf(margin=...) which is correct but confusing given ShapeConfig now has both margin (collision surface margin) and gap (broad-phase detection offset); update the call site (around build_sdf(...)) to add a short inline comment clarifying that we intentionally use shape_cfg.gap for the SDF narrow band because the SDF margin must cover the detection gap (not the collision surface margin), and mention the symbols ShapeConfig, gap, margin, and build_sdf to make intent explicit for future readers.newton/_src/usd/schemas.py (1)
70-71: Consider a temporarycontact_marginalias during migration.If you want to smooth the transition for existing USD pipelines, you could keep a deprecated alias that maps to the same USD attribute while docs migrate.
Optional backward-compat alias
PrimType.SHAPE: { # Mesh "max_hull_vertices": SchemaAttribute("newton:maxHullVertices", -1), # Collisions "gap": SchemaAttribute("newton:contactMargin", float("-inf")), + "contact_margin": SchemaAttribute("newton:contactMargin", float("-inf")), # deprecated },PrimType.SHAPE: { # Mesh "max_hull_vertices": SchemaAttribute("physxConvexHullCollision:hullVertexLimit", 64), # Collisions "gap": SchemaAttribute("physxCollision:contactOffset", float("-inf")), + "contact_margin": SchemaAttribute("physxCollision:contactOffset", float("-inf")), # deprecated },Also applies to: 152-152
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/usd/schemas.py` around lines 70 - 71, Add a deprecated alias mapping for contact_margin that points to the same USD attribute as the "gap" entry to ease migration: keep the existing "gap": SchemaAttribute("newton:contactMargin", float("-inf")) and add a "contact_margin" key (or similar snake_case alias) that returns/forwards to SchemaAttribute("newton:contactMargin", float("-inf")) with a clear deprecation comment; apply the same pattern for the other occurrence around the second instance noted (the entry at the other location referenced in the comment) so both places expose the temporary alias during migration.newton/_src/solvers/vbd/rigid_vbd_kernels.py (1)
1522-1523: Renamethicknessvariables tomargin_totalfor clarity.Now that the inputs are
rigid_contact_margin0/1, keepingthicknessnaming is a little misleading. Consider renaming the accumulated value in both kernels to match the new terminology.♻️ Suggested rename
- thickness = rigid_contact_margin0[contact_idx] + rigid_contact_margin1[contact_idx] - penetration = thickness - dist + margin_total = rigid_contact_margin0[contact_idx] + rigid_contact_margin1[contact_idx] + penetration = margin_total - dist- thickness_total = rigid_contact_margin0[idx] + rigid_contact_margin1[idx] - penetration = wp.max(0.0, thickness_total - dist) + margin_total = rigid_contact_margin0[idx] + rigid_contact_margin1[idx] + penetration = wp.max(0.0, margin_total - dist)Also applies to: 1579-1581, 2215-2216, 2274-2275
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/solvers/vbd/rigid_vbd_kernels.py` around lines 1522 - 1523, The code still uses a local accumulated variable named thickness while the inputs are now rigid_contact_margin0 and rigid_contact_margin1; rename that local variable to margin_total in the affected kernels and any related helper code so the name matches the new inputs. Locate the kernels that read rigid_contact_margin0/rigid_contact_margin1 and replace the thickness variable (and its uses in computations, conditionals, and comments) with margin_total, and update any nearby comments/docstrings to reflect the new terminology; apply the same rename to the other occurrences where thickness is computed/used alongside the rigid_contact_margin arrays.newton/_src/geometry/contact_data.py (1)
34-47: Add SI units for the new margin fields in the docstring.To comply with the docstring unit requirement, consider annotating the new margin fields with
[m].♻️ Proposed docstring tweak
- margin_a: Collision surface margin offset for shape A - margin_b: Collision surface margin offset for shape B + margin_a: Collision surface margin offset for shape A [m] + margin_b: Collision surface margin offset for shape B [m] ... - margin: Contact detection margin/threshold + margin: Contact detection margin/threshold [m]As per coding guidelines: “State SI units for all physical quantities in docstrings using inline [unit] notation (e.g., [m], [N]).”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/geometry/contact_data.py` around lines 34 - 47, The docstring entries for collision surface margins are missing SI units; update the docstring in contact_data.py (the ContactData/class-level docstring) to append "[m]" to margin_a, margin_b, and margin so they read e.g. "margin_a: Collision surface margin offset for shape A [m]" (and similarly for margin_b and margin) and also add "[m]" to the standalone "margin" entry, preserving existing wording and punctuation.newton/tests/test_narrow_phase.py (1)
1317-1426:test_per_shape_gapdoes not distinguishgap_a + gap_bfrommax(gap_a, gap_b)semantics.For all three sub-tests the chosen geometry values yield the same pass/fail outcome under either formula (since
sum ≥ max, the boundary cases never lie in the regionmax(gap_a, gap_b) < separation ≤ gap_a + gap_b). If the broad-phase change togap_a + gap_bshould propagate to narrow-phase detection too, a sub-test wheremax(gap_a, gap_b) < separation ≤ gap_a + gap_bwould create a concrete regression guard.Example discriminating scenario (sphere at
z = margin_sum - εwith two asymmetric gaps):# gap_a = 0.04, gap_b = 0.04 → sum = 0.08, max = 0.04 # sphere bottom at z = 0.06 (between max and sum) # With sum: 0.06 < 0.08 → contact # With max: 0.06 > 0.04 → no contact🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/tests/test_narrow_phase.py` around lines 1317 - 1426, Add a discriminating sub-test to ensure narrow-phase uses gap_a + gap_b (not max) by creating asymmetric per-shape gaps and placing the sphere at a separation between max(gap_a,gap_b) and gap_a+gap_b; specifically, set shape_gap so the two interacting shapes have gaps 0.04 and 0.04 (sum=0.08, max=0.04), set geom_transform for that pair so the sphere center is at z=0.06 (separation=0.06), call self.narrow_phase.launch with the same candidate pair/pair_count/pairs/pair setup used above, zero contact_count before the call, wp.synchronize(), and assertGreater(contact_count.numpy()[0], 0) to fail if narrow-phase wrongly used max(gap_a,gap_b) instead of the sum; reference shape_gap, geom_transform, pairs, contact_count, and self.narrow_phase.launch when adding this block.newton/examples/robot/example_robot_panda_hydro.py (1)
135-136: Extract the0.05SDF-margin fallback as a named constant.The expression
shape_cfg.gap if shape_cfg.gap is not None else 0.05appears four times with the same magic value. Sincegap=0.01is always set onshape_cfg, the else-branch is currently unreachable, and ifgapwere left atNonethe SDF would silently be built with a margin 5× larger than the configured broad-phase gap — the two values should at least be documented together.♻️ Suggested refactor
+_DEFAULT_SDF_MARGIN = 0.05 # fallback SDF build margin when no gap is configured + ... - mesh.build_sdf( - max_resolution=hydro_mesh_sdf_max_resolution, - narrow_band_range=shape_cfg.sdf_narrow_band_range, - margin=shape_cfg.gap if shape_cfg.gap is not None else 0.05, - ) + mesh.build_sdf( + max_resolution=hydro_mesh_sdf_max_resolution, + narrow_band_range=shape_cfg.sdf_narrow_band_range, + margin=shape_cfg.gap if shape_cfg.gap is not None else _DEFAULT_SDF_MARGIN, + )Apply the same substitution at the other three call sites (lines 185, 209, 259).
Also applies to: 185-186, 209-210, 259-260
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/examples/robot/example_robot_panda_hydro.py` around lines 135 - 136, Introduce a named constant (e.g., DEFAULT_SDF_MARGIN = 0.05) near the top of the module and replace all occurrences of the inline fallback expression shape_cfg.gap if shape_cfg.gap is not None else 0.05 with a single helper value (e.g., margin = shape_cfg.gap if shape_cfg.gap is not None else DEFAULT_SDF_MARGIN) at each call site; update all four sites that currently use that expression so the fallback is documented and controlled via the constant (keep the variable name DEFAULT_SDF_MARGIN or similar to make intent clear and colocate the constant with related comment about the broad-phase gap relationship).newton/tests/test_mujoco_solver.py (1)
2635-2700: Align test naming and variables with the margin rename.The test now validates
shape_margin/geom_margin, but the function name, comment, andnew_thicknessvariable still use the old terminology. Renaming them will keep the test consistent with the new API and reduce confusion later.♻️ Suggested rename for clarity
- def test_geom_margin_from_thickness(self): + def test_geom_margin_from_shape_margin(self): @@ - # Update thickness values at runtime - new_thickness = np.array([0.02 + i * 0.005 for i in range(model.shape_count)], dtype=np.float32) - model.shape_margin.assign(wp.array(new_thickness, dtype=wp.float32, device=model.device)) + # Update margin values at runtime + new_margin = np.array([0.02 + i * 0.005 for i in range(model.shape_count)], dtype=np.float32) + model.shape_margin.assign(wp.array(new_margin, dtype=wp.float32, device=model.device)) @@ - float(new_thickness[shape_idx]), + float(new_margin[shape_idx]),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/tests/test_mujoco_solver.py` around lines 2635 - 2700, The test and its variables still use "thickness" naming while the API uses "margin": rename the test function and variables/comments to reflect margin. Specifically, rename test_geom_margin_from_thickness to something like test_geom_margin_from_shape_margin (or test_geom_margin_updates_from_shape_margin), update the docstring to mention shape_margin/geom_margin instead of thickness, and rename the runtime update array new_thickness to new_margin (and its uses: model.shape_margin.assign(...), new_margin[shape_idx], etc.) so all identifiers (test name, docstring, and variable new_thickness) consistently use "margin" while leaving references to model.shape_margin, solver.mjw_model.geom_margin, and SolverNotifyFlags.SHAPE_PROPERTIES unchanged.newton/_src/geometry/sdf_hydroelastic.py (2)
1289-1290: Add a clarifying comment:contact_data.margin_a/bis the per-shape surface margin (formerlythickness), not the gap — setting 0.0 here is intentional.With the local variables
margin_a/margin_b(gap values) in scope just above, these zero-assignments look like a potential bug or omission. A brief inline comment prevents that misread:💬 Suggested comment
- contact_data.margin_a = 0.0 - contact_data.margin_b = 0.0 + contact_data.margin_a = 0.0 # surface collision margin (not gap); unused in hydroelastic path + contact_data.margin_b = 0.0 # surface collision margin (not gap); unused in hydroelastic pathThis becomes less necessary if
margin_a/margin_blocals are renamed togap_a/gap_bas suggested above.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/geometry/sdf_hydroelastic.py` around lines 1289 - 1290, The two assignments contact_data.margin_a = 0.0 and contact_data.margin_b = 0.0 are intentional: add a short inline comment on those lines clarifying that contact_data.margin_a/b are the per-shape surface margin (formerly called "thickness") and not the inter-shape gap (the locals margin_a/margin_b), so setting them to 0.0 here is deliberate; alternatively, rename the local variables margin_a/margin_b to gap_a/gap_b to avoid confusion and update any nearby references accordingly.
1073-1074: Local variable namesmargin_a/margin_b(assigned fromshape_gap) still use the old "margin" terminology — rename togap_a/gap_bfor consistency.The same pattern appears in three kernels:
Kernel Lines count_iso_voxels_block1073-1074 decode_contacts_kernel1265-1266 generate_contacts_kernel1376-1377 The local variables read from
shape_gap(the oldcontact_margin, i.e., the broad-phase detection offset) but are still namedmargin_a/margin_b. That conflicts with the newgapterminology and, more importantly, makes line 1289-1290 (contact_data.margin_a = 0.0) look like a silent overwrite of the computed value when it actually refers to a different field (ContactData.margin_a, formerlythickness_a— the surface collision margin).♻️ Suggested rename in all three kernels
- margin_a = shape_gap[shape_a] - margin_b = shape_gap[shape_b] + gap_a = shape_gap[shape_a] + gap_b = shape_gap[shape_b]Downstream uses (e.g.,
margin = margin_a + margin_b,va > r + margin_a) would becomegap_a + gap_bandva > r + gap_a, aligning with the new naming convention throughout. Then the localmarginvariable combining both could be renamedgapor left ascombined_gapfor clarity.Also applies to: 1265-1266, 1376-1377
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/geometry/sdf_hydroelastic.py` around lines 1073 - 1074, Rename the local variables that read from shape_gap in the three kernels to reflect the new "gap" terminology: in count_iso_voxels_block, decode_contacts_kernel, and generate_contacts_kernel replace margin_a/margin_b with gap_a/gap_b (and update any downstream uses like margin = margin_a + margin_b to gap = gap_a + gap_b or combined_gap as appropriate), and update expressions such as va > r + margin_a to va > r + gap_a so the local gap values are not confused with ContactData.margin_a (the surface collision margin).newton/_src/geometry/sdf_utils.py (3)
306-329: Kernel parameters still namedthickness— consider renaming for consistency.The
sdf_from_mesh_kernel(line 311) andsdf_from_primitive_kernel(line 338) still usethicknessas the parameter name, while the callers now passshape_margin. Since these are internal Warp kernels (not user-facing), this is low priority, but renaming them tomarginorshape_marginwould keep the terminology consistent end-to-end and reduce confusion for future maintainers.Also applies to: 332-366
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/geometry/sdf_utils.py` around lines 306 - 329, Rename the kernel parameter `thickness` to `shape_margin` (or `margin`) in the Warp kernels `sdf_from_mesh_kernel` and `sdf_from_primitive_kernel` so the parameter name matches callers; update the parameter list and all uses inside each function (e.g., where `thickness` is subtracted from `signed_distance`) to the new name to keep terminology consistent across call sites and kernels.
188-188: Naming inconsistency:SDF.create_from_meshstill usesthicknesswhile callees useshape_margin.
SDF.create_from_mesh(line 152) andSDF.create_from_points(line 113) still expose athicknessparameter, while the standalonecompute_sdf_from_shapeand_compute_sdf_from_shape_implnow useshape_margin. This creates a mixed public API where the same concept has two names.Line 719 does
thickness=shape_marginand line 188 doesshape_margin=thickness— both are correct bridging code, but the inconsistency in public parameter names will be confusing to users.If the plan is to rename these in a follow-up, consider adding a
# TODOto track it. Otherwise, renamingthickness→shape_margin(or justmargin) in theSDF.create_from_*methods would complete the rename in this file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/geometry/sdf_utils.py` at line 188, SDF.create_from_mesh and SDF.create_from_points expose a parameter named thickness while helper functions use shape_margin, causing an inconsistent public API; fix by renaming the public parameter thickness → shape_margin (or margin) in the SDF.create_from_mesh and SDF.create_from_points signatures and update all internal references and calls (including any callers that pass thickness) to use shape_margin, or if you prefer a staged change add a clear TODO on both SDF.create_from_* methods noting the planned rename and keep the bridging assignments (e.g., shape_margin=thickness) until callers are migrated.
458-468: Missing[m]unit annotation inshape_margindocstring.Line 480 documents
shape_marginas"Margin offset to subtract from SDF values."but omits the[m]unit, while the equivalent docstring incompute_sdf_from_shapeat line 696 correctly includes[m]. As per coding guidelines, SI units should be stated for all physical quantities in docstrings.Proposed fix
- shape_margin: Margin offset to subtract from SDF values. + shape_margin: Margin offset [m] to subtract from SDF values.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/geometry/sdf_utils.py` around lines 458 - 468, Update the docstring for the _compute_sdf_from_shape_impl function to include the SI unit for the shape_margin parameter (add "[m]" to "Margin offset to subtract from SDF values."). Locate the parameter description for shape_margin in the _compute_sdf_from_shape_impl docstring and append the unit annotation so it matches compute_sdf_from_shape (e.g., "shape_margin: float = 0.0 — Margin offset to subtract from SDF values [m]."). Ensure formatting and punctuation follow the existing docstring style.newton/_src/geometry/contact_reduction_hydroelastic.py (1)
740-757: Missing[m]unit inshape_gapdocstring.Line 753 documents
shape_gapas"Per-shape contact gap (detection threshold) (dtype: float)."— the[m]unit should be included per coding guidelines.The same applies to the
reduce_and_exportdocstring at line 802.Proposed fix
- shape_gap: Per-shape contact gap (detection threshold) (dtype: float). + shape_gap: Per-shape contact gap (detection threshold) [m] (dtype: float).Apply similarly at line 802.
As per coding guidelines: "State SI units for all physical quantities in docstrings using inline
[unit]notation."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/geometry/contact_reduction_hydroelastic.py` around lines 740 - 757, Update the docstrings for the physical parameter shape_gap in both the export method and the reduce_and_export method so they include the SI unit notation “[m]”; find the export(self, shape_gap: wp.array, ...) docstring (mentions “Per-shape contact gap (detection threshold) (dtype: float)”) and change it to include “[m]” after the parameter name or in the description, and do the same for the reduce_and_export docstring referenced at line ~802 so both declare the unit as “[m]” while keeping the dtype note.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@newton/_src/geometry/contact_reduction_global.py`:
- Around line 1017-1020: The contact margin calculation in
export_reduced_contacts_kernel and mesh_triangle_contacts_to_reducer_kernel is
using wp.max(margin_a, margin_b) but should use the sum (margin_a + margin_b) to
match the narrow-phase kernels; update the margin computation in both functions
to margin = margin_a + margin_b and adjust the stale comment ("matching other
kernels") to reflect that the threshold is the sum of per-shape margins so the
reduction path accepts the same contacts as the broad/narrow phases.
In `@newton/_src/sim/builder.py`:
- Around line 7986-8001: Update the _validate_shapes docstring to state SI units
for the physical quantities `margin` and `gap` using inline [m] notation;
mention that margin and gap are lengths in meters (e.g., "margin [m]" and "gap
[m]") and clarify that the effective expansion uses "margin + gap [m]"; keep
references to the collision flags `COLLIDE_SHAPES` and `COLLIDE_PARTICLES` and
preserve the warning text about negative gaps.
- Around line 179-185: Update the docstrings for the margin and gap fields to
state SI units (add “[m]”) and clarify that margin is a length-only collision
offset that affects collision detection and summed between shapes (margin_a +
margin_b); also note that for hollow or shell-like shapes this margin is used
for collision-only effective inertia/response rather than changing mass
distribution. For gap, state “[m]” and mention that when None it defaults to
builder.rigid_gap and that broad phase expands AABBs using (margin + gap) for
pair filtering. Use the existing symbols margin, gap, and builder.rigid_gap to
locate the documentation to modify.
- Around line 4721-4723: The tuple unpacking uses a single-letter name I which
triggers Ruff E741; update the unpacking from (m, c, I) to a clearer name like
(m, c, inertia) (or inertia_tensor) and rename any subsequent uses of I in this
scope to that new name; ensure references around compute_inertia_shape and any
later code that reads the inertia variable are updated accordingly.
In `@newton/tests/test_narrow_phase.py`:
- Around line 1833-1836: In the test functions where you conditionally check for
the warning string (e.g., the "if 'GJK candidate pair buffer overflowed' in
output:" block in test_narrow_phase and the similar block in
test_broad_phase_buffer_overflow), remove the tautological assert
(self.assertIn(...)) guarded by the same condition — either delete the entire
if/assert pair or keep only the non-flaky correctness assertion (the
overflow-counter assertion) and drop the optional warning assertion so the test
doesn't contain an assertion that can never fail.
- Line 192: TestNarrowPhase sets self.gap but never uses it; either remove the
unused assignment or make it the default cutoff used by geometry creation. If
you want a non-zero default, update _create_geometry_arrays to use
geom.get("cutoff", self.gap) (or otherwise accept a default_cutoff param) so
TestNarrowPhase::self.gap influences the created arrays; alternatively, delete
the self.gap assignment in TestNarrowPhase if the default should remain 0.0 and
ensure any tests that rely on a non-zero gap are updated to pass an explicit
cutoff.
---
Outside diff comments:
In `@newton/_src/geometry/sdf_contact.py`:
- Around line 686-874: The kernel docstring in mesh_sdf_collision_kernel is
outdated: replace references to geom_cutoff and thickness with the actual
parameter names (shape_gap and shape_data[...,3] for per-shape thickness) and
document their units as SI (e.g., shape_gap [m], triangle_mesh_thickness [m],
sdf_mesh_thickness [m], contact_threshold [m]); update the Args block to list
shape_data, shape_transform, shape_source, sdf_table, shape_sdf_index,
shape_gap, shape_pairs_mesh_mesh, shape_pairs_mesh_mesh_count, writer_data,
total_num_blocks and briefly describe each (include that shape_gap is the
per-shape margin in meters and that thickness values are read from
shape_data[...,3] in meters) while keeping Google-style formatting.
In `@newton/_src/sim/builder.py`:
- Around line 5374-5381: Remeshed/convex-decomposed shapes currently copy margin
but omit per-shape gap, losing custom broad-phase spacing; update the
ModelBuilder.ShapeConfig construction(s) (the block creating cfg =
ModelBuilder.ShapeConfig(...)) to include gap=self.shape_gap[shape] so new
shapes preserve per-shape gap, and apply the same change in the other analogous
block that builds ShapeConfig during remeshing/convex decomposition (the second
occurrence around the convex-decomposition remesh code).
In `@newton/_src/solvers/xpbd/solver_xpbd.py`:
- Around line 523-554: The kernels solve_body_contact_positions and
apply_rigid_restitution use parameter names contact_thickness0 and
contact_thickness1 but the call-site passes contacts.rigid_contact_margin0 and
contacts.rigid_contact_margin1; rename the kernel parameters to contact_margin0
and contact_margin1 (and update any internal references) so the parameter names
match the call-site terminology and improve clarity—update definitions in
newton/_src/solvers/xpbd/kernels.py for both solve_body_contact_positions and
apply_rigid_restitution accordingly.
In `@newton/examples/contacts/example_nut_bolt_hydro.py`:
- Line 106: The docstring for the parameter that documents shape_cfg still uses
the old term "contact margin" after the parameter rename; update the docstring
in example_nut_bolt_hydro.py so it refers to the renamed parameter "gap" (e.g.,
"Optional shape configuration used for gap [m]") and search for any other
occurrences of "contact margin" in that file or in comments near shape_cfg,
contact_margin, or gap to ensure all text reflects the new name.
In `@newton/tests/test_broad_phase.py`:
- Around line 2054-2149: The test and broad-phase kernel disagree on gap
combination semantics: the kernel uses additive combination (cutoff_combined =
box1_cutoff + box2_cutoff) while the test reference helpers
check_aabb_overlap_host and find_overlapping_pairs_np use max(cutoff[i],
cutoff[j]); update the reference helper logic to mirror the kernel by replacing
the max-based combination with additive combination (use cutoff_combined =
cutoff[i] + cutoff[j]) so the host-side overlap checks and pair-finding match
BroadPhaseAllPairs and BroadPhaseSAP behavior.
In `@newton/tests/test_import_mjcf.py`:
- Around line 2235-2266: Update the test_mjcf_geom_margin_parsing docstring to
use the term "margin" instead of "shape thickness" and state units for margin
and scale (meters) using inline [m] notation; e.g., describe that MJCF geom
margin values are parsed to shape_margin (in meters) and that the model scale
multiplies the margin value, referencing the test functions/variables builder,
builder_scaled, shape_margin, and scale so readers know which symbols the
docstring describes.
In `@newton/tests/test_import_usd.py`:
- Around line 4494-4539: Update the test_gap_parsing docstring and authored USD
attribute to use the renamed gap API: change any authored attribute
"newton:contactMargin" to "newton:gap" on collider1_prim and note the unit in
the test docstring as "[m]"; keep the rest of the logic that sets
builder.default_shape_cfg.gap and asserts model.shape_gap values, and optionally
add a small separate test verifying backward compatibility that still reads
"newton:contactMargin" if you want to ensure legacy support (reference symbols:
test_gap_parsing, newton.ModelBuilder, builder.default_shape_cfg.gap,
model.shape_gap, collider1_prim.GetAttribute).
- Around line 4494-4539: The regression shows the USD attribute
"newton:contactMargin" is being mapped to the wrong internal key; fix the
mapping in the USD schema so "newton:contactMargin" maps to the internal "gap"
key (not "contact_margin") so that ModelBuilder.add_usd and test_gap_parsing
(which asserts model.shape_gap values and uses builder.default_shape_cfg.gap)
work correctly; locate the mapping in the USD schema module (the dict/constant
that maps USD attribute names to internal schema keys in schemas.py) and change
the value for "newton:contactMargin" to "gap", then run the test to confirm it
passes.
---
Nitpick comments:
In `@newton/_src/geometry/contact_data.py`:
- Around line 34-47: The docstring entries for collision surface margins are
missing SI units; update the docstring in contact_data.py (the
ContactData/class-level docstring) to append "[m]" to margin_a, margin_b, and
margin so they read e.g. "margin_a: Collision surface margin offset for shape A
[m]" (and similarly for margin_b and margin) and also add "[m]" to the
standalone "margin" entry, preserving existing wording and punctuation.
In `@newton/_src/geometry/contact_reduction_hydroelastic.py`:
- Around line 740-757: Update the docstrings for the physical parameter
shape_gap in both the export method and the reduce_and_export method so they
include the SI unit notation “[m]”; find the export(self, shape_gap: wp.array,
...) docstring (mentions “Per-shape contact gap (detection threshold) (dtype:
float)”) and change it to include “[m]” after the parameter name or in the
description, and do the same for the reduce_and_export docstring referenced at
line ~802 so both declare the unit as “[m]” while keeping the dtype note.
In `@newton/_src/geometry/sdf_hydroelastic.py`:
- Around line 1289-1290: The two assignments contact_data.margin_a = 0.0 and
contact_data.margin_b = 0.0 are intentional: add a short inline comment on those
lines clarifying that contact_data.margin_a/b are the per-shape surface margin
(formerly called "thickness") and not the inter-shape gap (the locals
margin_a/margin_b), so setting them to 0.0 here is deliberate; alternatively,
rename the local variables margin_a/margin_b to gap_a/gap_b to avoid confusion
and update any nearby references accordingly.
- Around line 1073-1074: Rename the local variables that read from shape_gap in
the three kernels to reflect the new "gap" terminology: in
count_iso_voxels_block, decode_contacts_kernel, and generate_contacts_kernel
replace margin_a/margin_b with gap_a/gap_b (and update any downstream uses like
margin = margin_a + margin_b to gap = gap_a + gap_b or combined_gap as
appropriate), and update expressions such as va > r + margin_a to va > r + gap_a
so the local gap values are not confused with ContactData.margin_a (the surface
collision margin).
In `@newton/_src/geometry/sdf_utils.py`:
- Around line 306-329: Rename the kernel parameter `thickness` to `shape_margin`
(or `margin`) in the Warp kernels `sdf_from_mesh_kernel` and
`sdf_from_primitive_kernel` so the parameter name matches callers; update the
parameter list and all uses inside each function (e.g., where `thickness` is
subtracted from `signed_distance`) to the new name to keep terminology
consistent across call sites and kernels.
- Line 188: SDF.create_from_mesh and SDF.create_from_points expose a parameter
named thickness while helper functions use shape_margin, causing an inconsistent
public API; fix by renaming the public parameter thickness → shape_margin (or
margin) in the SDF.create_from_mesh and SDF.create_from_points signatures and
update all internal references and calls (including any callers that pass
thickness) to use shape_margin, or if you prefer a staged change add a clear
TODO on both SDF.create_from_* methods noting the planned rename and keep the
bridging assignments (e.g., shape_margin=thickness) until callers are migrated.
- Around line 458-468: Update the docstring for the _compute_sdf_from_shape_impl
function to include the SI unit for the shape_margin parameter (add "[m]" to
"Margin offset to subtract from SDF values."). Locate the parameter description
for shape_margin in the _compute_sdf_from_shape_impl docstring and append the
unit annotation so it matches compute_sdf_from_shape (e.g., "shape_margin: float
= 0.0 — Margin offset to subtract from SDF values [m]."). Ensure formatting and
punctuation follow the existing docstring style.
In `@newton/_src/solvers/semi_implicit/kernels_contact.py`:
- Around line 410-411: The kernel's formal parameter names contact_thickness0
and contact_thickness1 should be renamed to contact_margin0 and contact_margin1
to match the local variables margin_a/margin_b and the call-site arguments
rigid_contact_margin0/1; update any in-kernel references and the nearby comment
that still mentions "thickness" to use "margin" instead so naming is consistent
(look for the kernel signature parameters contact_thickness0/contact_thickness1,
the local variables margin_a/margin_b, and the call sites using
rigid_contact_margin0/1).
In `@newton/_src/solvers/vbd/rigid_vbd_kernels.py`:
- Around line 1522-1523: The code still uses a local accumulated variable named
thickness while the inputs are now rigid_contact_margin0 and
rigid_contact_margin1; rename that local variable to margin_total in the
affected kernels and any related helper code so the name matches the new inputs.
Locate the kernels that read rigid_contact_margin0/rigid_contact_margin1 and
replace the thickness variable (and its uses in computations, conditionals, and
comments) with margin_total, and update any nearby comments/docstrings to
reflect the new terminology; apply the same rename to the other occurrences
where thickness is computed/used alongside the rigid_contact_margin arrays.
In `@newton/_src/usd/schemas.py`:
- Around line 70-71: Add a deprecated alias mapping for contact_margin that
points to the same USD attribute as the "gap" entry to ease migration: keep the
existing "gap": SchemaAttribute("newton:contactMargin", float("-inf")) and add a
"contact_margin" key (or similar snake_case alias) that returns/forwards to
SchemaAttribute("newton:contactMargin", float("-inf")) with a clear deprecation
comment; apply the same pattern for the other occurrence around the second
instance noted (the entry at the other location referenced in the comment) so
both places expose the temporary alias during migration.
In `@newton/examples/contacts/example_nut_bolt_sdf.py`:
- Line 108: The code passes shape_cfg.gap into build_sdf(margin=...) which is
correct but confusing given ShapeConfig now has both margin (collision surface
margin) and gap (broad-phase detection offset); update the call site (around
build_sdf(...)) to add a short inline comment clarifying that we intentionally
use shape_cfg.gap for the SDF narrow band because the SDF margin must cover the
detection gap (not the collision surface margin), and mention the symbols
ShapeConfig, gap, margin, and build_sdf to make intent explicit for future
readers.
In `@newton/examples/robot/example_robot_panda_hydro.py`:
- Around line 135-136: Introduce a named constant (e.g., DEFAULT_SDF_MARGIN =
0.05) near the top of the module and replace all occurrences of the inline
fallback expression shape_cfg.gap if shape_cfg.gap is not None else 0.05 with a
single helper value (e.g., margin = shape_cfg.gap if shape_cfg.gap is not None
else DEFAULT_SDF_MARGIN) at each call site; update all four sites that currently
use that expression so the fallback is documented and controlled via the
constant (keep the variable name DEFAULT_SDF_MARGIN or similar to make intent
clear and colocate the constant with related comment about the broad-phase gap
relationship).
In `@newton/tests/test_mujoco_solver.py`:
- Around line 2635-2700: The test and its variables still use "thickness" naming
while the API uses "margin": rename the test function and variables/comments to
reflect margin. Specifically, rename test_geom_margin_from_thickness to
something like test_geom_margin_from_shape_margin (or
test_geom_margin_updates_from_shape_margin), update the docstring to mention
shape_margin/geom_margin instead of thickness, and rename the runtime update
array new_thickness to new_margin (and its uses: model.shape_margin.assign(...),
new_margin[shape_idx], etc.) so all identifiers (test name, docstring, and
variable new_thickness) consistently use "margin" while leaving references to
model.shape_margin, solver.mjw_model.geom_margin, and
SolverNotifyFlags.SHAPE_PROPERTIES unchanged.
In `@newton/tests/test_narrow_phase.py`:
- Around line 1317-1426: Add a discriminating sub-test to ensure narrow-phase
uses gap_a + gap_b (not max) by creating asymmetric per-shape gaps and placing
the sphere at a separation between max(gap_a,gap_b) and gap_a+gap_b;
specifically, set shape_gap so the two interacting shapes have gaps 0.04 and
0.04 (sum=0.08, max=0.04), set geom_transform for that pair so the sphere center
is at z=0.06 (separation=0.06), call self.narrow_phase.launch with the same
candidate pair/pair_count/pairs/pair setup used above, zero contact_count before
the call, wp.synchronize(), and assertGreater(contact_count.numpy()[0], 0) to
fail if narrow-phase wrongly used max(gap_a,gap_b) instead of the sum; reference
shape_gap, geom_transform, pairs, contact_count, and self.narrow_phase.launch
when adding this block.
ℹ️ Review info
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (51)
docs/concepts/collisions.rstdocs/concepts/usd_parsing.rstnewton/_src/geometry/broad_phase_common.pynewton/_src/geometry/broad_phase_nxn.pynewton/_src/geometry/broad_phase_sap.pynewton/_src/geometry/collision_core.pynewton/_src/geometry/contact_data.pynewton/_src/geometry/contact_reduction_global.pynewton/_src/geometry/contact_reduction_hydroelastic.pynewton/_src/geometry/narrow_phase.pynewton/_src/geometry/sdf_contact.pynewton/_src/geometry/sdf_hydroelastic.pynewton/_src/geometry/sdf_utils.pynewton/_src/sim/builder.pynewton/_src/sim/collide.pynewton/_src/sim/contacts.pynewton/_src/sim/model.pynewton/_src/solvers/featherstone/solver_featherstone.pynewton/_src/solvers/implicit_mpm/implicit_mpm_model.pynewton/_src/solvers/mujoco/kernels.pynewton/_src/solvers/mujoco/solver_mujoco.pynewton/_src/solvers/semi_implicit/kernels_contact.pynewton/_src/solvers/vbd/rigid_vbd_kernels.pynewton/_src/solvers/vbd/solver_vbd.pynewton/_src/solvers/xpbd/solver_xpbd.pynewton/_src/usd/schemas.pynewton/_src/utils/import_mjcf.pynewton/_src/utils/import_usd.pynewton/_src/viewer/viewer.pynewton/examples/cable/example_cable_bundle_hysteresis.pynewton/examples/cable/example_cable_pile.pynewton/examples/contacts/example_nut_bolt_hydro.pynewton/examples/contacts/example_nut_bolt_sdf.pynewton/examples/diffsim/example_diffsim_drone.pynewton/examples/ik/example_ik_cube_stacking.pynewton/examples/robot/example_robot_panda_hydro.pynewton/tests/test_broad_phase.pynewton/tests/test_collision_cloth.pynewton/tests/test_collision_pipeline.pynewton/tests/test_contact_reduction_global.pynewton/tests/test_hydroelastic.pynewton/tests/test_implicit_mpm.pynewton/tests/test_import_mjcf.pynewton/tests/test_import_usd.pynewton/tests/test_model.pynewton/tests/test_mujoco_solver.pynewton/tests/test_narrow_phase.pynewton/tests/test_rigid_contact.pynewton/tests/test_schema_resolver.pynewton/tests/test_sdf_compute.pynewton/tests/test_selection.py
adenzler-nvidia
left a comment
Member
There was a problem hiding this comment.
Review
Rename is thorough — verified no missed old names on the PR branch via git grep. USD wire attribute backward compat is well handled.
A few cleanup items:
Stale comments in narrow_phase.py
shape_gapparameter comment still says "Per-shape contact margins" (~line 969, 989)shape_datacomment still says "thickness w" — should be "margin w" (~line 970)- Comment "Sum margins for consistency with thickness summing" still references "thickness" (~line 976, 1012)
Kernel parameter names in semi_implicit/kernels_contact.py
eval_body_contact kernel signature still uses contact_thickness0/contact_thickness1 as parameter names. Local variables were renamed to margin_a/margin_b but the parameters weren't. Functionally correct (positional args) but inconsistent.
Stale variable names in tests
test_selection.py:thicknessesvariable used for margin valuestest_mujoco_solver.py:new_thicknessvariable for margin valuestest_broad_phase.py: docstring still says "per-shape contact margins"
geom_gap custom attribute in solver_mujoco.py
The geom_gap custom attribute and its plumbing in solver_mujoco.py / the MJCF importer should be removed. MuJoCo's geom_margin maps to Newton's shape_margin (the surface offset), not to gap (the AABB expansion). The current naming after this rename is misleading. This can be a follow-up — needs a ticket.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
> [!CAUTION]
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)newton/_src/sim/model.py (1)
293-300:⚠️ Potential issue | 🟡 MinorStale "thickness" references in
shape_collision_aabbdocstrings.Both
shape_collision_aabb_lowerandshape_collision_aabb_upperdocstrings still reference the oldthicknessterm, which this PR renames tomargin.📝 Proposed fix
self.shape_collision_aabb_lower = None """Local-space AABB lower bound [m] for each shape, shape [shape_count, 3], float. - Computed from base geometry only (excludes thickness - thickness is added during contact - margin calculations). Used for voxel-based contact reduction.""" + Computed from base geometry only (excludes margin - margin is added during contact + calculations). Used for voxel-based contact reduction.""" self.shape_collision_aabb_upper = None """Local-space AABB upper bound [m] for each shape, shape [shape_count, 3], float. - Computed from base geometry only (excludes thickness - thickness is added during contact - margin calculations). Used for voxel-based contact reduction.""" + Computed from base geometry only (excludes margin - margin is added during contact + calculations). Used for voxel-based contact reduction."""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/sim/model.py` around lines 293 - 300, The docstrings for shape_collision_aabb_lower and shape_collision_aabb_upper still mention the old term "thickness"; update both docstrings to replace "thickness" with the new term "margin" and ensure wording still matches the intent (e.g., "Computed from base geometry only (excludes margin - margin is added during contact margin calculations)"). Edit the docstrings attached to the attributes shape_collision_aabb_lower and shape_collision_aabb_upper in class Model to reflect this rename.
♻️ Duplicate comments (3)
newton/_src/geometry/contact_reduction_global.py (2)
1104-1107: Consistent additive gap computation — LGTM.Same
margin_a + margin_bpattern as inexport_reduced_contacts_kernel, keeping the mesh-triangle path aligned with the export path and all other narrow-phase kernels.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/geometry/contact_reduction_global.py` around lines 1104 - 1107, The code should compute the detection threshold as the sum of per-shape gaps (margin = margin_a + margin_b) to match the export_reduced_contacts_kernel and other narrow-phase kernels; ensure the variables margin_a, margin_b, and margin in contact_reduction_global.py use this additive pattern and replace any alternative gap computations in this file so the mesh-triangle path stays aligned with export_reduced_contacts_kernel.
1017-1020: Margin now uses additive combination — consistent with narrow-phase.The previous
wp.max(margin_a, margin_b)has been corrected tomargin_a + margin_b, matching the convention innarrow_phase.py,sdf_contact.py, and other kernels.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/geometry/contact_reduction_global.py` around lines 1017 - 1020, Replace the old max-based margin combination with an additive combination in contact_reduction_global.py: instead of using wp.max(margin_a, margin_b) compute margin_a = shape_gap[shape_a], margin_b = shape_gap[shape_b] and set margin = margin_a + margin_b (consistent with narrow_phase.py and sdf_contact.py); update any references to the previous variable usage in the contact reduction code paths that use margin to ensure they use the summed value.newton/_src/sim/builder.py (1)
4722-4725:⚠️ Potential issue | 🟡 MinorRename
Ito avoid Ruff E741.
Iis flagged as ambiguous; rename toinertia(or similar) to clear linting.🔧 Suggested rename
- (m, c, I) = compute_inertia_shape(type, scale, src, cfg.density, cfg.is_solid, cfg.margin) + (m, c, inertia) = compute_inertia_shape(type, scale, src, cfg.density, cfg.is_solid, cfg.margin) com_body = wp.transform_point(xform, c) - self._update_body_mass(body, m, I, com_body, xform.q) + self._update_body_mass(body, m, inertia, com_body, xform.q)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/sim/builder.py` around lines 4722 - 4725, The variable name I in the call to compute_inertia_shape is flagged by Ruff E741; change the tuple unpacking to use a non-ambiguous name such as inertia (e.g., (m, c, inertia) = compute_inertia_shape(...)) and pass that renamed variable into self._update_body_mass instead of I; update the surrounding lines in the same block where compute_inertia_shape is called and where self._update_body_mass(body, m, I, com_body, xform.q) is invoked so all references use the new name (refer to compute_inertia_shape, self._update_body_mass, and body_lock_inertia).
🧹 Nitpick comments (1)
newton/_src/solvers/mujoco/kernels.py (1)
141-141: Stale comment still references oldthicknessterminology.Line 141 was not updated as part of this rename PR. The justification "for consistency with thickness summing" is now circular since
thicknessno longer exists.✏️ Suggested comment update
- # Sum margins for consistency with thickness summing + # Sum per-shape margins (matches how radius_eff sums are stripped in contact conversion)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/solvers/mujoco/kernels.py` at line 141, Update the stale inline comment "Sum margins for consistency with thickness summing" to remove the old `thickness` term and explain the current rationale: locate the comment containing that exact text (in kernels.py) and replace it with a concise phrase such as "Sum margins for consistency with per-element margin accumulation" or "Sum margins to match how other per-element margins are accumulated", so the comment reflects the renamed concept and current implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@newton/_src/geometry/contact_reduction_global.py`:
- Around line 1009-1011: The inline comment above where margin_offset_a and
margin_offset_b are extracted should be updated to reflect the renamed
variables: replace the word "thickness" with "margin" (or "margin offset") so it
reads something like "Extract margin from shape_data (stored in w component)" to
match the variables margin_offset_a/margin_offset_b and the shape_data usage.
In `@newton/_src/solvers/mujoco/solver_mujoco.py`:
- Around line 2935-2941: The kernel call to
convert_newton_contacts_to_mjwarp_kernel is passing
contacts.rigid_contact_friction (which doesn't match the kernel's expected
rigid_contact_friction_scale parameter), causing a silent argument misalignment;
update the call site in solver_mujoco.py to pass
contacts.rigid_contact_friction_scale (or rename the kernel parameter to
rigid_contact_friction if you intend to use that field) so the 21st argument
matches the kernel signature—locate the call referencing
convert_newton_contacts_to_mjwarp_kernel and replace the passed symbol
contacts.rigid_contact_friction with contacts.rigid_contact_friction_scale (or
change the kernel parameter name in kernels.py to rigid_contact_friction) to
restore correct parameter alignment.
---
Outside diff comments:
In `@newton/_src/sim/model.py`:
- Around line 293-300: The docstrings for shape_collision_aabb_lower and
shape_collision_aabb_upper still mention the old term "thickness"; update both
docstrings to replace "thickness" with the new term "margin" and ensure wording
still matches the intent (e.g., "Computed from base geometry only (excludes
margin - margin is added during contact margin calculations)"). Edit the
docstrings attached to the attributes shape_collision_aabb_lower and
shape_collision_aabb_upper in class Model to reflect this rename.
---
Duplicate comments:
In `@newton/_src/geometry/contact_reduction_global.py`:
- Around line 1104-1107: The code should compute the detection threshold as the
sum of per-shape gaps (margin = margin_a + margin_b) to match the
export_reduced_contacts_kernel and other narrow-phase kernels; ensure the
variables margin_a, margin_b, and margin in contact_reduction_global.py use this
additive pattern and replace any alternative gap computations in this file so
the mesh-triangle path stays aligned with export_reduced_contacts_kernel.
- Around line 1017-1020: Replace the old max-based margin combination with an
additive combination in contact_reduction_global.py: instead of using
wp.max(margin_a, margin_b) compute margin_a = shape_gap[shape_a], margin_b =
shape_gap[shape_b] and set margin = margin_a + margin_b (consistent with
narrow_phase.py and sdf_contact.py); update any references to the previous
variable usage in the contact reduction code paths that use margin to ensure
they use the summed value.
In `@newton/_src/sim/builder.py`:
- Around line 4722-4725: The variable name I in the call to
compute_inertia_shape is flagged by Ruff E741; change the tuple unpacking to use
a non-ambiguous name such as inertia (e.g., (m, c, inertia) =
compute_inertia_shape(...)) and pass that renamed variable into
self._update_body_mass instead of I; update the surrounding lines in the same
block where compute_inertia_shape is called and where
self._update_body_mass(body, m, I, com_body, xform.q) is invoked so all
references use the new name (refer to compute_inertia_shape,
self._update_body_mass, and body_lock_inertia).
---
Nitpick comments:
In `@newton/_src/solvers/mujoco/kernels.py`:
- Line 141: Update the stale inline comment "Sum margins for consistency with
thickness summing" to remove the old `thickness` term and explain the current
rationale: locate the comment containing that exact text (in kernels.py) and
replace it with a concise phrase such as "Sum margins for consistency with
per-element margin accumulation" or "Sum margins to match how other per-element
margins are accumulated", so the comment reflects the renamed concept and
current implementation.
ℹ️ Review info
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
newton/_src/geometry/contact_reduction_global.pynewton/_src/sim/builder.pynewton/_src/sim/model.pynewton/_src/solvers/mujoco/kernels.pynewton/_src/solvers/mujoco/solver_mujoco.pynewton/_src/utils/import_mjcf.pynewton/tests/test_narrow_phase.py
🚧 Files skipped from review as they are similar to previous changes (2)
- newton/_src/utils/import_mjcf.py
- newton/tests/test_narrow_phase.py
andrewkaufman
added a commit
to andrewkaufman/newton-usd-schemas
that referenced
this pull request
Feb 23, 2026
1 task
Member
Author
Done |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
newton/_src/geometry/contact_reduction_global.py (1)
947-951:⚠️ Potential issue | 🟡 MinorStale comment:
"Per-shape contact margins"should read"Per-shape contact gaps".The adjacent parameter was renamed to
shape_gap(AABB expansion) on line 951, but the descriptive comment on line 947 was not updated. This is the same pattern flagged innarrow_phase.pyper the PR objectives.📝 Proposed fix
- # Per-shape contact margins + # Per-shape contact gaps shape_gap: wp.array(dtype=float),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/geometry/contact_reduction_global.py` around lines 947 - 951, Update the stale comment describing the shape_gap parameter: change the comment text "Per-shape contact margins" to "Per-shape contact gaps" adjacent to the shape_gap parameter (near shape_types and shape_data) so the comment matches the renamed parameter shape_gap (AABB expansion).newton/_src/solvers/mujoco/kernels.py (1)
141-143:⚠️ Potential issue | 🟡 MinorStale comment:
"thickness summing"should reference"margin summing"after the rename.The comment predates this PR's rename but is now inconsistent with the new terminology. The PR Objectives explicitly called out the same wording in
narrow_phase.py; the same fix applies here.📝 Proposed fix
- # Sum margins for consistency with thickness summing + # Sum margins for consistency with margin summing in the narrow-phase kernels margin = geom_margin[worldid, g1] + geom_margin[worldid, g2]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/solvers/mujoco/kernels.py` around lines 141 - 143, The inline comment above the margin calculation is stale: update the comment referencing "thickness summing" to the new terminology "margin summing" (the code computing margin = geom_margin[worldid, g1] + geom_margin[worldid, g2] and gap = geom_gap[worldid, g1] + geom_gap[worldid, g2] should keep same logic but the comment should read something like "Sum margins for consistency with margin summing" or equivalent to match the rename; locate the comment near variables margin, geom_margin, geom_gap, worldid, g1, g2 and replace the old wording only).newton/tests/test_mujoco_solver.py (1)
2635-2701:⚠️ Potential issue | 🟡 MinorRename the test to drop “thickness” and convert the docstring to Google style.
Line 2635 still references “thickness” even though the test now validates margin. Also, the updated docstring is not Google-style (missing Args/Returns). Consider renaming the test and formatting the docstring while preserving the [m] units.
🛠️ Proposed update
- def test_geom_margin_from_thickness(self): - """Test shape_margin to geom_margin conversion and runtime updates. - - Verifies that shape_margin [m] values are correctly propagated to - geom_margin [m] during solver initialization and after runtime updates - via notify_model_changed across multiple worlds. - """ + def test_geom_margin_conversion(self): + """Test shape_margin to geom_margin conversion and runtime updates. + + Args: + self: Test case instance. + + Returns: + None. + + Notes: + shape_margin [m] values should propagate to geom_margin [m] during + solver initialization and after runtime updates via + notify_model_changed across multiple worlds. + """As per coding guidelines: “Use Google-style docstrings with clear, concise explanations of what the function does, its parameters, and return values” and “newton/**/*.py: State SI units for all physical quantities in docstrings using inline [unit] notation (e.g., [m], [N·m]).”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/tests/test_mujoco_solver.py` around lines 2635 - 2701, The test function test_geom_margin_from_thickness should be renamed to reflect it validates margins (e.g., test_geom_margin) and its docstring converted to Google style while preserving SI unit annotations like [m]; update the function name reference and any calls to SolverMuJoCo.notify_model_changed or assertions if referenced by name, change the top docstring to a Google-style block with a one-line summary, an optional Args: (none) if there are no parameters, and Returns: (none) plus the note that shape_margin and geom_margin are in meters [m], and ensure the new name is used consistently in the file and any test discovery.newton/_src/geometry/narrow_phase.py (2)
1695-1695:⚠️ Potential issue | 🟡 MinorStale
thicknessreference inlaunch_custom_writesignature and docstring.The inline comment on
shape_data(line 1695) and its corresponding docstring description both still saythickness (w)— should bemargin (w)to match the rename.✏️ Proposed fix
- shape_data: wp.array(dtype=wp.vec4, ndim=1), # Shape data (scale xyz, thickness w) + shape_data: wp.array(dtype=wp.vec4, ndim=1), # Shape data (scale xyz, margin w)In the docstring:
- shape_data: Array of vec4 containing scale (xyz) and thickness (w) for each shape + shape_data: Array of vec4 containing scale (xyz) and margin (w) for each shape🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/geometry/narrow_phase.py` at line 1695, The docstring and inline comment for the parameter shape_data in launch_custom_write still refer to the fourth component as "thickness (w)"; update both the signature comment and the docstring description to say "margin (w)" to match the rename. Locate launch_custom_write and replace occurrences of "thickness (w)" with "margin (w)" in the parameter comment for shape_data (wp.array dtype=wp.vec4, ndim=1) and in any docstring parameter descriptions so they consistently reference margin (w).
2062-2062:⚠️ Potential issue | 🟡 MinorSame stale
thicknessreference inlaunchsignature and docstring.Line 2062 inline comment and the corresponding docstring entry (~line 2092) both still use
thickness (w)— should bemargin (w).✏️ Proposed fix
- shape_data: wp.array(dtype=wp.vec4, ndim=1), # Shape data (scale xyz, thickness w) + shape_data: wp.array(dtype=wp.vec4, ndim=1), # Shape data (scale xyz, margin w)In the docstring:
- shape_data: Array of vec4 containing scale (xyz) and thickness (w) for each shape + shape_data: Array of vec4 containing scale (xyz) and margin (w) for each shape🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/geometry/narrow_phase.py` at line 2062, The launch function's shape_data parameter comment and its docstring still refer to "thickness (w)" but the field is actually "margin (w)"; update the inline comment on the shape_data parameter (wp.array(dtype=wp.vec4, ndim=1)) and the launch() docstring entry to say "margin (w)" instead of "thickness (w)" so the signature, comment, and documentation are consistent (refer to the launch function and the shape_data parameter in narrow_phase.py).
♻️ Duplicate comments (1)
newton/_src/sim/builder.py (1)
4879-4881:⚠️ Potential issue | 🟡 MinorRename ambiguous
Ito avoid Ruff E741.
Ruff flagsIas an ambiguous name; renaming prevents lint errors.🔧 Proposed rename
- (m, c, I) = compute_inertia_shape(type, scale, src, cfg.density, cfg.is_solid, cfg.margin) + (m, c, inertia) = compute_inertia_shape(type, scale, src, cfg.density, cfg.is_solid, cfg.margin) com_body = wp.transform_point(xform, c) - self._update_body_mass(body, m, I, com_body, xform.q) + self._update_body_mass(body, m, inertia, com_body, xform.q)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/sim/builder.py` around lines 4879 - 4881, The variable name I from the call to compute_inertia_shape is flagged as ambiguous by Ruff E741; rename it to a clearer identifier (e.g., inertia_tensor or inertia_matrix) wherever it is returned and used: change the tuple unpacking in compute_inertia_shape(...) to (m, c, inertia_tensor) and update the subsequent call to self._update_body_mass(body, m, inertia_tensor, com_body, xform.q) plus any corresponding parameter/name inside compute_inertia_shape and _update_body_mass to match the new name so references remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@newton/_src/geometry/narrow_phase.py`:
- Around line 651-656: The extract_shape_data function in support_function.py
still uses the name "thickness" for the fifth returned value; update the
function signature, parameter/docstring, return type annotation, internal
variable assignment, and the return tuple to use "margin_offset" instead of
"thickness" so it matches callers like narrow_phase.py which expect
margin_offset_a and margin_offset_b; ensure the docstring and type hints
reference margin_offset and rename any local variable thickness -> margin_offset
within extract_shape_data.
In `@newton/_src/sim/model.py`:
- Around line 904-907: Update the docstring paragraph about rigid contact gaps
to state SI units: annotate the gap quantities as meters by adding "[m]" after
each mention of gap (e.g., Model.shape_gap, ShapeConfig.gap, builder.rigid_gap)
so the note reads that gaps are in meters; keep the rest of the text and
references to ModelBuilder.finalize unchanged.
In `@newton/tests/test_import_usd.py`:
- Around line 4544-4545: The docstring for test_gap_parsing is not Google-style
and omits the unit for the physical quantity `gap`; update the triple-quoted
docstring on the test_gap_parsing function to follow Google-style (short summary
line, blank line, and sections such as Args/Returns or Raises as appropriate)
and explicitly state the unit for gap as [m] (e.g., "gap: gap value parsed from
USD [m]"). Keep the description concise and, since this is a test, mention what
behavior is being verified (what is parsed and expected) in the Returns or
Summary section.
---
Outside diff comments:
In `@newton/_src/geometry/contact_reduction_global.py`:
- Around line 947-951: Update the stale comment describing the shape_gap
parameter: change the comment text "Per-shape contact margins" to "Per-shape
contact gaps" adjacent to the shape_gap parameter (near shape_types and
shape_data) so the comment matches the renamed parameter shape_gap (AABB
expansion).
In `@newton/_src/geometry/narrow_phase.py`:
- Line 1695: The docstring and inline comment for the parameter shape_data in
launch_custom_write still refer to the fourth component as "thickness (w)";
update both the signature comment and the docstring description to say "margin
(w)" to match the rename. Locate launch_custom_write and replace occurrences of
"thickness (w)" with "margin (w)" in the parameter comment for shape_data
(wp.array dtype=wp.vec4, ndim=1) and in any docstring parameter descriptions so
they consistently reference margin (w).
- Line 2062: The launch function's shape_data parameter comment and its
docstring still refer to "thickness (w)" but the field is actually "margin (w)";
update the inline comment on the shape_data parameter (wp.array(dtype=wp.vec4,
ndim=1)) and the launch() docstring entry to say "margin (w)" instead of
"thickness (w)" so the signature, comment, and documentation are consistent
(refer to the launch function and the shape_data parameter in narrow_phase.py).
In `@newton/_src/solvers/mujoco/kernels.py`:
- Around line 141-143: The inline comment above the margin calculation is stale:
update the comment referencing "thickness summing" to the new terminology
"margin summing" (the code computing margin = geom_margin[worldid, g1] +
geom_margin[worldid, g2] and gap = geom_gap[worldid, g1] + geom_gap[worldid, g2]
should keep same logic but the comment should read something like "Sum margins
for consistency with margin summing" or equivalent to match the rename; locate
the comment near variables margin, geom_margin, geom_gap, worldid, g1, g2 and
replace the old wording only).
In `@newton/tests/test_mujoco_solver.py`:
- Around line 2635-2701: The test function test_geom_margin_from_thickness
should be renamed to reflect it validates margins (e.g., test_geom_margin) and
its docstring converted to Google style while preserving SI unit annotations
like [m]; update the function name reference and any calls to
SolverMuJoCo.notify_model_changed or assertions if referenced by name, change
the top docstring to a Google-style block with a one-line summary, an optional
Args: (none) if there are no parameters, and Returns: (none) plus the note that
shape_margin and geom_margin are in meters [m], and ensure the new name is used
consistently in the file and any test discovery.
---
Duplicate comments:
In `@newton/_src/sim/builder.py`:
- Around line 4879-4881: The variable name I from the call to
compute_inertia_shape is flagged as ambiguous by Ruff E741; rename it to a
clearer identifier (e.g., inertia_tensor or inertia_matrix) wherever it is
returned and used: change the tuple unpacking in compute_inertia_shape(...) to
(m, c, inertia_tensor) and update the subsequent call to
self._update_body_mass(body, m, inertia_tensor, com_body, xform.q) plus any
corresponding parameter/name inside compute_inertia_shape and _update_body_mass
to match the new name so references remain consistent.
ℹ️ Review info
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
docs/concepts/usd_parsing.rstnewton/_src/geometry/contact_reduction_global.pynewton/_src/geometry/narrow_phase.pynewton/_src/sim/builder.pynewton/_src/sim/model.pynewton/_src/solvers/mujoco/kernels.pynewton/_src/solvers/mujoco/solver_mujoco.pynewton/_src/solvers/semi_implicit/kernels_contact.pynewton/_src/utils/import_mjcf.pynewton/_src/utils/import_usd.pynewton/tests/test_broad_phase.pynewton/tests/test_import_usd.pynewton/tests/test_mujoco_solver.pynewton/tests/test_selection.py
🚧 Files skipped from review as they are similar to previous changes (6)
- docs/concepts/usd_parsing.rst
- newton/tests/test_selection.py
- newton/_src/utils/import_mjcf.py
- newton/_src/utils/import_usd.py
- newton/_src/solvers/mujoco/solver_mujoco.py
- newton/tests/test_broad_phase.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
docs/concepts/usd_parsing.rst (1)
245-251:⚠️ Potential issue | 🟠 MajorCorrect the attribute name in the Newton example: use
newton:contactMargininstead ofnewton:contactGap.The documentation shows
newton:contactGap, but the schema (newton/_src/usd/schemas.py:71) defines the actual USD attribute asnewton:contactMargin. All tests and code usenewton:contactMargin. The attributecontactGapdoes not exist in the codebase.Suggested fix
- - ``newton:hullVertexLimit``, ``newton:contactGap`` + - ``newton:hullVertexLimit``, ``newton:contactMargin``🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/concepts/usd_parsing.rst` around lines 245 - 251, Replace the incorrect attribute name `newton:contactGap` in the Newton example with the actual attribute `newton:contactMargin` so the docs match the schema and tests; update the line containing `newton:contactGap` in the USD parsing examples to `newton:contactMargin`, ensuring the example now references the same attribute defined in the schema (`contactMargin`) used throughout the codebase.newton/_src/solvers/mujoco/kernels.py (1)
141-141:⚠️ Potential issue | 🟡 MinorStale "thickness" reference — should be updated as part of this rename.
The comment still says
"consistency with thickness summing". Since this PR explicitly replacesthickness→marginterminology, this line was missed.✏️ Suggested fix
- # Sum margins for consistency with thickness summing + # Sum margins (one per geom) for consistent surface-to-surface distance calculation margin = geom_margin[worldid, g1] + geom_margin[worldid, g2]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/solvers/mujoco/kernels.py` at line 141, The inline comment "# Sum margins for consistency with thickness summing" is stale after renaming thickness→margin; update it to something like "# Sum margins for consistency with margin summing" (or "# Sum margins for consistency with margin accumulation") and search within kernels.py for any remaining 'thickness' identifiers or comments and replace them with 'margin' to keep terminology consistent (check uses of the variable name margin and any related functions in this module).newton/_src/geometry/contact_reduction_global.py (1)
947-950:⚠️ Potential issue | 🟡 MinorStale comment: "thickness" should be "margin offset" to match the rename.
Line 947's comment
# Shape data for extracting thickness and effective radiusstill uses the old "thickness" terminology while all surrounding variable names have been updated tomargin_offset_a/bin this PR.📝 Proposed fix
- # Shape data for extracting thickness and effective radius + # Shape data for extracting margin offset and effective radius shape_types: wp.array(dtype=int), shape_data: wp.array(dtype=wp.vec4),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/geometry/contact_reduction_global.py` around lines 947 - 950, Update the stale comment that still says "thickness" to reflect the renamed terminology "margin offset": change the comment near the shape metadata declaration (around the shape_types and shape_data arrays) to mention "margin offset and effective radius" and/or reference the per-shape margin_offset_a / margin_offset_b fields so it matches the new variable names used elsewhere in contact_reduction_global.py.newton/_src/sim/builder.py (1)
5531-5546:⚠️ Potential issue | 🟠 MajorPropagate per‑shape gap when cloning/remeshing shapes.
Derived shapes created here don’t inherit the original shape’s gap, so decomposed meshes can silently fall back to
builder.rigid_gapand change collision distances. Please carry over the per‑shape gap.✅ Suggested fix
cfg = ModelBuilder.ShapeConfig( density=0.0, # do not add extra mass / inertia margin=self.shape_margin[shape], + gap=self.shape_gap[shape], is_solid=self.shape_is_solid[shape], has_shape_collision=False, has_particle_collision=False, is_visible=True, ) @@ cfg = ModelBuilder.ShapeConfig( density=0.0, # do not add extra mass / inertia ke=self.shape_material_ke[shape], kd=self.shape_material_kd[shape], kf=self.shape_material_kf[shape], ka=self.shape_material_ka[shape], mu=self.shape_material_mu[shape], restitution=self.shape_material_restitution[shape], mu_torsional=self.shape_material_mu_torsional[shape], mu_rolling=self.shape_material_mu_rolling[shape], kh=self.shape_material_kh[shape], margin=self.shape_margin[shape], + gap=self.shape_gap[shape], is_solid=self.shape_is_solid[shape], collision_group=self.shape_collision_group[shape], collision_filter_parent=self.default_shape_cfg.collision_filter_parent, )Also applies to: 5604-5619
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/sim/builder.py` around lines 5531 - 5546, The cloned/remeshed shapes are not preserving the per-shape collision gap; update the shape creation to propagate the per-shape gap by setting gap=self.shape_gap[shape] on the ShapeConfig (or passing the per-shape gap through add_shape_mesh if that function accepts a gap param) instead of letting the code fall back to builder.rigid_gap; apply the same change in the sibling block around the 5604–5619 region so both remeshing/cloning code paths carry self.shape_gap[shape].newton/_src/geometry/narrow_phase.py (2)
1692-1731:⚠️ Potential issue | 🟡 MinorUpdate stale “thickness” docs and add units for
shape_gap.
The signature comment and docstring still reference thickness; they should reflect margin offsets and include SI units for the gap.Proposed update
- shape_data: wp.array(dtype=wp.vec4, ndim=1), # Shape data (scale xyz, thickness w) + shape_data: wp.array(dtype=wp.vec4, ndim=1), # Shape data (scale xyz, margin offset w) ... - shape_data: Array of vec4 containing scale (xyz) and thickness (w) for each shape + shape_data: Array of vec4 containing scale (xyz) and margin offset [m] (w) for each shape ... - shape_gap: Array of per-shape contact gaps (detection threshold) for each shape + shape_gap: Array of per-shape contact gaps [m] (detection threshold) for each shapeAs per coding guidelines: “Use Google-style docstrings with clear, concise explanations of what the function does, its parameters, and return values” and “newton/**/*.py: State SI units for all physical quantities in docstrings using inline [unit] notation (e.g., [m], [N·m])”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/geometry/narrow_phase.py` around lines 1692 - 1731, Update the stale "thickness" wording and add SI units in the narrow phase function docs and signature comments: change references to the fourth component of shape_data (vec4) from "thickness" to "margin" or "margin offset" and annotate it with units [m]; add units [m] to shape_gap and shape_collision_radius descriptions in both the inline signature comments and the Google-style docstring; ensure other related phrases (e.g., "per-shape contact gap (detection threshold)") use the new terminology and include units. Refer to the symbols shape_data, shape_gap, shape_collision_radius and the function docstring/header to locate and update all occurrences.
2060-2110:⚠️ Potential issue | 🟡 MinorUpdate stale “thickness” docs and add units for
shape_gap.
Same documentation issue aslaunch_custom_write.Proposed update
- shape_data: wp.array(dtype=wp.vec4, ndim=1), # Shape data (scale xyz, thickness w) + shape_data: wp.array(dtype=wp.vec4, ndim=1), # Shape data (scale xyz, margin offset w) ... - shape_data: Array of vec4 containing scale (xyz) and thickness (w) for each shape + shape_data: Array of vec4 containing scale (xyz) and margin offset [m] (w) for each shape ... - shape_gap: Array of per-shape contact gaps (detection threshold) for each shape + shape_gap: Array of per-shape contact gaps [m] (detection threshold) for each shapeAs per coding guidelines: “Use Google-style docstrings with clear, concise explanations of what the function does, its parameters, and return values” and “newton/**/*.py: State SI units for all physical quantities in docstrings using inline [unit] notation (e.g., [m], [N·m])”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/geometry/narrow_phase.py` around lines 2060 - 2110, The docstring for the narrow-phase launcher has stale wording about "thickness" and lacks SI units for physical quantities; update the parameter descriptions for shape_data and shape_gap (and any other physical parameters in this docstring) to use Google-style phrasing and include SI units: explicitly state shape_data is a vec4 containing scale (xyz) in meters [m] and thickness (w) in meters [m] (or clarify if w is half-thickness), and mark shape_gap as a contact detection threshold in meters [m]; mirror the same unit/style changes you applied in launch_custom_write and ensure references use the exact parameter names shape_data, shape_gap, shape_collision_radius, and shape_transform so reviewers can find the edits easily.
♻️ Duplicate comments (1)
newton/_src/sim/builder.py (1)
4879-4881: Duplicate: Ruff E741 still triggered byIin tuple unpacking.This was already reported in prior review comments; still applies here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/sim/builder.py` around lines 4879 - 4881, The tuple unpack uses a single-letter variable I which triggers Ruff E741; rename the inertia variable returned from compute_inertia_shape (currently unpacked as I) to a clearer identifier (e.g., inertia, inertia_tensor, or I_body) and update its usage in the call to self._update_body_mass so the names match (locate compute_inertia_shape tuple unpack and the subsequent call to self._update_body_mass to change both the unpacked name and the argument passed, e.g., replace I with inertia_tensor).
🧹 Nitpick comments (2)
newton/_src/solvers/mujoco/kernels.py (1)
229-229: Optional:rigid_contact_frictionparameter name loses "scale" semantics.The old name
rigid_contact_friction_scaleclearly indicated a multiplier. The renamed parameterrigid_contact_friction(line 229) is consumed on line 360 asfriction_scaleand applied multiplicatively (friction[0] * friction_scale). The local alias restores clarity, but a reader inspecting only the kernel signature may confuse this with an absolute friction coefficient. Considerrigid_contact_friction_scaleor at minimum a brief inline comment on the parameter declaration.Also applies to: 360-360
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/solvers/mujoco/kernels.py` at line 229, The kernel parameter rigid_contact_friction in kernels.py loses the "scale" meaning and is used as friction_scale inside the kernel (see local alias/friction[0] * friction_scale around the friction computation), so either rename the parameter back to rigid_contact_friction_scale or add a short inline comment on the parameter declaration clarifying it is a multiplier (e.g., "scale/multiplier applied to per-contact friction"). Update the kernel signature (rigid_contact_friction → rigid_contact_friction_scale) and any call sites or keep the name but add the comment so readers immediately understand it is a multiplicative scale used as friction_scale in the kernel.newton/_src/geometry/contact_reduction_global.py (1)
1079-1102: Redundantshape_data[shape_a]read — reusescale_data_a[3]instead.
scale_data_a = shape_data[shape_a]is already loaded at line 1079. Themargin_offset_a = shape_data[shape_a][3]on line 1102 issues a second global memory read for the same element.♻️ Proposed fix
- # Extract margin offset for shape A (signed distance padding) - margin_offset_a = shape_data[shape_a][3] + # Extract margin offset for shape A (stored in w component of shape_data) + margin_offset_a = scale_data_a[3]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/geometry/contact_reduction_global.py` around lines 1079 - 1102, The code redundantly reads shape_data[shape_a] twice; reuse the already-loaded scale_data_a to get the margin instead of doing a second global lookup. Replace the second read that assigns margin_offset_a from shape_data[shape_a][3] by reading scale_data_a[3] (i.e., set margin_offset_a = scale_data_a[3]) so the function uses the cached scale_data_a value (symbols: scale_data_a, margin_offset_a, shape_data, shape_a).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@newton/_src/sim/model.py`:
- Around line 903-907: The doc note for contact gaps is missing SI units; update
the text around Model.shape_gap / ShapeConfig.gap / builder.rigid_gap /
ModelBuilder.finalize to annotate the gap quantity with [m] (e.g., "gap [m]") so
the note reads that gaps are in meters and follows the docstring unit
convention.
In `@newton/tests/test_mujoco_solver.py`:
- Around line 2635-2641: Rename the test function
test_geom_margin_from_thickness to test_shape_margin_to_geom_margin and update
its docstring to Google style: start with a one-line summary "Convert
shape_margin to geom_margin and verify runtime updates.", followed by a blank
line and a short "Details:" paragraph stating that the test verifies
shape_margin [m] values are propagated to geom_margin [m] at solver
initialization and after notify_model_changed across multiple worlds; there are
no parameters or return values so omit Args/Returns sections; keep the unit
notation "[m]" exactly as in the original description and ensure the docstring
remains triple-quoted above the test function.
---
Outside diff comments:
In `@docs/concepts/usd_parsing.rst`:
- Around line 245-251: Replace the incorrect attribute name `newton:contactGap`
in the Newton example with the actual attribute `newton:contactMargin` so the
docs match the schema and tests; update the line containing `newton:contactGap`
in the USD parsing examples to `newton:contactMargin`, ensuring the example now
references the same attribute defined in the schema (`contactMargin`) used
throughout the codebase.
In `@newton/_src/geometry/contact_reduction_global.py`:
- Around line 947-950: Update the stale comment that still says "thickness" to
reflect the renamed terminology "margin offset": change the comment near the
shape metadata declaration (around the shape_types and shape_data arrays) to
mention "margin offset and effective radius" and/or reference the per-shape
margin_offset_a / margin_offset_b fields so it matches the new variable names
used elsewhere in contact_reduction_global.py.
In `@newton/_src/geometry/narrow_phase.py`:
- Around line 1692-1731: Update the stale "thickness" wording and add SI units
in the narrow phase function docs and signature comments: change references to
the fourth component of shape_data (vec4) from "thickness" to "margin" or
"margin offset" and annotate it with units [m]; add units [m] to shape_gap and
shape_collision_radius descriptions in both the inline signature comments and
the Google-style docstring; ensure other related phrases (e.g., "per-shape
contact gap (detection threshold)") use the new terminology and include units.
Refer to the symbols shape_data, shape_gap, shape_collision_radius and the
function docstring/header to locate and update all occurrences.
- Around line 2060-2110: The docstring for the narrow-phase launcher has stale
wording about "thickness" and lacks SI units for physical quantities; update the
parameter descriptions for shape_data and shape_gap (and any other physical
parameters in this docstring) to use Google-style phrasing and include SI units:
explicitly state shape_data is a vec4 containing scale (xyz) in meters [m] and
thickness (w) in meters [m] (or clarify if w is half-thickness), and mark
shape_gap as a contact detection threshold in meters [m]; mirror the same
unit/style changes you applied in launch_custom_write and ensure references use
the exact parameter names shape_data, shape_gap, shape_collision_radius, and
shape_transform so reviewers can find the edits easily.
In `@newton/_src/sim/builder.py`:
- Around line 5531-5546: The cloned/remeshed shapes are not preserving the
per-shape collision gap; update the shape creation to propagate the per-shape
gap by setting gap=self.shape_gap[shape] on the ShapeConfig (or passing the
per-shape gap through add_shape_mesh if that function accepts a gap param)
instead of letting the code fall back to builder.rigid_gap; apply the same
change in the sibling block around the 5604–5619 region so both
remeshing/cloning code paths carry self.shape_gap[shape].
In `@newton/_src/solvers/mujoco/kernels.py`:
- Line 141: The inline comment "# Sum margins for consistency with thickness
summing" is stale after renaming thickness→margin; update it to something like
"# Sum margins for consistency with margin summing" (or "# Sum margins for
consistency with margin accumulation") and search within kernels.py for any
remaining 'thickness' identifiers or comments and replace them with 'margin' to
keep terminology consistent (check uses of the variable name margin and any
related functions in this module).
---
Duplicate comments:
In `@newton/_src/sim/builder.py`:
- Around line 4879-4881: The tuple unpack uses a single-letter variable I which
triggers Ruff E741; rename the inertia variable returned from
compute_inertia_shape (currently unpacked as I) to a clearer identifier (e.g.,
inertia, inertia_tensor, or I_body) and update its usage in the call to
self._update_body_mass so the names match (locate compute_inertia_shape tuple
unpack and the subsequent call to self._update_body_mass to change both the
unpacked name and the argument passed, e.g., replace I with inertia_tensor).
---
Nitpick comments:
In `@newton/_src/geometry/contact_reduction_global.py`:
- Around line 1079-1102: The code redundantly reads shape_data[shape_a] twice;
reuse the already-loaded scale_data_a to get the margin instead of doing a
second global lookup. Replace the second read that assigns margin_offset_a from
shape_data[shape_a][3] by reading scale_data_a[3] (i.e., set margin_offset_a =
scale_data_a[3]) so the function uses the cached scale_data_a value (symbols:
scale_data_a, margin_offset_a, shape_data, shape_a).
In `@newton/_src/solvers/mujoco/kernels.py`:
- Line 229: The kernel parameter rigid_contact_friction in kernels.py loses the
"scale" meaning and is used as friction_scale inside the kernel (see local
alias/friction[0] * friction_scale around the friction computation), so either
rename the parameter back to rigid_contact_friction_scale or add a short inline
comment on the parameter declaration clarifying it is a multiplier (e.g.,
"scale/multiplier applied to per-contact friction"). Update the kernel signature
(rigid_contact_friction → rigid_contact_friction_scale) and any call sites or
keep the name but add the comment so readers immediately understand it is a
multiplicative scale used as friction_scale in the kernel.
ℹ️ Review info
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
docs/concepts/usd_parsing.rstnewton/_src/geometry/contact_reduction_global.pynewton/_src/geometry/narrow_phase.pynewton/_src/sim/builder.pynewton/_src/sim/model.pynewton/_src/solvers/mujoco/kernels.pynewton/_src/solvers/mujoco/solver_mujoco.pynewton/_src/solvers/semi_implicit/kernels_contact.pynewton/_src/utils/import_mjcf.pynewton/_src/utils/import_usd.pynewton/tests/test_broad_phase.pynewton/tests/test_import_usd.pynewton/tests/test_mujoco_solver.pynewton/tests/test_selection.py
🚧 Files skipped from review as they are similar to previous changes (3)
- newton/tests/test_broad_phase.py
- newton/_src/utils/import_usd.py
- newton/tests/test_selection.py
4 tasks
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
newton/_src/geometry/support_function.py (1)
312-324:⚠️ Potential issue | 🟡 MinorAdd SI units to the
extract_shape_datadocstring.The docstring references physical quantities (scale, margin_offset, position) without inline SI units.
🔧 Suggested update
- shape_transform: World space transforms (already computed) + shape_transform: World space transforms (translation [m], rotation [unitless]) shape_types: Shape types - shape_data: Shape data (vec4 - scale xyz, margin_offset w) + shape_data: Shape data (vec4 - scale xyz [m], margin_offset w [m]) shape_source: Source pointers (mesh IDs etc.) Returns: - tuple: (position, orientation, shape_data, scale, margin_offset) + tuple: (position [m], orientation [unitless], shape_data, scale [m], margin_offset [m])As per coding guidelines: “
newton/**/*.py: State SI units for all physical quantities in docstrings using inline[unit]notation (e.g.,[m],[N·m]).”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/geometry/support_function.py` around lines 312 - 324, The docstring for extract_shape_data should state SI units inline: annotate returned position as position [m], orientation as orientation (quaternion, unitless), shape_data components as scale xyz [m] and margin_offset w [m], and the returned scale as scale [m] and margin_offset as margin_offset [m]; also annotate input shape_transform positions as world-space transforms with translation in [m] and any mesh/source IDs in shape_source as unitless identifiers. Update the textual descriptions in the docstring accordingly so each physical quantity uses the [unit] notation (e.g., position [m], scale [m], margin_offset [m]) while leaving non-physical items marked unitless.newton/_src/sim/model.py (1)
297-304:⚠️ Potential issue | 🟡 MinorStale terminology in AABB docstrings —
thicknessandcontact marginshould be updated.Both
shape_collision_aabb_lowerandshape_collision_aabb_upperdocstrings still reference the old terms. Per this PR's rename objectives, these should be updated tomarginandgap, respectively.📝 Proposed fix
- Computed from base geometry only (excludes thickness - thickness is added during contact - margin calculations). Used for voxel-based contact reduction.""" + Computed from base geometry only (excludes margin - margin is added during contact + gap calculations). Used for voxel-based contact reduction."""Apply the same diff to both
shape_collision_aabb_lowerandshape_collision_aabb_upperdocstrings.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/sim/model.py` around lines 297 - 304, Update the docstrings for shape_collision_aabb_lower and shape_collision_aabb_upper so they no longer use the stale terms "thickness" and "contact margin"; replace "thickness" with "margin" and replace "contact margin" with "gap" in both docstrings, keeping the rest of the wording intact (they still describe local-space AABB bounds computed from base geometry and used for voxel-based contact reduction).
♻️ Duplicate comments (2)
newton/tests/test_import_usd.py (1)
4544-4550:⚠️ Potential issue | 🟡 MinorUpdate
test_gap_parsingdocstring to Google style.The docstring still isn’t Google-style (missing Args/Returns). Please align it while keeping the [m] units.
✍️ Proposed docstring update
- """Verify USD contact margin parsing into shape gap values [m]. - - The ``newton:contactMargin`` USD attribute should map to per-shape - ``gap`` values [m], while colliders without an explicit value should - fall back to the configured default gap [m]. - """ + """Verify USD contact margin parsing into shape gap values. + + Args: + self: Test case instance. + + Returns: + None. Verifies that ``newton:contactMargin`` maps to per-shape ``gap`` [m], + and that missing values fall back to the default gap [m]. + """As per coding guidelines: "Use Google-style docstrings with clear, concise explanations of what the function does, its parameters, and return values" and "State SI units for all physical quantities in docstrings using inline [unit] notation (e.g., [m], [N·m])."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/tests/test_import_usd.py` around lines 4544 - 4550, Update the test_gap_parsing docstring to Google style: start with a one-line summary, then a short description, and add an Args: section (if the test takes no parameters, state "None") and a Returns: section (e.g., "None"), while preserving the [m] unit annotations for gap and default gap; modify the docstring in the test_gap_parsing function to follow that structure and wording, referencing the USD attribute newton:contactMargin and its mapping to per-shape gap values [m].newton/_src/sim/model.py (1)
904-907:⚠️ Potential issue | 🟡 MinorLine 904:
Rigid contact gapsstill missing[m]unit annotation.Lines 905–906 were updated with
[m], but the opening phrase on line 904 remains unannotated.📝 Proposed fix
- Rigid contact gaps are controlled per-shape via :attr:`Model.shape_gap`, which is populated + Rigid contact gaps [m] are controlled per-shape via :attr:`Model.shape_gap`, which is populatedAs per coding guidelines, "State SI units for all physical quantities in docstrings using inline
[unit]notation."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/sim/model.py` around lines 904 - 907, The opening phrase "Rigid contact gaps" in the docstring is missing the SI unit annotation; update that phrase to "Rigid contact gaps [m]" so the sentence reads consistently with the subsequent clauses and coding guidelines; ensure the docstring references remain correct for Model.shape_gap, ShapeConfig.gap, builder.rigid_gap and ModelBuilder.finalize.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@newton/_src/geometry/support_function.py`:
- Around line 312-324: The docstring for extract_shape_data should state SI
units inline: annotate returned position as position [m], orientation as
orientation (quaternion, unitless), shape_data components as scale xyz [m] and
margin_offset w [m], and the returned scale as scale [m] and margin_offset as
margin_offset [m]; also annotate input shape_transform positions as world-space
transforms with translation in [m] and any mesh/source IDs in shape_source as
unitless identifiers. Update the textual descriptions in the docstring
accordingly so each physical quantity uses the [unit] notation (e.g., position
[m], scale [m], margin_offset [m]) while leaving non-physical items marked
unitless.
In `@newton/_src/sim/model.py`:
- Around line 297-304: Update the docstrings for shape_collision_aabb_lower and
shape_collision_aabb_upper so they no longer use the stale terms "thickness" and
"contact margin"; replace "thickness" with "margin" and replace "contact margin"
with "gap" in both docstrings, keeping the rest of the wording intact (they
still describe local-space AABB bounds computed from base geometry and used for
voxel-based contact reduction).
---
Duplicate comments:
In `@newton/_src/sim/model.py`:
- Around line 904-907: The opening phrase "Rigid contact gaps" in the docstring
is missing the SI unit annotation; update that phrase to "Rigid contact gaps
[m]" so the sentence reads consistently with the subsequent clauses and coding
guidelines; ensure the docstring references remain correct for Model.shape_gap,
ShapeConfig.gap, builder.rigid_gap and ModelBuilder.finalize.
In `@newton/tests/test_import_usd.py`:
- Around line 4544-4550: Update the test_gap_parsing docstring to Google style:
start with a one-line summary, then a short description, and add an Args:
section (if the test takes no parameters, state "None") and a Returns: section
(e.g., "None"), while preserving the [m] unit annotations for gap and default
gap; modify the docstring in the test_gap_parsing function to follow that
structure and wording, referencing the USD attribute newton:contactMargin and
its mapping to per-shape gap values [m].
ℹ️ Review info
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
newton/_src/geometry/support_function.pynewton/_src/sim/model.pynewton/tests/test_import_usd.pynewton/tests/test_mujoco_solver.py
🚧 Files skipped from review as they are similar to previous changes (1)
- newton/tests/test_mujoco_solver.py
vastsoun
previously approved these changes
Feb 24, 2026
Member
There was a problem hiding this comment.
LGTM.
However, I have the following (minor) concern: I don't understand why the global rigid_gap defaults to 0.1. Doesn't this cause overly many false positives and therefore induce many preemptive contacts?
Also, @nvtw, I think the fantastic graphic you showed us recently should be contributed directly into the documentation and render in the collisions.rst. What do you think?
Member
Author
The graphic is already in an MR which integrates it into the docs (it actually originates from the omniverse PhysX docs). |
Member
|
Great. Thanks for the clarification. I've already blessed these PR changes, so from my side all good to proceed. |
This was referenced Feb 26, 2026
vastsoun
added a commit
to vastsoun/newton
that referenced
this pull request
Mar 3, 2026
* [Warp Raytrace] Added device parameter (newton-physics#1544) * [Warp Raytrace] Added device parameter to previously overlooked call (newton-physics#1545) * SolverMuJoCo: Fix tolerance clamping in update_solver_options_kernel (newton-physics#1546) * Change default shape_ke to align with MuJoCo, parse geom solref from MJCF for contact stiffness/damping (newton-physics#1491) Signed-off-by: Alain Denzler <adenzler@nvidia.com> * Fix log_shapes broadcasting for length-1 warp arrays (newton-physics#1550) * Fix XPBD restitution particle index (newton-physics#1557) * Out-of-Bound memory read in example_diffsim_bear newton-physics#1386 (newton-physics#1533) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Add versioned documentation deployment to GitHub Pages (newton-physics#1560) * Fix broken documentation links after versioned docs deployment (newton-physics#1566) * VBD New Features (newton-physics#1479) Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> * Add banners to membership verification workflow steps (newton-physics#1569) * Support cable junctions (newton-physics#1519) Signed-off-by: JC <jumyungc@nvidia.com> * Rename parameter I to inertia newton-physics#1543 (newton-physics#1551) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Fix example_robot_anymal_c_walk.py (newton-physics#1574) * Change everywhere linesearch to iterative (newton-physics#1573) * Remove standard collision pipeline (newton-physics#1538) * USD Plumbing MJC solver attributes through resolver and custom attribute framework (newton-physics#1463) Signed-off-by: Milad Rakhsha <mrakhsha@nvidia.com> Signed-off-by: Milad-Rakhsha-NV <167464435+Milad-Rakhsha-NV@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: adenzler-nvidia <adenzler@nvidia.com> * Fix child shape filtering (newton-physics#1559) * Fix ViewerRerun ignoring hidden parameter in log_mesh and log_instances (newton-physics#1555) * Make NxN and SAP broad phase respect filtered pairs (newton-physics#1554) * Add --quiet flag to examples to suppress Warp messages (newton-physics#1585) * Defer resolution of MESH_MAXHULLVERT default in importers (newton-physics#1587) * Fix TypeError when finalizing SDF geometry with device kwarg (newton-physics#1586) * Make MESH_MAXHULLVERT a static class attribute Mesh.MAX_HULL_VERTICES. (newton-physics#1598) * Significant non-determinism in unified collision pipeline for anymal_c_walking example newton-physics#1505 (newton-physics#1588) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Add test for non-contiguous case (newton-physics#1549) * Fix nightly Warp CI to resolve pre-release builds (newton-physics#1606) * Verify default class and value handling (newton-physics#1556) * SolverMuJoCo: Expand geom_margin to avoid OOB read with heterogeneous worlds (newton-physics#1607) * Fix bug in control clear method (newton-physics#1602) * Enable Use of Newton IK in Lab newton-physics#662 (newton-physics#1539) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Fix import of non-articulated joints (newton-physics#1535) * Deduplicate _process_joint_custom_attributes frequency handling (newton-physics#1584) * Add CI check for stale API docs and fix local build warnings (newton-physics#1570) * Update Pillow 12.0.0 to 12.1.1 (newton-physics#1612) * Prepare handling of mimic constraints in Newton (newton-physics#1523) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> Signed-off-by: Eric Heiden <eheiden@nvidia.com> * Support floating, base_joint and parent_body arguments for importers (newton-physics#1498) * Fix contact buffer memory overestimation (newton-physics#1614) * Configure banned-module-level-imports for ruff (newton-physics#1583) * Explicit `Contacts` instantiation with `Model.contacts()` and `CollisionPipeline.contacts()` (newton-physics#1445) * Fix the quadruped benchmark regression (newton-physics#1615) * Change default ignore_inertial_definitions from True to False (newton-physics#1537) * Finalize the Recording API (newton-physics#1600) * SolverMuJoCo: Fix ccd_iterations default (newton-physics#1631) * update gitignore to ignore Claude Code sandbox files (newton-physics#1628) * Add mimic joint support to SolverMuJoCo (newton-physics#1627) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Add --no-cache-clear flag to test runner (newton-physics#1629) * Update MuJoCo and MuJoCo Warp to 3.5.0 release (newton-physics#1633) * Improve inertia parsing from USD (newton-physics#1605) Signed-off-by: Eric Heiden <eheiden@nvidia.com> Signed-off-by: Eric Heiden <eric-heiden@outlook.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Remove standalone .typos.toml in favor of pyproject.toml config (newton-physics#1642) * Heightfield support newton-physics#1189 (newton-physics#1547) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Example_robot_policy: Replace ValueError with clean error for missing PhysX policy (newton-physics#1636) Co-authored-by: Viktor Reutskyy <33062116+vreutskyy@users.noreply.github.com> * Avoid unnecessary inflation of the contact reduction voxel aabb (newton-physics#1650) * Rename num_worlds to world_count (newton-physics#1634) Signed-off-by: Eric Heiden <eheiden@nvidia.com> * Support parsing autolimits from MJCF (newton-physics#1651) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Fix particle-shape restitution ignoring body velocity (newton-physics#1273) (newton-physics#1580) * Add overflow warnings for narrow-phase collision buffers (newton-physics#1643) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Documentation: add units to model/state docstrings (newton-physics#1649) * fix(viewer): add missing JointType.BALL support to contact line kernel (newton-physics#1640) * Make terrain mesh visual-only in anymal C walking example (newton-physics#1660) Signed-off-by: Eric Heiden <eheiden@nvidia.com> * Fix initialization of collider state in MPM finite difference mode (newton-physics#1652) * docs: document ModelBuilder.default_shape_cfg (newton-physics#1662) * Finalize the collision API (newton-physics#1581) * Remove hardcoded subnet ID from AWS workflow (newton-physics#1664) Signed-off-by: Eric Heiden <eheiden@nvidia.com> * Attempt to fix AWS config (newton-physics#1666) Signed-off-by: Eric Heiden <eheiden@nvidia.com> * Update AWS workflows to g7e.2xlarge with multi-AZ failover (newton-physics#1669) * fix(viewer-usd): disambiguate log_points colors for N=3 warp arrays (newton-physics#1661) * Viewer gl optimizations (newton-physics#1656) Signed-off-by: Miles Macklin <mmacklin@nvidia.com> Signed-off-by: Eric Heiden <eheiden@nvidia.com> Co-authored-by: Eric Heiden <eheiden@nvidia.com> * docs: add articulation workflow guidance and regression check (newton-physics#1663) * fix(examples): propagate IK solution to model state in Franka example (newton-physics#1637) * fix(deps,docs): bump nbconvert to 7.17.0 and fix ArticulationView doctest (newton-physics#1670) * Cleanup and improve some example (newton-physics#1625) Signed-off-by: adenzler-nvidia <adenzler@nvidia.com> Co-authored-by: adenzler-nvidia <adenzler@nvidia.com> * Handle zero-mass bodies and flip ensure_nonstatic_links default (newton-physics#1635) * Additional testing for ArticulationView (newton-physics#1527) Co-authored-by: Eric Heiden <eheiden@nvidia.com> * Update warp-lang nightly to 1.12.0.dev20260217 (newton-physics#1677) * Change default friction coefficients to match MuJoCo (newton-physics#1681) * Refactor mesh creation functions (newton-physics#1654) Signed-off-by: Eric Heiden <eheiden@nvidia.com> * Parse joint frictionloss from MJCF (newton-physics#1680) * Fix free joint body_pos and add ref/qpos0 support for MuJoCo solver (newton-physics#1645) * Fix root shapes in ArticulationView with fixed base (newton-physics#1639) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> Co-authored-by: Eric Heiden <eric-heiden@outlook.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Use model collision methods and remove `create_collision_pipeline` from examples (newton-physics#1648) Co-authored-by: nvtw <110816143+nvtw@users.noreply.github.com> * Fix XPBD apply_joint_forces ignoring child joint transform (newton-physics#1582) * Adjust SDF API (newton-physics#1644) Co-authored-by: Eric Heiden <eheiden@nvidia.com> * Optimize test suite runtime (~18% faster) (newton-physics#1689) * Remove ensure_nonstatic_links option from importers (newton-physics#1682) Signed-off-by: adenzler-nvidia <adenzler@nvidia.com> * SolverMuJoCo: Add geom_margin support, align thickness default with MuJoCo and schemas (newton-physics#1653) * refactor: privatize non-public solver internals (newton-physics#1683) * Fix option parsing with multiple <option> elements from includes (newton-physics#1692) * Bump warp-lang nightly and newton-usd-schemas (newton-physics#1693) * Get rid of tkinter dependency (newton-physics#1676) * Fix SDF example contact buffer overflow (newton-physics#1695) * Fix implicit biastype for position/velocity actuator shortcuts (newton-physics#1678) * Fix include processor to respect meshdir/texturedir (newton-physics#1685) * Reduce cold-cache Warp compile time for geometry modules (newton-physics#1618) Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: adenzler-nvidia <adenzler@nvidia.com> * Fix xyzw-to-wxyz quaternion conversion in body inertia kernel (newton-physics#1694) * Rename key to label and add hierarchical labels (newton-physics#1592) (newton-physics#1632) * Expose geometry SDF helpers on public API (newton-physics#1684) Signed-off-by: Eric Heiden <eheiden@nvidia.com> * Improve custom frequency handling from USD, parse MuJoCo actuators and tendons (newton-physics#1510) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> Signed-off-by: Eric Heiden <eheiden@nvidia.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Collapse fixed joints with non articulated bodies (newton-physics#1608) * Fix renaming joint_key -> joint_label (newton-physics#1700) Signed-off-by: Eric Heiden <eheiden@nvidia.com> * Bump .python-version from 3.11 to 3.12 (newton-physics#1702) * Replace CITATION.md with CITATION.cff (newton-physics#1706) * Respect MJCF contype=conaffinity=0 via collision_group=0 (newton-physics#1703) * Make ViewerUSD reuse existing USD layers for the same output path (newton-physics#1704) Signed-off-by: Eric Heiden <eheiden@nvidia.com> Signed-off-by: Eric Heiden <eric-heiden@outlook.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Remove dead up-axis conversion from MuJoCo solver (newton-physics#1707) * Skip IK cube stacking example test (newton-physics#1713) * Fix global pairs (world=-1) not exported to MuJoCo spec (newton-physics#1705) * Reduce the memory consumption of hydroelastic contacts (newton-physics#1609) * Fix flakiness cube stacking (newton-physics#1714) * Fix collision shapes not toggleable in viewer UI (newton-physics#1715) * Fix softbody examples table layout in README (newton-physics#1716) * Standardize sensor APIs: label matching, keyword args, and update() method naming (newton-physics#1665) * Fix intermittent crash in parallel test runner from Manager proxy race (newton-physics#1721) * Clean up inertia.py function arguments to match Mesh.create_* API (newton-physics#1719) * Reduce default test runner verbosity (newton-physics#1723) * Add menagerie comparison tests for SolverMuJoCo (newton-physics#1720) Signed-off-by: Alain Denzler <adenzler@nvidia.com> Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> Co-authored-by: Viktor Reutskyy <vreutskyy@nvidia.com> Co-authored-by: Viktor Reutskyy <33062116+vreutskyy@users.noreply.github.com> * Add spatial tendon support for MuJoCo solver (newton-physics#1687) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Expose qfrc_actuator from mujoco (newton-physics#1698) * Skip non-MODEL custom attributes in finalize validation (newton-physics#1734) * Resolve inheritrange for position actuators in MJCF parser (newton-physics#1727) * Support dampratio for position/velocity actuator shortcuts (newton-physics#1722) * Limit concurrency to 1 (newton-physics#1736) * Add a helper method for checking applied usd (newton-physics#1731) * Enhance playback URL handling in ViewerViser (newton-physics#1742) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> * Removed RenderShapeType (newton-physics#1748) * Override only MassAPI attributes that have been authored (newton-physics#1688) * Integration of newton-actuators (newton-physics#1342) Signed-off-by: jvonmuralt <jvonmuralt@nvidia.com> * Fix ArticulationView crash with fixed-joint-only articulations (newton-physics#1726) * Improve retrieval of Jupyter base URL in ViewerViser (newton-physics#1750) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> * Disable dynamics testing for UR5e and Apollo tests to avoid CI flakiness (newton-physics#1755) * Margin and Gap rename (newton-physics#1732) * Vbd Demos Fixing (newton-physics#1740) * Fix ViewerViser.log_lines method (newton-physics#1764) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> * Update warp-lang dependency to 1.12.0rc1 (newton-physics#1763) * Fix fromto capsule/cylinder orientation in MJCF parser (newton-physics#1741) * fix: multi-world particle BVH indexing (newton-physics#1641) Co-authored-by: Eric Heiden <eheiden@nvidia.com> * Clean up unused and internal-only kwargs in SolverMuJoCo (newton-physics#1766) * Parsing of the mimic joint and contact gap/margin from newton schemas (newton-physics#1690) Signed-off-by: Milad Rakhsha <mrakhsha@nvidia.com> * API Refactor v2 (newton-physics#1749) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> * Support explicit geom mass attributes in MJCF (newton-physics#1744) * Bump flask and werkzeug in lockfile for security (newton-physics#1769) Co-authored-by: Cursor <cursoragent@cursor.com> * Split MJCF worldbody root bodies into separate articulations (newton-physics#1754) * Expose VBD rigid contact forces for solver coupling (newton-physics#1745) Signed-off-by: JC <jumyungc@nvidia.com> * Add MJCF ellipsoid geom import and regression test (newton-physics#1772) Co-authored-by: Cursor <cursoragent@cursor.com> * Weld equality constraints parsed from mjcf are given Nan as the default value of torquescale. The correct default should be 1.0 (newton-physics#1760) * Improve picking accuracy and stability (newton-physics#1712) * Franka cloth demo improvement (newton-physics#1765) * Support computing sensing object transforms & API cleanup (newton-physics#1759) * Remove threading workaround (newton-physics#1751) * [Warp Raytrace] Consolidated ray intersect functions, renamed Options to Config (newton-physics#1767) * Improve README example gallery for PyPI compatibility (newton-physics#1776) * Fix issue with mesh in rerun viewer (newton-physics#1768) * Add PhysxMimicJointAPI parsing to USD importer (newton-physics#1735) * Move some math functions to Warp (newton-physics#1717) Signed-off-by: Eric Heiden <eheiden@nvidia.com> Signed-off-by: Eric Heiden <eric-heiden@outlook.com> * Add test to ensure MJCF xform argument is relative (newton-physics#1777) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> * Emit diaginertia instead of fullinertia for diagonal body inertia (newton-physics#1780) * Change default joint armature from 0.01 to 0 (newton-physics#1782) * Fix default kp/kv for position and velocity actuators (newton-physics#1786) * Lock body inertia after explicit MJCF <inertial> element (newton-physics#1784) * Fix for MJCF actuator custom attributes (newton-physics#1783) * Bump version to 0.2.3 Prepare the package metadata for the v0.2.3 release tag. * Fix ViewerRerun rendering of instances from hidden meshes (newton-physics#1788) * API cleanup (newton-physics#1789) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> * BODY actuator target name bypasses body name de-duplication in SolverMuJoCo (newton-physics#1729) * Use default density for visual geoms in MJCF import (newton-physics#1781) * Fix GL viewer crash on Wayland (newton-physics#1793) * Make USD xform parameter control absolute articulation placement (newton-physics#1771) * Fix CUDA context corruption in SDF implementation (newton-physics#1792) * Bump mujoco-warp dependency to 3.5.0.2 (newton-physics#1779) * Fix MuJoCo margin/gap conversion (newton-physics#1785) * Bump version to 1.1.0.dev0 (newton-physics#1798) * Missing unittest.main added back to test_import_mjcf.py. Helps with F5 debugging in VS Code. (newton-physics#1796) * Improve H1 example (newton-physics#1801) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> * Fix ViewerViser.set_camera() (newton-physics#1805) * Rename examples to follow prefix-first naming convention (newton-physics#1802) * Improve MuJoCo actuator domain randomization (newton-physics#1773) * Restore in_cup test in hydro example (newton-physics#1775) * Fix `newton.geometry` imports and change of Mesh maxhullverts global constant * Adapt to having both margin and gap arrays for each geom/shape * Fix for newton.geometry API changes in primitive/narrowphase.py * Fix removal of `BroadPhaseMode` IntEnum and revert to newton-sytle string literals * WIP: Adapt geometry/unified.py to fix breaking changes to `newton.geometry` API * First pass of API adjustments * Patch more gaps (with respect to margin and gap renaming) * GeoType.SDF was removed - reflect that in Kamino * Introduce CoM position offsets w.r.t body frame and operations to convert between body CoM state and generic local body-fixed reference frames. * Add caching of per-entity labels/names/keys in the Model subcontainers * Remove SDF shape wrapper since it's now internal to mesh handling in CD pipelines * Fix USD test assets * Add Newton <--> Kamino joint type conversion operations and per-space default limit constants * Add some cleanup to geometry and unified CD + UTs * Add Newton <--> Kamino shape type conversion operations * Apply new default joint coord limit constants to limits.py * Adapt foubrar model builder and USD asset to produce the same result in sim example * Purge "physical" goems and collapse all into a single group, and purge geometry "layers" * Disable allocation per-joint wrenches by default and make them optional * Make `Model` a dataclass * Separate ModelData* containers into own `core/data.py` module * Fix imports of ModelData * Rename `Model` as `ModelKamino` * Rename `ModelData` as `DataKamino` * Rename `State` as `StateKamino` * Rename `Control` as `ControlKamino` * Rename `Limits` as `LimitsKamino` * Rename `Contacts` as `ContactsKamino` * Rename `ModelBuilder` as `ModelBuilderKamino` * Make imports in test utilities relative * Revise CD meta-data attributes and their computation in GeometryModel and ModelBuilderKamino * Revise primitive CD pipeline * Revise unified CD pipeline * Revise CD front-end interfaces * Fix UTs and relevant utils for interface changes to CD * Change to `wp.DeviceLike` to account for upcoming deprecation of `Devicelike` * Depracate legacy HDF5 data io (will be replaced in the future) * Fix banned imports at module level * Modify USD importer to detect articulations and order geoms and joints similarly to how the Newton `parse_usd` function does. * Add conversion operation from `newton.Model` to `ModelKamino` * Add data, state and control container conversions * Add SolverKamino wrapper that fullfils newton integration interface * Add newton integration examples * Add SolverKamino to newton solver module imports * WIP: fix problem with lambda_j being allocated for only kinematic constraints and failing on array copying * Fix banned git import in benchmark * Rename *Settings to *Config (#213) * Rename SolverKaminoSettings -> SolverKaminoConfig * Rename DualProblemSettings -> DualProblemConfig * Rename CollisionDetectorSettings -> CollisionDetectorConfig * Rename PADMMSettings -> PADMMConfig * Rename ForwardKinematicsSolverSettings -> ForwardKinematicsSolverConfig * Rename SimulatorSettings -> SimulatorConfig * Add check for model compatibility in SolverKamino (#209) * Fix device assignment in sparse CG test on CPU (#216) * Replace Enum-type config attributes with Literal (#215) * Replace warmstart mode config param with literal * Replace contact warmstart mode config param with literal * Replace rotation correction config param with literal * Replace penalty update config param with literal * Replace FK preconditioner option config param with literal * Add post-init checks for dual/PADMM configs * Rename FKPreconditionerOptions to FKPreconditionerType * Remove WorldDescriptor from ModelKamino (#219) * Add geom index offset array to model info * Replace access to world description in model * Remove world descriptor from model * Fix computation of kinematics residual with sparse Jacobian (#220) * Migrates `ModelKaminoSize` to it's own module to avoid circular dependency between core/model.py and core/state.py and rename it as `SizeKamino`. * WIP: Fix SolverKamino wrapper * Fix circular dependency in conversions.py * Fix some broken unit tests and WIP to fix fourbar contact conversions and rendering * Make some conversions @staticmethods instead, because they dont need common class attributes * Fix declaration of custom state attributes and their conversion to/from newton and kamino * WIP: Debug model conversion and newton sim examples * Rename and cleanup start index array of per-world geoms * Model conversion and newton sim examples now work. * Make gravity conversion operation re-usable * Migrates boxes_fourbar builder using newton.ModelBuilder to it's own module to prepare for migration. * Use gravity conversion utility func in SolverKamino * Add reusable joint-parameterization conversion utility * Remove world-descriptor from model converter * Rename helper converter that handles entity-local transforms * Add some cleanup to DR Legs, ANYmal D and basic four-bar examples * Fix module-level imports of additional kamino-specific development dependencies * Fix module-level imports of additional kamino-specific development dependencies * Fix erroneous merge conflict. --------- Signed-off-by: Alain Denzler <adenzler@nvidia.com> Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> Signed-off-by: JC <jumyungc@nvidia.com> Signed-off-by: Milad Rakhsha <mrakhsha@nvidia.com> Signed-off-by: Milad-Rakhsha-NV <167464435+Milad-Rakhsha-NV@users.noreply.github.com> Signed-off-by: Eric Heiden <eric-heiden@outlook.com> Signed-off-by: Eric Heiden <eheiden@nvidia.com> Signed-off-by: Miles Macklin <mmacklin@nvidia.com> Signed-off-by: adenzler-nvidia <adenzler@nvidia.com> Signed-off-by: jvonmuralt <jvonmuralt@nvidia.com> Co-authored-by: Daniela Hase <116915287+daniela-hase@users.noreply.github.com> Co-authored-by: adenzler-nvidia <adenzler@nvidia.com> Co-authored-by: Viktor Reutskyy <33062116+vreutskyy@users.noreply.github.com> Co-authored-by: Eric Shi <97630937+shi-eric@users.noreply.github.com> Co-authored-by: Anka Chen <AnkaChan@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: JC-nvidia <116605903+jumyungc@users.noreply.github.com> Co-authored-by: Kenny Vilella <kvilella@nvidia.com> Co-authored-by: nvtw <110816143+nvtw@users.noreply.github.com> Co-authored-by: Milad-Rakhsha-NV <167464435+Milad-Rakhsha-NV@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Lennart Röstel <65088822+lenroe@users.noreply.github.com> Co-authored-by: Eric Heiden <eheiden@nvidia.com> Co-authored-by: jvonmuralt <jvonmuralt@nvidia.com> Co-authored-by: camevor <camevor@nvidia.com> Co-authored-by: mzamoramora-nvidia <mzamoramora@nvidia.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Alessandro Roncone <alecive87@gmail.com> Co-authored-by: gdaviet <57617656+gdaviet@users.noreply.github.com> Co-authored-by: Miles Macklin <mmacklin@nvidia.com> Co-authored-by: Gordon Yeoman <gyeomannvidia@users.noreply.github.com> Co-authored-by: Lukasz Wawrzyniak <lwawrzyniak@nvidia.com> Co-authored-by: Eric Heiden <eric-heiden@outlook.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Viktor Reutskyy <vreutskyy@nvidia.com> Co-authored-by: Lorenzo Terenzi <lorenzoterenzi96@gmail.com> Co-authored-by: smollerNV <164020096+smollerNV@users.noreply.github.com> Co-authored-by: twidmer <twidmer@nvidia.com> Co-authored-by: Christian Schumacher <christian.schumacher@disney.com> Co-authored-by: Guirec-Maloisel <25688871+Guirec-Maloisel@users.noreply.github.com>
vastsoun
added a commit
to vastsoun/newton
that referenced
this pull request
Mar 4, 2026
* [Warp Raytrace] Added device parameter (newton-physics#1544) * [Warp Raytrace] Added device parameter to previously overlooked call (newton-physics#1545) * SolverMuJoCo: Fix tolerance clamping in update_solver_options_kernel (newton-physics#1546) * Change default shape_ke to align with MuJoCo, parse geom solref from MJCF for contact stiffness/damping (newton-physics#1491) Signed-off-by: Alain Denzler <adenzler@nvidia.com> * Fix log_shapes broadcasting for length-1 warp arrays (newton-physics#1550) * Fix XPBD restitution particle index (newton-physics#1557) * Out-of-Bound memory read in example_diffsim_bear newton-physics#1386 (newton-physics#1533) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Add versioned documentation deployment to GitHub Pages (newton-physics#1560) * Fix broken documentation links after versioned docs deployment (newton-physics#1566) * VBD New Features (newton-physics#1479) Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> * Add banners to membership verification workflow steps (newton-physics#1569) * Support cable junctions (newton-physics#1519) Signed-off-by: JC <jumyungc@nvidia.com> * Rename parameter I to inertia newton-physics#1543 (newton-physics#1551) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Fix example_robot_anymal_c_walk.py (newton-physics#1574) * Change everywhere linesearch to iterative (newton-physics#1573) * Remove standard collision pipeline (newton-physics#1538) * USD Plumbing MJC solver attributes through resolver and custom attribute framework (newton-physics#1463) Signed-off-by: Milad Rakhsha <mrakhsha@nvidia.com> Signed-off-by: Milad-Rakhsha-NV <167464435+Milad-Rakhsha-NV@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: adenzler-nvidia <adenzler@nvidia.com> * Fix child shape filtering (newton-physics#1559) * Fix ViewerRerun ignoring hidden parameter in log_mesh and log_instances (newton-physics#1555) * Make NxN and SAP broad phase respect filtered pairs (newton-physics#1554) * Add --quiet flag to examples to suppress Warp messages (newton-physics#1585) * Defer resolution of MESH_MAXHULLVERT default in importers (newton-physics#1587) * Fix TypeError when finalizing SDF geometry with device kwarg (newton-physics#1586) * Make MESH_MAXHULLVERT a static class attribute Mesh.MAX_HULL_VERTICES. (newton-physics#1598) * Significant non-determinism in unified collision pipeline for anymal_c_walking example newton-physics#1505 (newton-physics#1588) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Add test for non-contiguous case (newton-physics#1549) * Fix nightly Warp CI to resolve pre-release builds (newton-physics#1606) * Verify default class and value handling (newton-physics#1556) * SolverMuJoCo: Expand geom_margin to avoid OOB read with heterogeneous worlds (newton-physics#1607) * Fix bug in control clear method (newton-physics#1602) * Enable Use of Newton IK in Lab newton-physics#662 (newton-physics#1539) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Fix import of non-articulated joints (newton-physics#1535) * Deduplicate _process_joint_custom_attributes frequency handling (newton-physics#1584) * Add CI check for stale API docs and fix local build warnings (newton-physics#1570) * Update Pillow 12.0.0 to 12.1.1 (newton-physics#1612) * Prepare handling of mimic constraints in Newton (newton-physics#1523) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> Signed-off-by: Eric Heiden <eheiden@nvidia.com> * Support floating, base_joint and parent_body arguments for importers (newton-physics#1498) * Fix contact buffer memory overestimation (newton-physics#1614) * Configure banned-module-level-imports for ruff (newton-physics#1583) * Explicit `Contacts` instantiation with `Model.contacts()` and `CollisionPipeline.contacts()` (newton-physics#1445) * Fix the quadruped benchmark regression (newton-physics#1615) * Change default ignore_inertial_definitions from True to False (newton-physics#1537) * Finalize the Recording API (newton-physics#1600) * SolverMuJoCo: Fix ccd_iterations default (newton-physics#1631) * update gitignore to ignore Claude Code sandbox files (newton-physics#1628) * Add mimic joint support to SolverMuJoCo (newton-physics#1627) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Add --no-cache-clear flag to test runner (newton-physics#1629) * Update MuJoCo and MuJoCo Warp to 3.5.0 release (newton-physics#1633) * Improve inertia parsing from USD (newton-physics#1605) Signed-off-by: Eric Heiden <eheiden@nvidia.com> Signed-off-by: Eric Heiden <eric-heiden@outlook.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Remove standalone .typos.toml in favor of pyproject.toml config (newton-physics#1642) * Heightfield support newton-physics#1189 (newton-physics#1547) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Example_robot_policy: Replace ValueError with clean error for missing PhysX policy (newton-physics#1636) Co-authored-by: Viktor Reutskyy <33062116+vreutskyy@users.noreply.github.com> * Avoid unnecessary inflation of the contact reduction voxel aabb (newton-physics#1650) * Rename num_worlds to world_count (newton-physics#1634) Signed-off-by: Eric Heiden <eheiden@nvidia.com> * Support parsing autolimits from MJCF (newton-physics#1651) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Fix particle-shape restitution ignoring body velocity (newton-physics#1273) (newton-physics#1580) * Add overflow warnings for narrow-phase collision buffers (newton-physics#1643) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Documentation: add units to model/state docstrings (newton-physics#1649) * fix(viewer): add missing JointType.BALL support to contact line kernel (newton-physics#1640) * Make terrain mesh visual-only in anymal C walking example (newton-physics#1660) Signed-off-by: Eric Heiden <eheiden@nvidia.com> * Fix initialization of collider state in MPM finite difference mode (newton-physics#1652) * docs: document ModelBuilder.default_shape_cfg (newton-physics#1662) * Finalize the collision API (newton-physics#1581) * Remove hardcoded subnet ID from AWS workflow (newton-physics#1664) Signed-off-by: Eric Heiden <eheiden@nvidia.com> * Attempt to fix AWS config (newton-physics#1666) Signed-off-by: Eric Heiden <eheiden@nvidia.com> * Update AWS workflows to g7e.2xlarge with multi-AZ failover (newton-physics#1669) * fix(viewer-usd): disambiguate log_points colors for N=3 warp arrays (newton-physics#1661) * Viewer gl optimizations (newton-physics#1656) Signed-off-by: Miles Macklin <mmacklin@nvidia.com> Signed-off-by: Eric Heiden <eheiden@nvidia.com> Co-authored-by: Eric Heiden <eheiden@nvidia.com> * docs: add articulation workflow guidance and regression check (newton-physics#1663) * fix(examples): propagate IK solution to model state in Franka example (newton-physics#1637) * fix(deps,docs): bump nbconvert to 7.17.0 and fix ArticulationView doctest (newton-physics#1670) * Cleanup and improve some example (newton-physics#1625) Signed-off-by: adenzler-nvidia <adenzler@nvidia.com> Co-authored-by: adenzler-nvidia <adenzler@nvidia.com> * Handle zero-mass bodies and flip ensure_nonstatic_links default (newton-physics#1635) * Additional testing for ArticulationView (newton-physics#1527) Co-authored-by: Eric Heiden <eheiden@nvidia.com> * Update warp-lang nightly to 1.12.0.dev20260217 (newton-physics#1677) * Change default friction coefficients to match MuJoCo (newton-physics#1681) * Refactor mesh creation functions (newton-physics#1654) Signed-off-by: Eric Heiden <eheiden@nvidia.com> * Parse joint frictionloss from MJCF (newton-physics#1680) * Fix free joint body_pos and add ref/qpos0 support for MuJoCo solver (newton-physics#1645) * Fix root shapes in ArticulationView with fixed base (newton-physics#1639) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> Co-authored-by: Eric Heiden <eric-heiden@outlook.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Use model collision methods and remove `create_collision_pipeline` from examples (newton-physics#1648) Co-authored-by: nvtw <110816143+nvtw@users.noreply.github.com> * Fix XPBD apply_joint_forces ignoring child joint transform (newton-physics#1582) * Adjust SDF API (newton-physics#1644) Co-authored-by: Eric Heiden <eheiden@nvidia.com> * Optimize test suite runtime (~18% faster) (newton-physics#1689) * Remove ensure_nonstatic_links option from importers (newton-physics#1682) Signed-off-by: adenzler-nvidia <adenzler@nvidia.com> * SolverMuJoCo: Add geom_margin support, align thickness default with MuJoCo and schemas (newton-physics#1653) * refactor: privatize non-public solver internals (newton-physics#1683) * Fix option parsing with multiple <option> elements from includes (newton-physics#1692) * Bump warp-lang nightly and newton-usd-schemas (newton-physics#1693) * Get rid of tkinter dependency (newton-physics#1676) * Fix SDF example contact buffer overflow (newton-physics#1695) * Fix implicit biastype for position/velocity actuator shortcuts (newton-physics#1678) * Fix include processor to respect meshdir/texturedir (newton-physics#1685) * Reduce cold-cache Warp compile time for geometry modules (newton-physics#1618) Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: adenzler-nvidia <adenzler@nvidia.com> * Fix xyzw-to-wxyz quaternion conversion in body inertia kernel (newton-physics#1694) * Rename key to label and add hierarchical labels (newton-physics#1592) (newton-physics#1632) * Expose geometry SDF helpers on public API (newton-physics#1684) Signed-off-by: Eric Heiden <eheiden@nvidia.com> * Improve custom frequency handling from USD, parse MuJoCo actuators and tendons (newton-physics#1510) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> Signed-off-by: Eric Heiden <eheiden@nvidia.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Collapse fixed joints with non articulated bodies (newton-physics#1608) * Fix renaming joint_key -> joint_label (newton-physics#1700) Signed-off-by: Eric Heiden <eheiden@nvidia.com> * Bump .python-version from 3.11 to 3.12 (newton-physics#1702) * Replace CITATION.md with CITATION.cff (newton-physics#1706) * Respect MJCF contype=conaffinity=0 via collision_group=0 (newton-physics#1703) * Make ViewerUSD reuse existing USD layers for the same output path (newton-physics#1704) Signed-off-by: Eric Heiden <eheiden@nvidia.com> Signed-off-by: Eric Heiden <eric-heiden@outlook.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Remove dead up-axis conversion from MuJoCo solver (newton-physics#1707) * Skip IK cube stacking example test (newton-physics#1713) * Fix global pairs (world=-1) not exported to MuJoCo spec (newton-physics#1705) * Reduce the memory consumption of hydroelastic contacts (newton-physics#1609) * Fix flakiness cube stacking (newton-physics#1714) * Fix collision shapes not toggleable in viewer UI (newton-physics#1715) * Fix softbody examples table layout in README (newton-physics#1716) * Standardize sensor APIs: label matching, keyword args, and update() method naming (newton-physics#1665) * Fix intermittent crash in parallel test runner from Manager proxy race (newton-physics#1721) * Clean up inertia.py function arguments to match Mesh.create_* API (newton-physics#1719) * Reduce default test runner verbosity (newton-physics#1723) * Add menagerie comparison tests for SolverMuJoCo (newton-physics#1720) Signed-off-by: Alain Denzler <adenzler@nvidia.com> Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> Co-authored-by: Viktor Reutskyy <vreutskyy@nvidia.com> Co-authored-by: Viktor Reutskyy <33062116+vreutskyy@users.noreply.github.com> * Add spatial tendon support for MuJoCo solver (newton-physics#1687) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Expose qfrc_actuator from mujoco (newton-physics#1698) * Skip non-MODEL custom attributes in finalize validation (newton-physics#1734) * Resolve inheritrange for position actuators in MJCF parser (newton-physics#1727) * Support dampratio for position/velocity actuator shortcuts (newton-physics#1722) * Limit concurrency to 1 (newton-physics#1736) * Add a helper method for checking applied usd (newton-physics#1731) * Enhance playback URL handling in ViewerViser (newton-physics#1742) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> * Removed RenderShapeType (newton-physics#1748) * Override only MassAPI attributes that have been authored (newton-physics#1688) * Integration of newton-actuators (newton-physics#1342) Signed-off-by: jvonmuralt <jvonmuralt@nvidia.com> * Fix ArticulationView crash with fixed-joint-only articulations (newton-physics#1726) * Improve retrieval of Jupyter base URL in ViewerViser (newton-physics#1750) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> * Disable dynamics testing for UR5e and Apollo tests to avoid CI flakiness (newton-physics#1755) * Margin and Gap rename (newton-physics#1732) * Vbd Demos Fixing (newton-physics#1740) * Fix ViewerViser.log_lines method (newton-physics#1764) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> * Update warp-lang dependency to 1.12.0rc1 (newton-physics#1763) * Fix fromto capsule/cylinder orientation in MJCF parser (newton-physics#1741) * fix: multi-world particle BVH indexing (newton-physics#1641) Co-authored-by: Eric Heiden <eheiden@nvidia.com> * Clean up unused and internal-only kwargs in SolverMuJoCo (newton-physics#1766) * Parsing of the mimic joint and contact gap/margin from newton schemas (newton-physics#1690) Signed-off-by: Milad Rakhsha <mrakhsha@nvidia.com> * API Refactor v2 (newton-physics#1749) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> * Support explicit geom mass attributes in MJCF (newton-physics#1744) * Bump flask and werkzeug in lockfile for security (newton-physics#1769) Co-authored-by: Cursor <cursoragent@cursor.com> * Split MJCF worldbody root bodies into separate articulations (newton-physics#1754) * Expose VBD rigid contact forces for solver coupling (newton-physics#1745) Signed-off-by: JC <jumyungc@nvidia.com> * Add MJCF ellipsoid geom import and regression test (newton-physics#1772) Co-authored-by: Cursor <cursoragent@cursor.com> * Weld equality constraints parsed from mjcf are given Nan as the default value of torquescale. The correct default should be 1.0 (newton-physics#1760) * Improve picking accuracy and stability (newton-physics#1712) * Franka cloth demo improvement (newton-physics#1765) * Support computing sensing object transforms & API cleanup (newton-physics#1759) * Remove threading workaround (newton-physics#1751) * [Warp Raytrace] Consolidated ray intersect functions, renamed Options to Config (newton-physics#1767) * Improve README example gallery for PyPI compatibility (newton-physics#1776) * Fix issue with mesh in rerun viewer (newton-physics#1768) * Add PhysxMimicJointAPI parsing to USD importer (newton-physics#1735) * Move some math functions to Warp (newton-physics#1717) Signed-off-by: Eric Heiden <eheiden@nvidia.com> Signed-off-by: Eric Heiden <eric-heiden@outlook.com> * Add test to ensure MJCF xform argument is relative (newton-physics#1777) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> * Emit diaginertia instead of fullinertia for diagonal body inertia (newton-physics#1780) * Change default joint armature from 0.01 to 0 (newton-physics#1782) * Fix default kp/kv for position and velocity actuators (newton-physics#1786) * Lock body inertia after explicit MJCF <inertial> element (newton-physics#1784) * Fix for MJCF actuator custom attributes (newton-physics#1783) * Bump version to 0.2.3 Prepare the package metadata for the v0.2.3 release tag. * Fix ViewerRerun rendering of instances from hidden meshes (newton-physics#1788) * API cleanup (newton-physics#1789) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> * BODY actuator target name bypasses body name de-duplication in SolverMuJoCo (newton-physics#1729) * Use default density for visual geoms in MJCF import (newton-physics#1781) * Fix GL viewer crash on Wayland (newton-physics#1793) * Make USD xform parameter control absolute articulation placement (newton-physics#1771) * Fix CUDA context corruption in SDF implementation (newton-physics#1792) * Bump mujoco-warp dependency to 3.5.0.2 (newton-physics#1779) * Fix MuJoCo margin/gap conversion (newton-physics#1785) * Bump version to 1.1.0.dev0 (newton-physics#1798) * Missing unittest.main added back to test_import_mjcf.py. Helps with F5 debugging in VS Code. (newton-physics#1796) * Improve H1 example (newton-physics#1801) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> * Fix ViewerViser.set_camera() (newton-physics#1805) * Rename examples to follow prefix-first naming convention (newton-physics#1802) * Improve MuJoCo actuator domain randomization (newton-physics#1773) * Restore in_cup test in hydro example (newton-physics#1775) * Fix `newton.geometry` imports and change of Mesh maxhullverts global constant * Adapt to having both margin and gap arrays for each geom/shape * Fix for newton.geometry API changes in primitive/narrowphase.py * Fix removal of `BroadPhaseMode` IntEnum and revert to newton-sytle string literals * WIP: Adapt geometry/unified.py to fix breaking changes to `newton.geometry` API * First pass of API adjustments * Patch more gaps (with respect to margin and gap renaming) * GeoType.SDF was removed - reflect that in Kamino * Introduce CoM position offsets w.r.t body frame and operations to convert between body CoM state and generic local body-fixed reference frames. * Add caching of per-entity labels/names/keys in the Model subcontainers * Remove SDF shape wrapper since it's now internal to mesh handling in CD pipelines * Fix USD test assets * Add Newton <--> Kamino joint type conversion operations and per-space default limit constants * Add some cleanup to geometry and unified CD + UTs * Add Newton <--> Kamino shape type conversion operations * Apply new default joint coord limit constants to limits.py * Adapt foubrar model builder and USD asset to produce the same result in sim example * Purge "physical" goems and collapse all into a single group, and purge geometry "layers" * Disable allocation per-joint wrenches by default and make them optional * Make `Model` a dataclass * Separate ModelData* containers into own `core/data.py` module * Fix imports of ModelData * Rename `Model` as `ModelKamino` * Rename `ModelData` as `DataKamino` * Rename `State` as `StateKamino` * Rename `Control` as `ControlKamino` * Rename `Limits` as `LimitsKamino` * Rename `Contacts` as `ContactsKamino` * Rename `ModelBuilder` as `ModelBuilderKamino` * Make imports in test utilities relative * Revise CD meta-data attributes and their computation in GeometryModel and ModelBuilderKamino * Revise primitive CD pipeline * Revise unified CD pipeline * Revise CD front-end interfaces * Fix UTs and relevant utils for interface changes to CD * Change to `wp.DeviceLike` to account for upcoming deprecation of `Devicelike` * Depracate legacy HDF5 data io (will be replaced in the future) * Fix banned imports at module level * Modify USD importer to detect articulations and order geoms and joints similarly to how the Newton `parse_usd` function does. * Add conversion operation from `newton.Model` to `ModelKamino` * Add data, state and control container conversions * Add SolverKamino wrapper that fullfils newton integration interface * Add newton integration examples * Add SolverKamino to newton solver module imports * WIP: fix problem with lambda_j being allocated for only kinematic constraints and failing on array copying * Fix banned git import in benchmark * Rename *Settings to *Config (#213) * Rename SolverKaminoSettings -> SolverKaminoConfig * Rename DualProblemSettings -> DualProblemConfig * Rename CollisionDetectorSettings -> CollisionDetectorConfig * Rename PADMMSettings -> PADMMConfig * Rename ForwardKinematicsSolverSettings -> ForwardKinematicsSolverConfig * Rename SimulatorSettings -> SimulatorConfig * Add check for model compatibility in SolverKamino (#209) * Fix device assignment in sparse CG test on CPU (#216) * Replace Enum-type config attributes with Literal (#215) * Replace warmstart mode config param with literal * Replace contact warmstart mode config param with literal * Replace rotation correction config param with literal * Replace penalty update config param with literal * Replace FK preconditioner option config param with literal * Add post-init checks for dual/PADMM configs * Rename FKPreconditionerOptions to FKPreconditionerType * Remove WorldDescriptor from ModelKamino (#219) * Add geom index offset array to model info * Replace access to world description in model * Remove world descriptor from model * Fix computation of kinematics residual with sparse Jacobian (#220) * Migrates `ModelKaminoSize` to it's own module to avoid circular dependency between core/model.py and core/state.py and rename it as `SizeKamino`. * WIP: Fix SolverKamino wrapper * Fix circular dependency in conversions.py * Fix some broken unit tests and WIP to fix fourbar contact conversions and rendering * Make some conversions @staticmethods instead, because they dont need common class attributes * Fix declaration of custom state attributes and their conversion to/from newton and kamino * WIP: Debug model conversion and newton sim examples * Rename and cleanup start index array of per-world geoms * Model conversion and newton sim examples now work. * Make gravity conversion operation re-usable * Migrates boxes_fourbar builder using newton.ModelBuilder to it's own module to prepare for migration. * Use gravity conversion utility func in SolverKamino * Add reusable joint-parameterization conversion utility * Remove world-descriptor from model converter * Rename helper converter that handles entity-local transforms * Add some cleanup to DR Legs, ANYmal D and basic four-bar examples * Fix module-level imports of additional kamino-specific development dependencies * Fix module-level imports of additional kamino-specific development dependencies * Fix erroneous merge conflict. --------- Signed-off-by: Alain Denzler <adenzler@nvidia.com> Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> Signed-off-by: JC <jumyungc@nvidia.com> Signed-off-by: Milad Rakhsha <mrakhsha@nvidia.com> Signed-off-by: Milad-Rakhsha-NV <167464435+Milad-Rakhsha-NV@users.noreply.github.com> Signed-off-by: Eric Heiden <eric-heiden@outlook.com> Signed-off-by: Eric Heiden <eheiden@nvidia.com> Signed-off-by: Miles Macklin <mmacklin@nvidia.com> Signed-off-by: adenzler-nvidia <adenzler@nvidia.com> Signed-off-by: jvonmuralt <jvonmuralt@nvidia.com> Co-authored-by: Daniela Hase <116915287+daniela-hase@users.noreply.github.com> Co-authored-by: adenzler-nvidia <adenzler@nvidia.com> Co-authored-by: Viktor Reutskyy <33062116+vreutskyy@users.noreply.github.com> Co-authored-by: Eric Shi <97630937+shi-eric@users.noreply.github.com> Co-authored-by: Anka Chen <AnkaChan@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: JC-nvidia <116605903+jumyungc@users.noreply.github.com> Co-authored-by: Kenny Vilella <kvilella@nvidia.com> Co-authored-by: nvtw <110816143+nvtw@users.noreply.github.com> Co-authored-by: Milad-Rakhsha-NV <167464435+Milad-Rakhsha-NV@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Lennart Röstel <65088822+lenroe@users.noreply.github.com> Co-authored-by: Eric Heiden <eheiden@nvidia.com> Co-authored-by: jvonmuralt <jvonmuralt@nvidia.com> Co-authored-by: camevor <camevor@nvidia.com> Co-authored-by: mzamoramora-nvidia <mzamoramora@nvidia.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Alessandro Roncone <alecive87@gmail.com> Co-authored-by: gdaviet <57617656+gdaviet@users.noreply.github.com> Co-authored-by: Miles Macklin <mmacklin@nvidia.com> Co-authored-by: Gordon Yeoman <gyeomannvidia@users.noreply.github.com> Co-authored-by: Lukasz Wawrzyniak <lwawrzyniak@nvidia.com> Co-authored-by: Eric Heiden <eric-heiden@outlook.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Viktor Reutskyy <vreutskyy@nvidia.com> Co-authored-by: Lorenzo Terenzi <lorenzoterenzi96@gmail.com> Co-authored-by: smollerNV <164020096+smollerNV@users.noreply.github.com> Co-authored-by: twidmer <twidmer@nvidia.com> Co-authored-by: Christian Schumacher <christian.schumacher@disney.com> Co-authored-by: Guirec-Maloisel <25688871+Guirec-Maloisel@users.noreply.github.com>
vastsoun
added a commit
to vastsoun/newton
that referenced
this pull request
Mar 4, 2026
* [Warp Raytrace] Added device parameter (newton-physics#1544) * [Warp Raytrace] Added device parameter to previously overlooked call (newton-physics#1545) * SolverMuJoCo: Fix tolerance clamping in update_solver_options_kernel (newton-physics#1546) * Change default shape_ke to align with MuJoCo, parse geom solref from MJCF for contact stiffness/damping (newton-physics#1491) Signed-off-by: Alain Denzler <adenzler@nvidia.com> * Fix log_shapes broadcasting for length-1 warp arrays (newton-physics#1550) * Fix XPBD restitution particle index (newton-physics#1557) * Out-of-Bound memory read in example_diffsim_bear newton-physics#1386 (newton-physics#1533) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Add versioned documentation deployment to GitHub Pages (newton-physics#1560) * Fix broken documentation links after versioned docs deployment (newton-physics#1566) * VBD New Features (newton-physics#1479) Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> * Add banners to membership verification workflow steps (newton-physics#1569) * Support cable junctions (newton-physics#1519) Signed-off-by: JC <jumyungc@nvidia.com> * Rename parameter I to inertia newton-physics#1543 (newton-physics#1551) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Fix example_robot_anymal_c_walk.py (newton-physics#1574) * Change everywhere linesearch to iterative (newton-physics#1573) * Remove standard collision pipeline (newton-physics#1538) * USD Plumbing MJC solver attributes through resolver and custom attribute framework (newton-physics#1463) Signed-off-by: Milad Rakhsha <mrakhsha@nvidia.com> Signed-off-by: Milad-Rakhsha-NV <167464435+Milad-Rakhsha-NV@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: adenzler-nvidia <adenzler@nvidia.com> * Fix child shape filtering (newton-physics#1559) * Fix ViewerRerun ignoring hidden parameter in log_mesh and log_instances (newton-physics#1555) * Make NxN and SAP broad phase respect filtered pairs (newton-physics#1554) * Add --quiet flag to examples to suppress Warp messages (newton-physics#1585) * Defer resolution of MESH_MAXHULLVERT default in importers (newton-physics#1587) * Fix TypeError when finalizing SDF geometry with device kwarg (newton-physics#1586) * Make MESH_MAXHULLVERT a static class attribute Mesh.MAX_HULL_VERTICES. (newton-physics#1598) * Significant non-determinism in unified collision pipeline for anymal_c_walking example newton-physics#1505 (newton-physics#1588) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Add test for non-contiguous case (newton-physics#1549) * Fix nightly Warp CI to resolve pre-release builds (newton-physics#1606) * Verify default class and value handling (newton-physics#1556) * SolverMuJoCo: Expand geom_margin to avoid OOB read with heterogeneous worlds (newton-physics#1607) * Fix bug in control clear method (newton-physics#1602) * Enable Use of Newton IK in Lab newton-physics#662 (newton-physics#1539) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Fix import of non-articulated joints (newton-physics#1535) * Deduplicate _process_joint_custom_attributes frequency handling (newton-physics#1584) * Add CI check for stale API docs and fix local build warnings (newton-physics#1570) * Update Pillow 12.0.0 to 12.1.1 (newton-physics#1612) * Prepare handling of mimic constraints in Newton (newton-physics#1523) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> Signed-off-by: Eric Heiden <eheiden@nvidia.com> * Support floating, base_joint and parent_body arguments for importers (newton-physics#1498) * Fix contact buffer memory overestimation (newton-physics#1614) * Configure banned-module-level-imports for ruff (newton-physics#1583) * Explicit `Contacts` instantiation with `Model.contacts()` and `CollisionPipeline.contacts()` (newton-physics#1445) * Fix the quadruped benchmark regression (newton-physics#1615) * Change default ignore_inertial_definitions from True to False (newton-physics#1537) * Finalize the Recording API (newton-physics#1600) * SolverMuJoCo: Fix ccd_iterations default (newton-physics#1631) * update gitignore to ignore Claude Code sandbox files (newton-physics#1628) * Add mimic joint support to SolverMuJoCo (newton-physics#1627) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Add --no-cache-clear flag to test runner (newton-physics#1629) * Update MuJoCo and MuJoCo Warp to 3.5.0 release (newton-physics#1633) * Improve inertia parsing from USD (newton-physics#1605) Signed-off-by: Eric Heiden <eheiden@nvidia.com> Signed-off-by: Eric Heiden <eric-heiden@outlook.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Remove standalone .typos.toml in favor of pyproject.toml config (newton-physics#1642) * Heightfield support newton-physics#1189 (newton-physics#1547) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Example_robot_policy: Replace ValueError with clean error for missing PhysX policy (newton-physics#1636) Co-authored-by: Viktor Reutskyy <33062116+vreutskyy@users.noreply.github.com> * Avoid unnecessary inflation of the contact reduction voxel aabb (newton-physics#1650) * Rename num_worlds to world_count (newton-physics#1634) Signed-off-by: Eric Heiden <eheiden@nvidia.com> * Support parsing autolimits from MJCF (newton-physics#1651) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Fix particle-shape restitution ignoring body velocity (newton-physics#1273) (newton-physics#1580) * Add overflow warnings for narrow-phase collision buffers (newton-physics#1643) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Documentation: add units to model/state docstrings (newton-physics#1649) * fix(viewer): add missing JointType.BALL support to contact line kernel (newton-physics#1640) * Make terrain mesh visual-only in anymal C walking example (newton-physics#1660) Signed-off-by: Eric Heiden <eheiden@nvidia.com> * Fix initialization of collider state in MPM finite difference mode (newton-physics#1652) * docs: document ModelBuilder.default_shape_cfg (newton-physics#1662) * Finalize the collision API (newton-physics#1581) * Remove hardcoded subnet ID from AWS workflow (newton-physics#1664) Signed-off-by: Eric Heiden <eheiden@nvidia.com> * Attempt to fix AWS config (newton-physics#1666) Signed-off-by: Eric Heiden <eheiden@nvidia.com> * Update AWS workflows to g7e.2xlarge with multi-AZ failover (newton-physics#1669) * fix(viewer-usd): disambiguate log_points colors for N=3 warp arrays (newton-physics#1661) * Viewer gl optimizations (newton-physics#1656) Signed-off-by: Miles Macklin <mmacklin@nvidia.com> Signed-off-by: Eric Heiden <eheiden@nvidia.com> Co-authored-by: Eric Heiden <eheiden@nvidia.com> * docs: add articulation workflow guidance and regression check (newton-physics#1663) * fix(examples): propagate IK solution to model state in Franka example (newton-physics#1637) * fix(deps,docs): bump nbconvert to 7.17.0 and fix ArticulationView doctest (newton-physics#1670) * Cleanup and improve some example (newton-physics#1625) Signed-off-by: adenzler-nvidia <adenzler@nvidia.com> Co-authored-by: adenzler-nvidia <adenzler@nvidia.com> * Handle zero-mass bodies and flip ensure_nonstatic_links default (newton-physics#1635) * Additional testing for ArticulationView (newton-physics#1527) Co-authored-by: Eric Heiden <eheiden@nvidia.com> * Update warp-lang nightly to 1.12.0.dev20260217 (newton-physics#1677) * Change default friction coefficients to match MuJoCo (newton-physics#1681) * Refactor mesh creation functions (newton-physics#1654) Signed-off-by: Eric Heiden <eheiden@nvidia.com> * Parse joint frictionloss from MJCF (newton-physics#1680) * Fix free joint body_pos and add ref/qpos0 support for MuJoCo solver (newton-physics#1645) * Fix root shapes in ArticulationView with fixed base (newton-physics#1639) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> Co-authored-by: Eric Heiden <eric-heiden@outlook.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Use model collision methods and remove `create_collision_pipeline` from examples (newton-physics#1648) Co-authored-by: nvtw <110816143+nvtw@users.noreply.github.com> * Fix XPBD apply_joint_forces ignoring child joint transform (newton-physics#1582) * Adjust SDF API (newton-physics#1644) Co-authored-by: Eric Heiden <eheiden@nvidia.com> * Optimize test suite runtime (~18% faster) (newton-physics#1689) * Remove ensure_nonstatic_links option from importers (newton-physics#1682) Signed-off-by: adenzler-nvidia <adenzler@nvidia.com> * SolverMuJoCo: Add geom_margin support, align thickness default with MuJoCo and schemas (newton-physics#1653) * refactor: privatize non-public solver internals (newton-physics#1683) * Fix option parsing with multiple <option> elements from includes (newton-physics#1692) * Bump warp-lang nightly and newton-usd-schemas (newton-physics#1693) * Get rid of tkinter dependency (newton-physics#1676) * Fix SDF example contact buffer overflow (newton-physics#1695) * Fix implicit biastype for position/velocity actuator shortcuts (newton-physics#1678) * Fix include processor to respect meshdir/texturedir (newton-physics#1685) * Reduce cold-cache Warp compile time for geometry modules (newton-physics#1618) Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: adenzler-nvidia <adenzler@nvidia.com> * Fix xyzw-to-wxyz quaternion conversion in body inertia kernel (newton-physics#1694) * Rename key to label and add hierarchical labels (newton-physics#1592) (newton-physics#1632) * Expose geometry SDF helpers on public API (newton-physics#1684) Signed-off-by: Eric Heiden <eheiden@nvidia.com> * Improve custom frequency handling from USD, parse MuJoCo actuators and tendons (newton-physics#1510) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> Signed-off-by: Eric Heiden <eheiden@nvidia.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Collapse fixed joints with non articulated bodies (newton-physics#1608) * Fix renaming joint_key -> joint_label (newton-physics#1700) Signed-off-by: Eric Heiden <eheiden@nvidia.com> * Bump .python-version from 3.11 to 3.12 (newton-physics#1702) * Replace CITATION.md with CITATION.cff (newton-physics#1706) * Respect MJCF contype=conaffinity=0 via collision_group=0 (newton-physics#1703) * Make ViewerUSD reuse existing USD layers for the same output path (newton-physics#1704) Signed-off-by: Eric Heiden <eheiden@nvidia.com> Signed-off-by: Eric Heiden <eric-heiden@outlook.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Remove dead up-axis conversion from MuJoCo solver (newton-physics#1707) * Skip IK cube stacking example test (newton-physics#1713) * Fix global pairs (world=-1) not exported to MuJoCo spec (newton-physics#1705) * Reduce the memory consumption of hydroelastic contacts (newton-physics#1609) * Fix flakiness cube stacking (newton-physics#1714) * Fix collision shapes not toggleable in viewer UI (newton-physics#1715) * Fix softbody examples table layout in README (newton-physics#1716) * Standardize sensor APIs: label matching, keyword args, and update() method naming (newton-physics#1665) * Fix intermittent crash in parallel test runner from Manager proxy race (newton-physics#1721) * Clean up inertia.py function arguments to match Mesh.create_* API (newton-physics#1719) * Reduce default test runner verbosity (newton-physics#1723) * Add menagerie comparison tests for SolverMuJoCo (newton-physics#1720) Signed-off-by: Alain Denzler <adenzler@nvidia.com> Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> Co-authored-by: Viktor Reutskyy <vreutskyy@nvidia.com> Co-authored-by: Viktor Reutskyy <33062116+vreutskyy@users.noreply.github.com> * Add spatial tendon support for MuJoCo solver (newton-physics#1687) Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> * Expose qfrc_actuator from mujoco (newton-physics#1698) * Skip non-MODEL custom attributes in finalize validation (newton-physics#1734) * Resolve inheritrange for position actuators in MJCF parser (newton-physics#1727) * Support dampratio for position/velocity actuator shortcuts (newton-physics#1722) * Limit concurrency to 1 (newton-physics#1736) * Add a helper method for checking applied usd (newton-physics#1731) * Enhance playback URL handling in ViewerViser (newton-physics#1742) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> * Removed RenderShapeType (newton-physics#1748) * Override only MassAPI attributes that have been authored (newton-physics#1688) * Integration of newton-actuators (newton-physics#1342) Signed-off-by: jvonmuralt <jvonmuralt@nvidia.com> * Fix ArticulationView crash with fixed-joint-only articulations (newton-physics#1726) * Improve retrieval of Jupyter base URL in ViewerViser (newton-physics#1750) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> * Disable dynamics testing for UR5e and Apollo tests to avoid CI flakiness (newton-physics#1755) * Margin and Gap rename (newton-physics#1732) * Vbd Demos Fixing (newton-physics#1740) * Fix ViewerViser.log_lines method (newton-physics#1764) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> * Update warp-lang dependency to 1.12.0rc1 (newton-physics#1763) * Fix fromto capsule/cylinder orientation in MJCF parser (newton-physics#1741) * fix: multi-world particle BVH indexing (newton-physics#1641) Co-authored-by: Eric Heiden <eheiden@nvidia.com> * Clean up unused and internal-only kwargs in SolverMuJoCo (newton-physics#1766) * Parsing of the mimic joint and contact gap/margin from newton schemas (newton-physics#1690) Signed-off-by: Milad Rakhsha <mrakhsha@nvidia.com> * API Refactor v2 (newton-physics#1749) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> * Support explicit geom mass attributes in MJCF (newton-physics#1744) * Bump flask and werkzeug in lockfile for security (newton-physics#1769) Co-authored-by: Cursor <cursoragent@cursor.com> * Split MJCF worldbody root bodies into separate articulations (newton-physics#1754) * Expose VBD rigid contact forces for solver coupling (newton-physics#1745) Signed-off-by: JC <jumyungc@nvidia.com> * Add MJCF ellipsoid geom import and regression test (newton-physics#1772) Co-authored-by: Cursor <cursoragent@cursor.com> * Weld equality constraints parsed from mjcf are given Nan as the default value of torquescale. The correct default should be 1.0 (newton-physics#1760) * Improve picking accuracy and stability (newton-physics#1712) * Franka cloth demo improvement (newton-physics#1765) * Support computing sensing object transforms & API cleanup (newton-physics#1759) * Remove threading workaround (newton-physics#1751) * [Warp Raytrace] Consolidated ray intersect functions, renamed Options to Config (newton-physics#1767) * Improve README example gallery for PyPI compatibility (newton-physics#1776) * Fix issue with mesh in rerun viewer (newton-physics#1768) * Add PhysxMimicJointAPI parsing to USD importer (newton-physics#1735) * Move some math functions to Warp (newton-physics#1717) Signed-off-by: Eric Heiden <eheiden@nvidia.com> Signed-off-by: Eric Heiden <eric-heiden@outlook.com> * Add test to ensure MJCF xform argument is relative (newton-physics#1777) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> * Emit diaginertia instead of fullinertia for diagonal body inertia (newton-physics#1780) * Change default joint armature from 0.01 to 0 (newton-physics#1782) * Fix default kp/kv for position and velocity actuators (newton-physics#1786) * Lock body inertia after explicit MJCF <inertial> element (newton-physics#1784) * Fix for MJCF actuator custom attributes (newton-physics#1783) * Bump version to 0.2.3 Prepare the package metadata for the v0.2.3 release tag. * Fix ViewerRerun rendering of instances from hidden meshes (newton-physics#1788) * API cleanup (newton-physics#1789) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> * BODY actuator target name bypasses body name de-duplication in SolverMuJoCo (newton-physics#1729) * Use default density for visual geoms in MJCF import (newton-physics#1781) * Fix GL viewer crash on Wayland (newton-physics#1793) * Make USD xform parameter control absolute articulation placement (newton-physics#1771) * Fix CUDA context corruption in SDF implementation (newton-physics#1792) * Bump mujoco-warp dependency to 3.5.0.2 (newton-physics#1779) * Fix MuJoCo margin/gap conversion (newton-physics#1785) * Bump version to 1.1.0.dev0 (newton-physics#1798) * Missing unittest.main added back to test_import_mjcf.py. Helps with F5 debugging in VS Code. (newton-physics#1796) * Improve H1 example (newton-physics#1801) Signed-off-by: Eric Heiden <eric-heiden@outlook.com> * Fix ViewerViser.set_camera() (newton-physics#1805) * Rename examples to follow prefix-first naming convention (newton-physics#1802) * Improve MuJoCo actuator domain randomization (newton-physics#1773) * Restore in_cup test in hydro example (newton-physics#1775) * Fix `newton.geometry` imports and change of Mesh maxhullverts global constant * Adapt to having both margin and gap arrays for each geom/shape * Fix for newton.geometry API changes in primitive/narrowphase.py * Fix removal of `BroadPhaseMode` IntEnum and revert to newton-sytle string literals * WIP: Adapt geometry/unified.py to fix breaking changes to `newton.geometry` API * First pass of API adjustments * Patch more gaps (with respect to margin and gap renaming) * GeoType.SDF was removed - reflect that in Kamino * Introduce CoM position offsets w.r.t body frame and operations to convert between body CoM state and generic local body-fixed reference frames. * Add caching of per-entity labels/names/keys in the Model subcontainers * Remove SDF shape wrapper since it's now internal to mesh handling in CD pipelines * Fix USD test assets * Add Newton <--> Kamino joint type conversion operations and per-space default limit constants * Add some cleanup to geometry and unified CD + UTs * Add Newton <--> Kamino shape type conversion operations * Apply new default joint coord limit constants to limits.py * Adapt foubrar model builder and USD asset to produce the same result in sim example * Purge "physical" goems and collapse all into a single group, and purge geometry "layers" * Disable allocation per-joint wrenches by default and make them optional * Make `Model` a dataclass * Separate ModelData* containers into own `core/data.py` module * Fix imports of ModelData * Rename `Model` as `ModelKamino` * Rename `ModelData` as `DataKamino` * Rename `State` as `StateKamino` * Rename `Control` as `ControlKamino` * Rename `Limits` as `LimitsKamino` * Rename `Contacts` as `ContactsKamino` * Rename `ModelBuilder` as `ModelBuilderKamino` * Make imports in test utilities relative * Revise CD meta-data attributes and their computation in GeometryModel and ModelBuilderKamino * Revise primitive CD pipeline * Revise unified CD pipeline * Revise CD front-end interfaces * Fix UTs and relevant utils for interface changes to CD * Change to `wp.DeviceLike` to account for upcoming deprecation of `Devicelike` * Depracate legacy HDF5 data io (will be replaced in the future) * Fix banned imports at module level * Modify USD importer to detect articulations and order geoms and joints similarly to how the Newton `parse_usd` function does. * Add conversion operation from `newton.Model` to `ModelKamino` * Add data, state and control container conversions * Add SolverKamino wrapper that fullfils newton integration interface * Add newton integration examples * Add SolverKamino to newton solver module imports * WIP: fix problem with lambda_j being allocated for only kinematic constraints and failing on array copying * Fix banned git import in benchmark * Rename *Settings to *Config (#213) * Rename SolverKaminoSettings -> SolverKaminoConfig * Rename DualProblemSettings -> DualProblemConfig * Rename CollisionDetectorSettings -> CollisionDetectorConfig * Rename PADMMSettings -> PADMMConfig * Rename ForwardKinematicsSolverSettings -> ForwardKinematicsSolverConfig * Rename SimulatorSettings -> SimulatorConfig * Add check for model compatibility in SolverKamino (#209) * Fix device assignment in sparse CG test on CPU (#216) * Replace Enum-type config attributes with Literal (#215) * Replace warmstart mode config param with literal * Replace contact warmstart mode config param with literal * Replace rotation correction config param with literal * Replace penalty update config param with literal * Replace FK preconditioner option config param with literal * Add post-init checks for dual/PADMM configs * Rename FKPreconditionerOptions to FKPreconditionerType * Remove WorldDescriptor from ModelKamino (#219) * Add geom index offset array to model info * Replace access to world description in model * Remove world descriptor from model * Fix computation of kinematics residual with sparse Jacobian (#220) * Migrates `ModelKaminoSize` to it's own module to avoid circular dependency between core/model.py and core/state.py and rename it as `SizeKamino`. * WIP: Fix SolverKamino wrapper * Fix circular dependency in conversions.py * Fix some broken unit tests and WIP to fix fourbar contact conversions and rendering * Make some conversions @staticmethods instead, because they dont need common class attributes * Fix declaration of custom state attributes and their conversion to/from newton and kamino * WIP: Debug model conversion and newton sim examples * Rename and cleanup start index array of per-world geoms * Model conversion and newton sim examples now work. * Make gravity conversion operation re-usable * Migrates boxes_fourbar builder using newton.ModelBuilder to it's own module to prepare for migration. * Use gravity conversion utility func in SolverKamino * Add reusable joint-parameterization conversion utility * Remove world-descriptor from model converter * Rename helper converter that handles entity-local transforms * Add some cleanup to DR Legs, ANYmal D and basic four-bar examples * Fix module-level imports of additional kamino-specific development dependencies * Fix module-level imports of additional kamino-specific development dependencies * Fix erroneous merge conflict. --------- Signed-off-by: Alain Denzler <adenzler@nvidia.com> Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> Signed-off-by: JC <jumyungc@nvidia.com> Signed-off-by: Milad Rakhsha <mrakhsha@nvidia.com> Signed-off-by: Milad-Rakhsha-NV <167464435+Milad-Rakhsha-NV@users.noreply.github.com> Signed-off-by: Eric Heiden <eric-heiden@outlook.com> Signed-off-by: Eric Heiden <eheiden@nvidia.com> Signed-off-by: Miles Macklin <mmacklin@nvidia.com> Signed-off-by: adenzler-nvidia <adenzler@nvidia.com> Signed-off-by: jvonmuralt <jvonmuralt@nvidia.com> Co-authored-by: Daniela Hase <116915287+daniela-hase@users.noreply.github.com> Co-authored-by: adenzler-nvidia <adenzler@nvidia.com> Co-authored-by: Viktor Reutskyy <33062116+vreutskyy@users.noreply.github.com> Co-authored-by: Eric Shi <97630937+shi-eric@users.noreply.github.com> Co-authored-by: Anka Chen <AnkaChan@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: JC-nvidia <116605903+jumyungc@users.noreply.github.com> Co-authored-by: Kenny Vilella <kvilella@nvidia.com> Co-authored-by: nvtw <110816143+nvtw@users.noreply.github.com> Co-authored-by: Milad-Rakhsha-NV <167464435+Milad-Rakhsha-NV@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Lennart Röstel <65088822+lenroe@users.noreply.github.com> Co-authored-by: Eric Heiden <eheiden@nvidia.com> Co-authored-by: jvonmuralt <jvonmuralt@nvidia.com> Co-authored-by: camevor <camevor@nvidia.com> Co-authored-by: mzamoramora-nvidia <mzamoramora@nvidia.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Alessandro Roncone <alecive87@gmail.com> Co-authored-by: gdaviet <57617656+gdaviet@users.noreply.github.com> Co-authored-by: Miles Macklin <mmacklin@nvidia.com> Co-authored-by: Gordon Yeoman <gyeomannvidia@users.noreply.github.com> Co-authored-by: Lukasz Wawrzyniak <lwawrzyniak@nvidia.com> Co-authored-by: Eric Heiden <eric-heiden@outlook.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Viktor Reutskyy <vreutskyy@nvidia.com> Co-authored-by: Lorenzo Terenzi <lorenzoterenzi96@gmail.com> Co-authored-by: smollerNV <164020096+smollerNV@users.noreply.github.com> Co-authored-by: twidmer <twidmer@nvidia.com> Co-authored-by: Christian Schumacher <christian.schumacher@disney.com> Co-authored-by: Guirec-Maloisel <25688871+Guirec-Maloisel@users.noreply.github.com>
vidurv-nvidia
pushed a commit
to vidurv-nvidia/newton
that referenced
this pull request
Mar 6, 2026
# Description Currently, the actions from the policy are directly applied to the environment and also often fed back to the policy using the last action as observation. Doing this can lead to instability during training since applying a large action can introduce a negative feedback loop. More specifically, applying a very large action leads to a large last_action observations, which often results in a large error in the critic, which can lead to even larger actions being sampled in the future. This PR aims to fix this for RSL-RL library, by clipping the actions to (large) hard limits before applying them to the environment. This prohibits the actions from growing continuously and greatly improves training stability. Fixes newton-physics#984, newton-physics#1732, newton-physics#1999 ## Type of change - Bug fix (non-breaking change which fixes an issue) - New feature (non-breaking change which adds functionality) ## Checklist - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [x] I have updated the changelog and the corresponding version in the extension's `config/extension.toml` file - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there
3 tasks
mmacklin
pushed a commit
to mmacklin/newton
that referenced
this pull request
Apr 7, 2026
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Margin and Gap rename - so far only for rigid bodies (soft bodies and particles keep to use the old terms)
The MR should close #1655
Newton Migration Guide
Please ensure the migration guide for warp.sim users is up-to-date with the changes made in this PR.
docs/migration.rstis up-to dateBefore your PR is "Ready for review"
newton/tests/test_examples.py)pre-commit run -aSummary by CodeRabbit
Breaking Changes
Documentation
Tests & Examples