Skip to content

Add physics variant selection to asset converters - #6881

Closed
hujc7 wants to merge 15 commits into
isaac-sim:developfrom
hujc7:jichuanh/converter-physics-variant
Closed

Add physics variant selection to asset converters#6881
hujc7 wants to merge 15 commits into
isaac-sim:developfrom
hujc7:jichuanh/converter-physics-variant

Conversation

@hujc7

@hujc7 hujc7 commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

The URDF and MJCF importers emit their physics description as payloads behind a "Physics" variant set on the generated asset, and leave that variant set unselected. USD has no implicit default, so an unselected variant set contributes nothing to composition and the converted USD opens with geometry only — no joints, no articulation root, no mass properties.

The converters already author a "physx" selection to compensate, but return early when the asset offers no such variant. A URDF whose joints are all fixed yields ["none", "physics"] and no "physx", so those assets still convert physics-free.

What changed

  • AssetConverterBaseCfg.physics_variant (new, defaults to "physics") chooses which variant the converter selects on the generated USD file.
  • Conversion raises when the asset does not offer the requested variant, naming the ones it does. Substituting another variant would hand back an asset authored for a different backend than the caller asked for.
  • Fixed the standalone importer install command in the import how-to. The wheel is published on PyPI, so the package index substitution it described is unnecessary.

Every variant stays in the generated USD file, so the selection is not destructive — it only decides what composes when the asset is opened without saying anything. UsdFileCfg.variants still overrides it at spawn time.

Why "physics" is the default

The variants are a shared base plus two supersets: physx.usda and mujoco.usda both carry subLayers = [@./physics.usda@]. Measured on the converted Franka:

variant what it adds over the base
physics this is the base — UsdPhysics joints / articulation root / mass, plus NewtonMimicAPI, NewtonArticulationRootAPI, NewtonCollisionAPI
physx PhysxJointAPI and physxJoint:maxJointVelocity per joint
mujoco one MjcActuator prim per joint

The base is the backend-portable layer by construction: the importer routes schemas matching Physics.* and Newton.* into it, Physx.* into the PhysX variant, and Mjc.* into the MuJoCo variant. Everything structural lives in the base — the importer no longer authors PhysX mimic joints or articulation roots — so the backend variants carry solver tuning only.

Defaulting to "physics" therefore produces an asset that runs on either backend, and leaves the backend choice to the caller who knows which one they want. Selecting "physx" or "mujoco" bakes that choice into the asset, which is the right thing to do explicitly and the wrong thing to do by default.

Testing

Isaac Sim 6.1.0-alpha.35, Kit 110.3.0:

./isaaclab.sh -p -m pytest source/isaaclab/test/sim/test_urdf_converter.py -q
Run Result
With this change 23 passed (20 existing + 3 new)
With the converter change reverted test_physics_variant_override and test_physics_variant_raises_when_requested_absent fail

The new tests cover the default selection, an explicit "mujoco" override, and the failure raised for an asset that offers no "physx" variant.

Tracked as nvbug 6527494.

hujc7 added 2 commits August 3, 2026 16:26
The URDF and MJCF importers emit physics as payloads behind a "Physics"
variant set and leave it unselected, so the generated USD composes with
geometry only. The converters already author a "physx" selection to
compensate, but bail out when the asset offers no such variant: a URDF
whose joints are all fixed yields only "none" and "physics", so those
assets still convert without joints, articulation roots, or mass.

Resolve the requested variant against what the asset actually offers,
falling back to the backend-neutral "physics" variant that both "physx"
and "mujoco" sublayer. Expose the request as physics_variant on the
converter config so that a Newton user can ask for "mujoco" at
conversion time, and "none" stays available as an explicit opt-out.
@github-actions github-actions Bot added the isaac-lab Related to Isaac Lab team label Aug 4, 2026
hujc7 added 4 commits August 3, 2026 21:51
The "physics" variant holds the backend-portable description: standard
UsdPhysics joints, articulation roots and mass, plus the Newton schemas.
The "physx" and "mujoco" variants sublayer it and add solver tuning, so
selecting one of them bakes a backend choice into the converted asset.

Default to "physics" instead, which leaves the asset usable on either
backend and leaves the backend choice to whoever knows it. Nothing is
lost by this: every variant stays in the generated USD file, so a caller
can request one at conversion time or override the selection at spawn.
The variant names were module-level string constants, which put the valid
values out of reach of callers. Expose them as a nested PhysicsVariant
string enum on the converter config instead, so a caller can name a
variant directly and the values document themselves.

Serializing an enum member needs class_to_dict to treat it as a leaf.
Enum members carry a __dict__ of enum internals, so the generic branch
recursed into it and emitted {_value_, _name_, _sort_order_} in place of
the value -- which would have landed in every converted asset's
config.yaml and in the hash that decides whether to reconvert.
The standalone URDF/MJCF importer instructions sat inside the legacy
installer section, so users following the recommended uv path never saw
them. The wheel is not specific to any installation method, so promote it
to its own section that all of them reach.

The instructions also predated the wheel being published, and told the
reader to substitute a package index URL for it. It is on PyPI now, so
install it by name.
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Aug 5, 2026
hujc7 added 4 commits August 4, 2026 22:30
The importers disagree on which physics variant to select. The Isaac Sim
importer extensions leave the variant set unselected, while the standalone
importer wheel selects "physx". The converter only authored a selection
when none was present, so on the standalone path physics_variant was
silently discarded and every asset came out as "physx".

Author the configured variant whenever it differs from what is already
selected, so the same config produces the same asset on either path.
Falling back to another variant handed the caller an asset authored for a
different backend than they asked for, silently. A caller that names
"physx" wants PhysX tuning; substituting the portable variant produces an
asset that loads and simulates, just not as configured.

Raise instead, naming the variants the asset does offer. Assets with no
"Physics" variant set at all are still left alone, since there is nothing
to select.
The installation doc cleanup upstream moved the standalone importer section
from the installation index into the import how-to and deleted the legacy
installer include. Keep upstream's location and drop the duplicate section,
then reapply the install command fix there: the wheel is on PyPI, so the
package index substitution it still described is unnecessary.
hujc7 added 5 commits August 6, 2026 15:57
Review of the physics variant work surfaced four defects.

class_to_dict serializes enum members as their value, but
update_class_from_dict type-checked the raw scalar against the enum
class, so reloading any config holding an enum default raised. This
reached beyond the converters: RecorderManagerBaseCfg round-trips
through Hydra for every manager-based environment. Rebuild the member
from its value instead, so the two functions are inverses again.

The asset hash was recorded before conversion ran, so a conversion that
raised still counted as cached and an identical retry skipped it and
returned the asset. Record the hash only once the asset is complete.

The nested PhysicsVariant enum was indexed once per converter cfg
subclass, and the docs build treats warnings as errors, so it failed
with three duplicate-object warnings. Document it on the base class
only.

The physics_variant docstring said conversion raises for an absent
variant, which does not hold for a flat asset carrying no variant set.
The previous commit swept in a re-resolution of the lockfile. This
branch changes no dependencies, so the lockfile belongs unchanged.
USD accepts a variant selection naming a variant the set does not offer,
and the prim then composes as if nothing were selected. For the
"Physics" variant set that silently drops the joints, articulation roots
and mass properties the set exists to carry, so the asset spawns as
plain geometry with no diagnostic -- the same symptom this branch fixes
on the conversion side.

Raise for a "Physics" set that is absent or lacks the requested variant,
and keep the existing warn-and-continue for every other set. Those were
introduced for optional variants such as a payload or a colour, where
one spawn configuration is expected to cover assets that do not all
expose the set.
@hujc7

hujc7 commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator Author

Folding this into #6935. That PR already contains these commits, and splitting the review across two PRs was costing more than it saved — the physics variant selection and the importer packaging both hinge on the same question of which importer serves a conversion.

Everything here is in #6935 unchanged:

  • physics_variant on the converter cfg, selecting the variant on the generated USD
  • REQUIRED_VARIANT_SETS in select_usd_variants, so a missing or unoffered "Physics" variant raises instead of silently composing nothing
  • the enum round-trip fix in class_to_dict / update_class_from_dict

Closing in favour of #6935.

@hujc7 hujc7 closed this Aug 7, 2026
kellyguo11 pushed a commit that referenced this pull request Aug 13, 2026
…lone importers by default (#6935)

## 1. Summary

- Asset conversion works on a default install. The standalone URDF/MJCF
importers are base dependencies (+7 packages, ~1 MB); no extra, no
second install step.
- `usd-exchange` is the sole OpenUSD provider on all platforms;
`usd-core` is gone. Both vendor a complete `pxr` into the same
directory, so co-installing silently replaces one with the other and
uninstalling either deletes files the survivor needs. No resolver
reports it.
- Converted assets no longer spawn without physics — the converters
author and select a `"Physics"` variant. Previously #6881, folded here.
- The converters choose their importer backend by **availability**, not
by what is installed.

Based on #6866 by @kellyguo11.

## 2. What changed

- `usd-exchange` on all platforms. Newton selects `[sim]`, not
`[sim,importers]`, whose extra pulls `usd-core` back in; the
mesh-processing packages it carried are declared directly.
- `isaacsim-asset-isolated` moved to `[project.dependencies]`. No
`importers` extra, no `--install importers` token — neither was ever
released.
- Converters author a `"Physics"` variant on the generated USD, selected
by the new `AssetConverterBaseCfg.physics_variant` (default `physics`).
The asset hash is recorded only after conversion succeeds, so a run that
raised is not cached.
- `select_usd_variants` raises when a variant set exists but does not
offer the requested variant — USD accepts such a selection and composes
the prim as if nothing were selected. An absent variant set still warns.
- MuJoCo's USD schemas go on OpenUSD's plugin search path — its schema
registry is built once and ignores plugins registered afterwards.
- Converter tests run in the kit-less image, selected by a `kitless`
marker.

## 3. Design notes

### 3.1 Installation is not availability

The wheel contributes `isaacsim.asset` as a PEP 420 namespace portion.
The Isaac Sim runtime ships `isaacsim` as a *regular* package, and a
regular package discards every namespace portion for that name — so an
installed wheel is unimportable whenever the runtime is on `sys.path`.

While the importers were opt-in, "installed" implied "kit-less
environment", so `metadata.distribution(...)` was a sound proxy for
"usable". As a base dependency it no longer is.
`standalone_importers_available()` resolves the spec instead, which
answers availability without importing `isaacsim` — keeping it usable
before a launch decision, where importing a simulation module is not
allowed.

The kit-less path stays preferred wherever it works; Kit is launched
only when the runtime owns the namespace.

### 3.2 No `tinyobjloader` pin

`tinyobjloader` publishes no usable stable release (`0.1`, then
`2.0.0rc*`). With the repo's existing `prerelease = "allow"`, the
converters' own requirement already resolves it to `2.0.0rc13`; an
explicit pin changed nothing (measured: identical lock, 431 packages).
`[tool.uv]` is untouched by this PR.

## 4. Testing

### 4.1 Documented install variants

Each ran the steps from `docs/source/setup/installation/index.rst`
verbatim in a fresh Python 3.12 environment, then probed how `isaacsim`
resolves, whether the importers import, the USD provider, and — the step
install-CI never takes — whether a conversion actually produces a USD.

| Variant | `isaacsim` | Importers | usd-exchange / usd-core |
Conversion |
|---|---|---|---|---|
| `uv run --extra ovphysx` | namespace | yes | 2.3.0 / absent | USD
produced |
| `./isaaclab.sh -i 'ov[ovrtx]'` | namespace | yes | 2.3.0 / absent |
USD produced |
| `./isaaclab.sh -i isaacsim` | regular pkg | yes | 2.3.0 / absent | USD
produced |
| venv + `uv pip install "isaacsim[all,extscache]"` + `./isaaclab.sh -i`
| regular pkg | yes | 2.3.0 / absent | USD produced |
| `uv init` + `uv add <isaaclab wheel>` | namespace | yes | 2.3.0 /
absent | n/a |
| Isaac Sim source build + `_isaac_sim` | regular pkg | via Kit | 2.3.0
/ absent | USD produced |

The provider invariant holds in both full Isaac Sim environments — the
ones most likely to drag `usd-core` back in.

### 4.2 Other

| Check | Result |
|---|---|
| nvbug 6583703 / 6583809 repro steps, verbatim | both signatures
absent; USD written |
| `install_ci` non-GPU suite (fresh env per test) | 31 passed, 14
skipped |
| Converter suites, kit-less and under Kit | 35 passed each |
| Newton kit-less, converters, CLI contracts | 217 + 96 passed |

## 5. Follow-up

`usd-exchange` 2.3.0 vendors OpenUSD 25.05, which predates the
`UsdPhysicsParsingUtility` fix (upstream issue 4002, public 26.05).
Measured on the shape upstream added as its regression test: 15/15
crashed, 0/15 with `PXR_WORK_THREAD_LIMIT=1`. usd-exchange MR !257 moves
the default flavor to 26.08 and resolves it.

`warp-lang[benchmark]` and `[examples]` still declare `usd-core` on
x86_64/Windows. Neither extra is selected here, so the collision is not
reachable through this repo.

`./isaaclab.sh -i` without an active 3.12 environment fails with
`ModuleNotFoundError: No module named 'tomllib'` on hosts whose system
Python is older than 3.11. Pre-existing, not touched here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant