Success series in benchmark schema

[AntoineRichard]

Description

Adds success series in benchmark schema

Type of change

• New feature (non-breaking change which adds functionality)

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

[greptile-apps]

Greptile Summary

This PR extends the benchmark schema from v1.0 to v1.1 by adding an optional LearningCurve success_rate field to the Learning dataclass, and wires each of the four RL training adapters (rsl_rl, rl_games, sb3, skrl) to populate it from their existing SuccessRateTracker.history. The top-level scalar TrainingBundle.success_rate and all PlayBundle fields are intentionally left unchanged.

• Schema / builders: SCHEMA_VERSION bumped to \"1.1\"; a new private _build_learning_curve helper DRYs up curve construction; build_learning() gains an optional success_rate_series parameter — absent or empty input correctly yields None.
• Adapters: all four scripts reorder get_success_tracker() to run before build_learning() so the tracker history can be forwarded; the guard tracker.history if tracker is not None else None handles tasks without a success metric.
• Smoke test: removes the per-library expect_success_rate flag and now asserts a non-None success rate and non-None success curve unconditionally for all four libraries, which conflicts with the previous expect_success_rate=False annotations for rl_games and rsl_rl on the Isaac-Cartpole-Direct task.

Confidence Score: 3/5

The schema and builder changes are clean and additive; the four adapter rewirings are mechanical and correct. The smoke test removes per-library guards that existed specifically because rl_games and rsl_rl do not emit a success metric on the Cartpole task, replacing them with unconditional assertions that would fail in CI for those two libraries.

The core implementation — schema dataclass, builder helper, and adapter wiring — is straightforward and well-tested at the unit level. The risk lives entirely in the smoke test rewrite: removing the expect_success_rate conditional and asserting non-None success rate and success curve for every library means the rl_games and rsl_rl smoke cases will fail unless the Cartpole task was recently updated to emit success metrics in those frameworks (which is not documented in this PR).

scripts/benchmarks/test/test_benchmark_smoke.py — the unconditional success assertions at lines 106-110 need to be guarded or the parametrisation needs confirmation that all four libraries now produce success data on the Cartpole task.

Important Files Changed

Filename	Overview
source/isaaclab/isaaclab/test/benchmark/schema.py	Bumps SCHEMA_VERSION to "1.1" and adds optional `success_rate: LearningCurve
source/isaaclab/isaaclab/test/benchmark/builders.py	Extracts a _build_learning_curve helper and adds success_rate_series parameter to build_learning; absent/empty series correctly produce None.
scripts/benchmarks/test/test_benchmark_smoke.py	Removes the per-library expect_success_rate guard and asserts success rate and success curve unconditionally; this breaks the rl_games and rsl_rl test cases which previously documented no success tracking for Cartpole.
scripts/benchmarks/rsl_rl/benchmark_rsl_rl_train.py	Moves get_success_tracker() before build_learning() and passes tracker.history as success_rate_series; correctly handles None tracker.
scripts/benchmarks/rl_games/benchmark_rl_games_train.py	Same adapter reordering as rsl_rl; correctly passes tracker history to build_learning.
source/isaaclab/test/benchmark/test_builders.py	Adds well-structured tests for populated, absent, empty, and keep_series=False success-rate curves; EMA expected value (0.6) correctly matches the builder implementation.
source/isaaclab/test/benchmark/test_schema.py	Updates round-trip and no-series tests to include a success-rate LearningCurve; assertions are correct and comprehensive.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Training Adapter] --> B[get_success_tracker]
    B -->|tracker not None| C[tracker.history]
    B -->|tracker is None| D[success_rate_series = None]
    C --> E[build_learning]
    D --> E
    E --> F{success_rates non-empty?}
    F -->|Yes| H[_build_learning_curve
final_raw, final_ema, series_per_iter]
    F -->|No| G[Learning.success_rate = None]
    H --> I[Learning.success_rate: LearningCurve]
    I --> J[TrainingBundle v1.1 JSON]
    G --> J
    B -->|has history| K[tracker.tail_mean
TrainingBundle.success_rate scalar]

Loading

%%{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"}}}%%
flowchart TD
    A[Training Adapter] --> B[get_success_tracker]
    B -->|tracker not None| C[tracker.history]
    B -->|tracker is None| D[success_rate_series = None]
    C --> E[build_learning]
    D --> E
    E --> F{success_rates non-empty?}
    F -->|Yes| H[_build_learning_curve
final_raw, final_ema, series_per_iter]
    F -->|No| G[Learning.success_rate = None]
    H --> I[Learning.success_rate: LearningCurve]
    I --> J[TrainingBundle v1.1 JSON]
    G --> J
    B -->|has history| K[tracker.tail_mean
TrainingBundle.success_rate scalar]

Loading

_{Reviews (1): Last reviewed commit: "Remove implementation plan from schema P..." | Re-trigger Greptile}

[greptile-apps]

Unconditional success-rate assertions will fail for rl_games and rsl_rl

The expect_success_rate parameter was removed and the assertions that training_data["success_rate"] is not None, success_curve is not None, and success_curve["series_per_iter"] is truthy are now unconditional. The original parametrisation explicitly documented that rl_games and rsl_rl with Isaac-Cartpole-Direct do not produce a success rate (expect_success_rate=False for both). When neither framework emits a success metric on that task, tracker will be None in each adapter, so success_rate_series will be None, learning.success_rate will be None, and TrainingBundle.success_rate will be None — causing all three assertions to raise at runtime.

[kellyguo11]

was this file intended?