Pre and post physics renderer initialization

[pbarejko]

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.
List any dependencies that are required for this change.

Fixes # (issue)

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (existing functionality will not work without user modification)
• Documentation update

Screenshots

Please attach before and after screenshots of the change if applicable.

Checklist

• I have read and understood the contribution guidelines
• I have run the pre-commit checks 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

[isaaclab-review-bot]

Code Review: Pre-Initialize Renderers

Thank you for this PR! The approach of front-loading renderer initialization to ensure deterministic ordering is a solid architectural improvement. Here are my observations:

✅ Strengths

1. Deterministic Initialization Order: Moving renderer backend creation before SimulationContext.reset() ensures consistent initialization sequence across runs, which is important for debugging and reproducibility.

2. Idempotent Design: The use of RenderContext.get_renderer() with config equality and the seen set tracking ensures the method can be safely called multiple times.

3. Consistent Integration: The method is properly called from all relevant environment classes (DirectRLEnv, DirectMARLEnv, ManagerBasedEnv, LeappDeploymentEnv, and the experimental Warp variants).

🔍 Suggestions

1. Consider Debug Logging (interactive_scene.py):
   The method silently iterates sensors without renderer_cfg. Consider adding debug-level logging to aid troubleshooting:

   import logging
   logger = logging.getLogger(__name__)
   # ... in initialize_renderers():
   logger.debug("Initialized %d unique renderer backends for %d sensors", len(backends), sensor_count)

2. Assertion Message (ovrtx_renderer.py, line 171):
   The assertion could be more descriptive for debugging:

   assert self._renderer is not None, "OVRTX Renderer creation failed - check log_file_path and log_level config"

3. CI Status: The pre-commit and changelog fragment checks are currently failing. Please verify these pass before merge.

📝 Notes

• The decision to defer prepare_stage() to camera initialization (documented in the docstring) is appropriate since it needs the final num_envs value.
• The OVRTX renderer change from lazy initialization in initialize() to eager initialization in __init__ aligns well with the pre-initialization pattern.

Overall, this is a well-structured change that improves initialization predictability. 👍

[greptile-apps]

Greptile Summary

This PR front-loads renderer backend creation by introducing InteractiveScene.initialize_renderers(), which walks registered sensors and eagerly constructs backends via RenderContext.get_renderer() before the first sim.reset(). The OVRTX renderer's Renderer instantiation is correspondingly moved from prepare_stage() into __init__().

• InteractiveScene.initialize_renderers() is a new idempotent method that deduplicates backends by object identity and is now called from all four env base classes (DirectRLEnv, DirectMARLEnv, ManagerBasedEnv, LeappDeploymentEnv) and their Warp variants after scene construction, making renderer construction order deterministic.
• OVRTXRenderer.__init__ now creates the Renderer(OVRTX_CONFIG) object immediately; prepare_stage() retains responsibility for USD export and initialize() for loading/binding, so the staged pipeline is preserved.
• The return value of initialize_renderers() (the list of unique backends) is discarded at every call site; the method is used purely for its side effect of registering backends with the shared RenderContext.

Confidence Score: 4/5

Safe to merge. The init-time renderer construction and the new initialize_renderers() call site in every env class are correctly ordered — sensor registration always precedes the renderer walk, and RenderContext is always available at that point.

The core logic is sound: render_context is always a live RenderContext() instance (initialized in SimulationContext.__init__), _setup_scene() is called before initialize_renderers() in every Direct env, and manager-based envs populate sensors during InteractiveScene construction so no sensors are missed. The two minor observations — unconditional ctx fetch before the loop guard, and an assert that optimized builds strip — prevent a clean 5.

interactive_scene.py and ovrtx_renderer.py are the two files where the substantive logic lives and are worth a second look; the env call-site changes are mechanical and low-risk.

Important Files Changed

Filename	Overview
source/isaaclab/isaaclab/scene/interactive_scene.py	Adds initialize_renderers() method that walks _sensors, deduplicates by id(backend), and returns the list of registered backends; core of this PR.
source/isaaclab_ov/isaaclab_ov/renderers/ovrtx_renderer.py	Moves Renderer(OVRTX_CONFIG) instantiation from prepare_stage() into __init__(), making renderer creation happen at backend-construction time rather than at stage-preparation time.
source/isaaclab/isaaclab/envs/direct_rl_env.py	Calls self.scene.initialize_renderers() after _setup_scene() inside the existing use_stage block — ordering is correct.
source/isaaclab/isaaclab/envs/direct_marl_env.py	Same one-line addition as direct_rl_env.py, after _setup_scene(), inside use_stage block.
source/isaaclab/isaaclab/envs/manager_based_env.py	Calls initialize_renderers() immediately after InteractiveScene(self.cfg.scene); correct because manager-based sensors are fully registered during scene construction.
source/isaaclab/isaaclab/envs/leapp_deployment_env.py	Calls initialize_renderers() inside the first use_stage block, before sim.reset(), matching the intended placement.
source/isaaclab_experimental/isaaclab_experimental/envs/direct_rl_env_warp.py	Calls initialize_renderers() after _setup_scene(), consistent with the non-Warp counterpart.
source/isaaclab_experimental/isaaclab_experimental/envs/manager_based_env_warp.py	Calls initialize_renderers() inside the use_stage block after InteractiveScene() construction; no _setup_scene() needed since sensors come from scene config.

Sequence Diagram

sequenceDiagram
    participant Env as Env.__init__
    participant IS as InteractiveScene
    participant RC as RenderContext
    participant OR as OVRTXRenderer

    Env->>IS: InteractiveScene(cfg.scene)
    IS-->>Env: scene (sensors populated)
    Env->>IS: _setup_scene() [Direct envs only]
    Env->>IS: initialize_renderers()
    IS->>RC: get_renderer(rcfg) per sensor
    RC->>OR: OVRTXRenderer.__init__(cfg)
    OR->>OR: Renderer(OVRTX_CONFIG) [was in prepare_stage]
    OR-->>RC: backend
    RC-->>IS: backend (deduplicated)
    IS-->>Env: [backends list, unused]

    Note over Env,OR: Later — first Camera._initialize_impl
    Env->>OR: prepare_stage(stage, num_envs)
    OR->>OR: export_stage_for_ovrtx(...)
    Env->>OR: initialize(spec)
    OR->>OR: open_usd / add_usd, bind cameras

Loading

_{Reviews (1): Last reviewed commit: "Pre-Initialize renderers" | Re-trigger Greptile}

[greptile-apps]

assert can be silently stripped in optimized builds

assert self._renderer is disabled when Python is run with -O or -OO, so a None or falsy renderer would go undetected until a subsequent attribute access raises an AttributeError. This line was pre-existing in prepare_stage() but is now in __init__(), making it the only guard between a failed construction and all later renderer calls. A direct if not self._renderer: raise RuntimeError(...) would be more robust.

[nvsekkin]

question: is there a reason not to implement this for all backends?

[nvsekkin]

question: should this live inside the new initialize() method form BaseRenderer?