Add hybrid LK loss (adaptive KL/TV blend) for speculative decoding#673
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR adds an ChangesLK hybrid loss plumbing
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/train.py (1)
966-971: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate
--loss-fnhelp to document thelk_hybridchoice.
lk_hybridwas added tochoices, but the help text still only describeskl_divandce, so users won't know what the new option does or that--lk-loss-etaapplies to it.📝 Proposed help text update
help=( "Loss function used during draft model training. " "'kl_div' = KL divergence (default). " - "'ce' = cross-entropy." + "'ce' = cross-entropy. " + "'lk_hybrid' = adaptive KL/TV blend (see --lk-loss-eta)." ),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/train.py` around lines 966 - 971, The `--loss-fn` help text in the training CLI is missing the newly added `lk_hybrid` option, so update the argument help near the `choices` definition in `train.py` to describe `lk_hybrid` alongside `kl_div` and `ce`, and mention that it uses `--lk-loss-eta` as its mixing parameter. Keep the description consistent with the existing `loss_fn`/`lk_loss_eta` argument names so users can understand the available training modes from the CLI help.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@scripts/train.py`:
- Around line 966-971: The `--loss-fn` help text in the training CLI is missing
the newly added `lk_hybrid` option, so update the argument help near the
`choices` definition in `train.py` to describe `lk_hybrid` alongside `kl_div`
and `ce`, and mention that it uses `--lk-loss-eta` as its mixing parameter. Keep
the description consistent with the existing `loss_fn`/`lk_loss_eta` argument
names so users can understand the available training modes from the CLI help.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 80a07b9c-2c60-4de6-8e9a-39ea47b11988
📒 Files selected for processing (6)
scripts/train.pysrc/speculators/models/dflash/core.pysrc/speculators/models/eagle3/core.pysrc/speculators/models/metrics.pysrc/speculators/models/peagle/core.pytests/unit/models/test_metrics.py
Addresses CodeRabbit review on vllm-project#673: the --loss-fn help only described kl_div and ce; add the lk_hybrid option and point to --lk-loss-eta. Signed-off-by: GIREESH7963 <abburigireesh@gmail.com>
|
addressed in 03bc603. The |
fynnsu
left a comment
There was a problem hiding this comment.
Looks good but thing to fix below.
|
This pull request has merge conflicts that must be resolved before it can be |
Adds lk_hybrid, a per-position loss that blends KL divergence and total variation with an acceptance-rate-adaptive weight: L = lambda*KL(p||q) + (1-lambda)*TV(p,q), lambda = exp(-eta * sg[alpha]), where alpha = sum_v min(p_v, q_v) is the (detached) acceptance rate. When overlap is low it leans on KL's strong gradient; as overlap grows it shifts to TV, which optimizes acceptance directly. eta defaults to the paper's best hybrid setting (3.0). Registered as "lk_hybrid" in _LOSS_FN_MAP, so it works both as a standalone --loss-fn name and inside the JSON weighted-combination spec. Adds unit tests covering the KL/TV limit cases, shape/finiteness, the stop-gradient on alpha, and resolution. Source: Samarin et al., "LK Losses: Direct Acceptance Rate Optimization for Speculative Decoding" (arXiv 2602.23881), hybrid objective. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: GIREESH7963 <abburigireesh@gmail.com>
03bc603 to
08019d3
Compare
|
Rebased onto
Net functional diff is now just the |
…llm-project#673) ## Summary Adds the hybrid LK loss: an adaptive per-position blend of KL and TV, L = lambda*KL + (1-lambda)*TV with lambda = exp(-eta * sg[alpha]), alpha = overlap. ## Motivation TV optimizes acceptance directly but has a vanishing gradient from a cold start; KL has a strong early gradient but optimizes a proxy. The hybrid leans on KL when overlap is low and shifts to TV as the draft aligns, making TV's acceptance-optimal target trainable from scratch. Source: Samarin et al., arXiv 2602.23881. Paper's best setting: eta=3. ## Changes - `lk_hybrid_loss(logits, targets, eta=3.0)` in metrics.py, reusing `kl_div_loss` and the TV overlap term; alpha in the blend weight is detached. - `resolve_loss_fn` gains `lk_loss_eta` (default 3.0), binding eta into the "lk_hybrid" entry via functools.partial (single binding point; call sites unchanged). - `--lk-loss-eta` CLI flag (mirrors --dflash-decay-gamma), threaded through each get_trainer_kwargs into resolve_loss_fn; "lk_hybrid" added to --loss-fn choices. - Tests: eta->0 == KL, eta->inf == TV, shape/finiteness, alpha-detach gradient check, resolve binds eta. ## Notes - The adaptive KL/TV blend and the DFlash positional decay are orthogonal weightings. - tv/nla are not in the --loss-fn choices list either; happy to add them here if desired. - Loss key name "lk_hybrid" open to preference. Signed-off-by: GIREESH7963 <abburigireesh@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…llm-project#673) ## Summary Adds the hybrid LK loss: an adaptive per-position blend of KL and TV, L = lambda*KL + (1-lambda)*TV with lambda = exp(-eta * sg[alpha]), alpha = overlap. ## Motivation TV optimizes acceptance directly but has a vanishing gradient from a cold start; KL has a strong early gradient but optimizes a proxy. The hybrid leans on KL when overlap is low and shifts to TV as the draft aligns, making TV's acceptance-optimal target trainable from scratch. Source: Samarin et al., arXiv 2602.23881. Paper's best setting: eta=3. ## Changes - `lk_hybrid_loss(logits, targets, eta=3.0)` in metrics.py, reusing `kl_div_loss` and the TV overlap term; alpha in the blend weight is detached. - `resolve_loss_fn` gains `lk_loss_eta` (default 3.0), binding eta into the "lk_hybrid" entry via functools.partial (single binding point; call sites unchanged). - `--lk-loss-eta` CLI flag (mirrors --dflash-decay-gamma), threaded through each get_trainer_kwargs into resolve_loss_fn; "lk_hybrid" added to --loss-fn choices. - Tests: eta->0 == KL, eta->inf == TV, shape/finiteness, alpha-detach gradient check, resolve binds eta. ## Notes - The adaptive KL/TV blend and the DFlash positional decay are orthogonal weightings. - tv/nla are not in the --loss-fn choices list either; happy to add them here if desired. - Loss key name "lk_hybrid" open to preference. Signed-off-by: GIREESH7963 <abburigireesh@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Eros483 <arnabmandal2912@gmail.com>
Summary
Adds the hybrid LK loss: an adaptive per-position blend of KL and TV,
L = lambda*KL + (1-lambda)*TV with lambda = exp(-eta * sg[alpha]), alpha = overlap.
Motivation
TV optimizes acceptance directly but has a vanishing gradient from a cold start; KL has a
strong early gradient but optimizes a proxy. The hybrid leans on KL when overlap is low and
shifts to TV as the draft aligns, making TV's acceptance-optimal target trainable from
scratch. Source: Samarin et al., arXiv 2602.23881. Paper's best setting: eta=3.
Changes
lk_hybrid_loss(logits, targets, eta=3.0)in metrics.py, reusingkl_div_lossand theTV overlap term; alpha in the blend weight is detached.
resolve_loss_fngainslk_loss_eta(default 3.0), binding eta into the "lk_hybrid"entry via functools.partial (single binding point; call sites unchanged).
--lk-loss-etaCLI flag (mirrors --dflash-decay-gamma), threaded through eachget_trainer_kwargs into resolve_loss_fn; "lk_hybrid" added to --loss-fn choices.
resolve binds eta.
Notes