Fix material combination inconsistencies by nvtw · Pull Request #2439 · newton-physics/newton

nvtw · 2026-04-14T10:53:56Z

Description

MjWarp currently uses a different material combination logic. This MR applies the same logic into the Newton-to-MjWarp contact converter. See #2422 and #2377

Checklist

New or existing tests cover these changes
The documentation is up to date with these changes
CHANGELOG.md has been updated (if user-facing change)

Test plan

Bug fix

Steps to reproduce:

Minimal reproduction:

import newton

# Code that demonstrates the bug

Summary by CodeRabbit

Bug Fixes
- Contact friction and parameter resolution in the MuJoCo solver now respect geometry priority ordering, correctly selecting higher-priority geometry values and applying component-wise clamping and stable mixing behavior for equal priorities.
Tests
- Added tests validating friction selection when priorities differ and element-wise mixing when priorities are equal.

coderabbitai · 2026-04-14T10:54:10Z

📝 Walkthrough

Walkthrough

Reworked MuJoCo solver contact parameter resolution to select mix, condim, and per-axis friction using explicit geom_priority ordering (values taken from the higher-priority geom when priorities differ; mixed when equal). Added tests validating friction priority and equal-priority mixing.

Changes

Cohort / File(s)	Summary
Kernel Contact Parameter Resolution `newton/_src/solvers/mujoco/kernels.py`	Added `safe_div` and `MJ_MINMU`; restructured `contact_params()` to resolve `mix`, `condim`, and per-axis friction by `geom_priority` (select from higher-priority geom when unequal; mix/element-wise max when equal). Moved `margin`/`gap` summation after priority/mix resolution and clamped friction per-component using `wp.max(MJ_MINMU, ...)`.
Friction Priority Tests `newton/tests/test_mujoco_solver.py`	Added `TestFrictionPriority` with helpers and tests that build two-geom scenarios using `mujoco:geom_priority` and per-geom `mu`; asserts that higher-priority geom supplies per-contact slide friction when priorities differ, and equal priorities use element-wise `max`.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

adenzler-nvidia

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Fix material combination inconsistencies' accurately summarizes the main objective of the changeset, which aligns Newton-to-MjWarp contact converter with MjWarp's material combination logic by fixing how friction, mix, and condim are resolved.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

🤖 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/solvers/mujoco/kernels.py`:
- Around line 145-159: The mix calculation and friction expansion must mirror
MJWarp: replace the raw division solmix1 / (solmix1 + solmix2) with
safe_div(solmix1, solmix1 + solmix2) from newton.math and keep the same wp.where
guards for near-zero solmix; additionally clamp each component of
resolved_friction (obtained from geom_friction[worldid, g1/g2] and later
expanded into vec5) to at least MJ_MINMU (1e-5) before constructing the vec5 so
all five friction entries use max(value, MJ_MINMU) to ensure parity with
mujoco_warp._src.collision_core.contact_params.

In `@newton/tests/test_mujoco_solver.py`:
- Around line 4029-4058: The two tests (test_higher_priority_friction_used and
test_equal_priority_uses_max) only cover one placement of the higher/larger mu
(always on body_a or body_b respectively), allowing order-dependent regressions
to slip; update each test to also run the swapped permutation (i.e., build the
model with the same priority/friction values but with the geom mu values and/or
body assignments swapped) or parameterize the test to iterate both placements,
using the same helpers (_build_model and _get_contact_friction) and asserting
the same expected friction (higher-priority mu wins; when priorities equal the
element-wise max) for both permutations so order-independence is actually
validated.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 959bfd9c-0634-41e6-9faa-d684b74462fd

📥 Commits

Reviewing files that changed from the base of the PR and between ffeb438 and 215f8fb.

📒 Files selected for processing (2)

newton/_src/solvers/mujoco/kernels.py
newton/tests/test_mujoco_solver.py

codecov · 2026-04-14T11:09:57Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

coderabbitai

Actionable comments posted: 1

🤖 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/solvers/mujoco/kernels.py`:
- Around line 34-36: Replace the local MJ_MINVAL usage with MuJoCo's mjMINVAL so
small denominators and the solmix near-zero branch use the same 1e-15 threshold
as upstream and the rest of the codebase: import or reference mujoco.mjMINVAL
and use that in safe_div and in the other places in this file that currently
rely on the module MJ_MINVAL (the occurrences around lines with the solmix
handling), rather than the hard-coded 2.220446049250313e-16 so behavior matches
MuJoCo and Newton constants.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: d868af28-f08c-4c31-9bce-6c714995ebcb

📥 Commits

Reviewing files that changed from the base of the PR and between 215f8fb and b69081b.

📒 Files selected for processing (2)

newton/_src/solvers/mujoco/kernels.py
newton/tests/test_mujoco_solver.py

🚧 Files skipped from review as they are similar to previous changes (1)

newton/tests/test_mujoco_solver.py

…crepancy

adenzler-nvidia

Looks good — the priority/solmix/friction logic now matches MjWarp's collision_core.contact_params and the tests cover both the unequal-priority and equal-priority paths well.

Minor suggestions (non-blocking):

Consider importing MJ_MINMU from mujoco_warp._src.types instead of hardcoding 1e-5, so it stays in sync if MuJoCo ever changes the constant.
Same for safe_div — could import from mujoco_warp._src.math to avoid the local copy, though it's trivial enough that either way is fine.

adenzler-nvidia

Looks good — the priority/solmix/friction logic now matches MjWarp's collision_core.contact_params and the tests cover both the unequal-priority and equal-priority paths well.

Longer-term, it might be worth factoring out the shared geom-level mixing logic into a common @wp.func upstream in MjWarp so both codebases stay in sync by construction, but that's outside the scope of this PR.

The pre-release audit found user-facing commits that landed since v1.1.0 without an [Unreleased] entry. Backfill five of them here. Kamino-related commits (newton-physics#2280, newton-physics#2517, newton-physics#2554, newton-physics#2575) are intentionally omitted: Kamino is still alpha and not currently tracked in CHANGELOG. - Changed: contact-conversion overhead reduction in SolverMuJoCo ([newton-physicsGH-2393](newton-physics#2393)) - Fixed: target_voxel_size ignored on the texture-SDF path ([newton-physicsGH-2464](newton-physics#2464)) - Fixed: Newton-to-mujoco-warp material-combination mismatch ([newton-physicsGH-2439](newton-physics#2439)) - Fixed: eq_objtype mismatch on joint equality / mimic constraints in SolverMuJoCo ([newton-physicsGH-2459](newton-physics#2459)) - Fixed: _tiled_sum_kernel launch-dim handling under warp-lang 1.13 ([newton-physicsGH-2546](newton-physics#2546))

Fix material combination inconsistencies

215f8fb

nvtw self-assigned this Apr 14, 2026

nvtw requested a review from adenzler-nvidia April 14, 2026 11:00

coderabbitai Bot reviewed Apr 14, 2026

View reviewed changes

Comment thread newton/_src/solvers/mujoco/kernels.py Outdated

Comment thread newton/tests/test_mujoco_solver.py Outdated

Implement CodeRabbit comments

b69081b

coderabbitai Bot reviewed Apr 14, 2026

View reviewed changes

Comment thread newton/_src/solvers/mujoco/kernels.py

Merge branch 'main' into dev/tw3/address_newton_vs_mjwarp_contact_dis…

3d45d69

…crepancy

adenzler-nvidia approved these changes Apr 15, 2026

View reviewed changes

nvtw added this pull request to the merge queue Apr 15, 2026

Merged via the queue into newton-physics:main with commit aab3d33 Apr 15, 2026
25 checks passed

coderabbitai Bot mentioned this pull request Apr 30, 2026

MuJoCo Contact Scaling for Articulated Geoms #2656

Closed

preist-nvidia mentioned this pull request May 4, 2026

Clean up CHANGELOG before 1.2 release branch cut #2684

Merged

coderabbitai Bot mentioned this pull request May 27, 2026

Fix MuJoCo shape_material force-space scaling (#2009) #2964

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix material combination inconsistencies#2439

Fix material combination inconsistencies#2439
nvtw merged 3 commits into
newton-physics:mainfrom
nvtw:dev/tw3/address_newton_vs_mjwarp_contact_discrepancy

nvtw commented Apr 14, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 14, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Apr 14, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

adenzler-nvidia left a comment

Uh oh!

adenzler-nvidia left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

nvtw commented Apr 14, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Test plan

Bug fix

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

adenzler-nvidia left a comment

Choose a reason for hiding this comment

Uh oh!

adenzler-nvidia left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nvtw commented Apr 14, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 14, 2026 •

edited

Loading

codecov Bot commented Apr 14, 2026 •

edited

Loading