Skip to content

[Bugfix] Fix Granite 4.1 Vision deepstack handoff between pipeline stages - #65818

Merged
njhill merged 4 commits into
vllm-project:mainfrom
xsxszab:granite4-vision-pp-deepstack
Nov 9, 2026
Merged

njhill merged 4 commits into
vllm-project:mainfrom
xsxszab:granite4-vision-pp-deepstack

Conversation

@xsxszab

@xsxszab xsxszab commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Overview

Granite 4.1 Vision (#40282) does not work with --pipeline-parallel-size > 1. Every non-first stage fails at startup with KeyError: 'ds_<layer>', and once that is fixed, later stages receive the forwarded deepstack features as zeros, which silently drops them from image requests. This PR fixes both: each stage declares only the deepstack buffers it receives, and the first stage forwards copies of the features instead of views of buffers it zeroes before the PP send.

Claims

  • Granite 4.1 Vision starts and serves with PP > 1; on main it fails at startup for any split.
  • With eager execution, PP2 output is identical to PP1 on image and text prompts: same greedy tokens and logprobs.
  • Adds a CI test that compares an image request between PP2 and no PP; it fails without the second fix and passes with this PR.

Validation

ibm-granite/granite-vision-4.1-4b, bf16, 1–2× B300, --max-model-len 8192. Six requests sent one at a time: three image chats and three text completions, 32 greedy tokens each with top-5 logprobs.

Check main Declaration fix only This PR
PP2 startup KeyError: 'ds_9' on stage 1 during warmup Starts Starts
Eager PP2 vs PP1, text prompts — 3/3 identical 3/3 identical
Eager PP2 vs PP1, image prompts — 2/3 identical tokens, logprobs differ by up to 0.27 3/3 identical, logprob difference 0

Two eager PP1 runs are identical (6/6, difference 0). With CUDA graphs, PP2 starts and serves; its outputs differ from PP1 at near-ties by the same amount as PP1 with CUDA graphs differs from PP1 in eager mode, because the two configurations capture different graph sizes.

vllm serve ibm-granite/granite-vision-4.1-4b --dtype bfloat16 --max-model-len 8192 \
  --pipeline-parallel-size 2 --enforce-eager

Test. test_pp_granite4_vision_deepstack serves a 4-layer dummy-weight variant with and without PP2 and compares a text and an image request (generate_with_image). With four layers, deepstack layers 0 and 3 land on different stages. It fails with only the declaration fix and passes with this PR:

pytest -v -s "tests/distributed/test_pipeline_parallel.py::test_pp_granite4_vision_deepstack"
declaration fix only:  AssertionError: Results for model='ibm-granite/granite-vision-4.1-4b' are not the same.
                       (the text_image comparison; pure_text matches)
this PR:               1 passed

On main the same test cannot start the PP2 server (the KeyError above). It runs in the "(L4) Distributed" CI step, which runs the whole file. It uses dummy weights because the registry still marks this checkpoint as not available online; the real-weight runs above were done outside CI.

Details

Startup failure. The deepstack and spatial features target layers 0, 3, 6, 9, 12, 15, 18 and 21. On a non-last stage, Granite4VisionLLMModel.forward forwards only ds_<l> with l >= end_layer, the features later stages inject. Granite4VisionLLMForCausalLM.make_empty_intermediate_tensors declared ds_<l> for every layer regardless of the stage, and Model Runner V2 copies every declared key from the received tensors (#42187), so the keys of earlier layers were missing. The fix declares ds_<l> only for l >= start_layer, which is exactly what the previous stage sends. The runner's strict copy is left as is: a declared key that never arrives should keep failing loudly rather than reuse a stale buffer.

Zeroed features. On the first stage, the forwarded ds_<l> are views of _ds_buffers. Granite4VisionForConditionalGeneration.forward zeroes those buffers right after the language model returns, so that stale features do not leak into the next request, and the worker sends the intermediate tensors only after the forward returns. Later stages therefore received zeros: with a 20/20 split, layer 21 lost its features, and with PP4, layers 12–21 lost theirs. Text requests are unaffected because their buffers are zero anyway. The fix forwards copies, at the cost of one (num_tokens, hidden_size) copy per forwarded layer.

No open PR changes this code path; #65412, the other open Granite 4.1 Vision PR, touches only the image preprocessing.

AI assistance (Claude Code) was used to find the bugs, write the change and the test, and draft this description. The submitter reviewed every changed line and ran the validation above.


Pull Request Checklist
  • I used vLLM's /pr-checklist skill. (Mandatory for agents, optional for humans).

  • AI assistance was used during the creation of this PR.

  • Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.

  • Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.

  • Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.

  • Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.

xsxszab and others added 2 commits November 4, 2026 17:15
…ages

With pipeline parallelism, Granite 4.1 Vision failed at startup on every
non-first stage with KeyError: 'ds_<layer>', and once that was avoided,
later stages received the forwarded deepstack features as zeros.

- Declare only the deepstack buffers a stage receives: the previous stage
  forwards the features of layers from this stage's first layer on.
- Forward copies of the features: on the first stage they are views of
  buffers the outer model zeroes right after the forward, before the PP send.

Add the model to the multimodal pipeline-parallel test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yifei Wang <yifei.w@inferact.ai>
…equest

Replace the text-only entry in the multimodal pipeline-parallel test with a
dedicated test that compares an image request between PP2 and no PP. With
four text layers, deepstack layers 0 and 3 land on different stages, so it
fails if a later stage receives missing or zeroed features.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yifei Wang <yifei.w@inferact.ai>
@mergify mergify Bot added the bug Something isn't working label Nov 5, 2026
@artem-spector

Copy link
Copy Markdown
Contributor

We hit the same startup failure independently on 2026-11-03 while working on #65412: main at c7a7607, ibm-granite/granite-vision-4.1-4b, pipeline_parallel_size=2 (offline LLM, mp executor, H100). Stage 1 failed during warmup with

vllm/v1/worker/gpu/model_runner.py:2090
    else v[:n].copy_(intermediate_tensors.tensors[k][:n])
KeyError: 'ds_9'

and the engine then hung on No available shared memory broadcast block found instead of exiting, so the job had to be cancelled. We didn't pursue it, so thanks for the fix, including the zeroed-features part, which we hadn't noticed.

Happy to rerun that setup with this branch on our H100s if a second data point is useful.

@xsxszab

xsxszab commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

Hi @artem-spector, thanks for confirming! This PR should fix the ds_9 startup error you hit. A rerun with this branch on your H100 setup would be very helpful

@artem-spector

Copy link
Copy Markdown
Contributor

Reran this on our setup (2× H100 80GB, ibm-granite/granite-vision-4.1-4b, bf16, --max-model-len 8192, offline LLM with the mp executor):

PR base (1ab4476) This PR (ed33170)
PP2 startup, CUDA graphs KeyError: 'ds_9' (model_runner.py:2099), then hangs starts, serves
test_pp_granite4_vision_deepstack — passed
Eager PP2 vs PP1, 3 image + 3 text prompts, 32 greedy tokens, top-5 logprobs — 6/6 identical, logprob difference 0

PP1 output is also identical between the base and this PR (6/6), so single-GPU behavior is unchanged. Thanks again for the fix.

@xsxszab
xsxszab marked this pull request as ready for review November 5, 2026 08:02
@xsxszab
xsxszab requested a review from youkaichao as a code owner November 5, 2026 08:02

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@njhill

njhill commented Nov 9, 2026

Copy link
Copy Markdown
Member

/ci run

@njhill njhill added the ready ONLY add when PR is ready to merge/full CI is needed label Nov 9, 2026
@njhill
njhill enabled auto-merge (squash) November 9, 2026 22:45
@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown

✅ @xsxszab, CI is now available for this PR.

  • /ci run starts upstream CI; /amd-ci run starts AMD CI only.
  • Run commands test your branch with every commit currently on its upstream target branch. If it is behind, the bot merges the target branch into it first (pull before pushing again). Append --allow-stale to a run command to test an outdated branch as is, at your own risk.
  • /ci retry retries failed jobs in the CI build for the current PR head. If the current head has no CI build, it starts a new CI build for the current head containing only jobs that failed in the latest earlier CI build for this PR.
  • /amd-ci retry retries failed jobs in AMD CI for the current PR head. Use /amd-ci run when the current head has no AMD CI build.
  • /ci cancel cancels scheduled or running CI builds for this PR branch; /amd-ci cancel does the same for AMD CI only.

@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #101280 for commit d17b0136d325.

ℹ️ This PR is 337 commits behind upstream main. Merged upstream main into this branch as d17b0136d325 before starting CI. Pull the branch before pushing more changes.

@njhill
njhill merged commit 8729aad into vllm-project:main Nov 9, 2026
122 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants