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

Add missing tokens from added_tokens_decoder into added_tokens #890

Merged
merged 5 commits into from
Feb 21, 2025

Conversation

sayanshaw24
Copy link
Contributor

@sayanshaw24 sayanshaw24 commented Feb 19, 2025

Fixes tokenizer issue where special tokens added into added_tokens_decoder attribute in tokenizer_config.json but missing in added_tokens in tokenizer.json are still populated correctly into added tokens map such that they are not tokenized incorrectly. This ensures compatibility even with HuggingFace models whose files have inconsistent tokenizer.json and tokenizer_config.json files.

Validation

  • Ran C++ test locally with Phi tokenizer files and validated with different tokens
  • Tested correct population of added_tokens with missing and complete cases

@sayanshaw24 sayanshaw24 requested a review from a team as a code owner February 19, 2025 22:14
@wenbingl
Copy link
Member

Fixes tokenizer issue where special tokens added into added_tokens_decoder attribute in tokenizer_config.json but missing in added_tokens in tokenizer.json are still populated correctly into added tokens map such that they are not tokenized incorrectly. This ensures compatibility even with HuggingFace models whose files have inconsistent tokenizer.json and tokenizer_config.json files.

Validation

  • Ran C++ test locally with PhiO tokenizer files and validated on numerous cases
  • Tested correct population of added_tokens with missing and complete cases

Good finding, thanks!

@wenbingl
Copy link
Member

Add a sentence which contains one or more tokens from added_tokens_decoder in the current unit test?

@baijumeswani baijumeswani merged commit 1bb7ab2 into main Feb 21, 2025
46 checks passed
@baijumeswani baijumeswani deleted the sayanshaw/phi4-tok branch February 21, 2025 19:02
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.

3 participants