Support graph-YAML environments in typed YAML Experiments - #970
Draft
alexmillane wants to merge 4 commits into
Draft
Support graph-YAML environments in typed YAML Experiments#970alexmillane wants to merge 4 commits into
alexmillane wants to merge 4 commits into
Conversation
The typed YAML experiment frontend previously resolved environment.type only against registered environment names; graph-spec YAML environments (e.g. the robolab tasks) were reachable only through the legacy JSON format or the CLI. An environment.type ending in .yaml/.yml now routes through the same LegacyGraphEnvironmentCfg compatibility path the JSON frontend uses, and the camera pre-launch guard recognizes such runs. Signed-off-by: Alex Millane <amillane@nvidia.com>
language_instruction is not a flag on the graph-environment parser (it is injected from the typed builder config after parsing, as is device), so rendering it as a token made argparse swallow the value as the example-environment positional. Signed-off-by: Alex Millane <amillane@nvidia.com>
osmo/submit_arena_experiment.py embeds the effective Experiment by re-serializing it, which failed for graph-YAML environments because the serializer only resolves registry-registered configs. The compatibility config now records its graph-spec path and source environment values, and the serializer emits them as the environment section. Graph runs also now execute with the Run's typed environment_builder config instead of one re-derived from CLI tokens, so post-load Hydra overrides (e.g. environment_builder.num_envs on an OSMO submission) take effect; tokens carry only environment values. Signed-off-by: Alex Millane <amillane@nvidia.com>
alexmillane
force-pushed
the
alex/feature/support_yaml_environments_on_osmo
branch
from
July 29, 2026 14:04
c4c0125 to
eb420ac
Compare
alexmillane
commented
Jul 31, 2026
alexmillane
left a comment
alexmillane
left a comment
Collaborator
Author
There was a problem hiding this comment.
Self review 1
Comment on lines
+69
to
+106
| @@ -80,6 +84,26 @@ def load_arena_experiment_from_config_file( | |||
| return ArenaExperimentCfg(runs=runs_with_process_device) | |||
|
|
|||
|
|
|||
| # TODO(cvolk, 2026-07-07): [typed-config-migration] Delete this factory when graph-YAML | |||
| # environments have a typed configuration and no longer use the argparse compatibility path. | |||
| def _graph_environment_cfg_from_yaml_values( | |||
| env_graph_spec_yaml: str, | |||
| environment_values: dict[str, Any], | |||
| ) -> LegacyGraphEnvironmentCfg: | |||
| """Create the temporary graph-YAML compatibility config from typed YAML Run values. | |||
|
|
|||
| The environment values are rendered as CLI tokens for the existing graph-environment | |||
| argparse path; the Run's environment_builder section stays typed and is applied | |||
| directly at execution (see build_arena_builder_from_legacy_graph). | |||
| """ | |||
| arena_env_args: dict[str, Any] = {"environment": env_graph_spec_yaml, **environment_values} | |||
| return LegacyGraphEnvironmentCfg( | |||
| arena_env_args=legacy_environment_args_to_cli_args(arena_env_args), | |||
| env_graph_spec_yaml=env_graph_spec_yaml, | |||
| environment_values=dict(environment_values), | |||
| ) | |||
|
|
|||
|
|
|||
Collaborator
Author
There was a problem hiding this comment.
Why are we threading this factor down into load_arena_experiment_from_yaml from the top here.
Can't we just move this down into the function and save the user at this levels the details of how the yaml environment is loaded?
Comment on lines
+50
to
+65
| def _assert_camera_support_enabled(experiment_cfg: ArenaExperimentCfg, enable_cameras: bool) -> None: | ||
| """Check that AppLauncher enabled camera support requested by typed Runs.""" | ||
| camera_run_names = [run_cfg.name for run_cfg in experiment_cfg.runs.values() if run_cfg.environment.enable_cameras] | ||
| camera_run_names = [ | ||
| run_cfg.name for run_cfg in experiment_cfg.runs.values() if _run_environment_requires_cameras(run_cfg) | ||
| ] | ||
| assert not camera_run_names or enable_cameras, ( | ||
| f"Runs {camera_run_names} enable environment cameras but AppLauncher started without camera support. " | ||
| "The camera requirements read from the Experiment before startup disagree with the composed Experiment." | ||
| ) | ||
|
|
||
|
|
||
| def _run_environment_requires_cameras(run_cfg: ArenaRunCfg) -> bool: | ||
| """Return whether a Run's environment enables cameras, including graph-YAML environments.""" | ||
| if isinstance(run_cfg.environment, LegacyGraphEnvironmentCfg): | ||
| return "--enable_cameras" in run_cfg.environment.arena_env_args | ||
| return run_cfg.environment.enable_cameras |
Collaborator
Author
There was a problem hiding this comment.
On main we've changed things such that the system automatically enables the cameras if required by the experiment file. Suggestion to pull main and check if this is still required.
Comment on lines
+186
to
+193
| assert graph_environment_cfg_factory is not None, ( | ||
| f"Run '{run_name}' selects graph-spec YAML environment '{graph_spec_yaml}', " | ||
| "but this loader was not given graph-YAML environment support" | ||
| ) | ||
| environment_values_without_selector = { | ||
| field_name: value for field_name, value in environment_values.items() if field_name != "type" | ||
| } | ||
| environment = graph_environment_cfg_factory(graph_spec_yaml, environment_values_without_selector) |
Collaborator
Author
There was a problem hiding this comment.
Hide this in a function.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Run graph-YAML environments with
experiment_runner.py.Detailed description
environment.typeending in.yaml/.ymlnow routes through the sameLegacyGraphEnvironmentCfgcompatibility path the JSON frontend uses.osmo/submit_arena_experiment.pyembeds the experiment in the OSMO yaml by re-serializing it, which failed for graph-YAML environments because the serializer only resolves registry-registered configs. The compatibility config now records its graph-spec path and source environment values, and the serializer emits them as the environment section.