Skip to content

[Bugfix] Fix Basic Models Test#34818

Merged
vllm-bot merged 26 commits intovllm-project:mainfrom
MatthewBonanni:fix_basic_extra_init
Feb 19, 2026
Merged

[Bugfix] Fix Basic Models Test#34818
vllm-bot merged 26 commits intovllm-project:mainfrom
MatthewBonanni:fix_basic_extra_init

Conversation

@MatthewBonanni
Copy link
Collaborator

@MatthewBonanni MatthewBonanni commented Feb 18, 2026 •
edited by github-actions bot
Loading

Purpose

Fixes #34806, #34810, #34814, and #34819

Test Plan

Test Result

Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

@MatthewBonanni
Fix
2a25c4b 
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
@mergify mergify bot added speculative-decoding bug Something isn't working labels Feb 18, 2026
gemini-code-assist[bot]
gemini-code-assist bot reviewed Feb 18, 2026
View reviewed changes
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces two small but useful changes. The first is an update to the model registry in tests/models/registry.py to provide a more specific reason for skipping tests related to H2OVLChatModel, which will improve test stability. The second change, in vllm/model_executor/models/minicpm_eagle.py, explicitly disallows the use of inputs_embeds for the EagleMiniCPMForCausalLM model by raising a NotImplementedError. This is a good defensive measure to prevent incorrect usage of the model and provides a clear error message. Both changes are correct and improve the robustness of the codebase. I approve these changes.

@mgoin mgoin added ready ONLY add when PR is ready to merge/full CI is needed ci-failure Issue about an unexpected test failure in CI labels Feb 18, 2026
@MatthewBonanni
Fix
1a40186 
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
@MatthewBonanni
Fix
c617228 
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
@DarkLight1337
Copy link
Member

DarkLight1337 commented Feb 18, 2026

Hmm this simply skips the tests, which we didn't have to do before #33600

@MatthewBonanni MatthewBonanni changed the title [Bugfix] Fix Basic Models Test (Extra Initialization) [Bugfix] Fix Basic Models Test Feb 18, 2026
@MatthewBonanni
Make _update_block_size_for_backend fault-tolerant
6ac74b8 
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
@mergify mergify bot added the nvidia label Feb 18, 2026
@MatthewBonanni
Copy link
Collaborator Author

MatthewBonanni commented Feb 18, 2026 •
edited
Loading

@DarkLight1337 good point - I changed this in 6ac74b8

DarkLight1337
DarkLight1337 reviewed Feb 18, 2026
View reviewed changes
@DarkLight1337 DarkLight1337 requested a review from mgoin February 18, 2026 17:29
@DarkLight1337
Copy link
Member

DarkLight1337 commented Feb 18, 2026 •
edited
Loading

Extra Initialization tests still fail

@mgoin
Copy link
Member

mgoin commented Feb 18, 2026

The extra init tests seem to be failing because of OOM, so unsure of clear debug

@DarkLight1337
Copy link
Member

DarkLight1337 commented Feb 18, 2026 •
edited
Loading

Might it be because #33600 caused the overrides/patches specific to the model initialization tests to not function correctly?

@MatthewBonanni
Copy link
Collaborator Author

MatthewBonanni commented Feb 18, 2026

@DarkLight1337 yes, I believe it's because _update_block_size_for_backend causes CUDA initialization

@MatthewBonanni
Workaround by limiting to MLA
dfed2a1 
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
@MatthewBonanni
Copy link
Collaborator Author

MatthewBonanni commented Feb 18, 2026 •
edited
Loading

As a workaround, I've decided to just only run _update_block_size_for_backend for MLA models. This was the behavior pre-33600. All standard attention backends support the default block size of 16. This avoids importing attention backends that initialize CUDA.

After this change, if the user sets a bad block size for a standard attention model, the attention backend selection won't factor this in. This was the pre-33600 behavior, though, so it should be alright.

I'll work on a fix that lets us get rid of the use_mla check in a follow up

EDIT: This workaround ended up not being any simpler than the full fix

@MatthewBonanni
Try-except should no longer be necessary
4065757 
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
mgoin
mgoin approved these changes Feb 18, 2026
View reviewed changes
Copy link
Member

@mgoin mgoin left a comment

Choose a reason for hiding this comment

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

The compromise is good with me to fix CI, thanks Matt

@github-project-automation github-project-automation bot moved this to Ready in NVIDIA Feb 18, 2026
@MatthewBonanni
Lazy allocate workspaces
40d3782 
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
@MatthewBonanni
Fix vllm config context
ce3fc1c 
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
@MatthewBonanni
Fix chunked local attention
5111418 
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
@MatthewBonanni
Fix config context
4bac453 
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
@MatthewBonanni
Fix ray executor
e5ac83c 
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
@MatthewBonanni
Re-add warning
99b3b3a 
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
@MatthewBonanni
Clean up
f7b337a 
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
@MatthewBonanni
Comment
982a892 
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
@MatthewBonanni
Clean up
9b741db 
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
@MatthewBonanni MatthewBonanni mentioned this pull request Feb 19, 2026
Closed
@MatthewBonanni
Use min across groups
3bb724a 
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
@vllm-bot vllm-bot merged commit 662205d into vllm-project:main Feb 19, 2026
64 of 67 checks passed
@github-project-automation github-project-automation bot moved this from Ready to Done in NVIDIA Feb 19, 2026
This was referenced Feb 20, 2026
Closed
Closed
Closed
@mgoin mgoin mentioned this pull request Feb 20, 2026
Closed
3 tasks
mgoin added a commit to neuralmagic/vllm that referenced this pull request Feb 20, 2026
@mgoin
Fix block_size mismatch for MLA models after vllm-project#34818
feed637 
Signed-off-by: mgoin <mgoin64@gmail.com>
@mgoin mgoin mentioned this pull request Feb 20, 2026
Closed
5 tasks
LucasWilkinson added a commit that referenced this pull request Feb 20, 2026
@LucasWilkinson
Revert "[Bugfix] Fix Basic Models Test (#34818)"
f8377bb 
This reverts commit 662205d.
@LucasWilkinson LucasWilkinson mentioned this pull request Feb 20, 2026
Merged
jmamou pushed a commit to jmamou/vllm that referenced this pull request Feb 23, 2026
@MatthewBonanni @robertgshaw2-redhat @jmamou
[Bugfix] Fix Basic Models Test (vllm-project#34818)
00ad10b 
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
Co-authored-by: Robert Shaw <114415538+robertgshaw2-redhat@users.noreply.github.com>
MatthewBonanni added a commit to MatthewBonanni/vllm that referenced this pull request Feb 23, 2026
@MatthewBonanni
Squashed commit of the following:
8d19437 
commit 4f9b8be
Author: mgoin <mgoin64@gmail.com>
Date:   Fri Feb 20 17:14:56 2026 +0000

    Cleanup

    Signed-off-by: mgoin <mgoin64@gmail.com>

commit feed637
Author: mgoin <mgoin64@gmail.com>
Date:   Fri Feb 20 17:08:37 2026 +0000

    Fix block_size mismatch for MLA models after vllm-project#34818

    Signed-off-by: mgoin <mgoin64@gmail.com>

Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
@MatthewBonanni MatthewBonanni mentioned this pull request Feb 23, 2026
Merged
5 tasks
llsj14 pushed a commit to llsj14/vllm that referenced this pull request Mar 1, 2026
@MatthewBonanni @robertgshaw2-redhat @llsj14
[Bugfix] Fix Basic Models Test (vllm-project#34818)
a442816 
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
Co-authored-by: Robert Shaw <114415538+robertgshaw2-redhat@users.noreply.github.com>
tunglinwood pushed a commit to tunglinwood/vllm that referenced this pull request Mar 4, 2026
@MatthewBonanni @robertgshaw2-redhat @tunglinwood
[Bugfix] Fix Basic Models Test (vllm-project#34818)
93c6fe4 
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
Co-authored-by: Robert Shaw <114415538+robertgshaw2-redhat@users.noreply.github.com>
sharvil10 pushed a commit to sharvil10/vllm that referenced this pull request Mar 4, 2026
@MatthewBonanni @robertgshaw2-redhat @sharvil10
[Bugfix] Fix Basic Models Test (vllm-project#34818)
6d9fb3e 
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
Co-authored-by: Robert Shaw <114415538+robertgshaw2-redhat@users.noreply.github.com>
askliar pushed a commit to askliar/vllm that referenced this pull request Mar 9, 2026
@MatthewBonanni @robertgshaw2-redhat
[Bugfix] Fix Basic Models Test (vllm-project#34818)
7a9ddc6 
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
Co-authored-by: Robert Shaw <114415538+robertgshaw2-redhat@users.noreply.github.com>
Signed-off-by: Andrii Skliar <askliar@nvidia.com>
Copilot AI pushed a commit to machov/vllm that referenced this pull request Mar 10, 2026
@MatthewBonanni @robertgshaw2-redhat
[Bugfix] Fix Basic Models Test (vllm-project#34818)
e6831e1 
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
Co-authored-by: Robert Shaw <114415538+robertgshaw2-redhat@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LucasWilkinson LucasWilkinson LucasWilkinson left review comments

@DarkLight1337 DarkLight1337 DarkLight1337 left review comments

@mgoin mgoin mgoin approved these changes

@ywang96 ywang96 Awaiting requested review from ywang96 ywang96 is a code owner

@pavanimajety pavanimajety Awaiting requested review from pavanimajety

@heheda12345 heheda12345 Awaiting requested review from heheda12345 heheda12345 is a code owner

@WoosukKwon WoosukKwon Awaiting requested review from WoosukKwon WoosukKwon is a code owner

@youkaichao youkaichao Awaiting requested review from youkaichao youkaichao is a code owner

@robertgshaw2-redhat robertgshaw2-redhat Awaiting requested review from robertgshaw2-redhat robertgshaw2-redhat is a code owner

@tlrmchlsmth tlrmchlsmth Awaiting requested review from tlrmchlsmth tlrmchlsmth is a code owner

@houseroad houseroad Awaiting requested review from houseroad houseroad is a code owner

@hmellor hmellor Awaiting requested review from hmellor hmellor is a code owner

@yewentao256 yewentao256 Awaiting requested review from yewentao256 yewentao256 is a code owner

@ProExpertProg ProExpertProg Awaiting requested review from ProExpertProg ProExpertProg is a code owner

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug Something isn't working ci-failure Issue about an unexpected test failure in CI multi-modality Related to multi-modality (#4194) nvidia ready ONLY add when PR is ready to merge/full CI is needed speculative-decoding v1

Projects

CI Failures
Status: Done
NVIDIA
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[CI Failure]: models/test_initialization.py::test_can_initialize_large_subset[EagleMiniCPMForCausalLM]

7 participants

@MatthewBonanni @DarkLight1337 @mgoin @robertgshaw2-redhat @NickLucche @LucasWilkinson @vllm-bot