Repository navigation
feat(eval): optionally record killed and unstarted rollouts in the failures sidecar - #5081
Merged
Merged
Conversation
…ilures sidecar A rollout that produced no result leaves no row anywhere: an attempt the agent reports as kill_shaped (Slurm SIGTERM, a dead Ray actor, an OOM) and a row dispatch_budget_s never started. The materialized inputs are then the only evidence the rollout was owed, and nothing says why it is missing. record_omitted_rollouts=true writes one failures-sidecar row for each. The row is a copy of the result, so it keeps the rollout's identity and agent, and it keeps the marker the result already carries (_ng_no_persist), its failure class (kill_shaped, or cancelled with _ng_dispatch_drained) and a reason in _ng_failure_message. No new keys or classes are introduced. Every reader of the sidecar now skips a row marked _ng_no_persist: scoring and count_failure_classes_as_zero, count_missing_rollouts_as_zero (which counts such a rollout as missing), exporter failure and retry metrics, the run health summary, gym dev compare, the resume attempt count and terminal gate, and gym eval reverify --judge-failed-only. A resumed run therefore runs the rollout again as its first attempt. Uploads are unchanged: the sidecar row is never uploaded. The option is off by default, so default artifacts are unchanged, until sidecar readers outside Gym read these rows as not run rather than failed. Signed-off-by: Marc Romeyn <mromeijn@nvidia.com>
AdamRajfer
reviewed
Oct 26, 2026
AdamRajfer
left a comment
Contributor
There was a problem hiding this comment.
The diagnostic capture scope makes sense. Two reader behaviors need attention before merge.
…hey are Review of the omitted-rollout rows found two problems. Readers dropped `_ng_no_persist` rows before picking a rollout's latest attempt, so an older judge failure followed by an interrupted attempt was counted again. Readers that take the latest attempt (scoring and batch status, exporter metrics, run health, gym dev compare) now let an interrupted row be the latest execution and then set it aside, so the rollout is missing. A row the dispatch budget never started never advances history. The readers keyed on `_ng_no_persist`, which ATIF reverification already writes on rows it counts, so ATIF failure counts and requested kill_shaped zeros changed even with recording off. The rows record_omitted_rollouts writes now carry `_ng_omitted_rollout`, and readers key on that; rows with only `_ng_no_persist` are read as before. Signed-off-by: Marc Romeyn <mromeijn@nvidia.com>
…les-main Signed-off-by: Marc Romeyn <mromeijn@nvidia.com> # Conflicts: # tests/unit_tests/test_rollout_collection.py
marcromeyn
marked this pull request as ready for review
October 26, 2026 18:48
AdamRajfer
self-requested a review
October 26, 2026 18:51
AdamRajfer
previously approved these changes
Oct 26, 2026
Contributor
Author
|
/ok to test 985b9ef |
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-5081-41ab898cc688.docs.buildwithfern.com/nemo/gym |
Signed-off-by: Marc Romeyn <mromeijn@nvidia.com>
Contributor
Author
|
/ok to test 41ab898 |
AdamRajfer
approved these changes
Oct 26, 2026
Contributor
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/40394147664 |
Contributor
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/40394677434 |
yuhezhang-ai
added a commit
that referenced
this pull request
Oct 26, 2026
#5081's record_omitted_rollouts writes a sidecar row for a kill_shaped attempt and for a row the dispatch budget never started. #3302's run history already saves a kill_shaped attempt as an interruption that spends no retry, so the option adds no second row for it. A row never started still gets #5081's marked row, written through RolloutStore, and the history skips it on load: it is not an attempt, and resume runs the rollout. Sidecars written without a manifest keep #5081's reader rules, and the history reads their rows the same way: an interrupted row is the rollout's latest execution and a row never started is skipped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Yuhe Zhang <yuhez@nvidia.com>
5 tasks done
yuhezhang-ai
added a commit
that referenced
this pull request
Oct 27, 2026
#4841 adds run.stats.timeouts to the run health summary: - Run accounting keeps #3302's split: a run with a manifest is counted from its history, a legacy output from its sidecar. The legacy branch now reads the sidecar through #4841's _read_failures. - The timeouts section reads each never-landed rollout's failed attempt from the same source: the attempt the run history selects, or the legacy sidecar's last row. - The built-in collector still writes the summary under its run lock, now with the run's model-call capture dirs, and exports the section; gym eval run and collect only check custom drivers, as before. - Run timing classifies the record collection saves, after #3302's native episode validation. #4638's per-run token-capture check replaces the per-row lookup, including for recoverable judge failures. Tests: #5081's accounting tests call #3302's _rollout_accounting signature; #4841's collection fakes return a response, its aggregation recomputes in place (a run is never merged onto itself), and #4638's test stubs the server client #3302's collector fingerprints. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Yuhe Zhang <yuhez@nvidia.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.
Scope: diagnostic capture only. Retry and recovery policy stay in #5072/#5073/#4996, and #3302's run manifest keeps its own interpretation of never-started, interrupted and omitted work. Reviewed with @AdamRajfer; his two findings are fixed in 7fa409f: readers keep the latest execution visible before setting these rows aside, and key on their own marker (
_ng_omitted_rollout), so existing ATIF rows are read as before.What and why
Today a rollout that produced no result leaves no row in any artifact:
kill_shaped(_ng_no_persist: true; Slurm SIGTERM, a dead Ray actor, an OOMoutside the rollout), and
dispatch_budget_snever started (_ng_dispatch_drained, classcancelled).Resume relies on that: no row means "run it again". But absence is then the only evidence. The materialized inputs
show the rollout was owed. Nothing says it was killed or never started, or why.
This is the "no-result attempt" half of GYM-327 ("define typed rollout outcomes and recovery"), which notes that
kill-shaped omissions leave no evidence. It is offered as an input to that work, not a separate design. It adds no
outcome taxonomy and no new failure class, and one new key,
_ng_omitted_rollout, that marks the rows it writes. Itwrites down what the collector already knows, using the markers the result already carries. GYM-327 can later map these rows onto a typed outcome. For example
nemo_gym/rollout_outcomes.py'sRolloutFailure(not yet used by the collector) hassource="collector"anddelivery="not_sent"for a drained row and"possibly_delivered"for a killed attempt. This PR does not decidethat mapping.
Writing (
nemo_gym/omitted_rollouts.py, called fromrollout_collection.py): with+record_omitted_rollouts=true, each_ng_no_persistresult is appended to<stem>_failures.jsonlas one row:_ng_task_index,_ng_rollout_index,agent_ref,_ng_attempt_index(on a retry) andany diagnostics the agent returned stay with it;
_ng_no_persist: trueis kept, and_ng_omitted_rollout: trueis added: readers key on the latter, because ATIFreverification already writes
_ng_no_persiston rows it counts as failures;_ng_failure_class: the result's own class. If the result has none,cancelledfor a drained row, otherwisekill_shaped;_ng_failure_message: the reason the result gave (_ng_failure_message,error_messageorerror). Without one itis
"<class>: the rollout produced no result". A drained row says, for example,not dispatched: dispatch budget exhausted.The rows are not attempts: no reward is added, no
_ng_failure_max_attemptsis stamped, and they never enterfailure_counts.Reading (unconditional, so a sidecar written with the option is read correctly by a run without it). A row is
never a failure or a failed attempt. Where a reader takes a rollout's latest attempt, an interrupted row
(
kill_shaped) is that latest execution, so an older failure is not counted again and the rollout is missing; a rowthe dispatch budget never started (
_ng_dispatch_drained) never advances history. Rows with only_ng_no_persistare read exactly as on
main.count_failure_classes_as_zero, batch status,gym eval aggregate(_latest_failure_rows)rollout_collection.pycount_missing_rollouts_as_zerocounts the rollout as missing_failure_export_metrics)rollout_collection.pyretries/attempts; an interrupted latest execution hides an oldercoverage/failed/<class>_load_from_cache)rollout_collection.py_ng_failure_terminalignored, the rollout runs again unstamped (retry policy unchanged)_rollout_accounting)rollout_health.pymissing, notfailedgym dev compare(load_rollouts)rollout_compare/rows.pygym eval reverify --judge-failed-onlyrollout_reverification.pygym eval reverify --judge-failed-onlystill selects judge failures as onmain; how a later execution affectsrejudging a saved answer belongs to the recovery work (#5073, #3879).
Why the option is off by default
EvalRDL's nemo-gym reader counts every failures-sidecar row of an unfinished rollout as a failed rollout. If this
were on by default, every run with a kill-shaped attempt or a drained budget would show "not run" rollouts as failures
in EvalRDL. That is the same missing-reported-as-failed mistake this change exists to avoid. A matching EvalRDL change
(read these rows as not run,
run.n_not_run) is in review separately. Even after it merges, anolder EvalRDL deployment reading newer Gym output would still miscount, so the default should flip only once that
reader has shipped. Until then the cost is one boolean: Gym's own readers already handle the rows, and the option can
be removed when the default flips. The option does not depend on that change. It can be turned on today for runs
whose sidecar is read only by Gym.
Validation
All commands from the repository root.
python -m pytest -q -p no:cacheprovider -o addopts="" -n 8 \ tests/unit_tests/test_rollout_collection.py tests/unit_tests/test_rollout_reverification.py \ tests/unit_tests/test_rollout_health.py tests/unit_tests/test_compare.py tests/unit_tests/test_dev_compare.py \ tests/unit_tests/test_batch_status.py resources_servers/gdpval/tests/test_multistage_orchestrator.py \ tests/unit_tests/test_fern_docs_links.py1350 passed, 24 subtests passed at 985b9ef (7fa409f merged with current
main), with thetelemetryextra installed (without ittest_rollout_collection.pyis skipped as a whole).New and extended tests (20 new test items):
TestOmittedRolloutRecords(5 cases, table-driven): killed; killed with no class or reason; no result under anotherclass; budget exhausted; below the drain margin. Each case runs
run_from_configand checks for exactly one markedrow, with its class, reason, identity and no added reward. It then resumes, and checks that the rollout is sent
again with no attempt index and lands in the rollouts JSONL.
TestSidecarReadersSkipOmittedRows(5): scoring and missing-as-zero, exporter metrics, run health,gym dev compare, and resume (five terminal copies of a marked row do not retire the rollout).test_run_from_config_counts_a_rollout_that_produced_no_row_as_zero: the score is the same, withcount_failure_classes_as_zero=[kill_shaped]set;test_rows_the_budget_never_started_count_as_zero_until_resume_runs_them: the score and the resume are the same;test_run_from_config_non_retaining_preserves_upload_semantics: the upload is the same, including on resume overa sidecar that already holds a marked row.
test_rows_for_rollouts_with_no_result_are_not_recovered(4) intest_rollout_reverification.py.TestOmittedRowsAndTheLatestAttempt(4, after review): in scoring, exporter metrics, run health andgym dev compare, an olderjudge_failedfollowed by an interrupted row is not counted again (the rollout ismissing), an older
judge_failedfollowed by a never-started row still stands, and an ATIF-style row with only_ng_no_persistis still counted. All four fail on the previous head (958e1e6).Red/green: with only the production files stashed, the selected tests fail 17 times. Three new items pass without
the production change and are kept as regression guards: the upload case (the option adds nothing to uploads) and the
reverification
killedandnever-startedcases (their classes were never recovered).The full
tests/unit_tests/run and the red/green check above were done before rebasing onto currentmainand have not been repeated.pre-commit run --files <changed files>passes on the rebased commit. The guardwords hook skippeditself because the private catalog is not available.
Rollout evidence
On the head before rebasing onto current
mainand before review; the rows now also carry_ng_omitted_rollout.Real run:
example_single_tool_call+simple_agentagainstopenai_model(gpt-6-luna), 3 tasks,--concurrency 1 --max-output-tokens 512 +dispatch_budget_s=2. Task 0 started. By the time it finished, the 2 sbudget was spent, so tasks 1 and 2 were never started.
rollouts.jsonlrollouts_failures.jsonlquality_summary.json)+record_omitted_rollouts=true--resumeEach recorded row:
_ng_no_persist: true,_ng_dispatch_drained: true,_ng_failure_class: "cancelled",_ng_failure_message: "not dispatched: dispatch budget exhausted",agent_ref, task and rollout index, no reward.mean/rewardwas 1.0 over 1 rollout. The completion report said "Rollouts missing from the score: 2 of3" with no failures routed.
0 prior failure attempts,2 rows that still need to be run, and ran both.A kill-shaped attempt was not run against a real model: the example agent cannot be made to report
kill_shapedfromconfiguration. The
killedcases above cover it with a stubbed agent reply.Compatibility
_ng_no_persistrows are read as before. ATIF reverification's kill-shaped rows carry no_ng_omitted_rollout, so their failure counts and requestedkill_shapedzeros are unchanged.rewardon a kill-shaped result (pinchbenchwritesreward: 0.0) keeps it in the row. External readers must check_ng_no_persistbefore readingreward.and still are not. The sidecar row is never uploaded.
rollout_collection_driverignores the option. GDPVal's multi-stage driver keeps its own routing andwrites nothing for
_ng_no_persistrows.count_*_as_zerooption is set. A rollout whose latestexecution was interrupted is then missing rather than counted under an older failure class. With the option off
(the default), no artifact, metric or score changes.
Docs:
reference/cli-commands.mdx(a "Rollouts with no result" bullet, plus "Counting lost rollouts" and "Dispatchbudget" wording) and
evaluation/aggregate-metrics.mdx.Proposed labels:
area:evaluation.Linear: FEP-1842 (part of FEP-1829)