[Refactor] Drop the GGUF draft workarounds now that vLLM covers them - #117
Draft
WhatGhost wants to merge 3 commits into
Draft
[Refactor] Drop the GGUF draft workarounds now that vLLM covers them #117WhatGhost wants to merge 3 commits into
WhatGhost wants to merge 3 commits into
Conversation
Converting this architecture to GGUF is not a pure requantization. Four things change on the way in, and each one loads without complaint and produces fluent but wrong output if it is not undone: the Q/K rows are re-laid out from the half-split NEOX order into llama.cpp's interleaved order, the per-layer norms have the architecture's `1 +` folded into the stored weight, the Q/K norms are synthesized from the config's scale factor rather than stored as learned parameters, and the vision patch embedding is reduced to the sum of its per-time-step blocks. The adapter reverses the first three exactly. The Q/K permutation is applied to the packed bytes directly, since GGUF splits super-blocks along the input dimension and so leaves each output row self-contained. The fourth is exact for still images, which depend on the sum alone because the encoder expands one patch to every time step. Video depends on the blocks individually and cannot be recovered, so it is declared unsupported and rejected during input validation rather than served from a reconstruction that is off by about 7% in the channel carrying motion. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: whatghost <yuyang.gao@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: whatghost <yuyang.gao@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: whatghost <yuyang.gao@amd.com>
WhatGhost
marked this pull request as draft
August 21, 2026 05:58
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.
Stacked on #115, which is stacked on #113 — the first two commits belong to those PRs; this PR is the third commit
ae13af7alone. I'll rebase onto main once both land.This stays a draft until vllm-project/vllm#53214 lands. It deletes the workarounds that PR makes unnecessary, so it cannot merge before it: without
SpeculativeConfig.hf_config_paththe key this PR passes through is rejected outright, and without the tolerant speculators probe a.gguftarget never reaches this plugin's config parser.Summary
#113 and #115 each reach into vLLM to get a GGUF draft past a place where a condition that is not an error is treated as one. Upstream now handles both, so the workarounds come out:
plugin.pyloses 34 lines net and one monkeypatch entirely. Nothing user-visible changes — the launch command is identical before and after, and the acceptance rate is unchanged.What comes out
The draft names its config source instead of having its weights reference rewritten.
SpeculativeConfignow takeshf_config_path, sospeculative_config["model"]can stay pointed at the.gguffile. What goes with the rewrite is all the state it needed: #115 poppedhf_config_pathout of the dict (the field was ours, andSpeculativeConfigrejects keys it does not declare), stashed the original path onengine_args._gguf_draft_weights, and put it back onmodel_weightsafterwards — all so that a second pass over the sameEngineArgs, which no longer saw a GGUF path, would not keep the config directory as the weights source and quietly load the unquantized checkpoint sitting there. Withmodelnever rewritten, idempotency stops being something to remember and the weights fall back tomodelthe way they do for every other model.The speculators probe patch is gone. #113 wrapped
maybe_override_with_speculatorsto short-circuit on a GGUF reference, and had to install the wrapper into botharg_utilsandtransformers_utils.config, since each holds its own reference to it. The probe now treats a reference it cannot read as "not a speculators model", so the wrapper, both module-level imports and both sentinel flags come out. The test that covered it stays, renamed: it pins upstream's behaviour now rather than the plugin's patch, which is also why this PR's suite goes red on an older vLLM.What does not come out, contrary to what #115 said
#115 listed the
{"quant_method": "gguf"}marker as a workaround that "comes out once that check stops raising upstream". That was wrong, and since the promise is on the record it is worth being explicit about. The marker has a second job that is load-bearing: a draft rebuilds its quantization config fromhf_config.quantization_config, so that dict is the only channel through which the loader's declaration — which of the draft's modules the GGUF file stores dense — can reach the draft's layers before they are built._publish_declaration_for_a_draftwrites into that dict and returns silently when it is absent, so removing the marker does not fail; it produces a draft that builds quantized layers for dense weights. The function therefore stays, renamed_open_the_drafts_declaration_channelwith a docstring that says what it actually does. Its body is unchanged.That also settles the limitation #115 recorded alongside it: the
hf_overridesfallback inget_quant_confignever rejects anything here, because planting the dict makes the lookup return at the branch above it. Nothing upstream needs to change there, which is why the upstream PR carries two fixes rather than the three or four first proposed.Dependencies
Testing
test_muse_glimmer_dflash_gguf.pyandtest_plugin.py, against a vLLM carrying the two upstream fixes. The dflash tests now pin the inverted invariant:modelstays on the.gguffile whilehf_config_pathnames the config directory, naming the source twice changes nothing, and an unquantized draft's dict comes back untouched key for key. Two new tests cover the declaration channel, including that an existing declaration is not clobbered.vllm serve, which is the path the removedhf_config_pathhandling has to survive: the server starts, the draft resolves its own architecture fromhf_config_path, and speculative decoding is live on the served request (42 of 66 draft tokens accepted).