-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
[TRANSFROMATIONS] Add support for 'inputs_embeds' input in SDPAToPA #27158
base: master
Are you sure you want to change the base?
Conversation
Add support for 'input_embeds' input in SDPAToPA transformation. The input is used in VLM instead of 'input_ids' in text-only models. The changes enable support of the SDPAToPA transformation for the following models: * llava-hf/llava-1.5-7b-hf * llava-hf/llava-v1.6-mistral-7b-hf * llava-hf/llava-v1.6-vicuna-7b-hf * llava-hf/llama3-llava-next-8b-hf * openbmb/MiniCPM-V-2_6 Signed-off-by: Andrii Staikov <[email protected]> - Tickets: * CVS-152288
should we continue with it? |
please mention it in the PR description |
3815c3e
to
a1eef49
Compare
if (auto casted_param = std::dynamic_pointer_cast<v0::Parameter>(param.get_node_shared_ptr())) { | ||
return casted_param; | ||
} else { | ||
OPENVINO_THROW("The model is in the inconsistent state. Found input '", | ||
name, | ||
"', but couldn't cast it to v0::Parameter."); | ||
} |
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.
if (auto casted_param = std::dynamic_pointer_cast<v0::Parameter>(param.get_node_shared_ptr())) { | |
return casted_param; | |
} else { | |
OPENVINO_THROW("The model is in the inconsistent state. Found input '", | |
name, | |
"', but couldn't cast it to v0::Parameter."); | |
} | |
auto casted_param = ov::as_type_ptr<v0::Paramter>(param.get_node_shared_ptr())); | |
OPENVINO_ASSERT(casted_param , "The model is in the inconsistent state. Input is not paramter: ", casted_param); | |
return casted_param; |
Use ov::as_type_ptr<>
instead of std::dynamic_pointer_cast
for operators.
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.
Fixed
const std::string& name) -> std::shared_ptr<v0::Parameter> { | ||
for (auto& param : model->inputs()) { | ||
const auto& names = param.get_names(); | ||
if (names.find(name) != names.end()) { |
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.
if (names.find(name) != names.end()) { | |
if (names.count(name)) { |
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.
Fixed
@@ -53,8 +53,28 @@ bool ov::pass::SDPAToPagedAttention::run_on_model(const std::shared_ptr<ov::Mode | |||
|
|||
auto sliding_window = v0::Constant::create(element::i32, Shape{}, {0}); // sliding_window | |||
|
|||
auto has_parameter = [=](const std::shared_ptr<ov::Model>& model, | |||
const std::string& name) -> std::shared_ptr<v0::Parameter> { | |||
for (auto& param : model->inputs()) { |
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.
for (auto& param : model->inputs()) { | |
for (const auto& param : model->inputs()) { |
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.
Fixed
@@ -53,8 +53,28 @@ bool ov::pass::SDPAToPagedAttention::run_on_model(const std::shared_ptr<ov::Mode | |||
|
|||
auto sliding_window = v0::Constant::create(element::i32, Shape{}, {0}); // sliding_window | |||
|
|||
auto has_parameter = [=](const std::shared_ptr<ov::Model>& model, |
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.
Consider rename as it not returns bool value.
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.
Yeah, I also don't like the name 100%, but I'd leave it as it is as it's used in cases when want to do something if a model has a parameter.
I've disabled a few failing models due to the latest version of transformers (4.46.2). The updated version of the package adds regression to the models, but enables the nanollava model. The regression will be fixed in 157416 |
[TRANSFROMATIONS] Add support for 'inputs_embeds' input in SDPAToPA
Add support for 'input_embeds' input in SDPAToPA transformation.
The input is used in VLM instead of 'input_ids' in text-only models.
The changes enable support of the SDPAToPA transformation for the following models:
CVS-156956
Signed-off-by: Andrii Staikov [email protected]