[Bugfix] Fix platform-specific routing in CustomOp implementations #24444
Conversation
Signed-off-by: Konrad Zawora <kzawora@habana.ai>
Signed-off-by: Konrad Zawora <kzawora@habana.ai>
![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 addresses a bug in CustomOp implementations where platform-specific routing logic was being bypassed due to forward method overrides. The fix involves renaming forward methods to forward_native and adding forward_cuda methods that call forward_native, thus preserving routing logic for out-of-tree backends. The review focuses on ensuring the correctness of these changes and identifying any potential issues.
Signed-off-by: Konrad Zawora <kzawora@habana.ai>
|
@MengqingCao @simon-mo @youkaichao @DarkLight1337 , may you help to take a quick look of this PR |
|
@gshtras , may you take a look of this PR |
jikunshang
left a comment
jikunshang
left a comment
There was a problem hiding this comment.
please fix precommit.
MengqingCao
left a comment
MengqingCao
left a comment
There was a problem hiding this comment.
This pr is very clean, LGTM
ProExpertProg
left a comment
ProExpertProg
left a comment
There was a problem hiding this comment.
Please fix pre-commit
Signed-off-by: Konrad Zawora <kzawora@habana.ai>
|
@jikunshang @MengqingCao @ProExpertProg pre-commit is green now |
github-actions
bot
added
the
ready
…llm-project#24444) Signed-off-by: Konrad Zawora <kzawora@habana.ai>
…llm-project#24444) Signed-off-by: Konrad Zawora <kzawora@habana.ai>
- HPU Mrope implementation had a bug which was exposed by vllm-project/vllm#24444 - Initial workaround was to use the default implementation: #162 - This PR fixes the bug in the HPU mrope --------- Signed-off-by: attafosu <thomas.atta-fosu@intel.com> Co-authored-by: Chendi.Xue <chendi.xue@intel.com>
|
this PR bring some issues. see https://github.com/vllm-project/vllm/pull/25145/files#r2359528376 |
- HPU Mrope implementation had a bug which was exposed by vllm-project/vllm#24444 - Initial workaround was to use the default implementation: vllm-project#162 - This PR fixes the bug in the HPU mrope --------- Signed-off-by: attafosu <thomas.atta-fosu@intel.com> Co-authored-by: Chendi.Xue <chendi.xue@intel.com> Signed-off-by: slokesha <slokeshappa@habana.ai>
…llm-project#24444) Signed-off-by: Konrad Zawora <kzawora@habana.ai>
…llm-project#24444) Signed-off-by: Konrad Zawora <kzawora@habana.ai> Signed-off-by: xuebwang-amd <xuebwang@amd.com>
…llm-project#24444) Signed-off-by: Konrad Zawora <kzawora@habana.ai> Signed-off-by: xuebwang-amd <xuebwang@amd.com>
5 participants
Some layers inheriting from
CustomOpare overwriting theforwardmethod, effectively disabling the platform routing logic defined inCustomOp.dispatch_forward(), resulting inself._forward_methodnot being called. This is problematic for any out-of-tree backends, as providing backend-specificforward_ootdoesn't result in calling the proper forward pass method, even though it's properly selected inself._forward_method. This PR movesforwardoverrides in layers inheriting fromCustomOptoforward_native, and addsforward_cudaalso callingforward_native- preserving the current behavior for all existing in-tree backends, but preserving routing logic for out-of-tree backends.