Skip to content

Conversation

@jikunshang
Copy link
Collaborator

@jikunshang jikunshang commented Sep 23, 2025

Purpose

previously, we will set compile_sizein init_with_cudagraph_size.
in #25161, xpu platform make support_static_graph_mode is False, which will not fall into previous path and leave compile size be None instead of a empty list even we don't set this.
this PR will set this in xpu.py to avoid runtime error.

Test Plan

CI

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.

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 runtime error on the XPU platform caused by an uninitialized compile_sizes configuration. While the proposed change correctly handles the case where compile_sizes is None, it is incomplete and fails to address scenarios where compile_sizes is configured with the special string "cudagraph_capture_sizes". My review includes a critical comment with a more robust solution that calls the proper initialization method, ensuring that compile_sizes is handled correctly in all cases for platforms that do not support static graphs.

Comment on lines +116 to +117
if compilation_config.compile_sizes is None:
compilation_config.compile_sizes = []
Copy link
Contributor

Choose a reason for hiding this comment

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

critical

This change correctly handles the case where compile_sizes is None. However, it is incomplete and can lead to a runtime error in other scenarios.

Specifically, if a user configures compile_sizes with the special value "cudagraph_capture_sizes", this if condition will not be met, and the special string will not be expanded. This will cause a TypeError later when the code attempts to iterate over it expecting integers.

A more robust solution is to call compilation_config.init_with_cudagraph_sizes(). This method is designed to correctly process the compile_sizes attribute from user configuration. Since XPU does not support CUDA graphs, we can pass an empty list for cudagraph_capture_sizes.

        # `init_with_cudagraph_sizes` is not called on platforms without static
        # graph support. We call it here to ensure `compile_sizes` is
        # correctly initialized from user config.
        compilation_config.init_with_cudagraph_sizes(cudagraph_capture_sizes=[])

@bigPYJ1151 bigPYJ1151 enabled auto-merge (squash) September 23, 2025 01:44
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 23, 2025
@bigPYJ1151 bigPYJ1151 merged commit f225ea7 into vllm-project:main Sep 23, 2025
52 checks passed
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com>
FeiDaLI pushed a commit to FeiDaLI/vllm that referenced this pull request Sep 25, 2025
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com>
charlifu pushed a commit to ROCm/vllm that referenced this pull request Sep 25, 2025
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com>
Signed-off-by: charlifu <charlifu@amd.com>
yewentao256 pushed a commit that referenced this pull request Oct 3, 2025
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com>
Signed-off-by: yewentao256 <zhyanwentao@126.com>
gjc0824 pushed a commit to gjc0824/vllm that referenced this pull request Oct 10, 2025
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com>
Signed-off-by: gaojc <1055866782@qq.com>
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 10, 2025
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com>
Signed-off-by: xuebwang-amd <xuebwang@amd.com>
choprahetarth pushed a commit to Tandemn-Labs/vllm that referenced this pull request Oct 11, 2025
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com>
lywa1998 pushed a commit to lywa1998/vllm that referenced this pull request Oct 20, 2025
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com>
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 24, 2025
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com>
Signed-off-by: xuebwang-amd <xuebwang@amd.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