feat(trainer): fold rl/ref_kl loss types into one pg skeleton, vectorize compute_loss#2951
Draft
hallerite wants to merge 1 commit into
Draft
feat(trainer): fold rl/ref_kl loss types into one pg skeleton, vectorize compute_loss#2951hallerite wants to merge 1 commit into
hallerite wants to merge 1 commit into
Conversation
…ize compute_loss The default (DPPO), IPO, and ref_kl loss fns are the same per-token loss -(keep · adv_tau·A · ratio) + kl_tau · mask · log_ratio² differing only in the advantage provider, the trust region, and the drift coefficient. Express them as PGParams instances of one pg_loss_fn skeleton, and run each component of compute_loss once over the concatenated bin instead of once per packed sequence — removing the per-sequence loop, the per-sequence bool(mask.any()) device syncs, and the graph anchor (the rl component always runs, so the loss is always backward-able and FSDP ranks stay in sync by construction). Gradients are bit-identical to the previous implementation across all component archetypes; loss values match to fp summation order. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Motivation
default_loss_fn(DPPO),ipo_loss_fn, andref_kl_loss_fnare three copies of the same per-token lossdiffering only in three parameters:
(ref_lp − trainer_lp).detach(), recomputed every forward1e-3This is the REINFORCE identity:
∇KL(π_θ‖π_ref) = −E[(log π_ref − log π_θ)·∇log π_θ]— reverse-KL distillation is policy gradient with a live advantage provider. Keeping three drifting copies hides that, and hides that "which trust region / which κ should OPD use" is a parameter choice, not a structural difference.Separately,
compute_losslooped over packed sequences, issuing O(sequences × components) kernel launches plus abool(mask.any())device sync per component per sequence. Benchmarked on a 32k-token micro-batch at 256 seqs/bin, that loop costs 120–190 ms of exposed critical-path time — and none of it hides behind the forward, because the driver launch queue only absorbs ~1–2k pending launches.Changes
PGParams— frozen dataclass selecting one instance of the skeleton:advantage: "shipped" | "ref_kl",trust_region: "dppo" | "ipo" | "one_sided",adv_tau,kl_tau, thresholds, metric prefix.pg_loss_fn— the single skeleton.default_loss_fn,ipo_loss_fn, andref_kl_loss_fnremain as thin adapters, sosetup_rl_loss_fn,trainer.lossconfig handling, and all existing imports are unchanged. ref_kl's hard-coded1e-3is now a visible parameter.compute_loss— concatenates the bin once and runs each component exactly once, instead of once per packed sequence. The per-sequence loop, the per-sequencebool(mask.any())D2H syncs, and the graph anchor are gone: the rl component always runs (an empty mask contributes an exact zero), so the loss is always backward-able and every rank runs backward — FSDP collectives stay in sync by construction.No config, wire, or orchestrator changes. The rl/ce/ref_kl streams and their semantics are untouched — the fold is trainer-internal.
Verification
tests/unit/train/rl/test_loss.py: 10/10 pass unmodified, including exact-value CE assertions, the empty-components backward test, and hot-path/explicit-ones equality.Performance (RTX PRO 6000, real
compute_loss+ backward)Cost is now flat in packing density, as a per-token loss should be.
Behavioral notes
CustomLossConfig): the fn now receives the packed sequences concatenated in a single call per micro-batch instead of one call per sequence. Per-token-sum losses (like all built-ins) are unaffected; a custom fn doing per-sequence normalization would change behavior._safe_meanand ce's boolean indexing still triggernonzerosyncs, but a fixed handful per micro-batch instead of O(sequences × components); included in the 1–4 ms above. Replacing them with mask-multiply reductions is a follow-up if we want the losstorch.compile-able.🤖 Generated with Claude Code