Skip to content

[MRV2] Fix for DS v3.2#38030

Merged
WoosukKwon merged 1 commit intomainfrom
woosuk/mrv2-fix-ds32
Mar 24, 2026
Merged

[MRV2] Fix for DS v3.2#38030
WoosukKwon merged 1 commit intomainfrom
woosuk/mrv2-fix-ds32

Conversation

@WoosukKwon
Copy link
Copy Markdown
Collaborator

No description provided.

Signed-off-by: Woosuk Kwon <woosuk@inferact.ai>
@WoosukKwon WoosukKwon requested a review from njhill as a code owner March 24, 2026 18:37
@WoosukKwon WoosukKwon added the ready ONLY add when PR is ready to merge/full CI is needed label Mar 24, 2026
@mergify mergify bot added the v1 label Mar 24, 2026
Copy link
Copy Markdown
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 modifies the _reshape_kv_cache function to accommodate more flexible KV cache specifications, allowing for both uniform and layer-specific AttentionSpec configurations. A review comment highlights a potential issue where the assert statement used for type checking kv_cache_spec could be bypassed in production if assertions are disabled, suggesting a more robust type validation using an explicit TypeError or ValueError to ensure consistent error handling.

kv_cache_spec = kv_cache_group_spec.kv_cache_spec
if isinstance(kv_cache_spec, UniformTypeKVCacheSpecs):
kv_cache_spec = kv_cache_spec.kv_cache_specs[layer_name]
assert isinstance(kv_cache_spec, AttentionSpec)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The assert statement on this line performs a critical type check. If assertions are disabled in a production environment, this check will be skipped, potentially leading to AttributeError or TypeError in subsequent operations if kv_cache_spec is not an AttentionSpec. For robust error handling, consider replacing this assert with an explicit TypeError or ValueError to ensure type validation always occurs, regardless of assertion settings.

Suggested change
assert isinstance(kv_cache_spec, AttentionSpec)
if not isinstance(kv_cache_spec, AttentionSpec):
raise TypeError(f"Expected kv_cache_spec to be AttentionSpec, but got {type(kv_cache_spec)}")

@WoosukKwon WoosukKwon merged commit 4b53740 into main Mar 24, 2026
62 checks passed
@WoosukKwon WoosukKwon deleted the woosuk/mrv2-fix-ds32 branch March 24, 2026 21:03
RhizoNymph pushed a commit to RhizoNymph/vllm that referenced this pull request Mar 26, 2026
Signed-off-by: Woosuk Kwon <woosuk@inferact.ai>
HenryTangDev pushed a commit to HenryTangMain/vllm that referenced this pull request Mar 27, 2026
Signed-off-by: Woosuk Kwon <woosuk@inferact.ai>
malaiwah pushed a commit to malaiwah/vllm that referenced this pull request Mar 27, 2026
Signed-off-by: Woosuk Kwon <woosuk@inferact.ai>
Signed-off-by: Michel Belleau <michel.belleau@malaiwah.com>
khairulkabir1661 pushed a commit to khairulkabir1661/vllm that referenced this pull request Mar 27, 2026
Signed-off-by: Woosuk Kwon <woosuk@inferact.ai>
Monishver11 pushed a commit to Monishver11/vllm that referenced this pull request Mar 27, 2026
Signed-off-by: Woosuk Kwon <woosuk@inferact.ai>
Signed-off-by: Monishver Chandrasekaran <monishverchandrasekaran@gmail.com>
nithinvc pushed a commit to nithinvc/vllm that referenced this pull request Mar 27, 2026
Signed-off-by: Woosuk Kwon <woosuk@inferact.ai>

Signed-off-by: Nithin Chalapathi <nithin.ch10@gmail.com>
JiantaoXu pushed a commit to JiantaoXu/vllm that referenced this pull request Mar 28, 2026
Signed-off-by: Woosuk Kwon <woosuk@inferact.ai>
vrdn-23 pushed a commit to vrdn-23/vllm that referenced this pull request Mar 30, 2026
Signed-off-by: Woosuk Kwon <woosuk@inferact.ai>
Signed-off-by: Vinay Damodaran <vrdn@hey.com>
EricccYang pushed a commit to EricccYang/vllm that referenced this pull request Apr 1, 2026
Signed-off-by: Woosuk Kwon <woosuk@inferact.ai>
Signed-off-by: EricccYang <yangyang4991@gmail.com>
bhargav-patel-29 pushed a commit to Bharatgen-Tech/vllm that referenced this pull request Apr 1, 2026
Signed-off-by: Woosuk Kwon <woosuk@inferact.ai>
Signed-off-by: bhargav-patel-29 <bhargav.patel@tihiitb.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed v1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants