Skip to content

Commit 4fce989

Browse files
committed
feat(configurator): env_params as first-class trial-identity field
Make env_params a first-class part of CloudAIGymEnv trial identity so the trajectory cache keys on (action, env_params) rather than action alone, fixing the domain-randomization correctness bug where the same action under a different env_params sample returned a stale reward. - Cache key now includes env_params; cache-key tests pin the contract (formerly the TDD-red specs of NVIDIA#900, folded in here). - Keep env.csv and trajectory.csv 1:1 step-aligned: a single TrajectoryEntry sinks both files coherently, including on constraint failure. - Reject env_params on non-DSE jobs; reject non-finite / negative weights. - Add cache-hit + declared-env_params integration coverage. Folds the test-only PR NVIDIA#900 (cache-key TDD) into this PR so the stack has no permanently-red standalone PR.
1 parent 88d35c5 commit 4fce989

8 files changed

Lines changed: 755 additions & 4 deletions

File tree

src/cloudai/_core/test_scenario.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ class TestRun:
9797
reports: Set[Type[ReportGenerationStrategy]] = field(default_factory=set)
9898
extra_srun_args: str | None = None
9999
num_nodes_explicit: bool = False
100+
current_env_params: dict[str, Any] = field(default_factory=dict)
100101

101102
def __hash__(self) -> int:
102103
return hash(self.name + self.test.name + str(self.iterations) + str(self.current_iteration))

src/cloudai/cli/handlers.py

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
System,
4040
TestParser,
4141
TestScenario,
42+
TestScenarioParsingError,
4243
)
4344
from cloudai.models.scenario import ReportConfig
4445
from cloudai.models.workload import TestDefinition
@@ -297,12 +298,35 @@ def _check_installation(
297298
return result
298299

299300

301+
def validate_dse_env_params(test_scenario: TestScenario) -> None:
302+
"""
303+
Reject prepped configs that declare env_params on a non-DSE test run.
304+
305+
env_params are sampled only during DSE (by CloudAIGymEnv); on a non-DSE run they would be
306+
silently ignored. is_dse_job is a property of the fully prepped config, so this is validated
307+
here rather than at parse time.
308+
"""
309+
offenders = [tr.name for tr in test_scenario.test_runs if tr.test.env_params and not tr.is_dse_job]
310+
if offenders:
311+
raise TestScenarioParsingError(
312+
f"Tests {offenders} declare env_params but are not DSE jobs. env_params are sampled only during "
313+
"DSE (by CloudAIGymEnv); add a sweep (a list-valued cmd_args/extra_env_vars entry or num_nodes) "
314+
"or remove env_params."
315+
)
316+
317+
300318
def handle_dry_run_and_run(args: argparse.Namespace) -> int:
301319
setup_result = _setup_system_and_scenario(args)
302320
if setup_result is None:
303321
return 1
304322
system, test_scenario, tests = setup_result
305323

324+
try:
325+
validate_dse_env_params(test_scenario)
326+
except TestScenarioParsingError as e:
327+
logging.error(str(e))
328+
return 1
329+
306330
if not _handle_single_sbatch(args, system):
307331
return 1
308332

@@ -491,7 +515,8 @@ def verify_test_scenarios(
491515
tests = Parser.parse_tests(test_tomls, system)
492516
hook_tests = Parser.parse_tests(hook_test_tomls, system)
493517
hooks = Parser.parse_hooks(hook_tomls, system, {t.name: t for t in hook_tests})
494-
Parser.parse_test_scenario(scenario_file, system, {t.name: t for t in tests}, hooks)
518+
scenario = Parser.parse_test_scenario(scenario_file, system, {t.name: t for t in tests}, hooks)
519+
validate_dse_env_params(scenario)
495520
except Exception:
496521
nfailed += 1
497522

src/cloudai/configurator/cloudai_gym.py

Lines changed: 63 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,14 @@
1919
import dataclasses
2020
import logging
2121
from pathlib import Path
22-
from typing import Any, Dict, Optional, Tuple
22+
from typing import Any, Dict, List, Optional, Tuple
2323

2424
from cloudai.core import METRIC_ERROR, BaseRunner, Registry, TestRun
2525
from cloudai.util.lazy_imports import lazy
2626

2727
from .base_agent import RewardOverrides
2828
from .base_gym import BaseGym
29+
from .env_params import CsvSink, EnvParamsObserver, EnvParamsSink, StepObserver
2930

3031

3132
@dataclasses.dataclass(frozen=True)
@@ -36,6 +37,7 @@ class TrajectoryEntry:
3637
action: dict[str, Any]
3738
reward: float
3839
observation: list
40+
env_params: dict[str, Any] = dataclasses.field(default_factory=dict)
3941

4042

4143
class CloudAIGymEnv(BaseGym):
@@ -61,8 +63,28 @@ def __init__(self, test_run: TestRun, runner: BaseRunner, rewards: RewardOverrid
6163
self.max_steps = test_run.test.agent_steps
6264
self.reward_function = Registry().get_reward_function(test_run.test.agent_reward_function)
6365
self.trajectory: dict[int, list[TrajectoryEntry]] = {}
66+
self._env_sink: EnvParamsSink | None = None
67+
self.observers: List[StepObserver] = self._build_observers()
6468
super().__init__()
6569

70+
def _build_observers(self) -> List[StepObserver]:
71+
"""
72+
Construct the per-step observers implied by the TestDefinition.
73+
74+
Workloads opt in to env_params via a TOML ``[env_params.<name>]`` block;
75+
an empty mapping yields no observers and zero overhead.
76+
"""
77+
observers: List[StepObserver] = []
78+
if self.test_run.test.env_params:
79+
seed = int((self.test_run.test.agent_config or {}).get("random_seed", 0))
80+
self._env_sink = CsvSink(self._env_csv_path())
81+
observers.append(EnvParamsObserver(self.test_run.test.env_params, seed))
82+
return observers
83+
84+
def _env_csv_path(self) -> Path:
85+
"""``env.csv`` lives alongside ``trajectory.csv`` so a plain ``merge`` joins them."""
86+
return self.trajectory_file_path.parent / "env.csv"
87+
6688
def define_action_space(self) -> Dict[str, list[Any]]:
6789
return self.test_run.param_space
6890

@@ -121,6 +143,9 @@ def step(self, action: Any) -> Tuple[list, float, bool, dict]:
121143
self.test_run.increment_step()
122144
self.test_run = self.test_run.apply_params_set(action)
123145

146+
for observer in self.observers:
147+
observer.before_step(self.test_run)
148+
124149
cached_result = self.get_cached_trajectory_result(action)
125150
if cached_result is not None:
126151
logging.info(
@@ -134,8 +159,11 @@ def step(self, action: Any) -> Tuple[list, float, bool, dict]:
134159
action=action,
135160
reward=cached_result.reward,
136161
observation=cached_result.observation,
162+
env_params=dict(self.test_run.current_env_params),
137163
)
138164
)
165+
for observer in self.observers:
166+
observer.after_step(self.test_run, cached_result.observation, cached_result.reward)
139167
return cached_result.observation, cached_result.reward, False, {}
140168

141169
if not self.test_run.test.constraint_check(self.test_run, self.runner.system):
@@ -162,6 +190,11 @@ def step(self, action: Any) -> Tuple[list, float, bool, dict]:
162190
self.test_run.step = new_tr.step
163191
self.test_run.output_path = new_tr.output_path
164192

193+
# Rebuilding/replacing test_run above can drop the per-trial env_params sample;
194+
# restore it so the trajectory entry, the cache key, and env.csv all record the
195+
# params the trial actually ran with.
196+
self.test_run.current_env_params = new_tr.current_env_params
197+
165198
observation = self.get_observation(action)
166199
reward = self.compute_reward(observation)
167200

@@ -171,9 +204,13 @@ def step(self, action: Any) -> Tuple[list, float, bool, dict]:
171204
action=action,
172205
reward=reward,
173206
observation=observation,
207+
env_params=dict(self.test_run.current_env_params),
174208
)
175209
)
176210

211+
for observer in self.observers:
212+
observer.after_step(self.test_run, observation, reward)
213+
177214
return observation, reward, False, {}
178215

179216
def render(self, mode: str = "human"):
@@ -230,7 +267,14 @@ def get_observation(self, action: Any) -> list:
230267
return observation
231268

232269
def write_trajectory(self, entry: TrajectoryEntry):
233-
"""Append the trajectory to the CSV file and to the local attribute."""
270+
"""
271+
Append the entry to the in-memory cache and trajectory.csv (plus env.csv when declared).
272+
273+
``trajectory.csv`` and the ``env.csv`` projection are sunk from the same
274+
``TrajectoryEntry`` here, so a trial that never produces an entry (e.g. a
275+
constraint failure returns before this call) lands in neither file and the
276+
two stay 1:1 step-aligned.
277+
"""
234278
self.current_trajectory.append(entry)
235279

236280
file_exists = self.trajectory_file_path.exists()
@@ -243,6 +287,9 @@ def write_trajectory(self, entry: TrajectoryEntry):
243287
writer.writerow(["step", "action", "reward", "observation"])
244288
writer.writerow([entry.step, entry.action, entry.reward, entry.observation])
245289

290+
if self._env_sink is not None:
291+
self._env_sink.write(entry.step, entry.env_params)
292+
246293
@property
247294
def trajectory_file_path(self) -> Path:
248295
return self.runner.scenario_root / self.test_run.name / f"{self.test_run.current_iteration}" / "trajectory.csv"
@@ -252,8 +299,21 @@ def current_trajectory(self) -> list[TrajectoryEntry]:
252299
return self.trajectory.setdefault(self.test_run.current_iteration, [])
253300

254301
def get_cached_trajectory_result(self, action: Any) -> TrajectoryEntry | None:
302+
"""
303+
Return a cached entry only when the full trial identity matches.
304+
305+
Trial identity is ``(action, env_params)``: env-randomized parameters
306+
change the workload's behaviour, so a trial repeating the same action
307+
under a different ``env_params`` sample must miss and re-run. Empty
308+
env_params on both sides is the back-compat path for workloads that
309+
do not declare any ``[env_params.*]`` block.
310+
"""
311+
current_env_params = getattr(self.test_run, "current_env_params", {}) or {}
255312
for entry in self.current_trajectory:
256-
if self._values_match_exact(entry.action, action):
313+
if not self._values_match_exact(entry.action, action):
314+
continue
315+
entry_env = getattr(entry, "env_params", {}) or {}
316+
if self._values_match_exact(entry_env, current_env_params):
257317
return entry
258318

259319
return None
Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
# SPDX-FileCopyrightText: NVIDIA CORPORATION & AFFILIATES
2+
# Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
3+
# SPDX-License-Identifier: Apache-2.0
4+
#
5+
# Licensed under the Apache License, Version 2.0 (the "License");
6+
# you may not use this file except in compliance with the License.
7+
# You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS,
13+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
# See the License for the specific language governing permissions and
15+
# limitations under the License.
16+
17+
"""
18+
Domain-randomization primitives for CloudAI DSE.
19+
20+
An env-randomized parameter is a workload knob whose value the environment
21+
samples per trial (categorical, optional weights). It is sibling to
22+
``cmd_args`` on a ``TestDefinition`` and does not enter the agent's action
23+
space; the policy learns a robust mapping under that variation.
24+
25+
This module owns the data schema (``EnvParamSpec``), the deterministic
26+
sampler (``EnvParamsSampler``), the persistence interface
27+
(``EnvParamsSink`` + ``CsvSink``) and the per-step observer
28+
(``EnvParamsObserver``). ``CloudAIGymEnv`` consumes these directly so the
29+
artifacts (``env.csv``) and the cache key align 1:1 with ``trajectory.csv``
30+
regardless of agent (PPO, BO, GA, MAB) or workload.
31+
"""
32+
33+
from __future__ import annotations
34+
35+
import csv
36+
import math
37+
import random
38+
from pathlib import Path
39+
from typing import Any, Dict, List, Optional, Protocol, runtime_checkable
40+
41+
from pydantic import BaseModel, ConfigDict, Field, model_validator
42+
from typing_extensions import Self
43+
44+
45+
class EnvParamSpec(BaseModel):
46+
"""Specification of one env-randomized parameter (categorical)."""
47+
48+
model_config = ConfigDict(extra="forbid")
49+
50+
values: List[Any] = Field(
51+
min_length=2,
52+
description="Candidate values; a single-valued parameter is just a fixed cmd_args entry.",
53+
)
54+
weights: Optional[List[float]] = Field(
55+
default=None,
56+
description="Optional probability weights aligned with values; uniform if omitted.",
57+
)
58+
59+
@model_validator(mode="after")
60+
def _validate_weights(self) -> Self:
61+
if self.weights is None:
62+
return self
63+
if len(self.weights) != len(self.values):
64+
raise ValueError(
65+
f"env_params weights length {len(self.weights)} does not match values length {len(self.values)}"
66+
)
67+
for w in self.weights:
68+
if not math.isfinite(w) or w < 0:
69+
raise ValueError(f"env_params weights must be finite and non-negative; got {w}")
70+
if sum(self.weights) <= 0:
71+
raise ValueError("env_params weights must have a positive sum")
72+
return self
73+
74+
75+
class EnvParamsSampler:
76+
"""
77+
Per-trial categorical sampler.
78+
79+
Determinism contract: ``sample(t)`` returns the same dict on every call
80+
(across processes) for the same ``(seed, env_params, t)``.
81+
82+
Independence contract: each parameter uses an RNG seeded by
83+
``f"{seed}:{name}:{trial}"`` so adding or removing an unrelated
84+
parameter does not perturb existing parameters' draw sequences.
85+
"""
86+
87+
def __init__(self, env_params: Dict[str, EnvParamSpec], seed: int) -> None:
88+
self._env_params = env_params
89+
self._seed = seed
90+
91+
def sample(self, trial: int) -> Dict[str, Any]:
92+
out: Dict[str, Any] = {}
93+
for name, spec in self._env_params.items():
94+
rng = random.Random(f"{self._seed}:{name}:{trial}")
95+
if spec.weights is not None:
96+
out[name] = rng.choices(spec.values, weights=spec.weights, k=1)[0]
97+
else:
98+
out[name] = rng.choice(spec.values)
99+
return out
100+
101+
102+
@runtime_checkable
103+
class EnvParamsSink(Protocol):
104+
"""Persist one trial's env_params sample; empty samples must be no-ops."""
105+
106+
def write(self, step: int, sample: Dict[str, Any]) -> None: ...
107+
108+
109+
class CsvSink:
110+
"""
111+
Append per-trial env_params samples to a step-aligned CSV.
112+
113+
The CSV mirrors how ``trajectory.csv`` serialises its ``action`` column
114+
(one row per env.step(), sample dict stringified in a single cell) so the
115+
two files align 1:1 on ``step`` and a plain ``merge`` joins them.
116+
"""
117+
118+
def __init__(self, path: Path) -> None:
119+
self._path = path
120+
121+
def write(self, step: int, sample: Dict[str, Any]) -> None:
122+
if step < 1:
123+
raise ValueError(f"step must be a positive trial index (cloudai DSE is 1-based); got {step}")
124+
if not sample:
125+
return
126+
new_file = not self._path.exists()
127+
self._path.parent.mkdir(parents=True, exist_ok=True)
128+
with self._path.open("a", newline="") as f:
129+
writer = csv.writer(f)
130+
if new_file:
131+
writer.writerow(("step", "env"))
132+
writer.writerow([step, sample])
133+
134+
135+
@runtime_checkable
136+
class StepObserver(Protocol):
137+
"""
138+
Hook fired by ``CloudAIGymEnv.step()`` around each trial.
139+
140+
``before_step`` runs before the cache lookup and before any workload
141+
execution. ``after_step`` runs after the trajectory row is written.
142+
"""
143+
144+
def before_step(self, test_run: Any) -> None: ...
145+
146+
def after_step(self, test_run: Any, observation: list, reward: float) -> None: ...
147+
148+
149+
class EnvParamsObserver:
150+
"""
151+
Sample env_params per step and stash them for the cache and the workload.
152+
153+
Pre-step: samples ``test_run.test.env_params`` for ``test_run.step`` and
154+
stashes the result on ``test_run.current_env_params`` so the cache key and
155+
the workload's substitution both see it. Persistence is owned by
156+
``CloudAIGymEnv.write_trajectory``, which sinks ``trajectory.csv`` and the
157+
``env.csv`` projection from the single ``TrajectoryEntry`` - so a trial that
158+
writes no trajectory row writes no env.csv row either, keeping the two files
159+
1:1 step-aligned. Post-step: no-op.
160+
"""
161+
162+
def __init__(self, env_params: Dict[str, EnvParamSpec], seed: int) -> None:
163+
self._sampler = EnvParamsSampler(env_params, seed=seed)
164+
165+
def before_step(self, test_run: Any) -> None:
166+
test_run.current_env_params = self._sampler.sample(test_run.step)
167+
168+
def after_step(self, test_run: Any, observation: list, reward: float) -> None:
169+
del test_run, observation, reward # persistence handled by CloudAIGymEnv.write_trajectory

0 commit comments

Comments
 (0)