Repository navigation
add option to not increment endpoint retry limit - #3079
Merged
Merged
Conversation
Signed-off-by: Julien Veron Vialard <jveronvialar@nvidia.com>
cmunley1
previously approved these changes
Sep 4, 2026
Contributor
|
/ok to test 0859232 |
adil-a
previously approved these changes
Sep 4, 2026
bxyu-nvidia
requested changes
Sep 4, 2026
|
|
||
| base_url: str | ||
| api_key: str | ||
| no_infinite_endpoint_retries: bool = False |
Contributor
There was a problem hiding this comment.
we should reuse the internal flag rather than adding a new one
Contributor
Author
There was a problem hiding this comment.
good suggestion, I made the changes
Signed-off-by: Julien Veron Vialard <jveronvialar@nvidia.com>
bxyu-nvidia
approved these changes
Sep 6, 2026
Contributor
|
/ok to test 65b8d21 |
This was referenced Sep 9, 2026
jkyi-nvidia
pushed a commit
that referenced
this pull request
Sep 11, 2026
## What does this PR do? Adds a no_infinite_endpoint_retries option for vLLM endpoints. When enabled, retryable endpoint errors no longer increase max_num_tries, so requests fail after the fixed retry limit instead of retrying indefinitely. ## Checklist - [X] I have read the [contributing guidelines](https://docs.nvidia.com/nemo/gym/latest/contribute/development-setup). - [X] The change is focused; unrelated "drive-by" edits are tracked as separate issues/PRs. - [X] Tests added or updated and pass locally, or N/A for docs-only / non-code changes (so CI unit/server checks pass when applicable). - [X] Pre-commit checks pass locally (`pre-commit run --all-files`) (so CI lint/format/copyright pass). - [X] All commits have DCO sign-off (`git commit -s`) (so the DCO check passes). --------- Signed-off-by: Julien Veron Vialard <jveronvialar@nvidia.com> Signed-off-by: Junkeun Yi <jkyi@nvidia.com>
6 of 7 tasks
khazic
pushed a commit
to khazic/Gym-lao
that referenced
this pull request
Oct 1, 2026
## What does this PR do? Makes multistage GDPVal ELO runs safe to resume across allocations, and adds the multistage MiniMax judge overlay. It carries the multistage orchestrator work from NVIDIA-NeMo#2588, NVIDIA-NeMo#3086, NVIDIA-NeMo#3131 and NVIDIA-NeMo#3615 on `gdpval-devel`, plus the overlay from NVIDIA-NeMo#2046 with the anchor refreshes from NVIDIA-NeMo#3184, NVIDIA-NeMo#3319 and NVIDIA-NeMo#3473. Multistage runs are long and get resumed across allocations. On main today: - the resume journal is keyed only on the stage shape, seed, references and task distribution, so a resume can replay a journal made from different rows, rollout settings or servers; - a stale journal deletes the rollouts, failure sidecar and journal it replaces; - every stage is marked complete whatever its failures, and the run always goes on to the final stage, so later stages are planned from an ELO fit that is missing rows; - the multistage path drops `route_failures_to_sidecar`, so a failed `/run` raises and kills the stage's in-flight rollouts. ### Changes - **Resume fingerprint.** It now also hashes: - the materialized rows, by content; - the rollout settings that affect results: `agent_name`, `limit`, `num_repeats`, `num_repeats_add_seed`, `responses_create_params`, `prompt_config`, `skills`; - every resolved server block, minus the top-level fields that change between allocations or resumes: `host`, `port`, `base_url`, `api_key`, `default_headers`, `*_base_url`, `*_api_key`, `*_default_headers`, `endpoint_*`, and the operational agent knobs `rerun_incomplete` and `concurrency`; - the `multistage.transport_assignment_repair` block, when set. Not hashed: `partial_completion`, `retry_inprocess`, `NEMO_GYM_MAX_ROLLOUT_ATTEMPTS`, and the contents of the reference deliverable directories. A new policy endpoint, rotated key, `RERUN_INCOMPLETE=true` or a different agent concurrency therefore keeps the journal. Every row carries the fingerprint as `verify_cache_namespace`, so cached `/verify` results are keyed per run. NVIDIA-NeMo#3686 adds the Stirrup code that reads it. - **Prior evidence is set aside, not deleted.** - A fresh start renames the rollouts, failure sidecar, journal and aggregate metrics to `<file>.stale.<ns>`. - Restarting from a stage moves later stages' rows to `<file>.pruned.<ns>` with atomic rewrites. The derived aggregate-metrics file is deleted. - The journal records `restart_from_stage` and `restart_cleanup_complete`, so an interrupted cleanup finishes on the next start. - A completed-stage marker is discarded while the failure sidecar still holds a retryable failure for that stage; the stage then replays its recorded plan. - **Non-final stages are strict.** A non-final stage completes only when every planned row has a usable judged battle, unless it sets `partial_completion`. - A stage with a retryable or undispatched row stays open. The run writes its rollouts and aggregate metrics, logs that it stopped and how to continue (`--resume`), and exits 0 with no `comparison/eval_elo`. NVIDIA-NeMo#3686's aggregation marks that result degraded. - When a stage is rejected on coverage with nothing left to retry (a terminal row, or one that used all `NEMO_GYM_MAX_ROLLOUT_ATTEMPTS`, default 3), the run writes its rollouts and then raises instead of emitting aggregate metrics (NVIDIA-NeMo#3615). - The final stage is never held open. A final stage with missing rows is written and aggregated; NVIDIA-NeMo#3686's aggregation reports its per-stage fit, marks it degraded and emits no headline `comparison/eval_elo`. - **Retry state carries across resumes.** Attempt counts, the longest failed-attempt duration, and the latest failure and no-persist dispositions are loaded per stage from the sidecar and the journal. The attempt cap therefore holds across allocations, `dispatch_longest_first` orders resumed rows by their longest earlier attempt, and a row whose deliverable survived a failed judging attempt is re-judged rather than re-run. - **Opt-in in-process retry.** `multistage.retry_inprocess: true` re-dispatches rows that failed with `timeout_exceeded` or `transient` in the same process, up to `NEMO_GYM_MAX_ROLLOUT_ATTEMPTS`. Other retryable classes wait for a resume. - **Opt-in partial calibration.** A non-final stage can set `partial_completion` (coverage floors, `waivable_failure_classes`, `tolerate_unresolved`) to accept the stage with bounded missing evidence. - The included and omitted rows and an evidence hash are frozen in the journal and revalidated on every resume. - A freshly planned policy-enabled stage uses a seeded, balanced assignment of tasks to references. - Setting it on the final stage is rejected. - **Opt-in transport-aware reference assignment** (`multistage.transport_assignment_repair.enabled`, new `transport_assignment.py`). At stage planning it reassigns the fewest tasks needed so that each candidate/reference attachment set fits the judge's file, raw, wire and per-section limits (defaults: the constants in `comparison.py`), keeping each reference's task count exact. It makes no provider calls. - Only tasks whose candidate deliverable already exists at planning time can move. On a fresh run nothing exists yet, so the default mode changes nothing and lists every task under `unrepairable_tasks` in the stage plan's receipt. - `reference_availability_only: true` needs no candidate deliverable: it moves tasks away from references with no completed deliverable (NVIDIA-NeMo#3131). - A task with no compatible reference stays on its original draw (`unroutable_tasks`), and if no count-preserving assignment exists the whole stage keeps its original draw (`infeasible`). Planning never aborts the run; those rows fail at dispatch as they would without repair. - The candidate root is the `persist_deliverables_dir` of the agent serving the GDPVal server. It must be set and absolute, as the Stirrup agent already requires; it may not exist yet. Every reference `deliverables_dir` must be a readable directory. Config errors fail at startup, before any prior state is set aside. - **Rollout settings forwarded into the multistage path.** `route_failures_to_sidecar` (already on main) plus NVIDIA-NeMo#3684's `dispatch_budget_s`, `drain_margin_s`, `dispatch_longest_first` and `DispatchLatencyTracker` summary. `dispatch_budget_s` is one wall-clock budget per process, counted from driver start and shared by all stages. - **The driver opts in to NVIDIA-NeMo#3684's failure contract.** On the single-pass path it sets `retry_terminal_timeouts` and `retry_invalid_judge_responses` to `true` only when the config leaves them unset; an explicit `false` is kept. The multistage bookkeeping always applies the contract, so a multistage run with either key explicitly `false` fails at startup. The bookkeeping uses the helpers NVIDIA-NeMo#3684 makes public. - **New overlay `resources_servers/gdpval/configs/gdpval_minimax_multistage_judge.yaml`.** Multistage comparison against a self-hosted MiniMax-M3 judge with 18 GDPval-AA v2 anchors. - Reference directories resolve from `$GDPVAL_REFS_ROOT`. Reference deliverables are not shipped with Gym, so the overlay can't run without your own copies. - The judge proxy sets `max_http_attempts: 1800`, because main caps non-internal client retries at 3 tries (NVIDIA-NeMo#3079, NVIDIA-NeMo#3538). - Its header documents the strict first stage and the resume command. Issue: none. This is part of the series that promotes `gdpval-devel` to main split by file. NVIDIA-NeMo#2046, still open against main, adds an earlier version of the same overlay file. ### Dependencies - **NVIDIA-NeMo#3684:** imports `DispatchLatencyTracker`, `get_max_rollout_attempts`, `is_terminal_failure`, `migrate_invalid_judge_main_rows` and `observed_elapsed` from `nemo_gym.rollout_collection`, and the `retry_terminal_timeouts` / `retry_invalid_judge_responses` config fields. - **NVIDIA-NeMo#3686:** the judge size limits in `comparison.py` that transport repair reads, the aggregation of `expected_final_stage_index`, the Stirrup reader of `verify_cache_namespace`, and `benchmarks/gdpval/config.yaml`, which the fingerprint test loads. The other PRs in the series come in through NVIDIA-NeMo#3686. - Merge order matters: on main alone, `multistage_orchestrator.py` fails to import (`DispatchLatencyTracker`), and main's `benchmarks/gdpval/config.yaml` already points `rollout_collection_driver` at it, so every `--benchmark gdpval` run would break. Keep this PR a draft until NVIDIA-NeMo#3684 and NVIDIA-NeMo#3686 merge. ## Validation macOS, Python 3.13. The suites ran on the promotion snapshot with this PR's fixes applied (base `1c8261080` plus every PR in the series; the two nested `pyproject.toml` files are left out). - `cd resources_servers/gdpval && pytest tests -q -p no:cacheprovider`: **582 passed**. - `cd responses_api_agents/stirrup_agent && pytest tests -q -p no:cacheprovider`: **320 passed, 2 skipped**. - This PR's test modules (`test_multistage_orchestrator.py`, `test_multistage_elo.py`, `test_transport_assignment.py`, `test_hardening_fixes.py`): **221 passed**. - The same tree with this PR's files reverted to base or removed (base plus the other PRs): GDPVal **437 passed**, Stirrup **320 passed, 2 skipped**. No earlier PR depends on this one. - `pytest tests/unit_tests -q -p no:cacheprovider` (no core file changes here; run for the combined tree): 5700 passed, 161 skipped, 145 subtests passed, 59 failed (20 tests and 39 subtests). All 59 are pre-existing macOS-only failures that also fail on plain main, in `telemetry/test_memory.py`, `test_ci_environment.py`, the `test_cli.py` memory-profiler test, `test_enroot_provider.py`, `test_opensandbox_compose.py` and `test_super_vllm_launcher.py`; the subtest count varies by one or two between runs because some `test_super_vllm_launcher.py` subtests are flaky on macOS. - `uvx ruff@0.9.9 format --check` and `uvx ruff@0.9.9 check` on the changed Python files, and `pre-commit run --files <changed files>`: clean, no files modified. - New tests cover: - fingerprint: the shipped `benchmarks/gdpval/config.yaml` plus `vllm_model.yaml` keep the same fingerprint when `policy_base_url`, `policy_api_key` or `RERUN_INCOMPLETE` change and a different one when the policy model changes; bare `base_url`/`api_key`/`default_headers`/`openai_default_headers` and agent `concurrency` are ignored, `agent_max_turns` is not; - transport repair: the candidate root comes from the linked agent (a resources-server `persist_deliverables_dir` or another server's agent is ignored), relative and non-directory roots are rejected, a root that does not exist yet pins every task, all-incompatible tasks keep their draw, and a capacity-infeasible stage keeps its original draw with `infeasible` set; - failure contract: an explicit `false` survives the single-pass driver, and multistage rejects it before touching any state; - a loop-level run with one `reference_missing` row under a 0.9-floor partial policy: without `tolerate_unresolved` the stage stays open (not coverage-blocked) and stage 2 is not planned; with it the stage is accepted as partial and stage 2 runs; - the stage planner's repair seam: the repaired mapping is dispatched and its receipt recorded in the plan, and a repair that drops a task or picks a reference outside the stage raises; - the retryable-stop log line. ## Rollouts **Live GDPVal smoke (final code):** all seven PRs of the series applied together on main `1c8261080`, run with the devel-only SLURM launcher on a 4-task AA-v2 comparison smoke (one 4-GPU policy node, CPU judging): rollout 4/4 tasks saved and finished (1h04m, 0 failure rows), Office preparation 7/7 converted, comparison judging 4/4 tasks judged with 0 imputed and 0 excluded, 3 wins / 1 loss. Per-task outcomes match an earlier gdpval-devel run on the same checkpoint, tasks and references. No rate limiting occurred, so the judge retry budget was not exercised live. Until then the multistage-specific paths (resume, partial completion, transport repair, the final-stage guard) are covered by the unit tests above only. ## Compatibility and benchmark impact Only runs that go through `rollout_collection_driver: resources_servers.gdpval.multistage_orchestrator:run_rollout_collection` are affected; `benchmarks/gdpval/config.yaml` sets it. Other environments are unchanged. - **Stages no longer advance past failures (GDPVal multistage only).** On main every stage is marked complete and the run always reaches the final stage. Now a non-final stage without `partial_completion` needs a usable battle for every planned row: - one retryable or undispatched row stops the run after that stage with exit status 0 and no `comparison/eval_elo`; rerun with `--resume`, or set `multistage.retry_inprocess`; - one terminal or max-attempt row makes the run raise after writing its rollouts; - under GDPVal's failure contract `reference_missing`, `eval_missing` and `transport_ineligible` are retried across resumes, so a missing reference deliverable costs up to `NEMO_GYM_MAX_ROLLOUT_ATTEMPTS` launches before the coverage rejection. `partial_completion.tolerate_unresolved`, or `transport_assignment_repair.reference_availability_only` when another reference has capacity, avoids that. - Strictness does not apply to single-stage runs or to the final stage. - **Existing multistage journals won't resume.** The fingerprint inputs changed, so the first run after upgrading starts fresh; the old state is renamed aside. After that, changing the dataset rows, `num_repeats`, the rollout settings above, or any server setting other than the ignored fields invalidates the journal. - **Verify cache is keyed per run.** Cached `/verify` results are namespaced by the fingerprint, so judgements cached by earlier runs are not reused under `RERUN_INCOMPLETE`, and any fingerprint change re-judges everything (judge cost). - **Old files stay on disk.** Fresh starts leave `*.stale.<ns>` files and stage restarts leave `*.pruned.<ns>` files next to the output; remove them by hand. - **Changing an accepted partial policy fails closed.** Removing or changing a stage's `partial_completion` (other than `tolerate_unresolved`) after that stage was accepted as partial makes resume raise; start a new output path instead. - **Failure contract on the single-pass path.** Resumed single-pass GDPVal runs retry legacy terminal timeouts and move invalid-judge rows back to the sidecar unless the config sets those keys to `false`. `benchmarks/gdpval/config.yaml` sets both to `true` in NVIDIA-NeMo#3686. Multistage runs reject an explicit `false`. - **`route_failures_to_sidecar` now reaches multistage.** Multistage runs that set it record a failed `/run` as a sidecar row instead of raising. - **`dispatch_budget_s` needs a worker limit.** On the multistage path it requires a positive `num_samples_in_parallel`, or the run raises `ValueError`. - **New row and journal fields.** Rollout rows gain `expected_final_stage_index`, `expected_stage_row_count` and `verify_cache_namespace`; retried rows carry `_ng_attempt_index`. The journal gains `attempt_dispositions`, `partial_complete` and restart records; a repaired stage plan carries a `transport_assignment_repair` receipt. - **New opt-in settings, all off by default:** - `multistage.retry_inprocess`: `false`. - `multistage.stages[i].partial_completion`: unset. When set, defaults are `min_success_fraction: 1.0`, `min_per_reference_success_fraction: 1.0`, `min_successful_rows_per_reference: 1`, `waivable_failure_classes: [timeout_exceeded]` (only `timeout_exceeded` and `transient` allowed) and `tolerate_unresolved: false`. `tolerate_unresolved: true` waives any persisted failure class, so the coverage floors are the only guard. - `multistage.transport_assignment_repair`: unset. Keys: `enabled`, `reference_availability_only: false`, `max_file_bytes`, `max_raw_bytes`, `max_wire_bytes`, `max_section_raw_bytes` (default: the judge limits in `comparison.py`) and `framing_reserve_bytes` (default `4194304`). - **Stage planning without the opt-ins is unchanged.** Task sampling and the uniform reference draw are the same, so the same seed gives the same plan unless `partial_completion` (balanced assignment) or transport repair is enabled. - **Benchmark results:** the ELO fit math is unchanged. Rows flagged `invalid_judge_response` are now left out of pooling and retried, and partial stages fit only their included rows. With NVIDIA-NeMo#3686's aggregation, the headline `comparison/eval_elo` is emitted only when the final stage has every expected row and a finite fit. For the new overlay, its 18 fixed anchors (read from the GDPval-AA v2 board on 2026-09-09, 09-11 and 09-17) set the scale; a stale anchor shifts every candidate. Judging-score changes come from NVIDIA-NeMo#3686. - **Docs:** the overlay header documents the strict first stage and resuming. `benchmarks/gdpval/README.md` (NVIDIA-NeMo#3686) documents `partial_completion`, stage strictness, `retry_inprocess`, `transport_assignment_repair`, what the resume fingerprint ignores, and the `.stale`/`.pruned` files. The other settings are described in code docstrings. ## Checklist - [ ] I have read the [contributing guidelines](https://github.com/NVIDIA-NeMo/Gym/blob/main/CONTRIBUTING.md). - [x] The change is focused; unrelated "drive-by" edits are tracked as separate issues/PRs. - [x] Tests added or updated and pass locally, or N/A for docs-only / non-code changes (so CI unit/server checks pass when applicable). - [x] Documentation added or updated, or N/A with justification. (Overlay header and docstrings here; the README sections for these settings — stage completion, transport-aware reference assignment, resume fingerprint — ship in NVIDIA-NeMo#3686, which owns `benchmarks/gdpval/README.md`.) - [x] Pre-commit checks pass locally (`pre-commit run --all-files`) (so CI lint/format checks pass). (Run on this PR's files.) - [x] New source files include the required Apache-2.0 SPDX header. - [x] All commits have DCO sign-off (`git commit -s`) (so the DCO check passes). --------- Signed-off-by: Serge Panev <spanev@nvidia.com> Co-authored-by: Marta Stepniewska-Dziubinska <marta-sd@users.noreply.github.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.
What does this PR do?
Adds a no_infinite_endpoint_retries option for vLLM endpoints. When enabled, retryable endpoint errors no longer increase max_num_tries, so requests fail after the fixed retry limit instead of retrying indefinitely.
Checklist
pre-commit run --all-files) (so CI lint/format/copyright pass).git commit -s) (so the DCO check passes).