Skip to content

Conversation

xuechendi
Copy link
Contributor

@xuechendi xuechendi commented Sep 20, 2025

Purpose

Fix failing for non cuda device when calling torch._C.Tag.cudagraph_unsafe
from #24281

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.

Signed-off-by: Chendi Xue <Chendi.Xue@intel.com>
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 correctly fixes a crash on non-CUDA platforms by conditionally including torch._C.Tag.cudagraph_unsafe. The approach is valid, but I've suggested a more robust implementation using a try-except block. This change would make the code more resilient by directly checking for the attribute's existence in the PyTorch library, rather than relying on vLLM's platform detection. I also recommend adding a regression test for non-CUDA environments to prevent this issue from recurring.

Signed-off-by: Chendi Xue <Chendi.Xue@intel.com>
@xuechendi
Copy link
Contributor Author

@ProExpertProg , please help to review, thanks

Signed-off-by: Chendi Xue <Chendi.Xue@intel.com>
Copy link
Member

@bigPYJ1151 bigPYJ1151 left a comment

Choose a reason for hiding this comment

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

the cpu ci recovered, thanks for the fix :)

@bigPYJ1151 bigPYJ1151 enabled auto-merge (squash) September 20, 2025 02:56
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 20, 2025
@bigPYJ1151 bigPYJ1151 merged commit 6c5f82e into vllm-project:main Sep 20, 2025
52 of 54 checks passed
FeiDaLI pushed a commit to FeiDaLI/vllm that referenced this pull request Sep 25, 2025
charlifu pushed a commit to ROCm/vllm that referenced this pull request Sep 25, 2025
…ion (vllm-project#25298)

Signed-off-by: Chendi Xue <Chendi.Xue@intel.com>
Signed-off-by: charlifu <charlifu@amd.com>
yewentao256 pushed a commit that referenced this pull request Oct 3, 2025
…ion (#25298)

Signed-off-by: Chendi Xue <Chendi.Xue@intel.com>
Signed-off-by: yewentao256 <zhyanwentao@126.com>
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants