Skip to content
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

resolve complicated chat templates during tokenizer saving #1151

Merged
merged 9 commits into from
Feb 17, 2025

Conversation

eaidova
Copy link
Collaborator

@eaidova eaidova commented Feb 7, 2025

What does this PR do?

Currently we have some issues with running some models with chat templates using C++ openvino API due to its chat template complexity parcing using C++. For smooth experience, we allow redefine them by own. To simplify such models usage, we want to provide C++ compatible simplified chat templates during tokenizer conversion (no impact on inference model with original python tokenizer) for several known and popular cases.

Additionally, it was found that sometimes running multimodal models (VLM), main chat template provided in processor instead of tokenizer as the result, our tokenizer conversion API ignore it. This PR fixes issue for such models.

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?

@HuggingFaceDocBuilderDev

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.

@eaidova eaidova marked this pull request as ready for review February 10, 2025 08:48
@eaidova eaidova force-pushed the ea/chat_template_simplification branch from d4ba3f2 to ccf3287 Compare February 10, 2025 10:09
@eaidova eaidova force-pushed the ea/chat_template_simplification branch from ccf3287 to 7cd9492 Compare February 10, 2025 11:34
@eaidova eaidova requested a review from AlexKoff88 February 10, 2025 13:14
@AlexKoff88
Copy link
Collaborator

I am fine with the changes. @eaidova, can you please describe the idea briefly for the rest of the reviewers?

Wovchena added a commit to Wovchena/openvino.genai-public that referenced this pull request Feb 11, 2025
Wovchena added a commit to Wovchena/openvino.genai-public that referenced this pull request Feb 11, 2025
github-merge-queue bot pushed a commit to openvinotoolkit/openvino.genai that referenced this pull request Feb 12, 2025
github-merge-queue bot pushed a commit to openvinotoolkit/openvino.genai that referenced this pull request Feb 12, 2025
github-merge-queue bot pushed a commit to openvinotoolkit/openvino.genai that referenced this pull request Feb 12, 2025
github-merge-queue bot pushed a commit to openvinotoolkit/openvino.genai that referenced this pull request Feb 13, 2025
ilya-lavrenov pushed a commit to openvinotoolkit/openvino.genai that referenced this pull request Feb 13, 2025
@eaidova eaidova force-pushed the ea/chat_template_simplification branch from 56823cd to df7e385 Compare February 13, 2025 08:56
@eaidova eaidova force-pushed the ea/chat_template_simplification branch from df7e385 to bbc48e4 Compare February 13, 2025 08:59
Copy link
Collaborator

@echarlaix echarlaix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Left a very minor comment

Co-authored-by: Ella Charlaix <80481427+echarlaix@users.noreply.github.com>
@eaidova
Copy link
Collaborator Author

eaidova commented Feb 14, 2025

LGTM! Left a very minor comment

@echarlaix, thanks for review. I applied your comment. Could this PR be merged?

@echarlaix echarlaix merged commit 0ff2bbe into huggingface:main Feb 17, 2025
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants