Skip to content

fix: additional special tokens being replaced#517

Merged
dushyantbehl merged 2 commits intofoundation-model-stack:mainfrom
dushyantbehl:main
Apr 4, 2025
Merged

fix: additional special tokens being replaced#517
dushyantbehl merged 2 commits intofoundation-model-stack:mainfrom
dushyantbehl:main

Conversation

@dushyantbehl
Copy link
Copy Markdown
Collaborator

@dushyantbehl dushyantbehl commented Apr 4, 2025

Description of the change

While adding additional special tokens it was noticed that the existing special tokens were being replaced and not present this was due to the fact that when calling add_special_tokens the argument replace_additional_special_tokens was being set to True by default and was causing the issue.

Note the HF documentation of this function states

   If `True`, the existing list of additional special tokens will be replaced by the list provided in
                `special_tokens_dict`. Otherwise, `self._special_tokens_map["additional_special_tokens"]` is just extended. In the former
                case, the tokens will NOT be removed from the tokenizer's full vocabulary - they are only being flagged
                as non-special tokens. Remember, this only affects which tokens are skipped during decoding, not the
                `added_tokens_encoder` and `added_tokens_decoder`. This means that the previous
                `additional_special_tokens` are still added tokens, and will not be split by the model.

Which means model accuracy and performance before this fix was not going to be affected but to fix the desired behavior this change sets replace_additional_special_tokens=False

Also adds a test case to check this internally.

Related issue number

How to verify the PR

Was the PR tested

  • I have added >=1 unit test(s) for every new method I have added.
  • I have ensured all unit tests pass

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 4, 2025

Thanks for making a pull request! 😃
One of the maintainers will review and advise on the next steps.

ashokponkumar
ashokponkumar previously approved these changes Apr 4, 2025
Signed-off-by: Dushyant Behl <dushyantbehl@in.ibm.com>
Signed-off-by: Dushyant Behl <dushyantbehl@in.ibm.com>
@dushyantbehl dushyantbehl merged commit 263264a into foundation-model-stack:main Apr 4, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants