A draft implementation of BPE tokenizer #39
Conversation
|
@gboduljak sorry for not getting to it a bit sooner. Thanks for the contribution this already looks great! As you wrote above, indeed this should be a different class as the tokenization algorithm is fundamentally very different. I would create a class I love the python tests. We really need to get to porting the tests we had to the open source repo. I will add some C++ test scaffolding ASAP so that we can later add some C++ tests as well. In the meantime I would not commit full merges and vocabulary files. For the test you can create some small merges and vocab on the fly or even better compare against a BPE tokenizer from HF using its merges file and vocab. I will definitely take a close look to the implementation over the weekend. |
|
@gboduljak I will close this. #62 implements a fast generic bpe tokenizer. It was quite involved in the end. Sorry if I dragged you into implementing it only to not be merged. I do appreciate your contributions here and in Let me know if you think I am missing something in the implementation in #62. Thanks! |
|
No worries. I will go through your implementation :) It is awesome to see this implemented in mlx. Great work 👍 |
This is a draft implementation of the
naiveBPE tokenizer. I tested it on CLIP tokenizer configuration. Some basics tests are working, but more tests are necessary.I realized that current tokenization methods are mostly based on
CharTrie. However, this data structure may not be the best for the BPE tokenization. This is because BPE tokenization kinda works bottom up from unigrams. Consequently, I think we may want a separate class for BPE tokenizer. To get quick feedback, I implemented everything within the currentTokenizer.I would appreciate your feedback on:
CharTrie.Closes #36.