Fix MJWarp USD friction loss import

[NeoZng]

Description

Fixes #6829.

Preserve MuJoCo-authored joint friction loss when Isaac Lab imports USD stages for the Newton MJWarp backend.

Before this change, both Newton production import paths passed only SchemaResolverNewton and SchemaResolverPhysx to Newton:

• vectorized clone replication; and
• standalone stage import.

Consequently, a joint authored with mjc:frictionloss=0.11 finalized with Model.joint_friction=0.0, even though Newton supports the attribute through SchemaResolverMjc.

This PR makes USD resolver selection an active-manager policy:

• NewtonManager defaults to Newton then PhysX resolvers;
• NewtonMJWarpManager appends the MuJoCo resolver;
• clone and standalone imports consume the same manager-owned resolver list; and
• resolver order remains Newton → PhysX → MuJoCo, so MJC values are fallbacks and do not override higher-priority authored values.

MuJoCo custom attributes continue to be registered through the existing _builder_attribute_solvers = (SolverMuJoCo,) mechanism. The follow-up removes the redundant MJWarp registration override instead of duplicating that base-class path.

Scope and alternative considered

PR #7386 fixes the immediate frictionloss symptom by appending the complete SchemaResolverMjc to every Newton physics import. That resolver also interprets additional joint, shape, contact, and scene attributes. Applying it unconditionally would therefore expand MJC semantics to Featherstone, XPBD, VBD, Kamino, and MPM rather than changing only MJWarp.

This PR intentionally keeps the immediate fix solver-scoped: the manager that registers and consumes MuJoCo-specific attributes also owns the MuJoCo resolver. Regression coverage verifies that MJWarp imports mjc:frictionloss and mjc:damping, while Featherstone preserves its previous behavior.

A broader cross-backend solution should separately classify portable MJC core semantics from MuJoCo-specific extensions, ideally by splitting those resolver responsibilities in Newton upstream. That larger architecture change is outside the scope of this bug fix.

No dependency or public API is added.

Type of change

• Bug fix (non-breaking change which fixes an issue)

Release backport

• Backport this pull request to the active release branch after it merges into develop

Screenshots

Not applicable; this fixes a non-visual USD import path.

Verification

Latest follow-up commit:

• focused production-path regression matrix: 5 passed;

  • MJWarp × clone/standalone preserves frictionloss=0.11 and damping=0.23;
  • Featherstone × clone/standalone leaves both MJC-authored values unconsumed;
  • the existing explicit-global clone import test still passes;

• full pre-commit suite passed;

• changelog validation passed;

• Python bytecode compilation passed; and

• git diff --check passed.

Previous PR head verification in the matching isolated environment:

• Python 3.12.13;
• isaaclab-newton==5.4.0;
• newton==1.5.0;
• warp-lang==1.16.0;
• mujoco-warp==3.11.0; and
• full Newton manager abstraction suite: 157 passed on a CUDA host.

Checklist

• I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html)
• I have run the pre-commit checks
• I have made corresponding changes to the documentation (not applicable: no public API or user workflow changed)
• My changes generate no new warnings
• I have added tests that prove my fix is effective
• I have added a changelog fragment under source/<pkg>/changelog.d/ for every touched package
• I have added my name to CONTRIBUTORS.md

[isaaclab-review-bot]

Isaac Lab Review Bot

The change centralizes USD schema-resolver selection in the active Newton manager and extends MJWarp imports with SchemaResolverMjc, preserving MuJoCo-authored joint friction and damping across clone-replication and standalone stage-import paths.

• Design and architecture: The protected resolver hook follows the existing solver-manager extension pattern and keeps MJWarp-specific schema handling within NewtonMJWarpManager. Both import paths now consume the same manager-owned resolver ordering. Resolver precedence still depends on Newton's ordered resolver semantics, but the explicit Newton/PhysX/MuJoCo ordering is consistent with the intended fallback behavior.
• API: No public API or dependency changes are introduced. The new protected classmethods use existing manager hooks, and MuJoCo custom-attribute registration is integrated through the established builder-registration mechanism. The package changelog fragment documents the user-visible fix.
• Implementation: The resolver list is passed through the clone path to both the main builder and source builders, and through the standalone stage-import path. Non-MJWarp managers retain the prior Newton and PhysX resolver defaults. The focused regression test exercises both production import paths and verifies mjc:frictionloss and mjc:damping in the finalized model.

No blocking issues. No inline issue met the actionable-evidence threshold; the assessment above records the review feedback.

Automated review; human maintainers own approval decisions.

[greptile-apps]

Greptile Summary

The PR preserves MuJoCo-authored joint friction loss during MJWarp USD imports by moving resolver selection behind the active Newton manager.

• Adds MJWarp-specific custom-attribute registration and the MuJoCo schema resolver.
• Uses manager-owned resolver lists in both clone and standalone stage-import paths.
• Adds production-path regression coverage for friction loss and damping.
• Adds a changelog fragment and contributor entry.

Confidence Score: 5/5

The PR appears safe to merge, with both affected USD import paths covered by focused regression tests.

The resolver hook is inherited by all reachable Newton managers, production clone dispatch occurs after manager initialization, and the MJWarp override preserves base behavior while adding the required MuJoCo import support.

Important Files Changed

Filename	Overview
source/isaaclab_newton/isaaclab_newton/physics/mjwarp_manager.py	Extends MJWarp builder setup with MuJoCo custom attributes and appends the MuJoCo USD resolver.
source/isaaclab_newton/isaaclab_newton/physics/newton_manager.py	Introduces a solver-overridable resolver-list hook and uses it for standalone stage import.
source/isaaclab_newton/isaaclab_newton/cloner/replicate.py	Routes clone imports through the active Newton manager’s schema resolvers.
source/isaaclab_newton/test/physics/test_newton_manager_abstraction.py	Covers clone and standalone MJWarp imports and verifies MuJoCo friction-loss and damping values survive finalization.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    USD[USD stage with mjc attributes] --> Path{Import path}
    Path -->|Clone| Clone[Replication builder]
    Path -->|Standalone| Standalone[Stage builder]
    Clone --> Manager[Active Newton manager]
    Standalone --> Manager
    Manager --> Resolvers[Newton → PhysX → MuJoCo resolvers]
    Manager --> Attributes[SolverMuJoCo custom attributes]
    Resolvers --> Builder[Newton ModelBuilder]
    Attributes --> Builder
    Builder --> Model[Finalized MJWarp model]

Loading

_{Reviews (1): Last reviewed commit: "Fix MJWarp USD friction loss import" | Re-trigger Greptile}

[AntoineRichard]

Note

AI-generated review. This review was produced by Claude (Claude Code) at @AntoineRichard's request, and reviewed by him before posting. Treat the findings as a starting point; the code references and behavioral claims below were verified against newton==1.5.0 sources, but a human maintainer owns the approval decision.

Thanks for this @NeoZng — I compared this PR against #7386, which fixes the same issue (#6829) by appending SchemaResolverMjc unconditionally on every Newton import path. I prefer this PR's approach and would like to converge on it, with two changes requested below.

Why this approach wins

I checked the claim that scoping matters, rather than taking it on faith:

• newton/_src/usd/schemas.py — SchemaResolverMjc is not joint-only. Beyond mjc:frictionloss and mjc:armature, it maps PrimType.SCENE (mjc:option:timestep → time_steps_per_second, mjc:option:iterations, mjc:flag:gravity), PrimType.SHAPE (mjc:margin, mjc:gap, mjc:solref → contact ke/kd, mjc:shellinertia → mass model), PrimType.MATERIAL (torsional/rolling friction, mjc:solmix, mjc:priority) and PrimType.ACTUATOR. Applying it unconditionally really would push MuJoCo contact and limit semantics into Featherstone, XPBD, VBD, Kamino and MPM for any MJCF-derived asset — so the scoping argument in your PR description holds up.
• newton/_src/usd/schema_resolver.py::SchemaResolverManager.get_value_with_resolver — precedence is first-authored-value-wins in resolver order, then the caller default, then the first non-None mapping default. Your Newton → PhysX → MuJoCo ordering therefore makes MJC a strict fallback, as documented. I also checked the mapping-default path for a possible leak (Mjc supplying defaults such as mu_torsional=0.005 on assets with no mjc:* authored): all three resolvers declare the same key sets and Newton is first, so Mjc's defaults are unreachable. No issue.

Secondary wins over the alternative: the _get_usd_import_schema_resolvers() hook matches the existing _get_usd_import_ignore_paths / _builder_attribute_solvers extension pattern; replicate.py loses its private newton._src.usd.schemas import in favour of the public newton.usd; and the parametrized clone × standalone × MJWarp/Featherstone test runs on CPU and includes a negative control.

Requested changes

1. Gate on the registered solvers, not on the manager class.

NewtonCoupledMJWarpVBDManager (source/isaaclab_contrib/isaaclab_contrib/custom_coupling/coupled_mjwarp_vbd_manager.py:22) extends NewtonVBDManager, not NewtonMJWarpManager, yet sets _builder_attribute_solvers = (SolverMuJoCo,). It runs a MuJoCo solver, so it wants the MJC resolver, but the current class-hook override does not reach it — the bug stays unfixed there.

Deriving the resolver list from the attributes the manager already registers keeps the same MJWarp-only scoping, covers the coupled manager, and picks up any future MuJoCo-based manager automatically. Roughly, in NewtonManager:

@classmethod
def _get_usd_import_schema_resolvers(cls) -> list[SchemaResolver]:
    resolvers: list[SchemaResolver] = [SchemaResolverNewton(), SchemaResolverPhysx()]
    if SolverMuJoCo in cls._builder_attribute_solvers:
        resolvers.append(SchemaResolverMjc())
    return resolvers

That would let the NewtonMJWarpManager override go away entirely. If you would rather keep the explicit override, an override on NewtonCoupledMJWarpVBDManager is the minimum, but the derived form is less likely to drift.

2. Note the call sites the hook deliberately does not cover.

visualization_builder.py:98 and isaaclab/assets/articulation/ordering_resolvers.py:481 still hardcode [SchemaResolverNewton(), SchemaResolverPhysx()]. That is correct — resolvers only select which attributes get parsed, so they cannot affect rendering or joint ordering — but after this PR there are two conventions in the tree. A one-line comment at each site (or at the hook) saying those paths intentionally stay on the fixed pair would keep the next reader from "fixing" the inconsistency.

Note on #7386

For the record, that PR's "no-op-safe when a solver has not registered MuJoCo custom attributes" claim is only half right, and the reason is worth knowing here too: SchemaResolverMjc.validate_custom_attributes raises when no mujoco-namespace custom attribute is registered, and newton/_src/utils/import_usd.py calls it for every resolver unconditionally — but a bare newton.ModelBuilder() already carries 17 mujoco-namespace attributes, so the guard never fires. It is safe from raising, and it does change behavior for non-MJWarp solvers. Your gating is what makes that a non-question.

Once the two points above are addressed I am happy to see this land, and #7386 closed in favour of it.

[NeoZng]

@AntoineRichard Thanks for the detailed review and the comparison with #7386. I have pushed the requested follow-up on top of the refreshed develop base as commit 924e407.

The implementation now:

• derives SchemaResolverMjc selection from registered SolverMuJoCo builder attributes rather than manager inheritance;

• reuses that policy for both standalone stage import and clone replication;

• covers the coupled MJWarp/VBD manager automatically and runtime-configured nested solvers through the generic coupler's capability query;

• documents why visualization and articulation-ordering paths intentionally retain the fixed Newton/PhysX resolver pair; and

• adds distinct-valued Newton/PhysX/MJC precedence coverage plus solver positive/negative controls.

This keeps the fix narrower than #7386: MJC schema interpretation is enabled only for managers that actually register SolverMuJoCo, so it does not leak MuJoCo semantics into plain Featherstone, XPBD, VBD, Kamino, or MPM managers.

Local validation:

• full repository pre-commit passed;

• Newton manager: 181 passed;

• Newton cloner: 43 passed;

• custom coupled manager: 12 passed;

• generic coupler: 57 passed;

• MJCF converter: 12 passed;