Skip to content

Add Bert Word Piece Tokenizer - #7141

Merged
treo merged 9 commits into
masterfrom
treo/bert_wordpiece_tokenizer
Feb 12, 2019
Merged

treo merged 9 commits into
masterfrom
treo/bert_wordpiece_tokenizer

Conversation

@treo

@treo treo commented Feb 11, 2019

Copy link
Copy Markdown
Member

Requires a Vocab File like the one from the published Bert models
(e.g. vocab.txt in https://storage.googleapis.com/bert_models/2018_11_23/multi_cased_L-12_H-768_A-12.zip)

Tests require https://github.com/deeplearning4j/dl4j-test-resources/pull/185

@AlexDBlack AlexDBlack left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall good, as expected. NavigableMap use here is cool :)
Some minor nitpicks here and there.
Only issue of note I can see here: some of the BERT models are lower case only... We should probably have that configurable somewhere, with an appropriate toLowerCase() call in the tokenizer when enabled?

treo added 6 commits February 11, 2019 15:53
* Update Copyright header

* Use @slf4j

* Share split pattern between Stream and non-stream versions

* Share vocab search between Stream and non-stream versions

* Continue search only in the sub tree that will have the result

* Allow lowerCase only vocab configuration

* Make BertWordPieceTokenzier safer to use with threads

* Allow passing an input stream in addition to a file to BertWordPieceTokenizerFactory

* Add documentation on the expected file format for vocab files
@treo

treo commented Feb 11, 2019

Copy link
Copy Markdown
Member Author

@AlexDBlack I've implemented the requested changes, fixed some more bugs I've found along the way,

AlexDBlack
AlexDBlack previously approved these changes Feb 11, 2019

@AlexDBlack AlexDBlack left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM 👍

@treo
treo merged commit 41e1a88 into master Feb 12, 2019
@treo
treo deleted the treo/bert_wordpiece_tokenizer branch February 12, 2019 05:20
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.

2 participants