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.

Sign up for GitHub

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

Jump to bottom

Add web binding Tokenizer.tokenToId() #41

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+32 −2
Open

Add web binding Tokenizer.tokenToId() #41

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/sentencepiece_tokenizer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,13 @@ class SentencePieceTokenizer : public Tokenizer {

std::string IdToToken(int32_t id) final { return sentence_piece_.IdToPiece(id); }

int32_t TokenToId(const std::string& token) final { return sentence_piece_.PieceToId(token); }
int32_t TokenToId(const std::string& token) final {
int32_t id = sentence_piece_.PieceToId(token);
if (id == sentence_piece_.unk_id()) {
return -1;
}
return id;
}
Comment on lines +40 to +46
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we still align with sentencepiece and still return unk when the token is not found?

Copy link
Author

Choose a reason for hiding this comment

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

Hello @Ubospica. Thanks for your comment!
I am not sure about current circumstances with sentencepiece, but I just wanted to enable tokenToId in web.
As I can see in the following description of Tokenizer.TokenToId(), -1 would be returned when it could not find the corresponding id.

tokenizers-cpp/include/tokenizers_cpp.h

Lines 65 to 68 in 5de6f65

/*!
* \brief Convert the given token to its corresponding id if it exists. If not, return -1.
*/
virtual int32_t TokenToId(const std::string& token) = 0;

The mentioned function is implementing that virtual method, so I wanted to make it as that.
(For HFTokenizer, it is already covered as I saw.)

As I understood, UNK token from sentencepiece tokenizer could be sth other than -1 due to the configuration. So I added explicit check with sentence_piece_ .unk_id(). But as I mentioned in the first comment, I am not sure this logic is really consistent considering practical uses, so you can suggest me to modify or revert that part to align it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hello @grf53, thank you for your explanation!

I think we should still align with Huggingface tokenizers, which means returning unk_token instead of -1 when the token does not exist. Additionally, please add tests to ensure that unk_token is returned for non-existent tokens.

For the current implementation of HFTokenizer, we will later modify it to return unk_token instead of -1.

Thanks again for your contribution!


private:
// the tokenizer
Expand Down
11 changes: 11 additions & 0 deletions web/src/tokenizers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,17 @@ export class Tokenizer {
return res;
}

/**
* Convert the given token to its corresponding id if it exists. If not, return -1.
*
* @param token the input token string.
* @returns The encoded token id.
*/
tokenToId(token: string): number {
const id = this.handle.TokenToId(token.slice());
return id;
}

/**
* Create a tokenizer from jsonArrayBuffer
*
Expand Down
3 changes: 2 additions & 1 deletion web/src/tokenizers_binding.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,6 @@ EMSCRIPTEN_BINDINGS(tokenizers) {
.function("Encode", &tokenizers::Tokenizer::Encode)
.function("Decode", &tokenizers::Tokenizer::Decode)
.function("GetVocabSize", &tokenizers::Tokenizer::GetVocabSize)
.function("IdToToken", &tokenizers::Tokenizer::IdToToken);
.function("IdToToken", &tokenizers::Tokenizer::IdToToken)
.function("TokenToId", &tokenizers::Tokenizer::TokenToId);
}
12 changes: 12 additions & 0 deletions web/tests/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,18 @@ async function testJSONTokenizer() {
if (tok49407 !== "<|endoftext|>") {
throw Error("Expect token 49407 to be <|endoftext|>");
}

const id0 = tok.tokenToId("!");
console.log("id0=" + id0);
if (id0 !== 0) {
throw Error("Expect token 0 to be !");
}

const id49407 = tok.tokenToId("<|endoftext|>");
console.log("id49407=" + id49407);
if (id49407 !== 49407) {
throw Error("Expect token 49407 to be <|endoftext|>");
}
}

async function testLlamaTokenizer() {
Expand Down