fix(sglang): request hidden states for every trainable drafter algorithm - #65
Open
khazic wants to merge 1 commit into
Open
Conversation
The SGLang request gate carried its own copy of the algorithm-to-layout mapping:
algorithm == "EAGLE3" -> last_hidden
algorithm in {"DFLASH", "DSPARK"} -> dflash aux hidden
Both sets have drifted from what speco_worker can actually train. EAGLE-1/2 and
P-EAGLE need last_hidden_states (their compute_loss raises without it, and both
reject use_logits=True), and Domino consumes the same aux context layers as
DFlash. None of the four appear in either set, so on an SGLang rollout with
collect_hidden_states_from_sgl they request nothing and drafter training is
starved.
EAGLE-1/2 are especially easy to miss here: Eagle1TrainerBackend.model_type
deliberately reports "eagle3" so it can reuse the EAGLE-3 data plumbing, but
this gate reads the raw config string instead, so the masquerade does not reach
it.
oldlogprob_layer_ids.resolve_drafter_hidden_states_layout is already the shared
answer to this question (speco_worker and speco_ray_trainer both use it, and it
lists DOMINO in DFLASH_FAMILY_ALGORITHMS). Route the SGLang gate through it
instead of keeping a second copy that can drift again.
Signed-off-by: khazic <khazzz1c@gmail.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.
Problem
The SGLang request gate carries its own copy of the algorithm-to-hidden-state-layout mapping:
Both sets have drifted from what
speco_workercan actually build a training backend for:last_hidden_states.Eagle1TrainerBackend.compute_lossandPEagleTrainerBackend.compute_lossboth raiseValueError("... requires last_hidden_states")without it, and bothbuild_modelimplementations rejectuse_logits=True, so there is no fallback.DominoTrainerBackendsubclassesDFlashTrainerBackendand itscompute_losssplitsbatch["hidden_states"]intonum_context_layers.None of the four appear in either set. On an SGLang rollout with
collect_hidden_states_from_sgl, they request nothing at all and drafter training is starved.EAGLE-1/2 are especially easy to miss here:
Eagle1TrainerBackend.model_typedeliberately reports"eagle3"so it can reuse the EAGLE-3 data plumbing, but this gate reads the raw config string, so the masquerade never reaches it.Fix
oldlogprob_layer_ids.resolve_drafter_hidden_states_layoutis already the shared answer to this exact question.speco_workerandspeco_ray_trainerboth use it, its docstring spells out the split ("DFlash-family drafters (DFlash, DSpark, Domino) consume the raw aux context layers, while EAGLE-family drafters also need the final hidden state"), andDFLASH_FAMILY_ALGORITHMSalready listsDOMINO.Route the SGLang gate through it instead of keeping a second copy that can drift again. The two predicates keep their names and their mutual exclusivity, and the shared collection preconditions move into one helper.
oldlogprob_layer_idsimports nothing fromverl_speco, so there is no import cycle.Validation
Ran a before/after repro on both
mainand this branch. For every algorithmspeco_workeraccepts, it asks what layout the trainer needs (via the shared resolver) and what the SGLang gate actually requests.Before/after repro output
Tests
Three new tests in
tests/integration/test_sglang_adapter_contract.py:test_every_trainable_drafter_requests_its_hidden_state_layoutwalks all seven algorithms through the real request path and pins which custom param each one gets.test_last_hidden_env_follows_the_shared_layout_resolvercoversVERL_SGLANG_DRAFTER_RETURN_LAST_HIDDENfor the same seven.test_hidden_state_gates_stay_off_without_drafter_collectionpins the preconditions that must keep both gates off (enable=False,enable_drafter_training=False,use_logits=True).Full CPU suite (
tests/integration tests/compat tests/config tests/examples) run on both sides:Test suite before/after
The same 9 tests fail on
mainand on this branch. They need optional dependencies (VeOmni, the NPU vLLM stack) that are absent in this environment, so they are pre-existing and unrelated. The+15on this branch are the new parametrized tests above.