Skip to content

Conversation

@kellyguo11
Copy link
Contributor

@kellyguo11 kellyguo11 commented Dec 19, 2025

Description

TiledCamera has some errors with initialization and reset. also removing --enable_cameras since that's not needed for the warp renderer.

Depends on #4115

Cartpole RGB and Depth can both train to ~300 reward.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

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

@github-actions github-actions bot added bug Something isn't working isaac-lab Related to Isaac Lab team labels Dec 19, 2025
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 19, 2025

Greptile Summary

This PR fixes critical bugs in the TiledCamera initialization and reset flow by migrating from Replicator API to Newton Warp renderer.

Key Changes:

  • TiledCamera Refactor: Replaced Replicator-based rendering with Newton Warp renderer, removing the --enable_cameras flag requirement and simplifying the initialization flow
  • Quaternion Convention Fix: Fixed quat_from_matrix() to correctly order quaternion components (r, i, j, k) and convert from wxyz to xyzw convention
  • Initialization Bug Fix: Fixed sensor initialization flag timing in SensorBase - now only set after successful initialization instead of being set even when exceptions occur
  • Numpy/Torch Conversion: Added proper torch.from_numpy() conversions when reading camera poses from USD to prevent type errors
  • Renderer Reset: Added renderer.reset() calls in camera reset methods to properly clear renderer state
  • Cartpole Updates: Fixed quaternion convention, find_joints() unpacking, and warp-to-torch conversions for joint data

Architecture:
The PR introduces a new renderer abstraction layer with lazy-loading registry pattern, allowing TiledCamera to work with different rendering backends. The Newton Warp renderer uses the TiledCameraSensor from the newton package for efficient parallel rendering.

Confidence Score: 4/5

  • This PR is mostly safe to merge with one clarification needed about the removed velocity write
  • The changes fix real bugs (initialization flag timing, quaternion convention, numpy/torch conversion) and successfully refactor TiledCamera to use Newton renderer. The PR author reports successful training (cartpole reaching ~300 reward). One minor concern: removal of write_root_velocity_to_sim() may be unintentional and should be verified.
  • Check cartpole_camera_env.py line 263 - verify whether removing the root velocity write during reset was intentional

Important Files Changed

Filename Overview
source/isaaclab/isaaclab/sensors/camera/tiled_camera.py Refactored from Replicator API to Newton Warp renderer, removed --enable_cameras flag check, added renderer reset in reset method, simplified initialization
source/isaaclab/isaaclab/utils/math.py Fixed quaternion conversion order and added wxyz to xyzw conversion for correct quaternion convention
source/isaaclab/isaaclab/sensors/camera/camera.py Added renderer reset call, fixed numpy to torch conversion for poses/quaternions, added renderer_type to string output
source/isaaclab/isaaclab/sensors/sensor_base.py Fixed initialization flag timing - now set after successful initialization, added early return in update if not initialized
source/isaaclab_tasks/isaaclab_tasks/direct/cartpole/cartpole_camera_env.py Fixed camera rotation quaternion convention from (w,x,y,z) to (x,y,z,w), added warp to torch conversions for joint data, fixed find_joints unpacking

Sequence Diagram

sequenceDiagram
    participant Env as Environment
    participant TC as TiledCamera
    participant SB as SensorBase
    participant Renderer as NewtonWarpRenderer
    participant Newton as TiledCameraSensor

    Note over Env,Newton: Initialization Flow
    Env->>TC: __init__(cfg)
    TC->>SB: Register initialization callback
    
    Env->>Env: sim.play()
    Env->>TC: _initialize_callback()
    TC->>TC: _initialize_impl()
    TC->>SB: _initialize_impl() (parent)
    
    TC->>TC: Check renderer_type == "newton_warp"
    TC->>Renderer: __init__(NewtonWarpRendererCfg)
    TC->>Renderer: initialize()
    Renderer->>Newton: TiledCameraSensor.alloc()
    Renderer->>Renderer: Setup camera rays & buffers
    TC->>TC: Set _is_initialized = True
    
    Note over Env,Newton: Update/Render Flow
    Env->>TC: update(dt)
    TC->>TC: _update_buffers_impl(env_ids)
    TC->>TC: _update_poses(env_ids)
    TC->>TC: Get world poses & convert to torch
    TC->>Renderer: render(pos, quat, intrinsics)
    Renderer->>Renderer: Convert to OpenGL convention
    Renderer->>Newton: TiledCameraSensor.render()
    Newton-->>Renderer: RGB & Depth buffers
    Renderer-->>TC: Output buffers (wp.array)
    TC->>TC: Convert wp.array to torch
    
    Note over Env,Newton: Reset Flow
    Env->>TC: reset(env_ids)
    TC->>SB: reset(env_ids) (parent)
    TC->>TC: Reset frame counters
    TC->>Renderer: reset()
    Renderer->>Renderer: Clear state (no-op currently)
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional Comments (1)

  1. source/isaaclab_tasks/isaaclab_tasks/direct/cartpole/cartpole_camera_env.py, line 263 (link)

    style: The write_root_velocity_to_sim() call was removed. Check whether this is intentional - if the velocities should be reset to zero on environment reset, they may not be properly cleared now. Was removing the root velocity write intentional, or should velocities be reset to default_root_state[:, 7:] on environment reset?

13 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@kellyguo11 kellyguo11 merged commit 5efe0cf into isaac-sim:dev/newton Dec 19, 2025
3 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants