Fix best checkpoints during async evaluation - #18
Conversation
|
/claude review |
|
Light review complete. Read the full diff and traced the new delayed-best-checkpoint promotion path (transactional actor/critic/HF copy with .tmp/.old rollback, orphaned-backup recovery, and the One minor typo flagged inline in the Not approving — comments only. |
47ce1d6 to
6ac1650
Compare
7700bda to
a15b494
Compare
|
Rebased this onto the latest |
a15b494 to
12aacde
Compare
Signed-off-by: Riad Darwish <darwishriad0@gmail.com>
Signed-off-by: Riad Darwish <darwishriad0@gmail.com>
Signed-off-by: Riad Darwish <darwishriad0@gmail.com>
12aacde to
56ca194
Compare
|
Rebased onto the latest |
Summary
Async evaluation receives a rollout slot before it acquires the vLLM lock. A
refit can win that race, so the slot number alone does not identify the policy
used to generate the evaluation samples.
The vLLM lock now carries the step of the policy loaded by vLLM. A successful
broadcast publishes its step while releasing the lock. Non-partial evaluation
records the version returned by that same lock. The metric is therefore attached
to the policy that generated the samples in either lock ordering.
When training has already advanced, the trainer promotes the matching
global_stepNcheckpoint instead of saving the current actor. It stages theactor, critic, and optional HF export before publishing any destination. The
actor checkpoint is published last as the durable commit marker. A copy or
rename failure rolls back every published destination, removes pending
directories, and leaves the in-memory best metric unchanged. Previous best
checkpoints are pruned only after publication succeeds.
Configurations that can delay a best metric require
--ckpt.save_steps 1, atleast
async_queue_size + 1rolling checkpoints, and no disk-size pruning.force_sync_modeis exempt only with one queue slot. The Qwen quick starts andthe 4B, 35B, and Omni3 Slurm recipes now use the required cadence. The 753B smoke recipe keeps its sparse checkpoint cadence and explicitly disables
best-checkpoint selection.
A resumed run recovers the best score from finalized actor metadata and ignores
unfinished
.tmpand.olddirectories. A fresh run with checkpoint loadingdisabled ignores best metadata left in its output directory. HF exports follow
the retained actor tags. Partial rollouts still permit several policy versions
within one evaluation, so that mode skips best-checkpoint selection.
Testing
python -m pytest -q tests/unit/test_rl_trainer.py(33 passed)python -m pytest -q(187 passedon the pre-v0.1.4 tree)python -m compileall -q molt examples/python testsbash -n examples/scripts/*.sh examples/scripts/quick_start/*.sh examples/scripts/slurm/*.shpre-commit run --files <changed files>git diff --checkThe unit tests cover both outcomes of the rollout-slot/refit race, delayed and
direct saves, two pending metrics, partial rollouts, restart behavior, fresh-run
isolation, HF retention, and the checkpoint configuration matrix. Filesystem
tests inject copy and publication failures independently for actor, critic, and
HF destinations. They check rollback, cleanup, source preservation, and
unchanged metric state, including an
ENOSPCcopy failure. They also coverrecovery of an orphaned
.oldbackup and ignoring aNaNmetric.Earlier distributed GPU validation
Before the final transaction and Slurm recipe changes, I ran the parent and a
patched revision consecutively in the same on-demand Verda session in FIN-02 on
2026-07-18 UTC. Both started from
f6d9191623c3bdc3eada42cde80c0877247636d7. The production diff executed inthat run has SHA-256
96a3f06f8edfd75c462f80a74b131a2b4107b4da3a0462c40726ecbdf4e1ffaa.That run validates the lock versioning, successful actor promotion, serialized
model and optimizer payloads, and vLLM refits for that exact revision. It is not
byte-for-byte GPU validation of the current diff. The final transaction,
critic/HF failure paths, and additional Slurm defaults are covered locally.
NV12between every GPU pair innvidia-smi topo -mhijkzzz/molt:0.1.2atsha256:b9c82365b0c65e9cd4daf0addc34c9a5eba89cfc4593fa2e480246dc7c1dfcd2Qwen/Qwen3-4B-Instruct-2507at Hugging Face snapshotcdbee75f17c01a7cc42f958dc650907174af0554async_queue_size=2,ckpt.save_steps=1, and non-partial rollouts5.8.1
Each run used mixed reward groups, completed four training steps, evaluated
policy step 2 while the trainer reached step 4, and fingerprinted the serialized
model and optimizer payloads. In the patched run, evaluation acquired the lock
first and the step-3 broadcast waited 65.65 seconds. The broadcast-first
ordering is covered by the lock interleaving unit tests.
global_step4global_step2Checkpoint fingerprints and refit details
Parent run:
ddff4b2b4bd24d9c3dd985df2b46b023b446ca25fd7643b4199757846c79da08,while step 2 =
215e9ce0b42c7e37523f8f0da6b3446d4bd5df92b8ea67c7c4e735445b1bad66991b9402af2c687b40d855307d3260c7b78ceed31f013b48c481771b59e2be4e,while step 2 =
c1243241a5b6c7cd1b66e32fa2bab003b171c81cf5c23ef7bfcbc3226c983319GPU-tested revision:
92072e0bd013d3ff30b90081ce55b8490acece961d5e4246568a46505f6d2aec,while step 4 =
341bb2423a0061d9d4941cee7012142caf126819ff2cc16a5570cad622eaada28fad15d336f9c11fb8deb85950008cfad3db41a2ff0563fd8efb9c413f69e6f8,while step 4 =
cd2696ee9a613066495def15f120ae755e6125e4df231a30d7aee6bc9b4b1bea4313755ec60143f5a3231717b19f16d2507d977d976010c087c7bcc538f6dfff,while step 4 =
05bac86c0bc42a7f1799dde2f29b4ebb0f46df1b3999d3293dc5d28cbea48733The run enabled
--train.check_weight_update_equal. After every broadcast,all floating-point parameters held by each vLLM worker were reported as
refreshed. vLLM separately ignored one
lm_head.weightflush per worker andbroadcast. Qwen ties that alias to its input embeddings
(
tie_word_embeddings=true), so it is not a separate vLLM parameter. Thewhole-worker check passed after all four broadcasts.
Harness SHA-256 values:
1941c78282d72b02aaff12d9a242e17c72d9c15a6672090755ca05cabfc99b3a6c1bb21877b91961a3571f42454a79b4a595dfd691bd91b4a728f1477b9b4a6481c123bf8a64fecc3b69bf91093f7086b6f52663da90182b5d12d41d2166962eThe evidence archive contains sanitized logs, GPU inventory and topology,
runtime and source provenance, payload manifests, copied optimizer metadata,
and four provider-console crops showing Verda, 8 x A100 40GB, FIN-02, and the
running state. It contains no account detail, network address, instance name,
resource identifier, financial field, or raw session identifier. Each run
produced about 180 GiB of checkpoint data. The archive represents the roughly
135 GiB compared subset with per-shard hashes instead of attaching the shards.
Evidence archive: pr3-verda-a100-evidence.zip, SHA-256
055b7e766e4442ac1063f6c712a39c200655fab6cc9ef68d8372e9b6d6023f93