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

[BPE PR 2.1] BPE Utils #7760

Merged
merged 27 commits into from
Jun 21, 2023
Merged

Conversation

pforderique
Copy link
Contributor

Included is bytesToUnicode, createStaticHashTable, and BytePairTokenizerCache with respective tests that will be needed for BPE.

@mattsoulanille mattsoulanille self-requested a review June 16, 2023 20:56
Copy link
Member

@mattsoulanille mattsoulanille left a comment

Choose a reason for hiding this comment

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

The tests look good! I have a few minor comments about the implementation.

tfjs-layers/src/layers/nlp/tokenizers_utils.ts Outdated Show resolved Hide resolved
tfjs-layers/src/layers/nlp/tokenizers_utils.ts Outdated Show resolved Hide resolved
tfjs-layers/src/layers/nlp/tokenizers_utils.ts Outdated Show resolved Hide resolved
tfjs-layers/src/layers/nlp/tokenizers_utils.ts Outdated Show resolved Hide resolved
tfjs-layers/src/layers/nlp/tokenizers_utils.ts Outdated Show resolved Hide resolved
tfjs-layers/src/layers/nlp/tokenizers_utils.ts Outdated Show resolved Hide resolved
tfjs-layers/src/layers/nlp/tokenizers_utils.ts Outdated Show resolved Hide resolved
tfjs-layers/src/layers/nlp/tokenizers_utils.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@Linchenn Linchenn left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@mattsoulanille mattsoulanille left a comment

Choose a reason for hiding this comment

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

LGTM!

@mattsoulanille mattsoulanille enabled auto-merge (squash) June 21, 2023 17:40
@mattsoulanille mattsoulanille merged commit 2ea2292 into tensorflow:master Jun 21, 2023
2 checks passed
@pforderique pforderique deleted the orderique branch June 21, 2023 18:32
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.

None yet

3 participants