Add physics variant selection to asset converters - #6881
Closed
hujc7 wants to merge 15 commits into
Closed
Conversation
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.
…er-physics-variant
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.
…er-physics-variant
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.
…er-physics-variant
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.
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.
…nt' into jichuanh/converter-physics-variant
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:
Closing in favour of #6935. |
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.
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
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.
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.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.variantsstill overrides it at spawn time.Why
"physics"is the defaultThe variants are a shared base plus two supersets:
physx.usdaandmujoco.usdaboth carrysubLayers = [@./physics.usda@]. Measured on the converted Franka:physicsUsdPhysicsjoints / articulation root / mass, plusNewtonMimicAPI,NewtonArticulationRootAPI,NewtonCollisionAPIphysxPhysxJointAPIandphysxJoint:maxJointVelocityper jointmujocoMjcActuatorprim per jointThe base is the backend-portable layer by construction: the importer routes schemas matching
Physics.*andNewton.*into it,Physx.*into the PhysX variant, andMjc.*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:
test_physics_variant_overrideandtest_physics_variant_raises_when_requested_absentfailThe 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.