fix: create the rollout output directory before the first write - #2313
Merged
ananthsub merged 1 commit intoAug 5, 2026
Merged
Conversation
ananthsub
force-pushed
the
fix/rollout-output-parent-dir
branch
from
August 4, 2026 15:00
4e80b93 to
e023e48
Compare
ffrujeri
previously approved these changes
Aug 5, 2026
…IA-NeMo#2217) `gym eval run --no-serve --output results/rollouts.jsonl` raised FileNotFoundError when `results/` did not exist yet. The mkdir was already there, but 11 lines too late: it ran at the top of the dispatch section, while the first thing the run writes is the materialized inputs. Move the mkdir to the top of `run_from_config`. All four artifacts a run produces derive from `output_fpath` and keep its parent -- materialized inputs and aggregate metrics via `with_stem`, the failures sidecar via `with_name` -- so one mkdir covers all of them, and it now precedes both the resume and fresh branches. Only `--no-serve` was affected. Without it, `gym eval run` dispatches to `e2e_rollout_collection`, which creates `<output parent>/preprocessed_datasets` during data preparation and so created the parent as a side effect. A git clone also hid this, because the repo tracks a `results/.gitignore` placeholder; a PyPI install running from an arbitrary cwd has no such directory. The other output-writing entry points already do this: `gym eval reverify` (rollout_reverification.py), `gym eval aggregate` (rollout_collection.py), and `gym dataset render` (prompt.py). This brings rollout collection in line. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
ananthsub
force-pushed
the
fix/rollout-output-parent-dir
branch
from
August 5, 2026 17:06
e023e48 to
2ba9a42
Compare
ananthsub
enabled auto-merge (squash)
August 5, 2026 17:10
ffrujeri
approved these changes
Aug 5, 2026
OlegSudakov
pushed a commit
to OlegSudakov/Gym
that referenced
this pull request
Aug 7, 2026
…IA-NeMo#2313) Fixes NVIDIA-NeMo#2217 ## Problem `gym eval run --no-serve --output results/mcqa_rollouts.jsonl` — the quickstart command in the README — raises `FileNotFoundError` when `results/` does not exist yet: ``` File "nemo_gym/rollout_collection.py", line 508, in run_from_config with config.materialized_jsonl_fpath.open("wb") as f: FileNotFoundError: [Errno 2] No such file or directory: 'results/mcqa_rollouts_materialized_inputs.jsonl' ``` The mkdir was already present in `run_from_config`, but it is called too late. It ran at the top of the dispatch section, while the first thing the run writes is the materialized inputs. ## Fix Move `output_fpath.parent.mkdir(parents=True, exist_ok=True)` to the top of `run_from_config`, above both the resume and fresh branches. The issue suggested adding a mkdir at each of the four output paths. One is enough: all four derive from `output_fpath` and keep its parent: materialized inputs and aggregate metrics via `with_stem`, the failures sidecar via `with_name`, rollouts is the path itself. Resume semantics are unchanged: creating an empty directory does not make `output_fpath.exists()` or `materialized_jsonl_fpath.exists()` true, so resume eligibility is decided the same way as before. ## Why this only showed up on the PyPI install path Two things were hiding it: - Without `--no-serve`, `gym eval run` dispatches to `e2e_rollout_collection`, which sets `output_dirpath = <output parent>/preprocessed_datasets` and lets `TrainDataProcessor` create it, incidentally creating the output parent. Only the `--no-serve` path goes straight to `collect_rollouts`. - The repo tracks a `results/.gitignore` placeholder, so `results/` exists in a git clone. A PyPI install running from an arbitrary cwd has no such directory. ## Scope This was the only affected entry point. The others already create their output parent first, so this brings rollout collection in line with the existing convention: - `gym eval reverify` — `rollout_reverification.py`, in `_prepare_output_fpaths` - `gym eval aggregate` — `rollout_collection.py`, in `RolloutAggregationHelper.run_from_config` - `gym dataset render` — `prompt.py`, in `materialize_prompts` - `gym eval profile` — writes next to an input rollouts file that must already exist ## Testing Added `test_run_from_config_creates_missing_output_dir`, parametrized over `resume_from_cache`, writing to a two-level-deep missing directory and asserting all four artifacts land there. Both parametrizations fail on `main` and pass with this change. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
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.
Fixes #2217
Problem
gym eval run --no-serve --output results/mcqa_rollouts.jsonl— the quickstart command in the README — raisesFileNotFoundErrorwhenresults/does not exist yet:The mkdir was already present in
run_from_config, but it is called too late. It ran at the top of the dispatch section, while the first thing the run writes is the materialized inputs.Fix
Move
output_fpath.parent.mkdir(parents=True, exist_ok=True)to the top ofrun_from_config, above both the resume and fresh branches.The issue suggested adding a mkdir at each of the four output paths. One is enough: all four derive from
output_fpathand keep its parent: materialized inputs and aggregate metrics viawith_stem, the failures sidecar viawith_name, rollouts is the path itself.Resume semantics are unchanged: creating an empty directory does not make
output_fpath.exists()ormaterialized_jsonl_fpath.exists()true, so resume eligibility is decided the same way as before.Why this only showed up on the PyPI install path
Two things were hiding it:
--no-serve,gym eval rundispatches toe2e_rollout_collection, which setsoutput_dirpath = <output parent>/preprocessed_datasetsand letsTrainDataProcessorcreate it, incidentally creating the output parent. Only the--no-servepath goes straight tocollect_rollouts.results/.gitignoreplaceholder, soresults/exists in a git clone. A PyPI install running from an arbitrary cwd has no such directory.Scope
This was the only affected entry point. The others already create their output parent first, so this brings rollout collection in line with the existing convention:
gym eval reverify—rollout_reverification.py, in_prepare_output_fpathsgym eval aggregate—rollout_collection.py, inRolloutAggregationHelper.run_from_configgym dataset render—prompt.py, inmaterialize_promptsgym eval profile— writes next to an input rollouts file that must already existTesting
Added
test_run_from_config_creates_missing_output_dir, parametrized overresume_from_cache, writing to a two-level-deep missing directory and asserting all four artifacts land there. Both parametrizations fail onmainand pass with this change.