Light intensity variation.

[alexmillane]

Summary

Add light intensity variation.

Detailed description

• Modifies the light intensity at runtime.

[greptile-apps]

Greptile Summary

This PR adds a LightIntensityVariation that samples a dome-light intensity at build time and applies it via a new DomeLight.set_intensity() helper. It also introduces the full Hydra-override plumbing (variations_hydra, variations_printing, --list-variations CLI flag, assert_hydra_overrides validator) and accompanying docs and tests.

• Adds LightIntensityVariation (build-time) to DomeLight, defaulting to a 100–2000 range; changes the global default intensity from 3000 to 500.
• Wires Hydra override parsing into policy_runner.py / ArenaEnvBuilder so users can toggle and configure any variation from the command line without code changes.

Confidence Score: 5/5

Safe to merge — the new variation integrates cleanly with the existing build-time variation machinery and all code paths are covered by tests.

The implementation is correct end-to-end: apply_cfg rebuilds the sampler before apply() is called, overrides are applied before build-time variations are sampled, and the test uses a degenerate sampler range to assert a deterministic outcome. The two findings are both cosmetic: a stale docs example that omits the new intensity entry, and a repr()-based string formatter that would produce quoted tokens for string-list fields in the catalogue. Neither affects runtime behavior.

docs/pages/concepts/variations/variations.rst (example output needs updating) and isaaclab_arena/variations/variations_printing.py (_format_value string formatting).

Important Files Changed

Filename	Overview
isaaclab_arena/variations/light_intensity_variation.py	New build-time variation; implementation is clean. apply_cfg correctly rebuilds the sampler, and apply() delegates to DomeLight.set_intensity().
isaaclab_arena/variations/variations_hydra.py	New Hydra-override engine. Error handling swallows all ConfigCompositionExceptions under the "Unknown variation path" message, which can mislead on type-mismatch errors.
isaaclab_arena/variations/variations_printing.py	Catalogue formatter. _format_value uses Python repr() for scalar non-numeric values (strings), producing quoted tokens that differ from Hydra's expected unquoted CLI syntax for string lists.
isaaclab_arena/assets/object_library.py	Adds set_intensity(), in-place HDR texture update, and auto-registers LightIntensityVariation. Intensity default changed from 3000 to 500.
isaaclab_arena/environments/arena_env_builder.py	Adds hydra_overrides parameter and applies them before build-time variations; correct ordering ensured.
docs/pages/concepts/variations/variations.rst	New docs page; the example --list-variations output is incomplete — it omits the intensity variation now always registered on every DomeLight.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    CLI["CLI args + Hydra override strings\n(parse_known_args)"] --> AHO["assert_hydra_overrides()\nvalidate leftover tokens"]
    AHO --> GAB["get_arena_builder_from_cli()\nArenaEnvBuilder(env, args, hydra_overrides)"]
    GAB --> LV{--list-variations?}
    LV -- Yes --> CAT["get_variations_catalogue_as_string()\ncompose_variations_cfg_and_apply_overrides\n(display only, no mutation)"]
    CAT --> EXIT["print + return"]
    LV -- No --> CMC["compose_manager_cfg()"]
    CMC --> AO["apply_overrides()\n→ variation.apply_cfg(new_cfg)\n→ sampler rebuilt"]
    AO --> ABTV["_apply_build_time_variations()\n→ LightIntensityVariation.apply()\n→ sampler.sample() → DomeLight.set_intensity()"]
    ABTV --> SI["DomeLight.set_intensity()\nspawner_cfg.intensity = value\nobject_cfg = _init_object_cfg()"]
    SI --> SCENE["Scene materialised\nwith new intensity baked in"]

Loading

_{Reviews (4): Last reviewed commit: "Rename test." | Re-trigger Greptile}

[isaaclab-review-bot]

🤖 Isaac Lab Review Bot

Summary

This PR adds a build-time variation that samples and applies dome light intensity before environment composition. The implementation follows the existing HDRImageVariation pattern well, with a clean separation between the variation class, the DomeLight.set_intensity() helper, and the test. However, there is a name mismatch between the variation registration and the test lookup that will cause test failures, a stale docstring, and a debug print left in production code.

Design Assessment

Design is sound. The approach correctly mirrors the HDRImageVariation pattern — a BuildTimeVariationBase subclass with a reference to the light asset, sampling once at build time. The refactor of add_hdr to mutate in-place rather than reconstructing the entire DomeLightCfg is a good simplification. Adding set_intensity() as a public method on DomeLight is the right level of abstraction.

Findings

See inline comments for details.

Test Coverage

• New code: Yes — dedicated test file covers both enabled and disabled paths
• Gaps: Test will fail due to name mismatch (see Critical finding); once fixed, coverage is adequate
• Quality: Good use of degenerate sampler range for deterministic assertions

CI Status

⏳ Pre-commit check is pending.

Verdict

Significant concerns

The variation name mismatch ("intensity" vs "dome_light_intensity") means the included tests cannot pass as-is, which is a blocking issue. The debug print() statement should also be removed before merge. Once these are addressed, the implementation is clean and follows established patterns well.

---

Update (d7888a3): All previous concerns have been addressed ✅

• 🔴 Name mismatch — Fixed. Test now uses "intensity" consistently matching the variation's default name.
• 🟡 Debug print() statement — Removed.
• 🟡 Stale docstring — Simplified; misleading range text removed.
• 🔵 Unused DEFAULT_INTENSITY constant — Removed.

No new issues found in the incremental changes. LGTM.

[cvolkcvolk]

Nice