Repository navigation
fix(math_with_judge): restore trajectory collection config cited by README - #4574
Merged
qiaochuz-nv merged 3 commits intoOct 14, 2026
Merged
Conversation
…EADME The README's "Prepare for trajectory collection" command loads configs/dapo17k_trajectory_collection.yaml, which #859 deleted, so it fails with ConfigPathNotFoundError. dapo17k.yaml is not a substitute: NeMo RL's is_trajectory_collection reads only the validation split, where dapo17k.yaml has AIME24 x32 instead of the DAPO17k train problems. Restore the config in the current schema: the DAPO17k train artifact (same GitLab source as dapo17k.yaml) as the validation split x16, in its own local file so collate checks it against its own metrics sidecar, which is added from a real collate run (278368 rows = 17398 x 16). Set judge_model_server.name to policy_model, since grading uses math-verify only and a '???' fails the strict missing-value check added in #1575. Document what the step writes and that --download needs the mlflow extra and GitLab registry credentials. Add a test that replays the README command through the CLI, checks the committed sidecar matches the split, and runs the collator offline on a fixture. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: qiaochuz <qiaochuz@nvidia.com>
download_jsonl_dataset opened its output path without creating the parent directory, so `gym dataset download --storage gitlab --output data/x.jsonl` (the math_with_judge README command) and `gym dataset collate --download` for GitLab-backed datasets failed with FileNotFoundError outside the Gym root, where data/ is tracked. The Hugging Face download already creates the directory. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: qiaochuz <qiaochuz@nvidia.com>
yaoyu-33
approved these changes
Oct 13, 2026
yaoyu-33
left a comment
Contributor
There was a problem hiding this comment.
LGTM. Reviewed the restored trajectory config and download-directory fix; 43 focused tests passed locally.
Contributor
|
/ok to test d6427eb |
Contributor
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/38633274932 |
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Oct 14, 2026
Contributor
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/38646874566 |
qiaochuz-nv
deleted the
qiaochuz/nvb-6847517-math-judge-readme-trajectory-config
branch
October 14, 2026 22:36
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.
What does this PR do?
The math_with_judge README's "Prepare for trajectory collection" command works again. It loads
configs/dapo17k_trajectory_collection.yaml, which no longer exists, sogym dataset collateexited withConfigPathNotFoundErrorbefore downloading anything. This PR restores that config in the current schema, documents the step's prerequisites, and adds a regression test that replays the README command.Running the README's other GitLab download, OpenMathReasoning, from outside the Gym root exposed a second bug: the GitLab downloader did not create the output directory. The second commit fixes it.
Fixes NVBug 6847517.
Closes #4498.
Cause
#859 ("Gym 0.2.0 huggingface dataset pointers") deleted three dapo17k variants under
resources_servers/math_with_judge/configs/, includingdapo17k_trajectory_collection.yaml, and removed their root-README rows. The math_with_judge README, which has cited that file since #291, was not updated. Config resolution (nemo_gym/global_config.pyload_extra_config_paths) then raisesConfigPathNotFoundError.dapo17k.yamlis not a drop-in replacement. The deleted config put the DAPO17k train problems in the validation split, repeated 16 times, because NeMo RL withis_trajectory_collection: truecollects from the validation dataloader only.dapo17k.yamlhas DAPO17k astrain×1 and AIME24 ×32 asvalidation, so trajectory collection would run on AIME24.Restoring the old file verbatim would not work either. It set
judge_model_server.name: ???, and since #1575 the parser rejects any???left after merging (ConfigMissingValuesError).The second bug is in
download_jsonl_dataset(nemo_gym/gitlab_utils.py). It openedoutput_fpathfor writing without creating its directory. A Gym checkout tracksdata/, so the README's--output data/open_math_reasoning_problems.jsonlworks from the Gym root but fails withFileNotFoundErrorfrom any other directory.gym dataset collate --downloadfails the same way for GitLab-backed datasets. The Hugging Face download already creates the directory (hf_utils.py).Fix
configs/dapo17k_trajectory_collection.yaml. The validation split is thedapo17k0.0.1dapo17k_train.jsonlartifact (the same GitLab source asdapo17k.yaml),num_repeats: 16. Grading uses math-verify only (should_use_judge: false), sojudge_model_server.nameispolicy_modeland the README command needs no override. The file hasdescription/valueand the environment-server block likedapo17k.yaml. Theupdate-readme-tablehook re-adds its root-README row.data/dapo17k_trajectory_collection_train.jsonl, as before Gym 0.2.0 huggingface dataset pointers #859. Collate checks each dataset against<jsonl stem>_metrics.json. Sharingdapo17k.yaml'sdapo17k_train.jsonlwould compare the ×16 validation stats with that file's train sidecar and fail (train != validation,17398 != 278368).data/dapo17k_trajectory_collection_train_metrics.json, written by a real collate run (278368 rows = 17398 × 16). Its word and turn statistics matchdapo17k_train_metrics.jsonexactly, so the rows are the same.--download) need themlflowextra (optional since (fix) Move mlflow/wandb out of core deps and into extras #3547) andmlflow_tracking_uri/mlflow_tracking_tokeninenv.yaml. The section says what it writes and why the split is validation.Test:
tests/unit_tests/test_math_with_judge_readme_trajectory_config.pytakes the README'sgym dataset collateblock and runs it through the real CLI withdispatchintercepted. It resolves the config offline (noenv.yaml), checks the single dataset isvalidation×16 withdapo17k.yaml's GitLab identifier, and checks the committed sidecar describes that split. It then runsTrainDataProcessoron a three-row fixture in a temporary directory, without network access, and checks that onlyvalidation.jsonland its metrics come out, with each question repeated 16 times in order.download_jsonl_datasetnow creates the output's parent directory, as the Hugging Face download does. A new test intests/unit_tests/test_gitlab_utils.pyfakes the registry lookup and the HTTP fetch, then downloads into a missing nested directory. It fails before this change with the sameFileNotFoundErrorand passes after.These plausible wrong fixes all fail the trajectory test:
???);dapo17k.yamlwith a judge override;num_repeats: 1;trainsplit;dapo17k_train.jsonl.Before the fix
Gym
977583d3d(main), the README command from the Gym root:gym dataset collate \ --config resources_servers/math_with_judge/configs/dapo17k_trajectory_collection.yaml \ --config responses_api_models/openai_model/configs/openai_model.yaml \ --output-dir data/dapo17k_trajectory_collection \ --mode train_preparation \ --downloadThe README's OpenMathReasoning GitLab download, from a directory without
data/, same base:gym dataset download --storage gitlab --name math_open_math_reasoning --revision 0.0.1 \ --artifact open_math_reasoning_problems.jsonl --output data/open_math_reasoning_problems.jsonlAfter the fix
This branch, same command, with the
mlflowextra and GitLab registry credentials inenv.yaml. No local data beforehand:There are 17398 runs of identical questions, each 16 rows long, and no
train.jsonl. A second run skips the download, reports the sidecaralready up to date, and exits 0.The OpenMathReasoning download, same command and directory: exit 0, with the file written (the QA case below checks its rows).
Validation
math_with_judge, which runs both README GitLab steps from a scratch directory with registry credentials:977583d3d: 0/2. Trajectory collation fails because the config is missing; the download fails withFileNotFoundError.ConfigPathNotFoundError), after 1 passed, and the five wrong fixes above fail it. Downloader test before 1 failed (FileNotFoundError), after 1 passed, also with the realmlflow3.16.1 installed.test_train_data_utils.py,test_cli_main.py,test_global_config.py,test_dataset_source.py,test_task_data.py::TestSchemaPresence,test_ray_opt_out.py,test_cli_resources_servers.py,test_model_registry.py,test_resources_server_registry.py,test_environment_scaffold.py,test_gitlab_utils.py,test_hf_utils.py, and the new tests: 592 passed.pre-commit run --all-files: passed.gym dataset collateruns no model. The full download-and-collate run above uses the real registry data.Compatibility
A new config file. Nothing that loads other configs changes. GitLab downloads now create a missing output directory instead of failing. The restored config uses the same GitLab artifact as
dapo17k.yaml, but a separate local file, so each keeps its own metrics sidecar.🤖 Generated with Claude Code