krickert commented on PR #1165: URL: https://github.com/apache/opennlp/pull/1165#issuecomment-5111103035
Since I pushed it, while enhancing it I didn't think it was as good of a contract. There are two subword engines and they needed one contract. `BertTokenizer` could not be it: it was pinned to `Tokenizer`, which promises spans into the input, and wordpiece pieces are not substrings of the text. That is why its `tokenizePos` threw from the day I added it. The new contract is `SubwordTokenizer`, returning `SubwordPiece(piece, id, start, end)`, so the piece and the original-text span are separate fields. `BertTokenizer` folded into `WordpieceEncoder` under it, and `SentencePieceTokenizer` implements the same one. Example: ```java WordpieceEncoder encoder = new WordpieceEncoder(vocabulary, lowerCase); List<SubwordPiece> pieces = encoder.encode(text); // piece, vocab id, and span String[] tokens = encoder.encodeToPieces(text); // what BertTokenizer.tokenize returned ``` The old pipeline is kept as `ReferenceBertPipeline` and the encoder is differential-tested against it, so the piece sequence is asserted identical. `AbstractDL.createTokenizer` now returns `Tokenizer` but there's no in-tree caller. It is binary-incompatible for anyone overriding it. `opennlp-dl` compiles against `opennlp-api` only and consumes wordpiece, so wordpiece stays in api. SentencePiece has no core consumer, so that engine sits in `opennlp-extensions/opennlp-subword` with only the contract in api. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
