Skip to content

Fix the failing test for basic model tests - #47077

Closed
aarushjain29 wants to merge 5 commits into
vllm-project:mainfrom
ROCm:fix-basic-models-extra-init-amd-v2
Closed

aarushjain29 wants to merge 5 commits into
vllm-project:mainfrom
ROCm:fix-basic-models-extra-init-amd-v2

Conversation

@aarushjain29

Copy link
Copy Markdown
Contributor

Purpose

Fix the three failing models in test_can_initialize_large_subset for the "Basic Models Tests (Extra Initialization)" group (AMD MI300 nightly, also reproducible upstream):

  • InternS1Pro: grouped router asserts 2 cannot be divided by 8 because dummy_hf_overrides ignores router_n_groups.
  • Eagle3DeepseekV3: reduced-layer Kimi-K2.5 returns a bare tensor while the runner unpacks hidden_states, _ = outputs; CUDA graph capture then trips an MLA decode-only assertion.
  • MistralLarge3 (675B NVFP4): OOM during the profile run.

Test Plan

pytest -v -s models/test_initialization.py -k 'test_can_initialize_large_subset[<model>]' on MI300X (gfx942).

Test Result

  • InternS1ProForConditionalGeneration: PASSED (was AssertionError)
  • Eagle3DeepseekV3ForCausalLM: PASSED (was ValueError unpack)
  • MistralLarge3ForCausalLM: skipped (offline), matching EagleMistralLarge3

cc @AndreasKaratzas

@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.

@mergify mergify Bot added the v1 label Jun 29, 2026
@AndreasKaratzas AndreasKaratzas added rocm Related to AMD ROCm ready ONLY add when PR is ready to merge/full CI is needed labels Jun 29, 2026
@github-project-automation github-project-automation Bot moved this to Todo in AMD Jun 29, 2026
Comment thread tests/models/registry.py Outdated
Comment thread tests/models/registry.py Outdated
Comment thread vllm/v1/attention/backends/mla/triton_mla.py Outdated
@AndreasKaratzas
AndreasKaratzas force-pushed the fix-basic-models-extra-init-amd-v2 branch from 55d735c to c6acc09 Compare July 2, 2026 03:00

@MatthewBonanni MatthewBonanni 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.

LGTM

@MatthewBonanni

Copy link
Copy Markdown
Member

Looks like MistralLarge3ForCausalLM is failing CI

@AndreasKaratzas
AndreasKaratzas force-pushed the fix-basic-models-extra-init-amd-v2 branch from 6b6ab01 to 3d73907 Compare July 7, 2026 23:06
Signed-off-by: aarushjain29 <aarushi.jain2@amd.com>
Signed-off-by: aarushjain29 <aarushi.jain2@amd.com>
Signed-off-by: aarushjain29 <aarushi.jain2@amd.com>
Signed-off-by: aarushjain29 <aarushi.jain2@amd.com>
Comment thread tests/models/registry.py Outdated
"mistralai/Mistral-Large-3-675B-Instruct-2512-NVFP4"
"mistralai/Mistral-Large-3-675B-Instruct-2512-NVFP4",
# Lower utilization so the KV cache fits on one GPU; OOMs at default 0.8.
gpu_memory_utilization=0.2,

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.

Maybe we can try without this after #47366
I dont like that we have to set 0.2 utilization to avoid ooming, i mean it should not oom at anything other than 100% utilization so there is something else going on there

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.

Signed-off-by: aarushjain29 <aarushi.jain2@amd.com>
@aarushjain29
aarushjain29 force-pushed the fix-basic-models-extra-init-amd-v2 branch 3 times, most recently from 9812ff9 to bf876ea Compare July 10, 2026 17:37
@mergify

mergify Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @aarushjain29.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@stefankoncarevic

Copy link
Copy Markdown
Contributor

Heads-up: #48527 merged (runs the init-test engine in-process on ROCm) and fixes this class of failures at the root. Might be worth re-checking whether this PR is still needed on the test side.

@AndreasKaratzas

Copy link
Copy Markdown
Member

Closing as #48527 was already merged.

@github-project-automation github-project-automation Bot moved this from Todo to Done in AMD Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-rebase ready ONLY add when PR is ready to merge/full CI is needed rocm Related to AMD ROCm v1

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants