variations in eval runner by alexmillane · Pull Request #779 · isaac-sim/IsaacLab-Arena

alexmillane · 2026-06-11T12:11:04Z

Summary

Connects the eval_runner.py to the variations.

Detailed description

Expands our Job/json to add a field for variations.
Passes those variations into the ArenaEnvBuilder for building.
Added a small example droid_pnp_variations_config.json that demonstrates how to use this feature and added it to the (placeholder) docs.

Introduce the variation framework: a VariationBase with build-time and run-time flavors, a SamplerBase abstraction with uniform and categorical samplers, and two concrete variations -- HDR dome-light image selection (build-time) and camera extrinsic decalibration (run-time). Variations attach to any Asset (scene objects or embodiments) and are collected by ArenaEnvBuilder, which applies build-time variations before scene composition and folds run-time variations into the event manager cfg. All variations default to disabled so existing envs are unchanged. The variations package exposes its API lazily (PEP 562 __getattr__): camera_decalibration pulls in torch and isaaclab.sensors, and importing that pair before the SimulationApp launches corrupts USD's Python bindings. Lazy exports keep importing the package -- or its lightweight base/sampler submodules -- safe at module-load time (e.g. pytest collection). Signed-off-by: alex <amillane@nvidia.com>

…riations

- Rename camera_decalibration.py -> camera_decalibration_variation.py - Drop package-level re-exports; import concrete classes from submodules - Remove sampler/variation listener plumbing (deferred to a follow-up MR) - Hoist a compulsory sampler_cfg field onto VariationBaseCfg and rename the per-variation sampler field to sampler_cfg - Replace UniformSampler.event_shape with an abstract SamplerBase.shape_per_sample - Drop the camera variation's unused mode field (all variations fire on reset) - Move attribute docs onto each member and trim docstrings to one line Signed-off-by: alex <amillane@nvidia.com>

…n order.

…Cfg.

…ure/variations

Agents drove the container with bare `docker exec` (root), while users attach via run_docker.sh as their host user (su $(id -un)). Root-run commands left root-owned files in shared caches (/tmp/Assets, ~/.config) that the host user could not read, breaking their later interactive runs. Update AGENTS.md and the dev-container/run-tests skills to route every in-container command through `su $(id -un) -c`, mirroring run_docker.sh. Signed-off-by: alex <amillane@nvidia.com>

…ariations_in_eval_runner

…re/variations_in_eval_runner

alexmillane

Self review.

alexmillane · 2026-06-12T07:28:54Z

+    Mirrors policy_runner's ``--list-variations`` but, because each eval job defines its own
+    environment via ``arena_env_args``, the catalogue is printed per job. Each job's own
+    ``variations`` overrides are applied so the catalogue reflects what the run would use.
+    Must be called inside a ``SimulationAppContext``.


Remove long docstring

alexmillane · 2026-06-12T07:29:13Z

 registered set.  The ``sampler_cfg.low`` / ``sampler_cfg.high`` vectors widen the camera
 extrinsics jitter range to ±10 mm per axis.

+Configuring variations in an eval jobs config


Move this section below the one below, to the end of the file.

alexmillane · 2026-06-12T07:29:38Z

    with open(args_cli.eval_jobs_config, encoding="utf-8") as f:
        eval_jobs_config = json.load(f)

+    # Print the variations catalogue for each job's environment and exit, without running anything.


shorten to "Print the variations catalogue for each job's environment and exit."

alexmillane · 2026-06-12T07:30:42Z

Delete this file. This was deleted on main and is being accidentally readded here.

alexmillane · 2026-06-12T07:31:44Z

+    """Drive the eval-config path (Job.from_dict -> load_env) and check the override reached a manager.
+
+    A runtime variation enabled via the job's ``variations`` block must surface as an
+    ``EventTermCfg`` in the composed ``env_cfg.events`` manager. We use the droid wrist-camera
+    extrinsics variation, which produces the event term ``wrist_camera_extrinsics_variation``.
+    """


Shorten to: "Enable a wrist camera extrinsics variation and check that it shows up as an event term in the cfg."

alexmillane · 2026-06-12T07:32:19Z

+        "num_steps": NUM_STEPS,
+        "policy_type": "zero_action",
+        "policy_config_dict": {},
+        "variations": {"droid_abs_joint_pos": {f"camera_extrinsics_{camera_name}": {"enabled": True}}},


Put a comment before this line "Enabling wrist camera extrinsics variation."

alexmillane · 2026-06-12T07:34:51Z

+def test_job_from_dict_with_variations():
+    """Job.from_dict populates job.variations from a nested variations block."""
+    job_dict = {
+        "name": "job_with_variations",
+        "arena_env_args": {"environment": "env1"},
+        "policy_type": "zero_action",
+        "policy_config_dict": {},
+        "variations": {"light": {"hdr_image": {"enabled": True}}},
+    }
+
+    job = Job.from_dict(job_dict)
+
+    assert job.variations == ["light.hdr_image.enabled=true"]


Combine this test with the test test_job_convert_variations_dict_to_hydra_overrides above.

alexmillane · 2026-06-12T07:35:03Z

+def test_job_from_dict_without_variations_defaults_empty():
+    """A job without a variations field defaults to no overrides."""
+    job_dict = {
+        "name": "job_without_variations",
+        "arena_env_args": {"environment": "env1"},
+        "policy_type": "zero_action",
+        "policy_config_dict": {},
+    }
+
+    job = Job.from_dict(job_dict)
+
+    assert job.variations == []


Combine this test with test_job_from_dict_with_variations above.

alexmillane · 2026-06-12T07:35:20Z

Delete this file. Added by accident.

greptile-apps · 2026-06-12T10:25:20Z

Greptile Summary

This PR wires the variations feature into eval_runner.py end-to-end: a new variations field is added to the JSON job config, converted from a nested dict to Hydra override strings, and passed through load_env into ArenaEnvBuilder. A --list-variations path is also plumbed in.

Job.from_dict / convert_variations_dict_to_hydra_overrides: a new classmethod walks the nested variations dict using an iterative DFS and flattens each leaf into a dotted.path=value Hydra override string using json.dumps for type-aware serialization (booleans, lists, numbers, strings).
eval_runner.py: load_env gains a variations parameter forwarded as hydra_overrides; a new list_variations helper prints the variations catalogue per job when --list-variations is passed, exiting before the main run loop.
Tests and example config: unit tests cover the flattening logic and round-trip through Job.from_dict; a new integration test verifies the override lands in env_cfg.events; a new example config and updated RST doc demonstrate the feature.

Confidence Score: 4/5

Safe to merge after addressing the null-variations crash in job_manager.py.

The one concrete defect is in Job.from_dict: a JSON config with "variations": null silently passes None into convert_variations_dict_to_hydra_overrides, which throws an opaque AssertionError about a non-empty path rather than indicating the actual problem. Everything else — the DFS flattener, load_env wiring, list_variations, and tests — looks correct and well-covered.

isaaclab_arena/evaluation/job_manager.py — the from_dict call to convert_variations_dict_to_hydra_overrides needs or {} to safely handle an explicit JSON null in the variations field.

Important Files Changed

Filename	Overview
isaaclab_arena/evaluation/job_manager.py	Adds `variations` field to `Job` and `convert_variations_dict_to_hydra_overrides` classmethod; `data.get("variations", {})` passes `None` to the converter when the JSON key is present with an explicit `null` value, causing a misleading AssertionError; asserts used for runtime input validation are stripped under -O.
isaaclab_arena/evaluation/eval_runner.py	Adds `variations` parameter to `load_env` and a new `list_variations` helper; both correctly forward the already-converted list of Hydra override strings; integration with `--list-variations` and the main run loop looks correct.
isaaclab_arena/tests/test_job_manager.py	New unit tests cover flattening logic (including booleans, lists, empty dict) and round-trip through `Job.from_dict`; all assertions look correct.
isaaclab_arena/tests/test_eval_runner.py	Adds subprocess test for variations end-to-end and an integration test that verifies the enabled variation appears in `env_cfg.events`; test structure is consistent with existing patterns.
isaaclab_arena_environments/eval_jobs_configs/droid_pnp_variations_config.json	New example config enabling three variations (HDR image, intensity, wrist-camera extrinsics); matches the documented schema exactly.
docs/pages/concepts/variations/variations.rst	New section documents the `variations` JSON field with an inline example config and two shell commands; content accurately reflects the implementation.

Sequence Diagram

sequenceDiagram
    participant User
    participant eval_runner as eval_runner.py
    participant JobManager
    participant Job
    participant ArenaEnvBuilder

    User->>eval_runner: --eval_jobs_config config.json
    eval_runner->>eval_runner: json.load(config)
    eval_runner->>JobManager: JobManager(eval_jobs_config["jobs"])
    JobManager->>Job: Job.from_dict(job_dict)
    Job->>Job: "convert_variations_dict_to_hydra_overrides({"light": {"hdr_image": {"enabled": true}}})"
    Job-->>JobManager: "job.variations = ["light.hdr_image.enabled=true"]"
    JobManager-->>eval_runner: job_manager

    alt --list-variations flag
        eval_runner->>ArenaEnvBuilder: "get_arena_builder_from_cli(hydra_overrides=job.variations)"
        ArenaEnvBuilder-->>eval_runner: catalogue string
        eval_runner->>User: print catalogue and return
    else normal run
        loop for each job rebuild
            eval_runner->>eval_runner: "load_env(job.arena_env_args, job.name, variations=job.variations)"
            eval_runner->>ArenaEnvBuilder: "get_arena_builder_from_cli(hydra_overrides=job.variations)"
            ArenaEnvBuilder->>ArenaEnvBuilder: apply Hydra overrides to env cfg
            ArenaEnvBuilder-->>eval_runner: env
            eval_runner->>eval_runner: rollout_policy(env, policy)
        end
    end

_{Reviews (2): Last reviewed commit: "Revert the submodule." | Re-trigger Greptile}

greptile-apps · 2026-06-12T10:25:24Z

            policy_config_dict=data["policy_config_dict"],
            status=status,
            language_instruction=data.get("language_instruction"),
+            variations=cls.convert_variations_dict_to_hydra_overrides(data.get("variations", {})),


"variations": null in JSON crashes with a misleading error

data.get("variations", {}) returns the default {} only when the key is absent. If the JSON contains "variations": null (explicit JSON null), it returns None. Passing None into convert_variations_dict_to_hydra_overrides pushes ("", None) onto the stack; when that item is popped, isinstance(None, dict) is False and the code falls through to assert prefix != "", which fires with AssertionError: "Variation override path must be non-empty" — a message that gives no hint that null is the actual problem. Replace with data.get("variations") or {} to treat both absent and null as "no overrides".

greptile-apps · 2026-06-12T10:25:25Z

+            if isinstance(node, dict):
+                for key, value in node.items():
+                    assert key != "", "Variation override keys must be non-empty"
+                    child_prefix = f"{prefix}.{key}" if prefix else str(key)
+                    stack.append((child_prefix, value))


The stack-based DFS pops in LIFO order, so sibling keys are emitted in reverse insertion order. While Hydra override semantics are generally order-independent, reversing the author's dict order is surprising. Switching to reversed(list(node.items())) before appending preserves input order with no algorithmic cost.

Suggested change

if isinstance(node, dict):

for key, value in node.items():

assert key != "", "Variation override keys must be non-empty"

child_prefix = f"{prefix}.{key}" if prefix else str(key)

stack.append((child_prefix, value))

if isinstance(node, dict):

for key, value in reversed(list(node.items())):

assert key != "", "Variation override keys must be non-empty"

child_prefix = f"{prefix}.{key}" if prefix else str(key)

stack.append((child_prefix, value))

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

greptile-apps · 2026-06-12T10:25:26Z

+                    assert key != "", "Variation override keys must be non-empty"
+                    child_prefix = f"{prefix}.{key}" if prefix else str(key)
+                    stack.append((child_prefix, value))
+                continue
+            assert prefix != "", "Variation override path must be non-empty"


assert for runtime input validation

Python strips assert statements when the interpreter runs with the -O (optimize) flag. Both assert key != "" and assert prefix != "" are meant to guard against bad user input, but they become no-ops under optimization. Prefer raising ValueError with a descriptive message so the check is always enforced regardless of how the process is launched.

cvolkcvolk · 2026-06-12T12:15:19Z

+               "num_rebuilds": 5,
+               "policy_type": "zero_action",
+               "policy_config_dict": {},
+               "variations": {


Really cool!

cvolkcvolk · 2026-06-12T12:17:06Z

Is this submodule bump intentional?

Not at all. Good catch! That would have reintroduced the texture bug no doubt.

cvolkcvolk

Nice! lgtm. Keep an eye on the submodule bump.

greptile-apps · 2026-06-12T14:02:17Z

            policy_config_dict=data["policy_config_dict"],
            status=status,
            language_instruction=data.get("language_instruction"),
+            variations=cls.convert_variations_dict_to_hydra_overrides(data.get("variations", {})),


data.get("variations", {}) only returns the {} default when the key is absent. If the JSON contains "variations": null (explicit JSON null), .get returns None, and convert_variations_dict_to_hydra_overrides(None) pushes ("", None) onto the stack. When that item is popped, isinstance(None, dict) is False and the code hits assert prefix != "" — raising AssertionError: "Variation override path must be non-empty", with no hint that null is the root cause. Using or {} treats both absent and explicit-null as "no overrides".

Suggested change

variations=cls.convert_variations_dict_to_hydra_overrides(data.get("variations", {})),

variations=cls.convert_variations_dict_to_hydra_overrides(data.get("variations") or {}),

alexmillane added 30 commits May 29, 2026 16:02

Add google style guide docstrings to AGENTS.md

089e7a2

Merge branch 'alex/feature/agents_md_docstrings' into alex/feature/va…

8e14ae5

…riations

Update comments with new AGENTS.md

2dad7e8

Simplify handling of the sampler configs.

4b9c4b3

Enable variation in the notebook.

aa9ad70

Add some tests for run-time variations.

ba3f315

Add tests for variation base.

9d15811

Add test for build time variations.

9f30398

Add test for hdr variation

1c7df49

Got extrinsic variations working in the notebook

cc6ad8d

Fix the camera extrinsic variation.

6ca83a8

Restore compile env notebook.

e61e7fa

Add a test that ensures get local poses returns the correct quaternio…

42c0663

…n order.

Add test for the numeric values of the extrinsics events.

f31abd7

Self review.

2eea71d

Merge branch 'main' into alex/feature/variations

a58de50

Revert the AGENTS.md

db8162b

Add attribute docstring guidance.

57e8fcb

Fix test.

d7641e6

Move isaaclab_arena_environment out of IsaacLabArenaManagerBasedRLEnv…

2636f7c

…Cfg.

Move isaaclab_arena_environment out of IsaacLabArenaManagerBasedRLEnv…

06199cd

…Cfg.

Merge branch 'alex/refactor/move_arena_env_out_of_cfg' into alex/feat…

e2daf2d

…ure/variations

Agentic move of Hydra stuff from POC branch.

6c56a8b

Add test for duplicate variations.

447ae01

Address unique naming bug on the Variations

3b72bff

Simplify sampler base classes as per the review.

d60c553

Add missing files.

7b18a0a

alexmillane added 12 commits June 10, 2026 18:50

Rename test.

d7888a3

Catch non-valid python identifier.

932fcc9

Delegate hydra parsing to the hydra library.

feb412d

Fix up some override stuff in the review.

c37e9d1

Respond to review.

380e8d4

Add variations to the eval runner.

c58b4eb

Merge branch 'alex/feature/build_time_resampling' into alex/feature/v…

d8d377e

…ariations_in_eval_runner

Merge branch 'alex/feature/light_intensity_variation' into alex/featu…

5fc2fc3

…re/variations_in_eval_runner

list variations in the eval runner.

1354e02

Add variations in eval runner to the docs.

e520350

Add missing docs file

fd2382b

Merge branch 'main' into alex/feature/variations_in_eval_runner

65f5d31

alexmillane commented Jun 12, 2026

View reviewed changes

Address some of the review comments.

095bae2

alexmillane marked this pull request as ready for review June 12, 2026 10:21

alexmillane requested review from cvolkcvolk, peterd-NV, qianl-nv, viiik-inside, xyao-nv and zhx06 as code owners June 12, 2026 10:21

greptile-apps Bot reviewed Jun 12, 2026

View reviewed changes

alexmillane changed the title ~~DRAFT: variations in eval runner~~ variations in eval runner Jun 12, 2026

cvolkcvolk reviewed Jun 12, 2026

View reviewed changes

cvolkcvolk approved these changes Jun 12, 2026

View reviewed changes

cvolkcvolk mentioned this pull request Jun 12, 2026

Sensitivity: record variation draws + self-describing episode summaries #788

Draft

Revert the submodule.

e4dd7f0

greptile-apps Bot reviewed Jun 12, 2026

View reviewed changes

	variations=cls.convert_variations_dict_to_hydra_overrides(data.get("variations", {})),
	variations=cls.convert_variations_dict_to_hydra_overrides(data.get("variations") or {}),

Conversation

alexmillane commented Jun 11, 2026

Summary

Detailed description

Uh oh!

alexmillane left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps Bot Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cvolkcvolk left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

greptile-apps Bot commented Jun 12, 2026 •

edited

Loading