Skip to content

model: fix vllm d3 multi seq case - #30408

Merged
ngxson merged 2 commits into
ggml-org:masterfrom
ngxson:xsn/d3_fix
Oct 12, 2026
Merged

ngxson merged 2 commits into
ggml-org:masterfrom
ngxson:xsn/d3_fix

Conversation

@ngxson

@ngxson ngxson commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator

Overview

Fix efe3626#commitcomment-204575113

Not 100% sure but seems like there is a problem with the case: non-causal + hybrid + multi seq

This seems to fix the problem

Requirements

@ngxson
ngxson requested a review from ggerganov as a code owner October 12, 2026 18:18

@ggerganov ggerganov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, when computing non-causal data, it has to fit in a single ubatch. The proposed fix should be good.

It can be improved by allowing split_seq to fit multiple sequences in the ubatch, as long as they can fit fully. This way for example, with an ubatch of 4096 tokens, one can process in a single pass 4 sequences of length 1024, instead of splitting them in 4 ubatches.

Add TODO to support that:

diff --git a/src/llama-batch.h b/src/llama-batch.h
index 8882c9941e..e1e43979b5 100644
--- a/src/llama-batch.h
+++ b/src/llama-batch.h
@@ -186,6 +186,7 @@ public:
     llama_ubatch split_equal(uint32_t n_ubatch, bool sequential, uint32_t n_keep_tail);
 
     // sequence-set-wise split - each ubatch contains a single sequence-set
+    // TODO: extend to support multiple _full_ sequences in the ubatch
     llama_ubatch split_seq(uint32_t n_ubatch);
 
     // a helper method for creating a well-defined ubatch of tokens

@ngxson
ngxson merged commit cbe28bb into ggml-org:master Oct 12, 2026
12 checks passed
edwardyoon pushed a commit to edwardyoon/focus-llama that referenced this pull request Oct 24, 2026
* model: fix vllm d3 multi seq case

* add TODO
auRose94 pushed a commit to auRose94/llama.cpp that referenced this pull request Oct 28, 2026
* model: fix vllm d3 multi seq case

* add TODO
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants