[GDPVal] Make partial-calibration waivable failure classes configurable - #2704
Merged
Kh4L merged 1 commit intoAug 24, 2026
Conversation
Partial calibration could only ever waive `timeout_exceeded`. A single
unresolved `transient` rollout — in practice one judge 400, e.g. the
empty-PDF "The document has no pages." from gemini-3.1-pro, which has no
model-group fallback — therefore held the whole run at stage 1.
Observed on invocation dc6c776f3af506df (Qwen3.5-122B-A10B, GDPVal-AA-v2):
45/45 stage-1 rollouts collected in 26 min, one `transient` row unresolved,
`stage 1/2 remains incomplete (1 rollout(s) retryable or undispatched)`,
stage 2 never planned, `comparison/eval_elo` null, 44 of 220 tasks delivered.
`PartialStagePolicy` gains `waivable_failure_classes`, defaulting to
`("timeout_exceeded",)` so existing behaviour is unchanged. The class is
validated against a closed set; `skipped` stays excluded because an unusable
sample is not evidence. The durable snapshot records the set and revalidation
compares it, so widening the policy correctly invalidates a frozen outcome.
A waived row is still omitted from the fit and still counts against every
coverage floor — this bounds which causes are acceptable, not how much
evidence may be missing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Alex Gronskiy <agronskiy@nvidia.com>
Contributor
|
LGTM! Thanks |
agronskiy
added a commit
that referenced
this pull request
Aug 27, 2026
The nemo_gym edit was redundant. The verify response already stamps the generic
_ng_failure_terminal flag, and _is_terminal_failure falls through to
return bool(record.get(NG_TERMINAL_KEY))
so teaching it a GDPVal-specific class name changed nothing. Reverted
nemo_gym/rollout_collection.py and its test, moved
REFERENCE_MISSING_FAILURE_CLASS into resources_servers/gdpval/app.py, and
relocated the terminal/success tests alongside the rest of the gdpval suite.
Better layering on its own terms -- reference_missing is GDPVal vocabulary and
the harness only needs the generic flag -- and it also fixes CI. Touching
nemo_gym/ flipped unit-tests.yml from the server-only path onto a full run, whose
Test job enforces fail_under=96 while measuring the whole repo. The gdpval
package is ~2300 uncovered statements there because its tests run in the separate
sharded server-suite job, whose coverage is never merged in, so the gate reported
81.55% and failed with 356 passed and 0 failures. Confining the diff to
resources_servers/gdpval keeps the run on the server path, which is how PR #2704
passed.
Signed-off-by: Alex Gronskiy <agronskiy@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
2 participants
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.
On top of #2588,
PartialStagePolicy.waivable_failure_classesbecomes a configurable tuple instead of being hardcoded to("timeout_exceeded",). The default is unchanged, so existing configs behave identically.Change
36767c8052c76c7047551262e19fb1afd481233e, branched off14125b7650317b1d1269987c840ea70deba7e5f4(the tip of [GDPVal] Harden production rollout and judging paths #2588 at the time).resources_servers/gdpval/multistage_elo.py(new field),resources_servers/gdpval/multistage_orchestrator.py(7 edits: policy record now emitsnewly_waivable_failure_classes; the waiver check consults the configured set;_WAIVABLE_FAILURE_CLASSESvalidation frozenset is{"timeout_exceeded", "transient"}; snapshot revalidation maps the record keynewly_waivable_failure_classesback to the config keywaivable_failure_classes), plus 3 new tests inresources_servers/gdpval/tests/test_multistage_orchestrator.py(test_transient_omission_advances_when_explicitly_waivable,test_unknown_waivable_failure_class_is_rejected,test_empty_waivable_failure_classes_is_rejected).resources_servers/gdpval/tests/.Motivation
The operative blocker was NOT the waiver scope:
partial_completionis opt-in and was never set, somultistage_orchestrator.py:1090took the strict branch and required every planned non-final rollout to carry battle evidence. That cost two GDPVal-AA-v2 runs their entire stage 2 (dc6c776f3af506dfand6c5ee8b81773b2b6, 44/220 each,eval_elonull). Oncepartial_completionis enabled, atransientjudge failure still stopped the stage because the waiver set was hardcoded — which this PR fixes.Production evidence
From Nemotron Super 3.5 run
590aeb2c72b1de1don this commit:[multistage-elo] stage 1/2 accepted partial calibration: success coverage 91.1%, omitted 4 rollout(s)timeout_exceeded, task 155timeout_exceeded, task 159timeout_exceeded, task 136transientsuccess_fraction0.9111, worst per-reference 0.8,multistage_statestage-0 statuspartial_completeeval ELO = 737.5over 4 refs. The predecessor run died at 44/220 witheval_elonull.[multistage-elo] resuming multi-stage run from cache (fingerprint match)and[multistage-elo] stage 1/2 reused partial result from cache: eval ELO = 627.4 (41 cached rollout(s))— so thenewly_waivable_failure_classes<->waivable_failure_classesround-trip works across legs.df318339a24e9384, completed ateval ELO = 882.6with a clean 45/45 stage-1 close (the waiver path did not fire there).Pre-merge validation
Against
36767c80, with policy{min_success_fraction: 0.9, min_per_reference_success_fraction: 0.5, min_successful_rows_per_reference: 1, waivable_failure_classes: [timeout_exceeded, transient]}and a synthetic 45-rollout stage: an omission of classtimeout_exceededaccepts atsuccess_fraction0.9778,transientaccepts at 0.9778, andlegitimateis rejected.🤖 Generated with Claude Code