Handle triton kernel import exception #25319
Conversation
|
@minosfuture has exported this pull request. If you are a Meta employee, you can view the originating diff in D82899108. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request aims to gracefully handle import exceptions for Triton kernels, which is a good addition. However, the implementation introduces a critical issue by changing the function call has_triton_kernels() to a simple reference has_triton_kernels. This will cause the condition to always be true, attempting to import Triton kernels even when they are not available, thus defeating the purpose of the check. I've provided a suggestion to correct this.
There was a problem hiding this comment.
The check if has_triton_kernels: is incorrect. has_triton_kernels is a function and needs to be called with parentheses, like has_triton_kernels(). Without the parentheses, this condition will always evaluate to True because it's checking the truthiness of the function object itself, not its return value. This would cause an ImportError when triton_kernels is not installed, which this check is supposed to prevent.
| if has_triton_kernels: | |
| if has_triton_kernels(): |
|
@minosfuture has exported this pull request. If you are a Meta employee, you can view the originating diff in D82899108. |
minosfuture
force-pushed
the
export-D82899108
branch
from
9701a2f to
0132441
Compare
|
@minosfuture has exported this pull request. If you are a Meta employee, you can view the originating diff in D82899108. |
minosfuture
force-pushed
the
export-D82899108
branch
from
0132441 to
68986ff
Compare
|
@minosfuture has exported this pull request. If you are a Meta employee, you can view the originating diff in D82899108. |
minosfuture
force-pushed
the
export-D82899108
branch
from
68986ff to
d7bb2ee
Compare
minosfuture
force-pushed
the
export-D82899108
branch
from
d7bb2ee to
00f9d83
Compare
|
@minosfuture has exported this pull request. If you are a Meta employee, you can view the originating diff in D82899108. |
minosfuture
force-pushed
the
export-D82899108
branch
from
00f9d83 to
d945fb3
Compare
Summary: Signed-off-by: Ming Yang <minos.future@gmail.com> Pull Request resolved: vllm-project#25319 Handle triton kernel import exception in cases it is not available. Test Plan: AMD DeepSeek test Reviewed By: houseroad Differential Revision: D82899108 Privacy Context Container: L1370295 Signed-off-by: Ming Yang <minos.future@gmail.com>
minosfuture
force-pushed
the
export-D82899108
branch
from
d945fb3 to
a97f222
Compare
mgoin
added
ready
Signed-off-by: Ming Yang <minos.future@gmail.com>
Signed-off-by: Ming Yang <minos.future@gmail.com> Signed-off-by: charlifu <charlifu@amd.com>
Signed-off-by: Ming Yang <minos.future@gmail.com> Signed-off-by: yewentao256 <zhyanwentao@126.com>
Summary: Handle triton kernel import exception in cases it is not available.
Test Plan: AMD DeepSeek test
Reviewed By: houseroad
Differential Revision:
D82899108
Privacy Context Container: L1370295