-
-
Notifications
You must be signed in to change notification settings - Fork 11k
Revert #26113 "[Frontend] CompilationConfig overhaul (#20283): deprecate use_inductor in favor of backend, simplify custom_ops" #26472
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
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 reverts a previous change that overhauled CompilationConfig, re-instating use_inductor as the primary flag over backend. The changes across test files and configuration files are consistent with this revert. However, I found a critical issue in vllm/platforms/cpu.py where a new logic block incorrectly disables custom ops for the 'eager' backend on CPU. My review includes a fix for this issue.
| if compilation_config.use_inductor: | ||
| compilation_config.custom_ops = ["none"] | ||
|
|
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.
The logic to disable custom ops is based on compilation_config.use_inductor, which defaults to True and is not updated based on the backend variable set earlier. This causes custom ops to be incorrectly disabled even when the backend is 'eager', which should support them.
To fix this, use_inductor should be explicitly set based on the chosen backend. This ensures that custom ops are only disabled when inductor is the backend, and use_inductor is consistent for any other logic that might depend on it.
| if compilation_config.use_inductor: | |
| compilation_config.custom_ops = ["none"] | |
| compilation_config.use_inductor = (backend == "inductor") | |
| if compilation_config.use_inductor: | |
| compilation_config.custom_ops = ["none"] |
…eprecate use_inducto…" This reverts commit 0c824fc. Signed-off-by: zjy0516 <riverclouds.zhu@qq.com>
0566bc3 to
8f72d11
Compare
|
@DarkLight1337 Only |
|
@DarkLight1337 in the future can we please go through review before reverting a large PR like this? We should forward fix instead of reverting |
…aul (vllm-project#20283): deprecate use_inductor in favor of backend, simplify custom_ops" (vllm-project#26472)" This reverts commit 5728da1.
…aul (vllm-project#20283): deprecate use_inductor in favor of backend, simplify custom_ops" (vllm-project#26472)" This reverts commit 5728da1.
|
The usual practice for CI failure is to revert the PR (given that this test has been failing for a while, I thought a fix couldn't be done quickly). I wasn't aware that this PR is so important. |
…m-project#20283): deprecate use_inductor in favor of backend, simplify custom_ops" (vllm-project#26472) Signed-off-by: zjy0516 <riverclouds.zhu@qq.com> Signed-off-by: yang926 <yang926@naver.com>
…m-project#20283): deprecate use_inductor in favor of backend, simplify custom_ops" (vllm-project#26472) Signed-off-by: zjy0516 <riverclouds.zhu@qq.com> Signed-off-by: xuebwang-amd <xuebwang@amd.com>
…m-project#20283): deprecate use_inductor in favor of backend, simplify custom_ops" (vllm-project#26472) Signed-off-by: zjy0516 <riverclouds.zhu@qq.com> Signed-off-by: Dhruvil Bhatt <bhattdbh@amazon.com>
…m-project#20283): deprecate use_inductor in favor of backend, simplify custom_ops" (vllm-project#26472) Signed-off-by: zjy0516 <riverclouds.zhu@qq.com>
…m-project#20283): deprecate use_inductor in favor of backend, simplify custom_ops" (vllm-project#26472) Signed-off-by: zjy0516 <riverclouds.zhu@qq.com>
…m-project#20283): deprecate use_inductor in favor of backend, simplify custom_ops" (vllm-project#26472) Signed-off-by: zjy0516 <riverclouds.zhu@qq.com> Signed-off-by: xuebwang-amd <xuebwang@amd.com>
…m-project#20283): deprecate use_inductor in favor of backend, simplify custom_ops" (vllm-project#26472) Signed-off-by: zjy0516 <riverclouds.zhu@qq.com> Signed-off-by: 0xrushi <6279035+0xrushi@users.noreply.github.com>
…m-project#20283): deprecate use_inductor in favor of backend, simplify custom_ops" (vllm-project#26472) Signed-off-by: zjy0516 <riverclouds.zhu@qq.com> Signed-off-by: 0xrushi <6279035+0xrushi@users.noreply.github.com>
FIX #26454
Reverts #26113.
Fix compile tests in distributed tests CI