fix(rollout): guard agentic sync and deterministic diffusion eval - #294
Merged
Conversation
2 tasks
haonan3
force-pushed
the
fix/engine-owned-weight-sync
branch
from
August 2, 2026 11:55
4439031 to
c077c35
Compare
…dger Closes the weight_version accounting gap documented as a follow-up in #287: ARTrainer.evaluate() pushed weights via _prepare_rollout without advancing the driver-side counter, leaving the engine on unnumbered weights until the next interval boundary. The async trainer families also disagreed on eval policy: AR synced-without-bump, diffusion explicitly passed sync_weights=False, agentic has no eval. Structure: both driver-side engines replace bump_weight_version() with sync_weights(weight_sync) - one call that pushes and advances the ledger, so the pairing stops being call-site discipline. The batch engine also enforces the quiesce invariant (raises before pushing if any generation is in flight). All async sync sites (boundary, resume, agentic drive) route through it; bump_weight_version drops to zero callers and is removed. Policy: async eval becomes read-only. ARTrainer.evaluate() gains sync_weights: bool = True (mirroring DiffusionTrainer.evaluate); the async trainers override evaluate() with async-correct defaults (no push; diffusion also defaults sleep_after=False), so bare evaluate() calls are safe outside train() too. A pre-train explicit push raises a lifecycle RuntimeError instead of an incidental AttributeError. Deliberate behavior changes: (1) async-AR eval no longer pushes weights, so BOTH the eval series and the training rollouts launched between an eval and the next interval boundary change - they previously ran under eval-synced fresher weights; deployment cadence is now governed solely by weight_sync_interval. At interval=1, or when eval_interval is a multiple of the sync interval, training is point-identical to main. (2) The scored eval policy is 1..interval optimizer steps old (exactly 1 at interval=1). (3) The resume-time push advances the ledger 0->1 (metric offset only - eviction math is relative; the launch ceiling is computed from rollout_id).
haonan3
force-pushed
the
fix/engine-owned-weight-sync
branch
from
August 2, 2026 13:13
c077c35 to
97329c3
Compare
2 tasks
haonan3
marked this pull request as ready for review
August 2, 2026 17:40
haonan3
requested review from
CjhHa1,
Ideny42,
celve,
leviking98z-rgb and
zzhuoxin1508
as code owners
August 2, 2026 17:40
github-actions
Bot
added
need review
eval_eta=0 previously rode along with the training-resolved sde_indices — a contradictory request the central kernel silently degrades to ODE but BAGEL's worker-resident scheduler refuses (RuntimeError at the first gated step). Clearing the gate at eval_eta<=0 makes the request say what eval means; SD3-family trajectories are unchanged (they already ran ODE).
The actor-side [LoRA-SYNC] lines never reach the driver log, leaving weight deployments invisible in stdout. sync_weights is now the single push path, so one driver-side INFO line covers every async push.
Resolve the async conflicts by retaining only the Agentic drive guard and sync logging, plus the deterministic pure-ODE eval fix.
CjhHa1
changed the title
CjhHa1
approved these changes
Aug 4, 2026
CjhHa1
left a comment
CjhHa1
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed after the main rebase and scope reduction. The remaining diff is limited to the Agentic live-drive sync guard/logging and the deterministic diffusion pure-ODE fix; the batch ledger and resident-policy eval changes were removed for #310.
github-actions
Bot
added
approved
14 tasks
CjhHa1
added a commit
to CjhHa1/UniRL
that referenced
this pull request
Aug 4, 2026
Preserve the optimizer-update batch control while carrying forward Tencent-Hunyuan#294's independent Agentic sync guard/logging and pure-ODE eval fix.
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
3 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.
Summary
AsyncAgenticRolloutEngine.sync_weights()the single Agentic push/version transaction, reject sync while a drive is live, and log each successful sync on the drivereval_eta <= 0) a pure-ODE request by clearing the SDE gate and schedulerThe earlier batch-engine version-ledger and resident-policy eval changes have been removed. Batch AR/diffusion policy-version semantics are handled by #310; eval there keeps the current-policy contract (
P=Uat the hard boundary).Related Issue
Follow-up to #287. Complementary to #310; this PR now contains only the independent Agentic guard/logging and diffusion eval fix.
Test Plan
ruff checkandruff format --checkon all changed Python filespython3 -m py_compileon all changed Python filesCompatibility / Risk
bump_weight_version()must usesync_weights(weight_sync)so push and version advancement cannot diverge.eta=0+ SDE-gate requests; stochastic eval is unchanged.