-
Notifications
You must be signed in to change notification settings - Fork 125
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
add sdpa for phi3 openvino model #705
Conversation
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
@echarlaix @AlexKoff88 could you please take a look? |
use_cache=use_cache, | ||
) | ||
|
||
from transformers.models.llama.modeling_llama import apply_rotary_pos_emb, repeat_kv |
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.
even if currently equivalent shouldn't be replace it with
from transformers.models.llama.modeling_llama import apply_rotary_pos_emb, repeat_kv | |
from transformers.models.phi3.modeling_phi3 import apply_rotary_pos_emb, repeat_kv |
to avoid any issue resulting from potential refactorization
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.
I believe it will breaks with current release, because phi3 code in transformers is not released yet on pypi... Can we add some TODO to change this in future (currently model is loaded using trust_remote_code using stable transformers release)?
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.
yes works for me, thanks @eaidova !
Co-authored-by: Ella Charlaix <80481427+echarlaix@users.noreply.github.com>
… into ea/phi3_sdpa
What does this PR do?
Fixes # (issue)
Before submitting