You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Token-based micro-batching (trainer.max_tokens_per_microbatch, added in #1477) is honored by the loss-less forward and by forward_backward, but the loss-path forward (i.e. forward(data, loss_fn=...), used for eval / loss-only inference such as Tinker FORWARD requests carrying a loss) silently ignores it and always chunks by micro_forward_batch_size_per_gpu:
Megatron: MegatronPolicyWorker.forward — the loss_fn is None branch goes through _forward_logprobs, which uses get_microbatch_iterator and reorders results back to input order; the loss_fn branch builds micro-batches with a plain BatchIterator.
FSDP / base worker: PolicyWorkerBase.forward in workers/worker.py — same split: the loss-less branch uses get_microbatch_iterator + reorder_and_combine_batches, the loss branch uses BatchIterator.
Consistency: forward, forward(loss_fn=...), and forward_backward should batch the same way so their compute/memory behavior (and micro-batch composition) matches.
Proposal
Mirror forward_backward in both workers' loss-path forward:
Build micro-batches with get_microbatch_iterator(data, micro_batch_size=micro_forward_batch_size_per_gpu, max_tokens_per_microbatch=...).
For Megatron, pad micro-batches to a uniform size with _pad_microbatch_to_size (Megatron's pipeline schedule requires uniform micro_batch_size) and track num_real_microbatches for metric normalization.
Skip padding-microbatch metrics when reducing, as forward_backward does.
Regression coverage can extend the stub-worker tests added in #2209 (tests/backends/skyrl_train/test_token_based_batching_utils.py) to the loss-path forward.
Problem
Token-based micro-batching (
trainer.max_tokens_per_microbatch, added in #1477) is honored by the loss-lessforwardand byforward_backward, but the loss-pathforward(i.e.forward(data, loss_fn=...), used for eval / loss-only inference such as Tinker FORWARD requests carrying a loss) silently ignores it and always chunks bymicro_forward_batch_size_per_gpu:MegatronPolicyWorker.forward— theloss_fn is Nonebranch goes through_forward_logprobs, which usesget_microbatch_iteratorand reorders results back to input order; theloss_fnbranch builds micro-batches with a plainBatchIterator.PolicyWorkerBase.forwardinworkers/worker.py— same split: the loss-less branch usesget_microbatch_iterator+reorder_and_combine_batches, the loss branch usesBatchIterator.Why it matters
forward,forward(loss_fn=...), andforward_backwardshould batch the same way so their compute/memory behavior (and micro-batch composition) matches.Proposal
Mirror
forward_backwardin both workers' loss-pathforward:get_microbatch_iterator(data, micro_batch_size=micro_forward_batch_size_per_gpu, max_tokens_per_microbatch=...)._pad_microbatch_to_size(Megatron's pipeline schedule requires uniformmicro_batch_size) and tracknum_real_microbatchesfor metric normalization.loss_fn_outputsback to input order withTokenBasedBatchIterator.reorder_and_combine_items, dropping entries produced for padding — token packing permutes samples, so this step is required for correctness (see [Tinker API] Restore variable-length result order #2043 and [fix] Restore input order for FSDP forward_backward loss_fn_outputs under token batching #2209, which fixed the missing reorder in the twoforward_backwardpaths).forward_backwarddoes.Regression coverage can extend the stub-worker tests added in #2209 (
tests/backends/skyrl_train/test_token_based_batching_utils.py) to the loss-pathforward.Related
forward_backwardoutputs)forward_backward)