Repository navigation
Gym CLI refactor - #1630
Merged
Merged
Gym CLI refactor#1630
Conversation
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
…nt deprecation note Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
… to stderr) Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
…a single --model flag Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Contributor
Author
|
/claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Contributor
Author
|
/ok to test 5067752 |
wprazuch
approved these changes
Jun 25, 2026
marta-sd
dismissed
cmunley1’s stale review
June 25, 2026 10:41
I hopefully addressed all feedback (no blockers has been raised during the daily)
ko3n1g
approved these changes
Jun 25, 2026
wprazuch
added a commit
that referenced
this pull request
Jun 25, 2026
…tion #12) (#1599) ## What Adds **`gym env validate`** (+ `ng_validate` / `nemo_gym_validate` deprecated shims) — runs the full config parse with **no Ray and no server subprocesses**, then exits **0 (valid) / 1 (invalid)** with a clean, rich-escaped message (**no traceback**). Returns in well under a second instead of after a ~30–60s Ray bootstrap. ```bash gym env validate --config resources_servers/<env>/configs/<env>.yaml --config responses_api_models/<model>/configs/<model>.yaml gym env validate --benchmark gsm8k --model-type openai_model ``` ## How `validate()` lives in `cli/env.py` and is registered as `env validate` in the `gym` router (`cli/main.py` COMMANDS) with the same config-selection flags as `env start` (`--config`, `--benchmark`, `--environment`, `--resources-server`, `--model-type`, `--search-dir`, `--model*`). It reuses the same `get_global_config_dict()` parse path the other commands use, so the validation checks stay in sync: - **config_paths** resolution — missing/typo'd ([#1488](#1488)) and malformed ([#1490](#1490)) - **server cross-references** — unknown `name:` refs ([#1561](#1561)) - **mandatory `???`** values ([#1575](#1575)) - **schema** (`BaseNeMoGymCLIConfig`) Wrapped in `exit_cleanly_on_config_error` (from #1609) so any `ConfigError` becomes a clean message + `exit 1`. A dummy `policy_model` is injected (the `NO_MODEL` parser config, as in `gym list` / `env compose`) so model interpolations like `${policy_base_url}` resolve without real creds — validation is about config **well-formedness**; the real model is supplied by the `--model*` flags at run time. ## Targets `main` Originally drafted on the unified-CLI epic branch; rebuilt directly on `main` now that [#1630](#1630) (and #1637/#1609/#1635/#1671) have merged. The old branch contents (a snapshot of the CLI refactor + unrelated CI commits) were superseded and replaced. ## Scope note The zero-server check ([#1489](#1489), "nothing configured to run") is intentionally **not** part of `validate`: `NO_MODEL` injects a dummy model server (which would defeat the check), and "is anything configured to run" is a *start*-time concern already enforced by `gym env start` before Ray init. `validate` focuses on config well-formedness. ## Why Epic [#1205](#1205) friction #12 (no config validation tooling) — the M1 "fast failure triage" deliverable. Config errors otherwise only surface after Ray starts (~30–60s). ## Tests - `test_cli_main.py`: `gym env validate --config X` routes to `nemo_gym.cli.env:validate` with `+config_paths=[X]` (added to the parametrized config-command matrix). - `test_cli.py`: `validate()` prints OK on a valid config; a raised `ConfigError` becomes `exit 1` (no traceback). - All `test_cli` + `test_cli_main` + `test_cli_legacy` pass (the only failures are the pre-existing Python-3.12 `TestDidYouMean` argparse issue on `main`); ruff + pre-commit clean. Smoke-tested end-to-end: `✓ Config is valid.` on a real benchmark, clean error + `exit 1` on a bad path, and the `ng_validate` deprecation shim. --------- Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
ritaneves
pushed a commit
that referenced
this pull request
Jun 25, 2026
:wq Gym CLI refactor (#1630) Implements #1434 Notes: - **Hydra dash-flags no longer supported.** The `gym` CLI parses its own flags, so Hydra's native `--multirun`, `--config-dir`, etc. are no longer supported. W assume blast radius is small (we don't have them in docs, most users already use overrides). - **Hydra diagnostics now go to stderr.** Hydra's log handler is redirected inside `get_global_config_dict()`, so its diagnostic lines move from stdout to stderr (this also affects legacy `ng_*` commands). This was needed to keep the stdout clean for `--json`/piping. --------- Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com> Co-authored-by: Ananth Subramaniam <ansubramania@nvidia.com> Signed-off-by: Rita Fernandes Neves <rfernandesne@nvidia.com>
gwarmstrong
added a commit
to gwarmstrong/Gym
that referenced
this pull request
Jun 25, 2026
Resolve conflict in fern/.../reference/cli-commands.mdx: upstream's CLI refactor (NVIDIA-NeMo#1630) replaced ng_collect_rollouts docs with the 'gym eval run' command. Re-applied the per-agent num_repeats (dict-form) documentation onto the new --num-repeats flag (table entry + example). rollout_collection.py and test_rollout_collection.py auto-merged cleanly; all 38 unit tests pass. Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
2 tasks done
7 tasks
khazic
pushed a commit
to khazic/Gym-lao
that referenced
this pull request
Oct 13, 2026
…DIA-NeMo#4538) ## What does this PR do? The `gym eval run` example in `resources_servers/ifbench/README.md` now runs. It passed `--prompt-config benchmarks/ifbench/prompts/default.yaml` together with `--input resources_servers/ifbench/data/example.jsonl`, but every example row already carries `responses_create_params.input`. `validate_prompt_compatibility` (`nemo_gym/prompt.py:76-86`) rejects that combination on purpose, so the documented command failed on all five rows before any generation. Fixes NVBug [6783016](https://nvbugspro.nvidia.com/bug/6783016) (Jira NMGYM-57). ### Cause `example.jsonl` is pre-rendered, and the README asked Gym to render the prompt a second time. The mismatch dates from NVIDIA-NeMo#999, which added IFBench with pre-rendered rows and an `ng_collect_rollouts +prompt_config=...` example after the validation (NVIDIA-NeMo#861) already existed. NVIDIA-NeMo#1630 carried the flag over into the `gym eval run` form. ### Fix Remove `--prompt-config` (and the trailing `\` before it) from the README command. The data and the validation stay as they are: - `example.jsonl` must keep `input`: it is the `type: example` dataset of `ifbench_simple_agent` (`resources_servers/ifbench/configs/ifbench.yaml`), which has no prompt config. - The model input does not change. For all five rows (ids 0, 79, 172, 260, 262) the shipped `input` is exactly `[{"role": "user", "content": <prompt>}]`, which is what `default.yaml` (`user: "{prompt}"`) renders. - The `--benchmark ifbench` path is unaffected: `benchmarks/ifbench/prepare.py` writes rows without `input`, matching the `prompt_config` in `benchmarks/ifbench/config.yaml`. ### Before the fix Gym `eed77e195` (main), the README command against the README's `gym env start --benchmark ifbench --model-type openai_model`: ```bash gym eval run --no-serve \ --agent ifbench_benchmark_simple_agent \ --input resources_servers/ifbench/data/example.jsonl \ --output /tmp/ifbench_rollouts.jsonl \ --num-repeats 1 \ --max-output-tokens 4096 \ --temperature 0.0 \ --prompt-config benchmarks/ifbench/prompts/default.yaml ``` ```text Using prompt config: benchmarks/ifbench/prompts/default.yaml Reading rows: 5it [00:00, 53773.13it/s] Traceback (most recent call last): ... File ".../nemo_gym/rollout_collection.py", in _preprocess_rows_from_config validate_prompt_compatibility([row for _, _, row in raw_rows], prompt_cfg) File ".../nemo_gym/prompt.py", line 83, in validate_prompt_compatibility raise ValueError( ValueError: Some rows have responses_create_params.input but prompt_config is also specified. These are mutually exclusive. Use one or the other. Violating rows: [0, 1, 2, 3, 4] exit 1 in 3.0s ``` ### After the fix Gym `07b4ac277` (this branch), the README command as now documented, same environment: ```bash gym eval run --no-serve \ --agent ifbench_benchmark_simple_agent \ --input resources_servers/ifbench/data/example.jsonl \ --output /tmp/ifbench_rollouts.jsonl \ --num-repeats 1 \ --max-output-tokens 4096 \ --temperature 0.0 ``` ```text exit 0 in 36.3s 5 rollout rows with unique identities every row carries responses_create_params/response/reward and a resp_* id materialized and rollout identity sets match (5 pairs) failures sidecar present (0 rows) ifbench_benchmark_simple_agent: mean/reward 0.4 agrees with persisted rollouts ifbench_benchmark_simple_agent: key_metrics is a subset of agent_metrics ``` ### Validation - QA testcase `testcases/environments/ifbench.py` (nmfw_tests), which runs the README's own commands with a live `openai_model` backend on an H100 host: - before, Gym `eed77e195`: 0/1 passed (the `ValueError` above); - after, Gym `07b4ac277`: 1/1 passed — five real generations, verify results recomputed independently, and twelve strict/loose/fractional/binary scoring controls. - Locally (macOS, Python 3.13.14): the README command without `--prompt-config` reads all five rows and gets past preprocessing; with it, the same `ValueError`. - `pre-commit run --all-files`: passed. - `pytest tests/unit_tests/test_fern_docs_links.py tests/unit_tests/test_cli_main.py`: 243 passed. - Rollout evidence: N/A beyond the QA run above — documentation-only change. ### Compatibility None. Documentation only; no code, config, or data changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: qiaochuz <qiaochuz@nvidia.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
yaoyu-33
pushed a commit
to yaoyu-33/Gym
that referenced
this pull request
Oct 13, 2026
…VIDIA-NeMo#4557) ## What does this PR do? The terminus_judge README launch command works again. `configs/terminus_judge.yaml` sends LLM-judge calls to a model server named `judge_model`, but the README command never defined one, so `gym env start` exited at config validation before any server started. Once the stack started, a second problem showed up: the README's 512-token judge budget left a reasoning judge no room to write a verdict. This PR fixes both. Fixes NVBug [6847480](https://nvbugspro.nvidia.com/bug/6847480). Closes NVIDIA-NeMo#4496. ### Cause `resources_servers/terminus_judge/configs/terminus_judge.yaml:5-7` sets `judge_model_server.name: judge_model`. The README command came in with NVIDIA-NeMo#597, when this config graded with `policy_model`. NVIDIA-NeMo#825 switched the config to a separate `judge_model` (with the `rubrics_v4` prompt) and moved the policy-as-judge setup to `terminus_judge_simple.yaml`, but left the README alone; NVIDIA-NeMo#1630 later rewrote the command from `ng_run` to `gym env start` unchanged. `--resources-server terminus_judge` loads only `configs/terminus_judge.yaml`, and `--model-type openai_model` defines only `policy_model`, so startup validation raises `ServerRefNotFoundError`. Nothing in Gym defines `judge_model` for this config: in-repo benchmarks use `terminus_judge_string_only.yaml`, and recipes that use `terminus_judge.yaml` bring their own judge. ### Fix Add `resources_servers/terminus_judge/configs/judge_model.yaml`. It defines `judge_model` with the `openai_model` adapter and reads `judge_base_url`, `judge_api_key`, and `judge_model_name`, falling back to the matching policy value for each one that is unset. The README command loads it with `--config`, and a new paragraph says the judge defaults to the policy endpoint and model and how to point it elsewhere. This follows the existing opt-in judge configs (`chemeval/configs/judge_model.yaml`, `sciknoweval/configs/judge_model.yaml`, `physics_judge/configs/judge_openai.yaml`). `terminus_judge.yaml` is unchanged. The README's judge override goes from `max_output_tokens=512` to `4096`, with a sentence on why. With 512, every judge call from a reasoning model used the whole budget on reasoning and returned no verdict, and terminus_judge scores that as 0. With 4096, every call returned a verdict after 1213 to 2346 output tokens (see *Validation*). Alternatives not taken, each checked offline with `GlobalConfigDictParser`: - **`judge_model: {_copy: policy_model}` in `terminus_judge.yaml`** (suggested in NVIDIA-NeMo#4496). A recipe that supplies its own `judge_model` with a different adapter then fails to start, because the copy merges the policy's adapter into it: `AlmostServerError ... responses_api_models: Dictionary should have at most 1 item after validation, not 2`. With `--model-type local_vllm_model`, the judge becomes a second local vLLM server (`gpu_memory_utilization: 0.9`). - **Point `judge_model_server` at `policy_model`.** This undoes NVIDIA-NeMo#825's separate judge: recipes that define `judge_model` would silently grade with the policy. Test: `tests/unit_tests/test_terminus_judge_readme_config.py` takes the README's `gym env start` block and runs it through the real CLI with `dispatch` intercepted. It then resolves the resulting config offline (placeholder endpoints, no `env.yaml`, no servers started). It checks that the judge reference is still `judge_model`, that the README's `max_output_tokens=4096` override applies, and that the judge resolves to the policy values, or to the `judge_*` values when those are set. Both alternatives above fail it: `_copy` fails the separate-judge case, and the redirect fails both. ### Before the fix Gym `bb032ae98` (`main`), the README command from the Gym root, with placeholder policy values: ```bash gym env start \ --resources-server terminus_judge \ --model-type openai_model \ +terminus_judge_resources_server.resources_servers.terminus_judge.judge_responses_create_params.max_output_tokens=512 ``` ```text Error: In server instance 'terminus_judge_resources_server', field 'judge_model_server' references responses_api_models/'judge_model', which is not defined in the merged config. Available responses_api_models: 'policy_model' ``` Exit 1; no server starts. The new test on the same commit: ```text E nemo_gym.config_types.ServerRefNotFoundError: In server instance 'terminus_judge_resources_server', field 'judge_model_server' references responses_api_models/'judge_model', which is not defined in the merged config. FAILED tests/unit_tests/test_terminus_judge_readme_config.py::test_readme_launch_defines_the_judge_server[policy-fallback] FAILED tests/unit_tests/test_terminus_judge_readme_config.py::test_readme_launch_defines_the_judge_server[separate-judge] ``` The NVBug's QA case, which runs the README flow (`gym env start`, then `gym eval run --agent terminus_judge_simple_agent` on the five examples) against a hosted model: ```text FAIL 6.1s the original Terminus README starts its judge topology and evaluates all five examples StackStartupError: `gym env start` exited with code 1 during startup 'judge_model_server' references responses_api_models/'judge_model', which is not defined in the merged config. ``` ### After the fix This branch, the README's new command, same placeholder policy values: ```bash gym env start \ --resources-server terminus_judge \ --config resources_servers/terminus_judge/configs/judge_model.yaml \ --model-type openai_model \ +terminus_judge_resources_server.resources_servers.terminus_judge.judge_responses_create_params.max_output_tokens=4096 ``` ```text All 5 / 5 servers ready! ``` The five servers are `policy_model` and `judge_model` (both `openai_model`), `terminus_judge_resources_server`, `terminus_judge_simple_agent`, and `terminus_judge_environment_server`. That startup log is from `bffe30099`; the budget commit after it doesn't change startup, and the QA run below starts the same five servers at `661425200`. The new test: ```text tests/unit_tests/test_terminus_judge_readme_config.py::test_readme_launch_defines_the_judge_server[policy-fallback] PASSED tests/unit_tests/test_terminus_judge_readme_config.py::test_readme_launch_defines_the_judge_server[separate-judge] PASSED ========================= 2 passed, 1 warning in 0.99s ========================= ``` The same QA case, updated to replay the README's new command (it adds the `--config`, uses the 4096 budget, expects five servers, and now also requires every judge call to return a verdict), Gym `661425200`: ```text PASS 62.5s the Terminus README starts its judge topology and evaluates all five examples terminus_judge: 1/1 passed in 62.5s ``` ### Validation - New test: before 2 failed, after 2 passed. Both alternatives in *Fix* fail it. - `tests/unit_tests/test_cli_resources_servers.py`, `test_cli_main.py`, `test_global_config.py`, `test_review_policy.py`, `test_model_registry.py`, `test_cli_agents.py`, `test_task_data.py::TestSchemaPresence`, `test_ray_opt_out.py`, and the new test: 483 passed. - `pre-commit run --all-files`: passed, no changes. - README command with placeholder policy values: before, exit 1 at config validation; after, 5 / 5 servers ready. - QA testcase `terminus_judge` (README stack, five examples, a hosted reasoning model as policy and therefore as judge): before 0/1 (startup), after 1/1 with every judge call returning a verdict. The case change ships separately in the QA suite. It also raises the case's 180 s startup budget, which a fresh checkout's server venv installs exceed, to the 1800 s the other README cases use. - Rollout evidence, all on the README stack with five real rollouts each: - With only the `judge_model` fix (`bffe30099`) and the old 512 budget, the QA case passed 1/1, but every judge call failed. Four rows reached the judge through the new `judge_model` server; each reply used 512/512 output tokens on reasoning and had no verdict (`judge_parsing_failed`, reward 0.0). The fifth row's policy output was invalid JSON (`model_output_invalid`). - Same commit, judge budget 4096: all five judge calls returned `[[A=B]]` after 1213 to 2346 output tokens, and all five rewards were 1.0. - This branch (`661425200`), the QA case above: all five rows reached the judge, and all five judge calls returned a verdict after 1195 to 2760 output tokens. Two were `[[A=B]]` (reward 1.0) and three `[[A!=B]]` (reward 0.0), all with `mask_sample: false`. ### Compatibility The README command gains `--config resources_servers/terminus_judge/configs/judge_model.yaml`. Configs that don't load the new file behave as before, including recipes that bring their own `judge_model`. With only policy values set, the README's judge is the policy endpoint and model (self-judging); set `judge_base_url`, `judge_api_key`, and `judge_model_name` for an independent judge. The README's judge calls may now use up to 4096 output tokens instead of 512. The shipped config sets no judge budget, so recipes are unaffected. Not changed: a judge reply with no verdict still scores an unmasked 0 (`terminus_judge/app.py`, `judge_parsing_failed`). Whether to set `mask_sample` there is a separate scoring decision. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: qiaochuz <qiaochuz@nvidia.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This branch was previously deployed
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
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.
Implements #1434
Notes:
gymCLI parses its own flags, soHydra's native
--multirun,--config-dir, etc. are no longer supported. W assume blast radius is small (we don't have them in docs, most users already use overrides).get_global_config_dict(), so its diagnostic lines move from stdout to stderr (this alsoaffects legacy
ng_*commands). This was needed to keep the stdout clean for--json/piping.