Skip to content

Conversation

@LucasWilkinson
Copy link
Collaborator

#28718 inadvertently undid part of #28404

Signed-off-by: Lucas Wilkinson <[email protected]>
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.

Makes sense, thank you

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 aims to fix a CI test on Blackwell by removing an assertion. However, this change is problematic as it hides a deeper issue. The assertion assert out.is_contiguous() was added for a valid reason: downstream distributed operations like reduce_scatter require contiguous tensors. Removing it will likely lead to runtime errors. The root cause is that correct_attn_out no longer guarantees a contiguous output after a previous partial revert. The correct solution is to restore the logic in correct_attn_out to ensure its output is always contiguous.

@mgoin mgoin added bug Something isn't working ready ONLY add when PR is ready to merge/full CI is needed ci-failure Issue about an unexpected test failure in CI labels Nov 24, 2025
@mgoin mgoin enabled auto-merge (squash) November 24, 2025 22:33
@vllm-bot vllm-bot merged commit 2d9ee28 into vllm-project:main Nov 25, 2025
49 of 51 checks passed
bringlein pushed a commit to bringlein/vllm that referenced this pull request Nov 26, 2025
devpatelio pushed a commit to SumanthRH/vllm that referenced this pull request Nov 29, 2025
kitaekatt pushed a commit to kitaekatt/vllm that referenced this pull request Dec 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ci-failure Issue about an unexpected test failure in CI ready ONLY add when PR is ready to merge/full CI is needed

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants