IsaacLab icon indicating copy to clipboard operation
IsaacLab copied to clipboard

[wip] Add initial design for Visualizers

Open matthewtrepte opened this issue 1 month ago • 1 comments

Description

Initial design to support multiple visualizers with Isaac Lab 3.0

The visualizers

  • OV Visualizer
  • Newton OpenGL Visualizer
  • Newton Rerun Visualizer

Type of change

  • 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

matthewtrepte avatar Nov 08 '25 03:11 matthewtrepte

Greptile Overview

Greptile Summary

This PR introduces a multi-visualizer architecture for Isaac Lab 3.0, enabling support for Newton OpenGL, Omniverse, and Rerun visualizers through a factory pattern with scene data providers. The Newton visualizer implementation is functional, but the Rerun and Omniverse visualizers contain critical bugs.

Key Changes

  • Added abstract Visualizer base class with lifecycle methods (initialize(), step(), close(), is_running())
  • Implemented SceneDataProvider abstraction to decouple physics backends from visualizers
  • Created visualizer factory registry pattern for extensibility
  • Integrated visualizers into SimulationContext with pause/resume controls
  • Updated RL scripts to use new enable_visualizers() utility

Critical Issues Found

  • Rerun visualizer: Syntax error in config (/tmp/test.rrd unquoted), incorrect attribute access (cfg.enable_markers should be self.cfg.enable_markers)
  • Simulation context: Potential infinite loop in training pause handler (line 625-628) with no timeout mechanism
  • OV visualizer: Stub implementation missing full functionality
  • enable_visualizers(): Function doesn't create default Newton visualizer when None configured, contradicting its docstring

Architecture Strengths

  • Clean separation of concerns with abstract base classes
  • Factory pattern enables easy addition of new visualizers
  • Scene data provider abstraction supports multiple physics backends
  • Well-documented lifecycle and pause semantics

Confidence Score: 2/5

  • PR has critical runtime errors in Rerun visualizer and potential simulation hangs
  • The architecture and Newton visualizer are solid (score 4), but multiple syntax errors and logic bugs in Rerun visualizer will cause immediate runtime failures. The infinite loop in simulation context pause handling poses a reliability risk. These issues must be fixed before merging.
  • source/isaaclab/isaaclab/sim/visualizers/rerun_visualizer.py and rerun_visualizer_cfg.py require immediate fixes for syntax errors and attribute references. simulation_context.py needs timeout logic for pause handling.

Important Files Changed

File Analysis

Filename Score Overview
source/isaaclab/isaaclab/sim/visualizers/newton_visualizer.py 3/5 Functional Newton visualizer with custom viewer subclass, but silently suppresses rendering exceptions
source/isaaclab/isaaclab/sim/visualizers/rerun_visualizer.py 1/5 Contains multiple syntax errors and undefined attribute references throughout implementation
source/isaaclab/isaaclab/sim/visualizers/ov_visualizer.py 2/5 Stub implementation missing all required abstract methods from base class
source/isaaclab/isaaclab/sim/simulation_context.py 2/5 Visualizer integration with potential infinite loop in pause handling and inconsistent rendering pause behavior
source/isaaclab/isaaclab/sim/visualizers/rerun_visualizer_cfg.py 1/5 Syntax error in default value - unquoted string literal will cause NameError

Sequence Diagram

sequenceDiagram
    participant Script as RL Script
    participant SimCtx as SimulationContext
    participant Provider as SceneDataProvider
    participant VizCfg as VisualizerCfg
    participant Viz as Visualizer
    participant Newton as NewtonManager

    Script->>SimCtx: __init__(cfg)
    SimCtx->>Provider: create NewtonSceneDataProvider()
    
    Script->>SimCtx: reset()
    SimCtx->>Newton: start_simulation()
    SimCtx->>Newton: initialize_solver()
    SimCtx->>SimCtx: initialize_visualizers()
    
    SimCtx->>VizCfg: for each cfg in visualizers
    VizCfg->>Viz: create_visualizer()
    Note over VizCfg,Viz: Factory pattern via registry
    
    SimCtx->>Provider: get_scene_data()
    Provider-->>SimCtx: {model, state, usd_stage, metadata}
    SimCtx->>Viz: initialize(scene_data)
    Viz->>Viz: validate physics backend
    Viz->>Viz: create viewer (Newton/Rerun/OV)
    
    loop Training Loop
        Script->>SimCtx: step(render=True)
        SimCtx->>Newton: step()
        SimCtx->>SimCtx: step_visualizers(dt)
        
        SimCtx->>Viz: is_running()?
        alt Visualizer closed
            SimCtx->>Viz: close()
            SimCtx->>SimCtx: remove from list
        else Visualizer paused (training)
            loop While paused
                SimCtx->>Viz: step(0.0, provider)
                SimCtx->>Viz: is_training_paused()?
            end
        else Visualizer paused (rendering)
            Note over SimCtx,Viz: Skip step, continue
        else Normal step
            SimCtx->>Provider: get_state()
            Provider->>Newton: get current state
            Provider-->>SimCtx: state
            SimCtx->>Viz: step(dt, provider)
            Viz->>Provider: get_state()
            Viz->>Viz: render frame
        end
    end
    
    Script->>SimCtx: close()
    SimCtx->>SimCtx: close_visualizers()
    SimCtx->>Viz: close()

greptile-apps[bot] avatar Nov 11 '25 05:11 greptile-apps[bot]

Let's get it in and worry about these later.

AntoineRichard avatar Dec 01 '25 11:12 AntoineRichard