Skip to content

Conversation

@kellyguo11
Copy link
Contributor

Description

Allows running benchmark_non_rl, benchmark_rsl_rl, and benchmark_rlgames scripts without launching Isaac Sim. This will allow us to gather similar timing data for startup time and FPS metrics without using the Isaac Sim benchmarking extension.

Without Isaac Sim, we are not able to capture Render and Physics simulation data directly from the scripts, but there's other benchmarking work going on where we can inject timers into the code to capture these.

By default, it will not launch Isaac Sim unless --kit is added to the command.

Type of change

  • New feature (non-breaking change which adds functionality)
  • Breaking change (existing functionality will not work without user modification)

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 documentation Improvements or additions to documentation isaac-lab Related to Isaac Lab team labels Jan 31, 2026
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 31, 2026

Greptile Overview

Greptile Summary

This PR adds support for running benchmark scripts (benchmark_non_rl, benchmark_rsl_rl, and benchmark_rlgames) without launching Isaac Sim by introducing a --kit flag that defaults to False. When --kit is not specified, the scripts use a new standalone benchmarking infrastructure that replicates the functionality of isaacsim.benchmark.services.

Key Changes

  • New --kit flag: Controls whether Isaac Sim is launched. Default is False (standalone mode).
  • Standalone benchmark infrastructure: New StandaloneBenchmark class in standalone_benchmark.py provides benchmarking capabilities without Isaac Sim dependencies, including system metrics collection (CPU, memory, GPU via multiple detection methods).
  • Conditional imports: AppLauncher and Isaac Sim-specific modules are only imported when --kit is enabled, avoiding import errors in standalone mode.
  • Unified logging interface: benchmark_utils.py provides factory functions that return appropriate logging implementations for both modes.
  • Breaking change: Default behavior now runs without Isaac Sim unless --kit is explicitly added.

Issues Found

  • Minor: Duplicate AppLauncher imports in all three benchmark scripts when --kit is enabled
  • Minor: Duplicate copyright header in benchmark_rsl_rl.py

Confidence Score: 4/5

  • This PR is safe to merge after addressing the duplicate imports
  • The implementation is well-structured with a clean separation between kit and standalone modes. The standalone benchmark infrastructure is comprehensive and handles edge cases (GPU detection failures, missing dependencies). The issues found are minor style problems (duplicate imports and copyright header) that don't affect functionality.
  • All three benchmark scripts have duplicate imports that should be cleaned up before merging

Important Files Changed

Filename Overview
scripts/benchmarks/benchmark_non_rl.py Added --kit flag to conditionally launch Isaac Sim, with duplicate import that should be removed
scripts/benchmarks/benchmark_rlgames.py Added --kit flag to conditionally launch Isaac Sim, with duplicate import that should be removed
scripts/benchmarks/benchmark_rsl_rl.py Added --kit flag, duplicate copyright header and duplicate import that should be fixed
scripts/benchmarks/utils/standalone_benchmark.py New standalone benchmark infrastructure with comprehensive system metrics collection
scripts/benchmarks/utils/benchmark_utils.py Factory functions for creating kit and standalone logging functions

Sequence Diagram

sequenceDiagram
    participant User
    participant Script as Benchmark Script
    participant ArgParser as Argument Parser
    participant AppLauncher as AppLauncher (Kit)
    participant BenchmarkKit as BaseIsaacBenchmark (Kit)
    participant BenchmarkStandalone as StandaloneBenchmark
    participant Env as Gym Environment
    
    User->>Script: Run benchmark with/without --kit flag
    Script->>ArgParser: Parse arguments
    
    alt --kit flag present
        ArgParser->>ArgParser: Import AppLauncher
        ArgParser->>ArgParser: Add AppLauncher args
        Script->>AppLauncher: Launch Isaac Sim
        AppLauncher-->>Script: simulation_app
        Script->>BenchmarkKit: Create BaseIsaacBenchmark
        BenchmarkKit-->>Script: benchmark instance
    else --kit flag absent (default)
        Script->>BenchmarkStandalone: Create StandaloneBenchmark
        BenchmarkStandalone-->>Script: benchmark instance
    end
    
    Script->>Script: Import required libraries
    Script->>Env: Create gym environment
    Env-->>Script: env
    
    Script->>Script: Set benchmark phase
    
    loop For each frame/iteration
        Script->>Env: env.step(action)
        Env-->>Script: observation, reward, done, info
        Script->>Script: Record timing metrics
    end
    
    Script->>Script: store_measurements()
    Script->>Script: Log metrics (startup, runtime)
    Script->>Script: benchmark.stop()
    
    alt --kit flag present
        Script->>AppLauncher: Close simulation_app
    end
    
    Script-->>User: Benchmark results saved
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.

5 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

Comment on lines 63 to 64
if args_cli.kit:
from isaaclab.app import AppLauncher
Copy link
Contributor

Choose a reason for hiding this comment

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

AppLauncher is imported twice when --kit is enabled (lines 44 and 64). Remove this duplicate import.

Suggested change
if args_cli.kit:
from isaaclab.app import AppLauncher
if args_cli.kit:

Comment on lines 63 to 64
if args_cli.kit:
from isaaclab.app import AppLauncher
Copy link
Contributor

Choose a reason for hiding this comment

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

AppLauncher is imported twice when --kit is enabled (lines 44 and 64). Remove this duplicate import.

Suggested change
if args_cli.kit:
from isaaclab.app import AppLauncher
if args_cli.kit:

Comment on lines 79 to 80
if args_cli.kit:
from isaaclab.app import AppLauncher
Copy link
Contributor

Choose a reason for hiding this comment

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

AppLauncher is imported twice when --kit is enabled (lines 60 and 80). Remove this duplicate import.

Suggested change
if args_cli.kit:
from isaaclab.app import AppLauncher
if args_cli.kit:

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 31, 2026

Additional Comments (1)

scripts/benchmarks/benchmark_rsl_rl.py
Duplicate copyright header

# SPDX-License-Identifier: BSD-3-Clause

@AntoineRichard
Copy link
Collaborator

AntoineRichard commented Feb 2, 2026

@kellyguo11 I think there is some overlapp with #4497 wrt to the benchmarking too. I don't think we shoud even rely on IS tools this way we don't have to follow their template / format.

@kellyguo11
Copy link
Contributor Author

@kellyguo11 I think there is some overlapp with #4497 wrt to the benchmarking too. I don't think we shoud even rely on IS tools this way we don't have to follow their template / format.

I'm good with that as well if we can capture all the necessary data around simulation and rendering. Sounds like Isaac Sim is also making some breaking changes to their benchmarking tool, so we can also save some work to migrate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants