Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/nemo_evaluator_sdk/examples/gym/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ Working from a Gym checkout also makes its components take precedence over the p

Useful flags: `--resources-server`, `--agent`, `--model-type` (`inference_provider` for OpenAI-compatible **chat** endpoints; `openai_model` uses the OpenAI **Responses API** and 500s against chat-only endpoints), `--num-repeats`, `--dataset`, `--output-dir`.

For the full set of knobs the underlying `gym env start` / `gym eval run` commands accept, see the [NeMo Gym documentation](https://github.com/NVIDIA-NeMo/Gym). Anything `GymRuntimeConfig` does not expose as a field can be passed through with its `env_overrides` escape hatch — nested data such as `{"model": {"temperature": 0.7}}`, flattened to Hydra's override grammar and applied to `gym env start`.
For the full set of knobs the underlying `gym env start` / `gym eval run` commands accept, see the [NeMo Gym documentation](https://github.com/NVIDIA-NeMo/Gym). Anything `GymRuntimeConfig` does not expose as a field can be passed through with its `hydra_params` escape hatch — nested data such as `{"model": {"temperature": 0.7}}`, flattened to Hydra's override grammar and applied to `gym env start`.

Each run writes its bundle to a fresh temporary directory by default. Pass `--output-dir` to choose one, but give every run its own: the runner refuses to reuse a directory that already holds Gym rollout output (Gym appends to its failures sidecar, so reusing one would mix runs) and raises rather than clearing a prior run's results.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,10 @@ def _flatten_overrides(overrides: Mapping[str, Any], _prefix: str = "") -> list[
return arguments


def _redact_env_overrides(overrides: Mapping[str, Any], _prefix: str = "") -> dict[str, Any]:
def _redact_hydra_params(overrides: Mapping[str, Any], _prefix: str = "") -> dict[str, Any]:
"""Redact credential-looking values from overrides before they are recorded as provenance.

``env_overrides`` is a free-form escape hatch forwarded to Gym, so nothing stops a caller passing
``hydra_params`` is a free-form escape hatch forwarded to Gym, so nothing stops a caller passing
``{"model": {"api_key": "sk-..."}}``. ``RunnerInfo.config`` is persisted into the run bundle, so a
value that looks like a credential must not be written there.

Expand All @@ -147,7 +147,7 @@ def _redact_env_overrides(overrides: Mapping[str, Any], _prefix: str = "") -> di
for key, value in overrides.items():
path = f"{_prefix}{key}"
if isinstance(value, Mapping):
redacted[key] = _redact_env_overrides(value, f"{path}.")
redacted[key] = _redact_hydra_params(value, f"{path}.")
elif any(marker in path.casefold() for marker in _SECRET_KEY_MARKERS):
redacted[key] = _REDACTED
elif isinstance(value, (list, tuple)):
Expand All @@ -158,9 +158,9 @@ def _redact_env_overrides(overrides: Mapping[str, Any], _prefix: str = "") -> di


def _redact_list_item(item: Any, path: str) -> Any:
"""Redact inside one element of a list-valued override. See :func:`_redact_env_overrides`."""
"""Redact inside one element of a list-valued override. See :func:`_redact_hydra_params`."""
if isinstance(item, Mapping):
return _redact_env_overrides(item, f"{path}.")
return _redact_hydra_params(item, f"{path}.")
if isinstance(item, (list, tuple)):
return [_redact_list_item(nested, path) for nested in item]
return item
Expand All @@ -186,11 +186,11 @@ def _selection_args(config: GymRuntimeConfig, work_dir: Path) -> list[str]:
# env we're running. Assumes the agent config's top-level key equals the agent name (the
# simple_agent convention) *and* that the resources-server is registered under the
# environment's own name — not universally true, so self-contained or differently-named
# servers set bind_resources_server=False and bind themselves via env_overrides.
# servers set bind_resources_server=False and bind themselves via hydra_params.
selection.append(
f"+{config.agent}.responses_api_agents.{config.agent}.resources_server.name={config.resources_server}"
)
selection.extend(_flatten_overrides(config.env_overrides))
selection.extend(_flatten_overrides(config.hydra_params))
# Gym is a Hydra app, so each invocation writes a timestamped run directory — by default
# `outputs/<date>/<time>/` under the *current* directory. Since the subprocesses inherit this
# process's cwd (so Gym can find env.yaml), the default would litter whatever directory the
Expand Down Expand Up @@ -229,13 +229,23 @@ class GymRuntimeConfig(BaseModel):
"(the composable/Pattern-A agent case, e.g. simple_agent whose config leaves it '???'). Set False for "
"self-contained agents that already bind their own resources-server.",
)
env_overrides: dict[str, Any] = Field(
hydra_params: dict[str, Any] = Field(
default_factory=dict,
description="Nested config overrides merged into Gym's config, applied after the auto-derived "
description="Parameters merged into Gym's Hydra config, applied after the auto-derived "
"resources-server binding. Structured rather than pre-serialized Hydra strings so the config "
"travels as JSON — `{'a': {'b': 1}}` becomes `++a.b=1` at invocation. This is the escape "
"hatch for what Gym does not standardize: an environment whose resources-server is registered "
"under a different name, or which references a model server no shipped config defines.",
"under a different name, or which references a model server no shipped config defines. "
"Distinct from `env_vars`: these configure the Gym *environment*, not the OS environment.",
)
env_vars: dict[str, str] = Field(
default_factory=dict,
description="Environment variables set on the `gym` invocation, merged over the ones this "
"process already has. Some Gym environments are configurable only this way — `wmt_translation` "
"reads `WMT_TRANSLATION_COMET_PY_CACHE` for its model-cache root, defaulting to a path that "
"exists only inside NVIDIA's container image — and requiring the caller to export those turns "
"a property of the environment into a property of whoever launched the run. Redacted from "
"recorded provenance on the same rules as `hydra_params`.",
)
num_repeats: int = Field(default=1, ge=1, description="Attempts per row; each attempt becomes one trial.")
concurrency: int = Field(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
from collections.abc import Sequence
from pathlib import Path

from nemo_evaluator_sdk.agent_eval.runtimes.gym.config import GymRuntimeConfig

logger = logging.getLogger(__name__)


Expand Down Expand Up @@ -186,3 +188,24 @@ async def _terminate(proc: asyncio.subprocess.Process, *, grace_s: float = 30.0)
except asyncio.TimeoutError:
_signal_group(pgid, signal.SIGKILL)
await proc.wait()


def _gym_invocation_env(config: GymRuntimeConfig) -> dict[str, str]:
"""The environment the ``gym`` CLI is invoked with.

Three layers, lowest precedence first:

1. **This process's environment.** Gym reads credentials from its own gitignored ``env.yaml``,
but honours plenty of ordinary variables (``HF_TOKEN``, proxies) a caller expects to carry
over.
2. **``RAY_ENABLE_UV_RUN_RUNTIME_ENV=0``.** Gym launches each server from its own subdir with its
own ``.venv``; Ray (>=2.56) otherwise detects a ``uv run`` ancestor and tries to replicate that
uv project onto its workers, asserting the project's ``pyproject.toml`` lives in the driver's
cwd — which aborts startup. That hook is wrong for Gym, whose servers manage their own deps.
3. **``config.env_vars``.** Named explicitly in the run config, so it wins over both. That
includes the Ray setting: it is a default that makes Gym work, not an invariant, and someone
debugging that hook needs a way to put it back.

Lives here rather than in the runtime so the precedence is assertable without starting Ray.
"""
return {**os.environ, "RAY_ENABLE_UV_RUN_RUNTIME_ENV": "0", **config.env_vars}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

import asyncio
import logging
import os
import re
import tempfile
from collections import deque
Expand All @@ -24,7 +23,7 @@
_HYDRA_SUBDIR,
GymRuntimeConfig,
_hydra_scalar,
_redact_env_overrides,
_redact_hydra_params,
_selection_args,
)
from nemo_evaluator_sdk.agent_eval.runtimes.gym.dataset import _materialize_dataset, _source_datasets
Expand All @@ -33,6 +32,7 @@
_VALIDATE_TIMEOUT_S,
_drain_pumps,
_gym_executable,
_gym_invocation_env,
_pending_servers,
_pump_stream,
_terminate,
Expand Down Expand Up @@ -80,8 +80,10 @@ def runner_info(self) -> RunnerInfo:
"""Identify this runner and the Gym settings that shape its results.

Credentials normally live in the Gym checkout's gitignored ``env.yaml`` and never reach this
object — but ``env_overrides`` is a free-form escape hatch, so its values are redacted by key
(see :func:`_redact_env_overrides`) rather than trusted.
object — but ``hydra_params`` and ``env_vars`` are free-form escape hatches, so their values
are redacted by key (see :func:`_redact_hydra_params`) rather than trusted. ``env_vars``
needs it at least as much: environment variables are the conventional way to pass an API
key, so a caller doing the obvious thing would otherwise write one into the run bundle.
"""
cfg = self._config
return RunnerInfo(
Expand All @@ -95,7 +97,8 @@ def runner_info(self) -> RunnerInfo:
"num_repeats": cfg.num_repeats,
"concurrency": cfg.concurrency,
"bind_resources_server": cfg.bind_resources_server,
"env_overrides": _redact_env_overrides(cfg.env_overrides),
"hydra_params": _redact_hydra_params(cfg.hydra_params),
"env_vars": _redact_hydra_params(cfg.env_vars),
"reward_key": cfg.reward_key,
},
)
Expand Down Expand Up @@ -201,7 +204,7 @@ async def _run_two_step(self, input_path: Path, output_path: Path, work_dir: Pat
# detects a `uv run` ancestor and tries to replicate that uv project onto its workers,
# asserting the project pyproject.toml lives in the driver's cwd — which aborts startup. That
# hook is wrong for Gym (servers manage their own deps), so disable it for the subprocesses.
subprocess_env = {**os.environ, "RAY_ENABLE_UV_RUN_RUNTIME_ENV": "0"}
subprocess_env = _gym_invocation_env(cfg)

selection = _selection_args(cfg, work_dir)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
#: answer keeps "the plumbing works" separable from "the agent produced nothing".
_CANNED_ANSWER = "This is a stub response from the NeMo Platform test suite."

#: Stands in for a per-run absolute directory inside a case's `env_overrides`. A committed
#: Stands in for a per-run absolute directory inside a case's `hydra_params`. A committed
#: absolute path would be wrong on every machine, so the run substitutes a real one.
_ABS_ASSETS_PLACEHOLDER = "_ABS_ASSETS"

Expand Down Expand Up @@ -213,7 +213,7 @@ class GymEnvironmentCase:
#: Nested config overrides this environment needs beyond the runner's automatic binding. Empty
#: where the standard `policy_model` wiring is enough; populated as the sweep discovers what an
#: environment actually requires.
env_overrides: Mapping[str, Any] = field(default_factory=dict)
hydra_params: Mapping[str, Any] = field(default_factory=dict)
#: Row field holding this environment's ground truth, and how it wants an answer spelled. When
#: set, the stub answers each prompt with that row's own correct answer and the rollout test can
#: assert a perfect score — which is what makes per-task attribution testable (see
Expand Down Expand Up @@ -277,7 +277,7 @@ def id(self) -> str:
# hiding them behind guesswork would make the runner wrong for environments that follow the
# convention. Recorded here so the cost to a caller is visible.
bind_resources_server=False,
env_overrides={
hydra_params={
"simple_agent": {
"responses_api_agents": {"simple_agent": {"resources_server": {"name": "gdpval_resources_server"}}}
},
Expand Down Expand Up @@ -322,7 +322,7 @@ def id(self) -> str:
#
# `_ABS_ASSETS` is a placeholder: the rollout test substitutes a real tmp_path, since a
# committed absolute path would be wrong on every machine.
env_overrides={
hydra_params={
"legal_agent_bench": {
"resources_servers": {
"legal_agent_bench": {
Expand Down Expand Up @@ -417,7 +417,7 @@ def _require_environment(gym: str, case: GymEnvironmentCase) -> Path:
return environment_dir


def _selection(case: GymEnvironmentCase, hydra_dir: Path, env_overrides: Mapping[str, Any] | None = None) -> list[str]:
def _selection(case: GymEnvironmentCase, hydra_dir: Path, hydra_params: Mapping[str, Any] | None = None) -> list[str]:
"""The selection arguments the runner passes to both `gym env validate` and `gym env start`.

``hydra.run.dir`` mirrors what the runner does: Gym is a Hydra app and writes a timestamped run
Expand All @@ -436,7 +436,7 @@ def _selection(case: GymEnvironmentCase, hydra_dir: Path, env_overrides: Mapping
argv.append(f"+{case.agent}.responses_api_agents.{case.agent}.resources_server.name={case.resources_server}")
# Resolved overrides when the caller has them, so validate checks the *same* values the rollout
# would run — an unresolved `_ABS_ASSETS` placeholder would validate a config nothing ever uses.
argv.extend(_flatten_overrides(case.env_overrides if env_overrides is None else env_overrides))
argv.extend(_flatten_overrides(case.hydra_params if hydra_params is None else hydra_params))
argv.append(f"hydra.run.dir={hydra_dir}")
return argv

Expand Down Expand Up @@ -494,7 +494,7 @@ def _case_overrides(case: GymEnvironmentCase, tmp_path: Path) -> dict[str, Any]:
"""
assets = tmp_path / "assets"
assets.mkdir(parents=True, exist_ok=True)
rendered = json.dumps(case.env_overrides).replace(_ABS_ASSETS_PLACEHOLDER, str(assets))
rendered = json.dumps(case.hydra_params).replace(_ABS_ASSETS_PLACEHOLDER, str(assets))
return json.loads(rendered)


Expand Down Expand Up @@ -563,7 +563,7 @@ async def test_gym_environment_runs_end_to_end(case: GymEnvironmentCase, tmp_pat
# Omit rather than pass None, so the runner's own default stays the single source of
# truth for every environment that does not need more.
**({"startup_timeout_s": case.startup_timeout_s} if case.startup_timeout_s else {}),
env_overrides={
hydra_params={
"policy_base_url": policy_base_url,
"policy_api_key": "stub-key",
"policy_model_name": "stub-model",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
_LOG_TAIL_LINES,
_drain_pumps,
_gym_executable,
_gym_invocation_env,
_pending_servers,
_pump_stream,
)
Expand Down Expand Up @@ -769,7 +770,7 @@ def test_selection_omits_the_binding_when_the_caller_binds_it_themselves(tmp_pat
selection = _selection_args(
_config(
bind_resources_server=False,
env_overrides={
hydra_params={
"simple_agent": {"responses_api_agents": {"simple_agent": {"resources_server": {"name": "other"}}}}
},
),
Expand Down Expand Up @@ -848,6 +849,34 @@ async def test_validate_config_raises_with_gyms_own_report(tmp_path: Path) -> No
assert "mcqa" in str(excinfo.value)


def test_invocation_env_inherits_the_process_environment(monkeypatch: pytest.MonkeyPatch) -> None:
# Gym honours ordinary variables a caller expects to carry over — proxies, HF_TOKEN — so the
# parent environment is the base layer rather than being replaced.
monkeypatch.setenv("A485_INHERITED", "from-parent")
assert _gym_invocation_env(_config())["A485_INHERITED"] == "from-parent"


def test_invocation_env_disables_rays_uv_hook_by_default() -> None:
# Gym launches each server from its own subdir with its own .venv; Ray's uv-project replication
# asserts the driver's cwd holds the pyproject and aborts startup.
assert _gym_invocation_env(_config())["RAY_ENABLE_UV_RUN_RUNTIME_ENV"] == "0"


def test_env_vars_win_over_the_inherited_environment(monkeypatch: pytest.MonkeyPatch) -> None:
# The point of the field: naming a variable in the run config makes it a property of the
# environment being run, not of whoever launched the runner.
monkeypatch.setenv("WMT_TRANSLATION_COMET_PY_CACHE", "/whatever-the-launcher-had")
env = _gym_invocation_env(_config(env_vars={"WMT_TRANSLATION_COMET_PY_CACHE": "/from-the-run-config"}))
assert env["WMT_TRANSLATION_COMET_PY_CACHE"] == "/from-the-run-config"


def test_env_vars_can_restore_rays_uv_hook() -> None:
# The Ray default exists to make Gym work, not as an invariant — someone debugging that hook
# needs a way to put it back, so an explicit value has to outrank it.
env = _gym_invocation_env(_config(env_vars={"RAY_ENABLE_UV_RUN_RUNTIME_ENV": "1"}))
assert env["RAY_ENABLE_UV_RUN_RUNTIME_ENV"] == "1"


def test_gym_executable_reports_how_to_install_when_absent(monkeypatch: pytest.MonkeyPatch) -> None:
monkeypatch.setattr(shutil, "which", lambda _: None)
with pytest.raises(RuntimeError, match="own environment"):
Expand Down
Loading
Loading