-
-
Couldn't load subscription status.
- Fork 10.8k
[XPU] Fix compile_size is None case.
#25433
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[XPU] Fix compile_size is None case.
#25433
Conversation
There was a problem hiding this 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.
| if compilation_config.compile_sizes is None: | ||
| compilation_config.compile_sizes = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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=[])Signed-off-by: Kunshang Ji <kunshang.ji@intel.com>
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com>
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com> Signed-off-by: charlifu <charlifu@amd.com>
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com> Signed-off-by: gaojc <1055866782@qq.com>
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com> Signed-off-by: xuebwang-amd <xuebwang@amd.com>
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com>
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com>
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com> Signed-off-by: xuebwang-amd <xuebwang@amd.com>
Purpose
previously, we will set
compile_sizeininit_with_cudagraph_size.in #25161, xpu platform make
support_static_graph_modeis 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
supported_models.mdandexamplesfor a new model.