Allow for automatic switching between ovrtx and isaac sim RTX - #6246
Conversation
Greptile SummaryThis PR introduces a new
Confidence Score: 5/5Safe to merge — the auto RTX resolution logic is correct for all documented use cases and the previously-flagged livestream ordering bug is properly fixed. The core decision logic in scan() is sound: auto_rtx placeholders are collected before the visited guard so aliased instances are all updated, and _ensure_livestream_kit_visualizer is now correctly called before scan() so livestream-forced Kit is visible during resolution. The only inconsistency found is that config_scan.needs_kit can remain False when use_isaac_sim was triggered solely by a kit visualizer and the auto_rtx node is not inside a CameraCfg — but launch_simulation reads _uses_isaac_sim_runtime() rather than config_scan.needs_kit for its actual decision, so runtime behavior is correct in all currently-exercised paths. sim_launcher.py — the post-resolution needs_kit update uses the has_kit_camera/kitless-physics formula, which can underreport for non-camera auto_rtx in kit-visualizer-only scenarios. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User as User CLI
participant LS as launch_simulation()
participant EL as _ensure_livestream_kit_visualizer()
participant SC as scan()
participant VT as _uses_isaac_sim_runtime()
participant VL as _validate_runtime()
User->>LS: "launcher_args (presets=rtx, livestream?, visualizer?)"
LS->>EL: launcher_args
EL-->>LS: "mutates visualizer to [kit] if livestream>0"
LS->>SC: cfg, launcher_args (with kit already injected if livestream)
SC->>SC: walk tree, record auto_rtx locations, physics, kit cameras
SC->>VT: config_scan (needs_kit from initial walk), launcher_args
VT-->>SC: "use_isaac_sim = needs_kit OR has_kit_visualizer"
alt "use_isaac_sim == True"
SC->>SC: setattr(camera, renderer_cfg, IsaacRtxRendererCfg())
SC->>SC: "update has_kit_camera, needs_kit = True"
else "use_isaac_sim == False"
SC->>SC: setattr(camera, renderer_cfg, OVRTXRendererCfg())
SC->>SC: "update has_ovrtx = True, needs_kit = False"
end
SC-->>LS: resolved Scan
LS->>VL: config_scan, launcher_args
VL-->>LS: raises if ovrtx+kit or ovphysx+kit
alt needs_kit
LS->>LS: AppLauncher (Kit / Isaac Sim)
LS->>LS: auto-enable cameras if has_kit_camera
else kitless
LS->>LS: sync visualizer settings to carb
end
LS-->>User: yields resolved physics_cfg
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User as User CLI
participant LS as launch_simulation()
participant EL as _ensure_livestream_kit_visualizer()
participant SC as scan()
participant VT as _uses_isaac_sim_runtime()
participant VL as _validate_runtime()
User->>LS: "launcher_args (presets=rtx, livestream?, visualizer?)"
LS->>EL: launcher_args
EL-->>LS: "mutates visualizer to [kit] if livestream>0"
LS->>SC: cfg, launcher_args (with kit already injected if livestream)
SC->>SC: walk tree, record auto_rtx locations, physics, kit cameras
SC->>VT: config_scan (needs_kit from initial walk), launcher_args
VT-->>SC: "use_isaac_sim = needs_kit OR has_kit_visualizer"
alt "use_isaac_sim == True"
SC->>SC: setattr(camera, renderer_cfg, IsaacRtxRendererCfg())
SC->>SC: "update has_kit_camera, needs_kit = True"
else "use_isaac_sim == False"
SC->>SC: setattr(camera, renderer_cfg, OVRTXRendererCfg())
SC->>SC: "update has_ovrtx = True, needs_kit = False"
end
SC-->>LS: resolved Scan
LS->>VL: config_scan, launcher_args
VL-->>LS: raises if ovrtx+kit or ovphysx+kit
alt needs_kit
LS->>LS: AppLauncher (Kit / Isaac Sim)
LS->>LS: auto-enable cameras if has_kit_camera
else kitless
LS->>LS: sync visualizer settings to carb
end
LS-->>User: yields resolved physics_cfg
Reviews (5): Last reviewed commit: "Merge branch 'mataylor/rtx-preset' of gi..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Signed-off-by: Matthew Taylor <mataylor@nvidia.com>
…cLab into mataylor/rtx-preset
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
kellyguo11
changed the title
| * **presets=** — environment-specific (domain) preset name passed as | ||
| ``presets=NAME[,NAME,...]`` | ||
| (e.g. ``rgb``, ``depth``, ``single_camera``, ``duo_camera``) | ||
| (e.g. ``rtx``, ``rgb``, ``depth``, ``single_camera``, ``duo_camera``) |
There was a problem hiding this comment.
rtx should be part of the renderer presets and not the environment-specific presets? similar to isaacsim_rtx_renderer... listed under renderer= above
There was a problem hiding this comment.
could we maybe also have another ticket to rename the renderer presets to remove the _renderer suffix in the name?
There was a problem hiding this comment.
rtx should be part of the renderer presets and not the environment-specific presets?
Yes I had it under renderers but changed it back :( , however ticket states preset=rtx should I update the ticket?
If a user specifies presets=rtx, runs physx from isaac sim or uses the kit visualizer, we will use isaac sim RTX.
There was a problem hiding this comment.
I have updated this to renderer=rtx
There was a problem hiding this comment.
thanks! yes let's update the ticket, it might have been created before we refactored the presets logic
| ``--task=<name> --help`` to list them. | ||
|
|
||
| Use ``renderer=rtx`` for automatic RTX selection on tasks that expose the multi-backend renderer selector. | ||
| It uses Isaac Sim RTX with PhysX or the Kit visualizer, and OVRTX for fully kit-less runs. |
There was a problem hiding this comment.
if the user uses ovphyx as the physics backend - should the auto rtx select ovrtx?
There was a problem hiding this comment.
yes I have added an additional test for this as you noted below. 👌
| use_isaac_sim = _uses_isaac_sim_runtime(config_scan, launcher_args) | ||
| renderer_factory = IsaacRtxRendererCfg if use_isaac_sim else OVRTXRendererCfg | ||
|
|
||
| has_auto_camera = False | ||
| for location in auto_rtx_locations: | ||
| if location[0] is None: | ||
| raise ValueError("Automatic RTX renderer placeholders cannot be resolved as the root config.") | ||
| setattr(location[0], location[1], renderer_factory()) | ||
| has_auto_camera = has_auto_camera or location[2] | ||
|
|
||
| if use_isaac_sim: | ||
| config_scan.has_kit_camera = config_scan.has_kit_camera or has_auto_camera | ||
| else: | ||
| config_scan.has_ovrtx = True | ||
| config_scan.needs_kit = config_scan.has_kit_camera or not config_scan.has_kitless_physics |
There was a problem hiding this comment.
Could we add some comments explaining this resolution logic
| assert launcher_args.visualizer == ["kit"] | ||
| assert launcher_args.enable_cameras is True | ||
| assert isinstance(env_cfg.tiled_camera.renderer_cfg, IsaacRtxRendererCfg) | ||
|
|
There was a problem hiding this comment.
do we need a test for if user chooses ovphysx?
rilei-nvidia
left a comment
rilei-nvidia
left a comment
There was a problem hiding this comment.
Thanks for addressing the comments around adding the tests for the ovphysx case, and the clear comments & documentation @mataylor-nvidia!
| * - ``ovrtx_renderer`` | ||
| - OV RTX renderer | ||
| * - ``rtx`` | ||
| - Automatic RTX renderer selection (Isaac Sim RTX for PhysX or Kit visualizer, OVRTX for fully kit-less runs) |
There was a problem hiding this comment.
maybe we can clarify physx --> isaac sim physx
or just simply it as Isaac Sim RTX when running with Isaac Sim, and ovrtx for kitless
| use_isaac_sim = _uses_isaac_sim_runtime(config_scan, launcher_args) | ||
| renderer_factory = IsaacRtxRendererCfg if use_isaac_sim else OVRTXRendererCfg | ||
|
|
||
| # Update every auto RTX placeholder in place, and track whether any of them were |
There was a problem hiding this comment.
is the comment maybe incomplete?
3 participants
Description
Introduce a new general "RTX" preset, where a user does not need to understand the differences and caveats between Isaac Sim RTX vs. ovrtx. As a user, they can specify presets=rtx, and we will have logic to determine whether Isaac Sim RTX or ovrtx should be used. Users can still explicitly specify presets=ovrtx or presets=isaacsim_rtx, and we will produce an error if any invalid combinations are provided (e.g. ovrtx + isaac sim).
If a user specifies presets=rtx, runs physx from isaac sim or uses the kit visualizer, we will use isaac sim RTX.
If a user specifies presets=rtx, does not launch any isaac sim instances (e.g. no kit visualizer and no isaac sim physx), then we can use ovrtx (or whichever option is more performant)
Fixes OMPE-92354
Type of change
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there