Simplify TokenizerArgs.__post_init__ with Enum Tokenizer Type
#1535
Conversation
Summary: Simplify `TokenizerArgs.__post_init__` with enum tokenizer type, since only one of the tokenizer type can be true. We want to touch as less code outside of `__post_init__` as possible at the moment. Test Plan: python torchchat.py generate llama2|llama3|granite-code Reviewers: @Jack-Khuu Subscribers: Issue: #1518
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/torchchat/1535
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit c846de9 with merge base 5f8f35d ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
facebook-github-bot
added
the
CLA Signed
torchchat/cli/builder.py
Outdated
| self.tokenizer_type = TokenizerType.NONE | ||
| self.t = None |
There was a problem hiding this comment.
Do we really have to set these as none again since we already set them at the very top.
There was a problem hiding this comment.
We can actually drop all the logic here after the HF tokenizer check, tokenizer_type and .t are already set to these by default
torchchat/cli/builder.py
Outdated
| is_sentencepiece = self.is_sentencepiece() | ||
| is_hf_tokenizer = self.is_hf_tokenizer() | ||
|
|
||
| if sum([is_tiktoken, is_hf_tokenizer, is_sentencepiece]) != 1: |
There was a problem hiding this comment.
We can replace this by just checking if the tokenizer enum is None
torchchat/cli/builder.py
Outdated
| self.tokenizer_type = TokenizerType.NONE | ||
| self.t = None |
There was a problem hiding this comment.
We can actually drop all the logic here after the HF tokenizer check, tokenizer_type and .t are already set to these by default
Summary: Simplify `TokenizerArgs.__post_init__` with enum tokenizer type, since only one of the tokenizer type can be true. We want to touch as less code outside of `__post_init__` as possible at the moment. Test Plan: python torchchat.py generate llama2|llama3|granite-code Reviewers: @Jack-Khuu Subscribers: Issue: #1518
Summary:
Simplify
TokenizerArgs.__post_init__with enum tokenizer type, since only one of the tokenizer type can be true.We want to touch as less code outside of
__post_init__as possible at the moment.Test Plan:
python torchchat.py generate llama2|llama3|granite-code
Reviewers:
@Jack-Khuu
Subscribers:
Issue:
#1518