eval: CheckpointEvalFn — one contract for checkpoint-consuming eval backends#1782
Open
yueming-yuan wants to merge 9 commits into
Open
eval: CheckpointEvalFn — one contract for checkpoint-consuming eval backends#1782yueming-yuan wants to merge 9 commits into
yueming-yuan wants to merge 9 commits into
Conversation
Three call sites (in-job fleet, external service, and the shared-engine pin loop) each reimplemented "load an HF snapshot, confirm weight_version, retry on mismatch" against different transports (Ray actor vs HTTP). Extract that into WeightTarget (RayEngineTarget/HttpServerTarget) + pin_and_verify() in miles/utils/weight_target.py; the external service also gains the same retry-on-mismatch behavior it was missing. Thread generate_state/weight_version through RolloutFnEvalInput so RolloutManager is the only place that decides fleet-vs-shared eval; InferenceRolloutFn and FullyAsyncRolloutFn no longer read --eval-num-gpus or own an EvalFleetSession, they just use input.generate_state if given. EvalFleetSession now only builds+caches the fleet GenerateState (RolloutManager calls it), rather than also running the eval datasets.
…o example An eval fn that declares eval_needs_snapshot = True now receives each eval point's HF snapshot path (RolloutFnEvalInput.hf_dir) and owns delivering it to its own backend. EvalDispatcher gates snapshot export + async dispatch on "needs a snapshot" instead of eval_num_gpus > 0, so external backends get the same export/overflow/skip machinery without an in-job fleet; RolloutManager routes fleet / external-fn / shared-engine in one place and the fleet and external paths share the run+log+GC tail. Because such a fn runs inside the training job, it reads the real training args — the hand-copied EVAL_ARG_DEFAULTS config of the standalone service (silently wrong for e.g. LoRA runs) is no longer the only way to eval on external GPUs. examples/fully_async/external_eval_fn.py implements the contract against any sglang-compatible HTTP server; the standalone service moves from tools/ to examples/fully_async/ and now drives that same fn, so "how to eval a snapshot" has exactly one implementation. Also fix test_eval_runs_on_dedicated_fleet monkeypatching the source module instead of the consumer binding (the patch never took effect).
Collapse the three-way eval routing (fleet / external-fn / shared) to the real two-way split: eval runs against the live training engines (pinned by blocking call order) or against a checkpoint file (pinned by the file). CheckpointEvalFn in miles/rollout/checkpoint_eval.py is the contract for the second posture: __init__ prepares the backend, evaluate_checkpoint(dir, input) returns results, EvalSkip(reason) gives every backend the attributable-skip channel the fleet used to own privately, dispose() tears down. FleetEvalFn implements the contract for the in-job fleet (absorbing EvalFleetSession and the health/pin/router logic that lived in RolloutManager) and delegates generation to the inner eval fn, so custom eval fns keep working on the fleet. resolve_checkpoint_eval_fn() at manager construction is now the only discrimination point; the eval_needs_snapshot attribute, the driver-side fn loading, and EvalDispatcher's mirrored validation asserts are gone — dispatch asks the manager once via eval_uses_snapshots(). ExternalSglangEvalFn becomes a true black box: it launches its own sglang server on user-named GPUs (MILES_EXTERNAL_EVAL_GPUS) or attaches to one (MILES_EXTERNAL_EVAL_URL); the standalone service drops its launch/health code and just drives the fn.
Follow the example idiom (one launcher runs the whole job, features attach via --xxx-function-path): run_qwen3_5_4b_fully_async_eval.py launches fully-async training with --eval-backend fleet|external behind the same CheckpointEvalFn contract, replacing the fleet-only shell script. ExternalSglangEvalFn is configured by env only (URL to attach, GPUS to launch, SERVER_ARGS passes arbitrary sglang flags through); the constructor kwargs existed only for the standalone service, which is deleted — it duplicated the trainer surface with hand-copied EVAL_ARG_DEFAULTS (silently wrong for e.g. LoRA runs). This drops the detached use cases the service carried (post-hoc backlog eval without a training job; external eval for --colocate runs, which use train.py and never export snapshots); the fn is importable if someone needs a custom driver.
Both pin call sites now live inside CheckpointEvalFn implementations, so the Protocol + RayEngineTarget/HttpServerTarget adapter layer no longer earns its ceremony. pin_and_verify keeps the one correctness-critical piece — the load-all / read-back / all-must-match / retry loop — and takes per-target load/read callables; the fleet builds them over actor handles, the example over two nested HTTP closures.
The generic callable-list interface was fleet-shaped (multi-target gather, all-must-match, empty-fails) with the external server as a degenerate single-element caller. FleetEvalFn gets _pin_fleet over its own engine handles; the example inlines a 12-line single-server pin — the reference implementation now shows the pin discipline (read back the version, retry, never trust a silent load) explicitly instead of hiding it behind a core helper. pin_and_verify leaves the public API; its unit tests fold into the fleet and example tests.
Removed: make_eval_args (3-line wrapper), run_eval_datasets merge (gather + dict-merge), router-not-ready (a try/except reason mapping), base-checkpoint marker exemption (a single != guard, covered by any e2e with eval-before-train), dispatcher drain (mirror of a 4-line loop). Merged the two fleet pin tests into one: two engines, one stale — all-must-match, one retry, attributable skip.
yueming-yuan
requested review from
Shi-Dong,
Zhichenzzz,
fzyzcjy,
guapisolo,
jybsuper and
maocheng23
as code owners
July 24, 2026 06:03
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
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.
Chained on #1740. Refactors the eval backend surface it introduced; no change to the shared-engine default path (byte-identical behavior, guarded by a regression test).
Design
Eval has exactly two postures: against the live training engines (version pinned by blocking call order) or against a checkpoint file (version pinned by the file itself). This PR gives the second posture one contract:
__init__prepares the backend (launch or attach); each call receives a snapshot path and returns results. The trainer owns everything else: per-point HF export, async dispatch/overflow policy, logging at the snapshot's step, snapshot GC. Because the fn runs in-job, all eval config (datasets, sampling, rm, lora) comes from the real training args — nothing is hand-copied.FleetEvalFn(core): the dedicated fleet (--eval-num-gpus) as a backend of this contract — absorbs the health-probe/pin/router logic that lived inRolloutManagerandEvalFleetSession, then delegates generation to the inner eval fn with the fleet'sGenerateState(custom eval fns keep working on the fleet).ExternalSglangEvalFn(example): self-contained black box — launches its own sglang server on user-named GPUs (MILES_EXTERNAL_EVAL_GPUS, extra flags viaMILES_EXTERNAL_EVAL_SERVER_ARGS) or attaches to one (MILES_EXTERNAL_EVAL_URL); pins each snapshot (load → read back version → retry) and runs the standard eval datasets.checkpoint_dirto its API and mapping the response intoRolloutFnEvalOutput.Routing collapses to one discrimination point (
resolve_checkpoint_eval_fnat manager construction, fail-fast validation included) and one snapshot path (RolloutManager._eval_checkpoint);EvalDispatcherno longer loads the eval fn in the driver — it asks the manager once viaeval_uses_snapshots().examples/fully_async/run_qwen3_5_4b_fully_async_eval.pylaunches either backend behind one flag (--eval-backend fleet|external), replacing the fleet-only shell script.Removed (deliberate capability regressions vs #1740)
tools/checkpoint_eval_service.pyis deleted rather than kept as a second driver: its hand-copiedEVAL_ARG_DEFAULTSconfig was silently wrong for any run whose eval-relevant args differ from miles defaults (e.g. LoRA evals ran the bare base weights). This drops two detached use cases the service carried:--colocateruns (they usetrain.py, which never exports snapshots).Testing
tests/fast: 30 tests (fleet pin ordering/all-must-match/retry, dead-engine recovery, EvalSkip attribution, GC ring + never-delete-outside-staging, shared-path shape regression, dispatcher overflow policies, example fn pin discipline and launch mode).pre-commit run --all-filesclean.Commit
84359491f(weight_target fold) is a pure mechanical move with a reproducible transform script (mechanical-refactor-verify PASS); later commits then collapsed that abstraction into the two backends.