Share one timestamped output directory across Experiment chunks

[cvolkcvolk]

Summary

Share timestamped output across chunks

Detailed description

• Expose execute_run(...) as the focused building block for executing one ArenaRunCfg inside a caller-owned SimulationApp.
• Preserve the existing --output_base_dir behavior: every invocation creates one timestamped Experiment directory below the configured base.
• Choose that directory once in the parent and pass it privately to every legacy chunk worker.
• Keep a fresh SimulationApp per chunk while producing one directory and one combined index.html for the whole Experiment.
• Leave OSMO-specific timestamp discovery to the distributed workflow in Distribute Arena Experiment Runs on OSMO #920.

Output layout

Normal and chunked Experiments now use the same layout:

<output_base_dir>/
└── <timestamp>/
    ├── index.html
    ├── run-a/
    │   └── episode_results_rebuild0.jsonl
    └── run-b/
        └── episode_results_rebuild0.jsonl

For example, --output_base_dir ./other-directory writes the Experiment below ./other-directory/<timestamp>/. Chunk workers reuse that timestamped directory rather than creating one directory per chunk.

[arena-review-bot]

🤖 Isaac Lab-Arena Review Bot

Summary

This PR renames the single-Run execution primitive (build_and_run → execute_run) with a clearer docstring on SimulationApp/env ownership, and adds an --experiment_output_directory option (mutually exclusive with --output_base_dir) so managed runners like OSMO can write straight into their allocated directory instead of a timestamped local one. The change is small, well-scoped, and nicely covered by tests. Boundaries hold and the wrapped-env access still goes through env.unwrapped; the report scan uses rglob, so the non-timestamped managed layout works correctly.

Design, Boundaries & Scope

One question, not a blocker: the new experiment_output_layout.py module wraps a single output_dir / run_name expression and, despite a docstring calling it "shared by producers and consumers," has exactly one caller and no second consumer (OSMO passes the directory without importing it; build_report discovers results by rglob). Worth asking whether it earns its own module yet, or could stay inline until a real second consumer appears. (Raised inline.)

Findings

🔵 Improvement: experiment_output_layout.py — single-caller helper module whose "shared contract" has only a producer today; consider inlining until a second consumer exists.

Test Coverage

Good. The new CLI option, the mutually-exclusive rejection, and both branches of _resolve_experiment_output_directory are covered; the OSMO managed-output path asserts the emitted command. All renames are reflected in the existing tests. Minor gap: the new --chunk_size + --experiment_output_directory guard in main() has no regression test — low priority given it is a simple assert.

Verdict

Ship it (one optional simplification question).

[greptile-apps]

Greptile Summary

This PR introduces a deterministic output directory layout for managed Arena Experiment execution, allowing OSMO runners to write directly into their pre-allocated output directory instead of generating a timestamped subdirectory. It also renames build_and_run → execute_run in run_execution.py to better reflect that the caller owns the SimulationApp lifecycle.

• New --experiment_output_directory flag: mutually exclusive with --output_base_dir; when set, the experiment writes directly to the given path without inserting a reverse-dated subdirectory. The OSMO task immediately adopts this flag, passing {{output}} as the value.
• New experiment_output_layout.py module: canonicalises the experiment_dir / run_name path formula so both producers (run_execution) and future consumers share one definition.
• build_and_run → execute_run rename: all call-sites (production and test) updated; improved docstring clarifies SimulationApp ownership semantics.

Confidence Score: 4/5

Safe to merge; the new flag is opt-in, local invocations fall back to the existing timestamped path, and the rename is fully propagated across all call-sites and tests.

The new flag and path resolution are straightforward and well-tested. Two minor concerns remain: shlex.join in experiment_runner_task.py shell-quotes {{output}} to '{{output}}' (inconsistent with policy_runner_task.py and not directly exercised by the test), and the --chunk_size/--experiment_output_directory conflict guard is a bare assert that disappears under -O.

osmo/tasks/experiment_runner_task.py (quoting inconsistency with policy_runner_task.py) and isaaclab_arena/evaluation/experiment_runner.py (bare assert for the --chunk_size conflict guard).

Important Files Changed

Filename	Overview
isaaclab_arena/evaluation/experiment_output_layout.py	New module exposing a single get_experiment_run_output_directory helper that encodes the canonical experiment_dir / run_name layout; trivially correct.
isaaclab_arena/evaluation/experiment_runner.py	Adds _resolve_experiment_output_directory to support an exact output path, always creates the resolved directory, and threads the new option through main(). The --chunk_size/--experiment_output_directory conflict is guarded only by a bare assert rather than a parser error.
isaaclab_arena/evaluation/experiment_runner_cli.py	Converts --output_base_dir and the new --experiment_output_directory into a proper add_mutually_exclusive_group, preventing both from being passed simultaneously at the argparse level.
isaaclab_arena/evaluation/run_execution.py	Renames build_and_run to execute_run with clearer SimulationApp-ownership docs, and uses get_experiment_run_output_directory for run subdirectory resolution. All call-sites updated.
osmo/tasks/experiment_runner_task.py	Switches from --output_base_dir to --experiment_output_directory so OSMO writes directly into its allocated directory; uses shlex.join which will shell-quote {{output}} to '{{output}}', differing from the plain " ".join() pattern in policy_runner_task.py.
isaaclab_arena/tests/test_experiment_runner.py	Adds well-structured unit tests for the new CLI flag, mutual-exclusivity rejection, and both branches of _resolve_experiment_output_directory using monkeypatching.
isaaclab_arena/tests/test_osmo_experiment_workflow.py	New test verifies that the generated OSMO task script uses --experiment_output_directory with {{output}} and omits --output_base_dir.
isaaclab_arena/tests/test_run_execution.py	Mechanical rename of build_and_run to execute_run throughout tests, no logic changes.
isaaclab_arena/tests/test_experiment_hydra.py	Single import and call-site updated from build_and_run to execute_run; no logic changes.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant CLI as experiment_runner CLI
    participant Resolver as _resolve_experiment_output_directory
    participant Runner as execute_experiment
    participant Layout as experiment_output_layout
    participant Run as execute_run

    CLI->>Resolver: output_base_dir, experiment_output_directory
    alt --experiment_output_directory provided (OSMO managed)
        Resolver-->>CLI: Path(exact_experiment_output_directory)
    else default local execution
        Resolver->>Resolver: timestamped_run_dir(output_base_dir)
        Resolver-->>CLI: Path(timestamped_dir)
    end
    CLI->>CLI: experiment_output_directory.mkdir(parents, exist_ok)
    CLI->>Runner: "execute_experiment(cfg, output_dir=experiment_output_directory)"
    loop for each run_config
        Runner->>Layout: get_experiment_run_output_directory(output_dir, run_config.name)
        Layout-->>Runner: output_dir / run_name
        Runner->>Run: execute_run(run_config, run_output_directory)
        Run-->>Runner: ArenaRunResult
    end
    Runner-->>CLI: list[ArenaRunResult]

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"}}}%%
sequenceDiagram
    participant CLI as experiment_runner CLI
    participant Resolver as _resolve_experiment_output_directory
    participant Runner as execute_experiment
    participant Layout as experiment_output_layout
    participant Run as execute_run

    CLI->>Resolver: output_base_dir, experiment_output_directory
    alt --experiment_output_directory provided (OSMO managed)
        Resolver-->>CLI: Path(exact_experiment_output_directory)
    else default local execution
        Resolver->>Resolver: timestamped_run_dir(output_base_dir)
        Resolver-->>CLI: Path(timestamped_dir)
    end
    CLI->>CLI: experiment_output_directory.mkdir(parents, exist_ok)
    CLI->>Runner: "execute_experiment(cfg, output_dir=experiment_output_directory)"
    loop for each run_config
        Runner->>Layout: get_experiment_run_output_directory(output_dir, run_config.name)
        Layout-->>Runner: output_dir / run_name
        Runner->>Run: execute_run(run_config, run_output_directory)
        Run-->>Runner: ArenaRunResult
    end
    Runner-->>CLI: list[ArenaRunResult]

Loading

_{Reviews (1): Last reviewed commit: "Define managed Experiment output layout" | Re-trigger Greptile}

[greptile-apps]

Runtime assertion could be argparse-level validation

The guard against combining --chunk_size and --experiment_output_directory is a bare assert, which is silently disabled when Python runs with -O. The guard would be more robust as an explicit if + parser.error(...) call (or a check in parse_experiment_runner_args) so it fires in all execution modes and produces a user-readable error message consistent with the mutually-exclusive-group error produced by argparse for --output_base_dir/--experiment_output_directory.

[cvolkcvolk]

Closing for now. This PR addresses the separate legacy JSON --chunk_size output TODO, while #920 runs typed single-Run Experiments and no longer depends on it. We can revisit legacy chunk output sharing in a focused PR with a simpler parent/child path contract.