Unify env random seeding using ManagerBasedRLEnv seed

[peterd-NV]

Summary

Arena was previously setting the seed using a custom set_seed function in Arena. The seeding was inconsistent and not always being set (for example it was not used for batched eval). This gave non-deterministic results.

This PR removes the inconsistent env seeding and uses Isaac Lab's built in env random seeding mechanism. Any time a user provides a seed via CLI, it is passed to the arena environment builder which sets it in the env_cfg. Isaac Lab then handles setting all global seeds.

Note: Isaac Lab's seed mechanism sets all of the seeds that Arena was setting plus additional seeds for replicator and warp. It is more comprehensive than what Arena had.

[greptile-apps]

Greptile Summary

This PR replaces Arena's bespoke set_seed helper with Isaac Lab's built-in seeding mechanism. By assigning the CLI seed to env_cfg.seed (after the env_cfg_callback) inside build_env_cfg, all code paths through ArenaEnvBuilder — including paths that previously skipped seeding — now receive a consistent seed. The distributed per-rank seed offset is also moved earlier so it is in place before make_registered_and_return_cfg reads from args_cli.seed.

• arena_env_builder.py: Adds env_cfg.seed = self.args.seed after the env-cfg callback, delegating all RNG initialization (torch, numpy, random, replicator, warp) to Isaac Lab.
• policy_runner.py: Removes the post-build set_seed call; moves the per-rank seed += local_rank offset to before the ArenaEnvBuilder is constructed so the correct seed flows into env_cfg.seed.
• utils/random.py: Deletes the now-unused set_seed function and its gymnasium/numpy imports.

Confidence Score: 5/5

Safe to merge — the change is a focused seeding refactor with no functional regressions on any known code path.

The seed is correctly assigned after the env-cfg callback runs, all callers go through the standard CLI parser that always defines --seed, the per-rank distributed offset is now applied before the environment is constructed (rather than after), and the deleted set_seed helper has no remaining callers in the codebase. Isaac Lab's built-in seeding is a superset of what Arena's helper provided.

No files require special attention.

Important Files Changed

Filename	Overview
isaaclab_arena/environments/arena_env_builder.py	Adds env_cfg.seed = self.args.seed after the env-cfg callback; seed is always available via the standard CLI parser (default 42).
isaaclab_arena/evaluation/policy_runner.py	Moves per-rank seed offset before ArenaEnvBuilder construction, removes the now-redundant post-build set_seed call.
isaaclab_arena/utils/random.py	Deletes set_seed and its gymnasium/numpy imports; remaining utilities (get_random_rotation, get_rngs) are unaffected.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant CLI as CLI / args_cli
    participant PR as policy_runner.main()
    participant AB as ArenaEnvBuilder
    participant IL as Isaac Lab (ManagerBasedRLEnv)

    PR->>CLI: "parse args (--seed default=42)"
    alt distributed
        PR->>CLI: "args_cli.seed += local_rank"
    end
    PR->>AB: get_arena_builder_from_cli(args_cli)
    PR->>AB: make_registered_and_return_cfg()
    AB->>AB: compose_manager_cfg()
    Note over AB: env_cfg_callback (if set)
    AB->>AB: "env_cfg.seed = self.args.seed"
    AB->>IL: "gym.make(name, cfg=env_cfg)"
    IL->>IL: seed(env_cfg.seed) torch/numpy/random/replicator/warp

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 CLI / args_cli
    participant PR as policy_runner.main()
    participant AB as ArenaEnvBuilder
    participant IL as Isaac Lab (ManagerBasedRLEnv)

    PR->>CLI: "parse args (--seed default=42)"
    alt distributed
        PR->>CLI: "args_cli.seed += local_rank"
    end
    PR->>AB: get_arena_builder_from_cli(args_cli)
    PR->>AB: make_registered_and_return_cfg()
    AB->>AB: compose_manager_cfg()
    Note over AB: env_cfg_callback (if set)
    AB->>AB: "env_cfg.seed = self.args.seed"
    AB->>IL: "gym.make(name, cfg=env_cfg)"
    IL->>IL: seed(env_cfg.seed) torch/numpy/random/replicator/warp

Loading

_{Reviews (3): Last reviewed commit: "address comments" | Re-trigger Greptile}

[isaaclab-review-bot]

Review Summary

Clean refactoring that unifies environment seeding through Isaac Lab's built-in ManagerBasedRLEnv.seed mechanism, eliminating the inconsistent custom set_seed utility. The approach is sound — centralizing seed propagation in compose_manager_cfg ensures all code paths through ArenaEnvBuilder (including batched eval) get consistent seeding.

---

Findings

1. 🟡 Seed mutation of args_cli is a side effect that persists beyond its intended scope

File: isaaclab_arena/evaluation/policy_runner.py (line ~172)

if args_cli.seed is not None:
    args_cli.seed += local_rank

This mutates args_cli.seed in-place before passing it to get_arena_builder_from_cli. Since args_cli is a namespace object shared throughout main(), any subsequent code that reads args_cli.seed will see the modified value (base_seed + local_rank) rather than the original user-supplied seed. While nothing currently reads it afterwards, this is fragile — future code (e.g., logging the user's requested seed, or a second env build) would silently get the wrong value.

Suggestion: Use a local variable instead:

env_seed = args_cli.seed
if env_seed is not None and is_distributed(args_cli):
    env_seed += local_rank
args_cli.seed = env_seed  # or pass env_seed explicitly

Or at minimum, add a comment noting the intentional mutation.

2. 🟡 env_cfg.seed is set before the env_cfg_callback, which could silently override it

File: isaaclab_arena/environments/arena_env_builder.py (line ~295-296)

# Set seed for Isaac Lab env.
env_cfg.seed = getattr(self.args, "seed", None)

# Apply the environment configuration callback if it is set
if self.arena_env.env_cfg_callback is not None:
    env_cfg = self.arena_env.env_cfg_callback(env_cfg)

The seed is set before the callback runs. If any env_cfg_callback also sets env_cfg.seed (e.g., for environment-specific reproducibility requirements), it will silently override the CLI-provided seed without warning. Conversely, if the intent is that CLI --seed should always be authoritative, consider moving the seed assignment after the callback (similar to how --presets is applied after the callback with the comment "the user's CLI choice is the final authority").

3. 🟢 Minor: The --seed CLI default is 42, so getattr(self.args, "seed", None) will never return None

File: isaaclab_arena/environments/arena_env_builder.py (line ~296)

The CLI defines --seed with default=42, meaning args.seed will always be an integer (never None) when invoked through standard CLI paths. The getattr(..., None) fallback only activates if self.args is a manually constructed namespace without a seed attribute. This isn't a bug, but the code reads as if None is a normal case. A brief comment clarifying the intent (e.g., "None when seed is explicitly disabled or args built programmatically") would improve readability.

4. 🟢 Dead code removal is incomplete — set_seed function body removed but not the function itself

File: isaaclab_arena/utils/random.py

The diff removes the set_seed function entirely, which is correct since no other module imports it (confirmed: only policy_runner.py imported it). The cleanup is complete. ✅ (No action needed — this is confirmation that the removal is safe.)

5. 🟡 No test coverage for the new seeding path

The PR removes the set_seed call but doesn't add tests verifying that env_cfg.seed is correctly propagated through compose_manager_cfg. Given that the PR description mentions this fixes non-deterministic results in batched eval, a regression test (even a simple unit test asserting compose_manager_cfg().seed == args.seed) would protect against future refactors accidentally breaking seed propagation.

---

Overall Assessment

This is a well-motivated simplification. Isaac Lab's native seeding is indeed more comprehensive (covers replicator + warp in addition to torch/numpy/random). The main concerns are:

1. The in-place mutation pattern for distributed seeds (minor robustness issue)
2. Ordering relative to env_cfg_callback (design decision worth documenting)
3. Lack of test coverage for the new path

None of these are blocking. Nice cleanup! 👍

---

Update (commit f940f58): ✅ Finding #2 addressed — seed assignment moved after env_cfg_callback, ensuring CLI --seed is always authoritative (matching --presets pattern). No new issues in this commit. Remaining findings (#1 args mutation, #3 minor readability, #5 test coverage) are non-blocking and unchanged.

[xyao-nv]

Thanks for fixing!

[xyao-nv]

How about for the seed you modify here, we call it runtime_seed? To differentiate it from placement_seed.

[alexmillane]

Looks good to me!!! Thanks for doing this!

We still have this variable placement_seed around. I wonder if we can also simplify by getting rid of that? But that's a question for another time.

[xyao-nv]

Looks good to me!!! Thanks for doing this!

We still have this variable placement_seed around. I wonder if we can also simplify by getting rid of that? But that's a question for another time.

I think Peter's idea is placement seed is at build time, and this seed is at run time. Leave it for future if unifying these two explicitly is needed.