-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Honor replicate_physics via cfg-registered replication lists #6550
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
0e90593
20ee5a3
50767e2
479c8fa
6eea5fa
6644069
d83afbd
6db2d22
d470a6d
1403655
41e6622
5a007ed
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| Fixed | ||
| ^^^^^ | ||
|
|
||
| * Fixed :attr:`~isaaclab.scene.InteractiveSceneCfg.replicate_physics` being ignored since the | ||
| replication-session refactor: scenes configured with ``replicate_physics=False`` invoked | ||
| native physics replication anyway, silently discarding per-environment USD differences | ||
| (e.g. prestartup scale randomization). Cloning is now USD-only in that case, so the physics | ||
| engine parses each environment's USD prims directly; assets whose only cloning mechanism is | ||
| physics replication are not cloned. | ||
|
|
||
| Added | ||
| ^^^^^ | ||
|
|
||
| * Added :attr:`~isaaclab.cloner.CloneCfg.replicate_physics` as the cloner-side home of the | ||
| policy; :class:`~isaaclab.scene.InteractiveScene` pipes the scene flag into it and | ||
| :func:`~isaaclab.cloner.replicate` applies it. | ||
| * Added :attr:`~isaaclab.assets.AssetBaseCfg.cloning_contexts` to override, per asset, the | ||
| cloning contexts resolved at replication time; ``None`` uses the active backend's default | ||
| stack (``isaaclab_<backend>.cloner.REPLICATION``). | ||
|
|
||
| Changed | ||
| ^^^^^^^ | ||
|
|
||
| * **Breaking:** Changed :data:`~isaaclab.cloner.REPLICATION_QUEUE` to hold asset cfgs instead | ||
| of ``(cfg, BackendCtxCls)`` pairs: construction registers *which* cfgs participate via | ||
| :func:`~isaaclab.cloner.queue_replication`, and *how* each is cloned resolves inside | ||
| :func:`~isaaclab.cloner.replicate`. Code appending pairs should register the cfg and direct | ||
| contexts through :attr:`~isaaclab.assets.AssetBaseCfg.cloning_contexts`. | ||
|
|
||
| Removed | ||
| ^^^^^^^ | ||
|
|
||
| * Removed ``queue_usd_replication``: register the cfg with | ||
| :func:`~isaaclab.cloner.queue_replication` and direct contexts through the cfg's | ||
| ``cloning_contexts`` field instead. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,10 +7,16 @@ | |
|
|
||
| from __future__ import annotations | ||
|
|
||
| import importlib | ||
| from collections.abc import Callable, Iterable | ||
| from typing import TYPE_CHECKING, Any | ||
|
|
||
| from isaaclab.utils.backend_utils import FactoryBase | ||
| from isaaclab.utils.string import string_to_callable | ||
| from isaaclab.utils.version import has_kit | ||
|
|
||
| from .cloner_strategies import sequential | ||
| from .usd import UsdReplicateContext | ||
|
|
||
| if TYPE_CHECKING: | ||
| import torch | ||
|
|
@@ -20,32 +26,71 @@ | |
| from .clone_plan import ClonePlan | ||
|
|
||
|
|
||
| REPLICATION_QUEUE: list[tuple[Any, type]] = [] | ||
| """``(cfg, BackendCtxCls)`` pairs appended by ``queue_<backend>_replication`` and drained by :func:`replicate`.""" | ||
| REPLICATION_QUEUE: list[Any] = [] | ||
| """Asset cfgs registered by :func:`queue_replication` and drained by :func:`replicate`. | ||
|
|
||
| The queue only records *which* cfgs participate in cloning; how each cfg is cloned is | ||
| resolved at dispatch from :attr:`~isaaclab.assets.AssetBaseCfg.cloning_contexts` or the | ||
| active backend's default stack. | ||
| """ | ||
|
|
||
|
|
||
| def queue_replication(cfg: Any) -> None: | ||
| """Register ``cfg`` for cloning when :func:`replicate` next runs. | ||
|
|
||
| Args: | ||
| cfg: Asset cfg with resolved ``prim_path``. | ||
| """ | ||
| REPLICATION_QUEUE.append(cfg) | ||
|
|
||
| def replicate(plan: ClonePlan, *, stage: Usd.Stage) -> None: | ||
|
|
||
| def replicate(plan: ClonePlan, *, stage: Usd.Stage, replicate_physics: bool = True) -> None: | ||
| """Drain :data:`REPLICATION_QUEUE` against ``plan``, dispatch each backend, publish the plan. | ||
|
|
||
| Physics contexts come from :attr:`~isaaclab.assets.AssetBaseCfg.cloning_contexts` when | ||
| set, otherwise from the backend's ``PHYSICS_CONTEXT`` class. | ||
| :class:`~isaaclab.cloner.UsdReplicateContext` is added automatically when the cfg has a | ||
| spawner and Kit is available. With ``replicate_physics=False`` physics contexts are | ||
| dropped; USD replication still fires when the spawner+Kit condition is met. | ||
|
|
||
| Cfgs absent from ``plan.cfg_rows`` are silently skipped. Backend contexts run in | ||
| ascending ``replicate_priority`` order. The queue is cleared up front, so a backend | ||
| failure cannot leak stale entries into the next call. | ||
|
|
||
| Args: | ||
| plan: Replication layout to dispatch. | ||
| stage: USD stage to author replicated prim specs into. | ||
| replicate_physics: Whether physics replication clones each environment. If False, | ||
| cloning is USD-only; an asset whose contexts are all physics-based is not cloned. | ||
| """ | ||
| from isaaclab.sim import SimulationContext # noqa: PLC0415 | ||
|
|
||
| queued = list(REPLICATION_QUEUE) | ||
| REPLICATION_QUEUE.clear() | ||
|
|
||
| # Group queued cfgs by backend, taking the union of row indices each backend owns. | ||
| # In the homogeneous plan every cfg maps to row 0, so multiple queue_<backend>_replication | ||
| # In the homogeneous plan every cfg maps to row 0, so multiple queue_replication | ||
| # calls (e.g. one per body type in RigidObjectCollection) all contribute {0} and the set | ||
| # union keeps it as a single row — no redundant copy specs are authored. | ||
| backend_rows: dict[type, set[int]] = {} | ||
| for cfg, BackendCtxCls in queued: | ||
| for cfg in queued: | ||
| rows = plan.cfg_rows.get(id(cfg)) | ||
| if rows is None: | ||
| continue | ||
| backend_rows.setdefault(BackendCtxCls, set()).update(rows) | ||
| if cfg.cloning_contexts is None: | ||
| physics_ctx = getattr( | ||
| importlib.import_module(f"isaaclab_{FactoryBase._get_backend()}.cloner"), "PHYSICS_CONTEXT", None | ||
| ) | ||
| contexts = [physics_ctx] if physics_ctx else [] | ||
| else: | ||
| contexts = [string_to_callable(c) if isinstance(c, str) else c for c in cfg.cloning_contexts] | ||
| if not replicate_physics: | ||
| contexts = [c for c in contexts if c is UsdReplicateContext] | ||
| ctx_set = dict.fromkeys(contexts) | ||
| if getattr(cfg, "spawn", None) is not None and has_kit(): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Preserve explicit This unconditional USD insertion means a spawned cfg with an explicit override is not authoritative: |
||
| ctx_set.setdefault(UsdReplicateContext, None) | ||
|
Comment on lines
+87
to
+91
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The PR description states "every context except The correct filter keeps only if not replicate_physics:
contexts = [c for c in contexts if c is UsdReplicateContext]The subsequent |
||
| for BackendCtxCls in ctx_set: | ||
| backend_rows.setdefault(BackendCtxCls, set()).update(rows) | ||
|
|
||
| backend_ctxs: dict[type, Any] = {} | ||
| for BackendCtxCls, row_set in backend_rows.items(): | ||
|
|
@@ -70,7 +115,7 @@ class ReplicateSession: | |
| """Folds :func:`make_clone_plan` and :func:`replicate` into a ``with`` block. | ||
|
|
||
| ``__enter__`` builds the plan (and mutates each cfg's ``spawn_path``); asset | ||
| constructors inside the block register backend replication into | ||
| constructors inside the block register their cfgs into | ||
| :data:`REPLICATION_QUEUE`; ``__exit__`` drains and dispatches. | ||
|
|
||
| Example: | ||
|
|
@@ -92,6 +137,7 @@ def __init__( | |
| stage: Usd.Stage, | ||
| clone_strategy: Callable = sequential, | ||
| valid_set: torch.Tensor | None = None, | ||
| replicate_physics: bool = True, | ||
| ): | ||
| """Capture arguments for :func:`make_clone_plan` and :func:`replicate`. | ||
|
|
||
|
|
@@ -104,9 +150,12 @@ def __init__( | |
| clone_strategy: Prototype-to-env assignment function. | ||
| valid_set: Optional ``[num_combos, num_groups]`` long tensor of valid | ||
| prototype combinations; ``None`` uses the full cartesian product. | ||
| replicate_physics: Whether physics replication clones each environment; | ||
| forwarded to :func:`replicate`. | ||
| """ | ||
| self._cfgs = cfgs | ||
| self._stage = stage | ||
| self._replicate_physics = replicate_physics | ||
| self._kwargs = dict( | ||
| num_clones=num_clones, | ||
| env_spacing=env_spacing, | ||
|
|
@@ -125,7 +174,7 @@ def __enter__(self) -> ReplicateSession: | |
| def __exit__(self, exc_type, exc_value, traceback) -> None: | ||
| if exc_type is None: | ||
| assert self._plan is not None | ||
| replicate(self._plan, stage=self._stage) | ||
| replicate(self._plan, stage=self._stage, replicate_physics=self._replicate_physics) | ||
| else: | ||
| # Drop cfgs registered before the failure so the next session is clean. | ||
| REPLICATION_QUEUE.clear() | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Document the backend symbol that actually exists
No backend in this head exports
REPLICATION; PhysX, Newton, and OvPhysX export onlyPHYSICS_CONTEXT. The new package changelog fragments also advertiseREPLICATION, so users will be directed to a nonexistent API and the compiled changelog will carry invalid symbol references. Please either implement the documented stack API or update this guide and all affected fragments to describePHYSICS_CONTEXTplus the USD auto-add behavior.