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 tokenizer to cache key #876

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ekagra-ranjan
Copy link

Fixes: #872

elif hasattr(tokenizer, "path"):
tokenizer.name_or_path = tokenizer.path
else:
print("[WARNING]: tokenizer has no name or path. Setting cache key to `unknown`")
Copy link
Contributor

Choose a reason for hiding this comment

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

Use warnings.

@@ -52,6 +52,17 @@ def adapt_tokenizer(tokenizer: PreTrainedTokenizerBase) -> PreTrainedTokenizerBa
tokenizer.vocabulary = tokenizer.get_vocab()
tokenizer.special_tokens = set(tokenizer.all_special_tokens)

# tokenizer.name_or_path is one of the keys for `create_states_mapping()` cache
if hasattr(tokenizer, "name_or_path"):
Copy link
Contributor

Choose a reason for hiding this comment

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

To what do all these cases correspond? Regardless, we'll need tests for them.

@br3no
Copy link
Contributor

br3no commented May 8, 2024

@ekagra-ranjan, nice.

Would an alternative be to devise a function that will compute a unique hash given the current class members? This would not require changing the API.

@brandonwillard
Copy link
Contributor

Would an alternative be to devise a function that will compute a unique hash given the current class members? This would not require changing the API.

Yes, that would be much better.

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.

State mapping cache ignores the tokenizer used to build the state machine
3 participants