Aggregate metrics at log time only - #6943
Draft
qgallouedec wants to merge 10 commits into
Draft
Conversation
…gface/trl into sft-metrics-single-reduce
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.
Branch
sft-metrics-single-reduce(main merged in at 67dfbe2). Alternative to #6678 (@michaelbenayoun), which first moved SFT's metric collectives to log time; this takes the same idea with running sums instead of per-step buffers, and applies it to all main-code trainers (SFT, Distillation, GRPO, RLOO, DPO, KTO, Reward).How it works
Same
(total, count)pair thatRLOOTrainerandGRPOTraineralready reduce today, kept as a running sum instead of being reduced every step:_metric_mins/_metric_maxs, onereduce("max")each (min as -max(-x)); ±inf sentinels keep key sets rank-symmetric; empty window logs None._metricsfloat lists remain only for values already global for free: stats derived from gathers the training needs anyway (completion lengths →num_input_tokens_seen, rewards → advantages) and Python constants (entropy_coef,step_time).num_items_in_batch, KTO's loss KL, ref-logp precompute).Communication removed
log_metric)Plus the matching
.item()host syncs. Under gradient accumulation these gathers were the only communication on K−1 of K micro-batches (grad all-reduce fires once, underno_sync); in eval there is no grad sync at all.Measured on 8×H100 (SmolLM2-135M, seq ≤256, bs 2, gas=64; two interleaved main/branch runs each):
The win grows with rank count, accumulation, and cheaper micro-batches; expect more on multi-node interconnects (unmeasured here).
Benchmark script
Run once on
mainand once on this branch:torchrun --nproc_per_node 8 bench_metrics.py main # then: bench_metrics.py branchDeliberate value changes
logging_steps=1, and no longer depend onlogging_steps.gather_for_metricsflat truncation at uneven last batches (it silently dropped whole ranks' scalars).rewards/*log 0.0 instead of omitting the key; margins still need both sides.num_tokensadvances at train-timelog(); an eval log in between can lag by ≤1 window.Verification
logging_steps=1, where the old and new semantics coincide: Reward exact (including the cross-rank min/max path), DPO 4e-6, GRPO and RLOO exact apart fromstep_time, SFT and KTO exact apart from epoch-boundary steps, which is the truncation fix above.test_log_averages_over_the_window_weighted_by_countpins the weighting: two steps of 1 token at entropy 3.0 and 9 tokens at entropy 1.0 log 1.2, not 2.0.Follow-up (not here, to bound review)
Experimental copies keep the old pattern: async_grpo, async_distillation, gmpo, grpo_with_replay_buffer (still work: GRPO keeps
_metrics), and the DPO-family forks (cpo, orpo, bco, tpo, online_dpo, xpo, nash_md, ppo).