Repository navigation
[mirror #240] qwen4exp: run MTP heads that share the target's token_embd and output - #11
LeoBorcherding wants to merge 6 commits into
Conversation
MTP heads exported with nextn_shared_target_tensors carry no token_embd or output, which load_arch_tensors required, so mainline refused them. For such a head created as an MTP draft, take both from the target the draft context is created against. ctx_other itself stays unset: for draft-mtp it means the draft shares the target's memory, which this head does not. Any other use of such a head fails with a clear error rather than a null dereference. Heads converted before upstream MTP support also mark the MTP block dense (compress ratio 0). graph_mtp built the k-pool input regardless, but the draft's indexer cache then holds no layer, so the input was never allocated and the first decode asserted in set_input. Build it only for a QSA MTP block; build_layer_attn already treats ratio 0 as dense.
A shared MTP draft placed on other devices than the target (for example --spec-draft-device none or CUDA1) read token_embd/output from target buffers on devices its context had no backend for, and context creation aborted. Initialize a backend for every target device the draft context does not already have.
|
@codex review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 924234599b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (cparams.ctx_type != LLAMA_CONTEXT_TYPE_MTP || !tgt || !tgt->tok_embd || !tgt->output || | ||
| tgt->hparams.n_embd != model.hparams.n_embd || tgt->vocab.n_tokens() != model.vocab.n_tokens()) { | ||
| throw std::runtime_error(model.arch_name() + " head without token_embd/output only works as an MTP draft (--spec-type draft-mtp) of a matching target (this warning is normal during memory fitting)"); |
There was a problem hiding this comment.
When --fit measures a separate MTP-only Qwen4Exp sidecar, common_fit_apply() creates its extra-model context without ctx_other, so this guard rejects the probe and the fallback in common/fit.cpp drops the sidecar's memory from the estimate. The real speculative setup supplies the target context and loads the draft, so --fit can choose a target split that leaves no room for the MTP block and OOMs when the draft is initialized. Please make the fit probe account for the sidecar using the target heads.
Useful? React with 👍 / 👎.
|
Not actioned at |
status at
08490ff: not converged · codex: clean at 08490ff · fork CI: 0 pass, 0 fail, 0 pending · final diff review: pending · phases: 0/9 doneMirror of unslothai#240 at
924234599b, base13ec8f614c(merge base withbase/upstream-13ec8f614). A sandbox: night-shift-tester tests and pushes fixes here, never to the author's branch.