Skip to content

Commit 261d077

Browse files
matthewtreptekellyguo11huidongcmyurasov-nvhujc7
authored
Patch newton model reqs (isaac-sim#5398)
# Description <!-- Thank you for your interest in sending a pull request. Please make sure to check the contribution guidelines. Link: https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html 💡 Please try to keep PRs small and focused. Large PRs are harder to review and merge. --> Patch newton model reqs issue where in some caes env configs were constructed in a way where newton model reqs were not correctly determined, leading to downstream issues. Also, revert the revert to physx scene data provider, since the fix above no longer requires the revert, which is non ideal since building newton model from usd fallback is slow. <!-- As a practice, it is recommended to open an issue to have discussions on the proposed pull request. This makes it easier for the community to keep track of what is being developed or added, and if a given feature is demanded by more than one party. --> ## Type of change <!-- As you go through the list, delete the ones that are not applicable. --> - Bug fix (non-breaking change which fixes an issue) ## Screenshots Please attach before and after screenshots of the change if applicable. <!-- Example: | Before | After | | ------ | ----- | | _gif/png before_ | _gif/png after_ | To upload images to a PR -- simply drag and drop an image while in edit mode and it should upload the image directly. You can then paste that source into the above before/after sections. --> ## 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](https://pre-commit.com/) with `./isaaclab.sh --format` - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] I have updated the changelog and the corresponding version in the extension's `config/extension.toml` file - [ ] I have added my name to the `CONTRIBUTORS.md` or my name already exists there <!-- As you go through the checklist above, you can mark something as done by putting an x character in it For example, - [x] I have done this task - [ ] I have not done this task --> --------- Signed-off-by: matthewtrepte <mtrepte@nvidia.com> Co-authored-by: Kelly Guo <kellyg@nvidia.com> Co-authored-by: HuiDong Chen <huidongc@nvidia.com> Co-authored-by: myurasov-nv <168484206+myurasov-nv@users.noreply.github.com> Co-authored-by: hujc <leo.hu.sh@gmail.com> Co-authored-by: Antoine Richard <antoiner@nvidia.com> Co-authored-by: peterd-NV <peterd@nvidia.com>
1 parent 7166229 commit 261d077

8 files changed

Lines changed: 152 additions & 168 deletions

File tree

source/isaaclab/isaaclab/scene/interactive_scene.py

Lines changed: 31 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
RigidObjectCollection,
2929
RigidObjectCollectionCfg,
3030
)
31-
from isaaclab.physics.scene_data_requirements import resolve_scene_data_requirements
31+
from isaaclab.physics.scene_data_requirements import aggregate_requirements, resolve_scene_data_requirements
3232
from isaaclab.sensors import ContactSensorCfg, FrameTransformerCfg, SensorBase, SensorBaseCfg
3333
from isaaclab.sim import SimulationContext
3434
from isaaclab.sim.utils.stage import get_current_stage, get_current_stage_id
@@ -140,7 +140,6 @@ def __init__(self, cfg: InteractiveSceneCfg):
140140
self.stage_id = get_current_stage_id()
141141
self.sim.clear_scene_data_visualizer_prebuilt_artifact()
142142
self.physics_backend = self.sim.physics_manager.__name__.lower()
143-
visualizer_clone_fn = None
144143
requested_viz_types = set(self.sim.resolve_visualizer_types())
145144
if self.physics_backend.startswith("ovphysx"):
146145
from isaaclab_ovphysx.cloner import ovphysx_replicate
@@ -200,26 +199,7 @@ def __init__(self, cfg: InteractiveSceneCfg):
200199
if has_scene_cfg_entities:
201200
self._add_entities_from_cfg()
202201

203-
requirements = resolve_scene_data_requirements(
204-
visualizer_types=requested_viz_types,
205-
renderer_types=self._sensor_renderer_types(),
206-
)
207-
self.sim.update_scene_data_requirements(requirements)
208-
visualizer_clone_fn = cloner.resolve_visualizer_clone_fn(
209-
physics_backend=self.physics_backend,
210-
requirements=requirements,
211-
stage=self.stage,
212-
set_visualizer_artifact=self.sim.set_scene_data_visualizer_prebuilt_artifact,
213-
)
214-
if visualizer_clone_fn is not None:
215-
logger.debug(
216-
"Enabling visualizer artifact prebuild for clone path "
217-
"(backend=%s, requires_newton_model=%s, requires_usd_stage=%s).",
218-
self.physics_backend,
219-
requirements.requires_newton_model,
220-
requirements.requires_usd_stage,
221-
)
222-
self.cloner_cfg.visualizer_clone_fn = visualizer_clone_fn
202+
self._refresh_visualizer_clone_fn_from_requirements(requested_viz_types)
223203

224204
if has_scene_cfg_entities:
225205
self.clone_environments(copy_from_source=(not self.cfg.replicate_physics))
@@ -236,6 +216,8 @@ def clone_environments(self, copy_from_source: bool = False):
236216
If True, clones are independent copies of the source prim and won't reflect its changes (start-up time
237217
may increase). Defaults to False.
238218
"""
219+
self._refresh_visualizer_clone_fn_from_requirements()
220+
239221
# PhysX-only: set env id bit count for replicated physics. Newton handles env separation in its own API.
240222
# Intentionally matches both physx and ovphysx (both are PhysX-based)
241223
if self.cfg.replicate_physics and "physx" in self.physics_backend:
@@ -267,6 +249,33 @@ def clone_environments(self, copy_from_source: bool = False):
267249
if self.cloner_cfg.clone_usd:
268250
cloner.usd_replicate(self.stage, *replicate_args)
269251

252+
def _refresh_visualizer_clone_fn_from_requirements(self, visualizer_types=()) -> None:
253+
"""Refresh clone-time visualizer prebuild hook from current scene-data requirements."""
254+
discovered_req = resolve_scene_data_requirements(
255+
visualizer_types=visualizer_types,
256+
renderer_types=self._sensor_renderer_types(),
257+
)
258+
current_req = self.sim.get_scene_data_requirements()
259+
requirements = aggregate_requirements((current_req, discovered_req))
260+
if requirements != current_req:
261+
self.sim.update_scene_data_requirements(requirements)
262+
263+
visualizer_clone_fn = cloner.resolve_visualizer_clone_fn(
264+
physics_backend=self.physics_backend,
265+
requirements=requirements,
266+
stage=self.stage,
267+
set_visualizer_artifact=self.sim.set_scene_data_visualizer_prebuilt_artifact,
268+
)
269+
if visualizer_clone_fn is not None:
270+
logger.debug(
271+
"Enabling visualizer artifact prebuild for clone path "
272+
"(backend=%s, requires_newton_model=%s, requires_usd_stage=%s).",
273+
self.physics_backend,
274+
requirements.requires_newton_model,
275+
requirements.requires_usd_stage,
276+
)
277+
self.cloner_cfg.visualizer_clone_fn = visualizer_clone_fn
278+
270279
def _sensor_renderer_types(self) -> list[str]:
271280
"""Return renderer type names used by scene sensors."""
272281
renderer_types: list[str] = []

source/isaaclab/isaaclab/sensors/camera/camera.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ def __init__(self, cfg: CameraCfg):
106106
self._check_supported_data_types(cfg)
107107
# initialize base class
108108
super().__init__(cfg)
109+
self._register_renderer_scene_data_requirements()
109110

110111
# TODO(follow-up PR): move this flag flip out of Camera. The cleanest path is
111112
# an apply_pre_reset_settings() hook on RendererCfg (default no-op) that
@@ -133,6 +134,30 @@ def __init__(self, cfg: CameraCfg):
133134
self._renderer: BaseRenderer | None = None
134135
self._render_data = None
135136

137+
def _register_renderer_scene_data_requirements(self) -> None:
138+
"""Register renderer requirements early enough for clone-time prebuilds."""
139+
renderer_type = getattr(getattr(self.cfg, "renderer_cfg", None), "renderer_type", None)
140+
if renderer_type is None:
141+
return
142+
143+
from isaaclab.physics.scene_data_requirements import aggregate_requirements, requirement_for_renderer_type
144+
from isaaclab.sim import SimulationContext
145+
146+
sim = SimulationContext.instance()
147+
if sim is None:
148+
logger.debug("SimulationContext not available; deferring renderer requirements registration.")
149+
return
150+
151+
try:
152+
renderer_req = requirement_for_renderer_type(renderer_type)
153+
except ValueError:
154+
return
155+
156+
current_req = sim.get_scene_data_requirements()
157+
merged_req = aggregate_requirements((current_req, renderer_req))
158+
if merged_req != current_req:
159+
sim.update_scene_data_requirements(merged_req)
160+
136161
def __del__(self):
137162
"""Unsubscribes from callbacks and cleans up renderer resources."""
138163
# unsubscribe callbacks

source/isaaclab/isaaclab/visualizers/visualizer_cfg.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ class VisualizerCfg:
4040
lookat: tuple[float, float, float] = (0.0, 0.0, 0.0)
4141
"""Initial camera look-at point (x, y, z) in world coordinates."""
4242

43-
cam_source: Literal["cfg", "prim_path"] = "cfg"
43+
cam_source: Literal["cfg", "prim_path"] = "prim_path"
4444
"""Camera source mode: 'cfg' uses eye/lookat, 'prim_path' follows a camera prim."""
4545

4646
cam_prim_path: str = "/World/envs/env_0/Camera"

source/isaaclab/test/scene/test_interactive_scene.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import isaaclab.sim as sim_utils
2222
from isaaclab.actuators import ImplicitActuatorCfg
2323
from isaaclab.assets import ArticulationCfg, RigidObjectCfg
24+
from isaaclab.physics.scene_data_requirements import SceneDataRequirement
2425
from isaaclab.scene import InteractiveScene, InteractiveSceneCfg
2526
from isaaclab.sim import build_simulation_context
2627
from isaaclab.utils import configclass
@@ -134,6 +135,13 @@ def test_clone_environments_non_cfg_invokes_visualizer_clone_fn(monkeypatch: pyt
134135
scene = object.__new__(InteractiveScene)
135136
scene.cfg = SimpleNamespace(replicate_physics=False, num_envs=3)
136137
scene.stage = object()
138+
scene.physics_backend = "physx"
139+
scene._sensors = {}
140+
scene.sim = SimpleNamespace(
141+
get_scene_data_requirements=lambda: SceneDataRequirement(),
142+
update_scene_data_requirements=lambda requirements: None,
143+
set_scene_data_visualizer_prebuilt_artifact=lambda artifact: None,
144+
)
137145
scene.env_fmt = "/World/envs/env_{}"
138146
scene._ALL_INDICES = torch.arange(3, dtype=torch.long)
139147
scene._default_env_origins = torch.zeros((3, 3), dtype=torch.float32)
@@ -189,6 +197,38 @@ def _usd_replicate(stage, *args, **kwargs):
189197
assert len(usd_calls) == 1
190198

191199

200+
def test_refresh_visualizer_clone_fn_uses_registered_requirements(monkeypatch: pytest.MonkeyPatch):
201+
"""Clone-time prebuild hook should be installed from requirements registered after scene init."""
202+
scene = object.__new__(InteractiveScene)
203+
scene.physics_backend = "physx"
204+
scene.stage = object()
205+
scene._sensors = {}
206+
scene.cloner_cfg = SimpleNamespace(visualizer_clone_fn=None)
207+
208+
requirements = SceneDataRequirement(requires_newton_model=True)
209+
scene.sim = SimpleNamespace(
210+
get_scene_data_requirements=lambda: requirements,
211+
update_scene_data_requirements=lambda requirements: None,
212+
set_scene_data_visualizer_prebuilt_artifact=lambda artifact: None,
213+
)
214+
215+
captured = {}
216+
217+
def _resolve_visualizer_clone_fn(**kwargs):
218+
captured.update(kwargs)
219+
return "visualizer-clone-fn"
220+
221+
monkeypatch.setattr(
222+
"isaaclab.scene.interactive_scene.cloner.resolve_visualizer_clone_fn",
223+
_resolve_visualizer_clone_fn,
224+
)
225+
226+
scene._refresh_visualizer_clone_fn_from_requirements()
227+
228+
assert captured["requirements"].requires_newton_model
229+
assert scene.cloner_cfg.visualizer_clone_fn == "visualizer-clone-fn"
230+
231+
192232
def assert_state_equal(s1: dict, s2: dict, path=""):
193233
"""
194234
Recursively assert that s1 and s2 have the same nested keys

source/isaaclab/test/sensors/test_camera.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import copy
1919
import random
20+
from types import SimpleNamespace
2021

2122
import numpy as np
2223
import pytest
@@ -27,7 +28,9 @@
2728
from pxr import Gf, Usd, UsdGeom
2829

2930
import isaaclab.sim as sim_utils
31+
from isaaclab.physics.scene_data_requirements import SceneDataRequirement
3032
from isaaclab.sensors.camera import Camera, CameraCfg
33+
from isaaclab.sim import SimulationContext
3134

3235
pytestmark = pytest.mark.isaacsim_ci
3336

@@ -45,6 +48,23 @@
4548
WIDTH = 320
4649

4750

51+
def test_camera_registers_renderer_scene_data_requirements(monkeypatch: pytest.MonkeyPatch):
52+
"""Camera creation path should register renderer-driven scene-data requirements."""
53+
camera = object.__new__(Camera)
54+
camera.cfg = SimpleNamespace(renderer_cfg=SimpleNamespace(renderer_type="newton_warp"))
55+
updates = []
56+
sim = SimpleNamespace(
57+
get_scene_data_requirements=lambda: SceneDataRequirement(),
58+
update_scene_data_requirements=updates.append,
59+
)
60+
61+
monkeypatch.setattr(SimulationContext, "instance", staticmethod(lambda: sim))
62+
63+
camera._register_renderer_scene_data_requirements()
64+
65+
assert updates == [SceneDataRequirement(requires_newton_model=True)]
66+
67+
4868
def setup() -> tuple[sim_utils.SimulationContext, CameraCfg, float]:
4969
camera_cfg = CameraCfg(
5070
height=HEIGHT,

source/isaaclab/test/sim/test_physx_scene_data_provider_visualizer_contract.py

Lines changed: 4 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -67,68 +67,11 @@ def test_load_prebuilt_artifact_populates_provider_state():
6767
assert provider._xform_mask_buf is None
6868

6969

70-
def test_load_prebuilt_artifact_missing_falls_back_to_usd_build():
71-
"""When no artifact is registered, the USD-traversal fallback is invoked."""
70+
def test_load_prebuilt_artifact_missing_sets_error_state():
71+
"""When no artifact is registered, model/state stay unset."""
7272
provider = _make_provider()
73-
fallback_artifact = VisualizerPrebuiltArtifacts(
74-
model="usd-built-model",
75-
state="usd-built-state",
76-
rigid_body_paths=["/World/envs/env_0/A"],
77-
articulation_paths=[],
78-
num_envs=2,
79-
)
80-
stored: list[VisualizerPrebuiltArtifacts] = []
81-
provider._simulation_context = SimpleNamespace(
82-
get_scene_data_visualizer_prebuilt_artifact=lambda: None,
83-
set_scene_data_visualizer_prebuilt_artifact=stored.append,
84-
)
85-
provider._stage = None
86-
provider._xform_views = {}
87-
provider._view_body_index_map = {}
88-
provider._view_order_tensors = {}
89-
provider._pose_buf_num_bodies = 0
90-
provider._positions_buf = None
91-
provider._orientations_buf = None
92-
provider._covered_buf = None
93-
provider._xform_mask_buf = None
94-
95-
with (
96-
patch.object(
97-
PhysxSceneDataProvider,
98-
"_build_newton_artifact_from_usd_fallback",
99-
autospec=True,
100-
return_value=fallback_artifact,
101-
),
102-
patch(
103-
"isaaclab_physx.scene_data_providers.physx_scene_data_provider.replace_newton_shape_colors",
104-
lambda m, s: None,
105-
),
106-
):
107-
provider._load_newton_model_from_prebuilt_artifact()
108-
109-
assert provider._last_newton_model_build_source == "usd_fallback"
110-
assert provider._newton_model == "usd-built-model"
111-
assert provider._newton_state == "usd-built-state"
112-
assert provider._rigid_body_paths == ["/World/envs/env_0/A"]
113-
assert provider._num_envs_at_last_newton_build == 2
114-
# The fallback artifact is cached on the simulation context so subsequent providers see it.
115-
assert stored == [fallback_artifact]
116-
117-
118-
def test_load_prebuilt_artifact_missing_and_fallback_failed_sets_missing_state():
119-
"""When both the prebuilt artifact and the USD-traversal fallback fail, model/state stay unset."""
120-
provider = _make_provider()
121-
provider._simulation_context = SimpleNamespace(
122-
get_scene_data_visualizer_prebuilt_artifact=lambda: None,
123-
set_scene_data_visualizer_prebuilt_artifact=lambda artifact: None,
124-
)
125-
with patch.object(
126-
PhysxSceneDataProvider,
127-
"_build_newton_artifact_from_usd_fallback",
128-
autospec=True,
129-
return_value=None,
130-
):
131-
provider._load_newton_model_from_prebuilt_artifact()
73+
provider._simulation_context = SimpleNamespace(get_scene_data_visualizer_prebuilt_artifact=lambda: None)
74+
provider._load_newton_model_from_prebuilt_artifact()
13275
assert provider._last_newton_model_build_source == "missing"
13376
assert provider._newton_model is None
13477
assert provider._newton_state is None

source/isaaclab/test/sim/test_simulation_context_visualizers.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,9 @@ def get_newton_state(self):
183183
self.state_calls.append(None)
184184
return {"state_call": len(self.state_calls)}
185185

186+
def get_camera_transforms(self):
187+
return {}
188+
186189

187190
class _DummyViserViewer:
188191
def __init__(self):
@@ -354,6 +357,9 @@ def get_newton_model(self):
354357
def get_newton_state(self):
355358
return {"ok": True}
356359

360+
def get_camera_transforms(self):
361+
return {}
362+
357363
monkeypatch.setattr(rerun_visualizer, "NewtonViewerRerun", _FakeNewtonViewerRerun)
358364
monkeypatch.setattr(
359365
rerun_visualizer, "_ensure_rerun_server", lambda **kwargs: ("rerun+http://127.0.0.1:9876/proxy", False)

0 commit comments

Comments
 (0)