OPENNLP-1838: Adopt BertTokenizer in opennlp-dl components#1075
Conversation
|
Will look into this once the other stacked onces are done. |
|
@krickert Ready for rebase, as all other changes in in main now. |
Rebased, ready for review |
There was a problem hiding this comment.
Pull request overview
This PR updates the opennlp-dl ONNX-based components to use BertTokenizer (basic normalization + wordpiece) instead of raw WordpieceTokenizer, and introduces a tri-state lower-casing option to match common model expectations (uncased vs cased).
Changes:
- Switch tokenization in
SentenceVectorsDL,DocumentCategorizerDL, andNameFinderDLtoBertTokenizer, with component-appropriate lower-casing defaults. - Add
InferenceOptions.lowerCase(nullable) and resolution logic inAbstractDLto combine component defaults with per-instance overrides. - Add unit/eval tests covering tokenizer creation behavior, lower-casing resolution, and an eval assertion for uncased-model capitalization invariance.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| opennlp-eval-tests/src/test/java/opennlp/dl/vectors/SentenceVectorsDLEval.java | Adds an eval assertion that capitalization does not change vectors for an uncased model. |
| opennlp-core/opennlp-ml/opennlp-dl/src/test/java/opennlp/dl/CreateTokenizerTest.java | New unit tests for tokenizer selection (BERT vs RoBERTa special tokens) and lower-casing option resolution. |
| opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/vectors/SentenceVectorsDL.java | Defaults sentence-vector tokenization to uncased behavior; adds constructor to control lower-casing. |
| opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/namefinder/NameFinderDL.java | Uses BertTokenizer with default case-preserving behavior for NER, overrideable via InferenceOptions. |
| opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/InferenceOptions.java | Introduces nullable lowerCase option to support component defaults with overrides. |
| opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/doccat/DocumentCategorizerDL.java | Uses BertTokenizer with default lower-casing enabled for classification, overrideable via InferenceOptions. |
| opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/AbstractDL.java | Adds BertTokenizer factory + lower-case resolution helper shared by components. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You find the eval data here: https://nightlies.apache.org/opennlp/ You can also trigger a eval build using your ASF credentials here: https://ci-builds.apache.org/job/OpenNLP/job/eval-tests-configurable/ I have triggered one for you: https://ci-builds.apache.org/job/OpenNLP/job/eval-tests-configurable/48/ |
@rzo1 Something seems odd with this run: Run time is way too fast, confirmed by this output: |
|
Thanks @rzo1 @mawiesne. Eval build #48 didn't actually exercise the new DL evals. You can see it in the build log: the module finishes in ~1.2s and no This also means the The surefire skip issue itself is a pre-existing bug on |
|
Not related to this PR, see https://ci-builds.apache.org/job/OpenNLP/job/eval-tests/ (a few minutes only for the last days), so yes what Kristian says needs to be fixed on main |
mawiesne
added
java
Without basic tokenization, uncased models received the unknown token for every capitalized or accented word. SentenceVectorsDL, DocumentCategorizerDL and NameFinderDL now tokenize with BertTokenizer. Lower casing defaults follow each component's commonly used models: true for SentenceVectorsDL and DocumentCategorizerDL (uncased), false for NameFinderDL (cased NER models rely on capitalization). The default is overridable via InferenceOptions.setLowerCase or the new SentenceVectorsDL constructor.
Validate that a RoBERTa-style vocabulary contains a supported unknown token instead of silently selecting one that is absent, and compare eval vectors with a tolerance rather than exact float equality.
Lower casing now sends correct tokens to the uncased sentiment models, shifting the score distributions. All predicted categories are unchanged; the models are simply more confident. Values regenerated against the canonical eval-data ONNX models from the nightlies archive.
|
@krickert I just noticed you're working via a branch on a fork. Forget my last question / comment related to a rebase... For the records, here's a eval run kicked for fixed main branch: |
|
In roughly 4 hours time, this build should kick in for your fork + branch: https://ci-builds.apache.org/job/OpenNLP/job/eval-tests-configurable/51 Note: URL will only work once build no. 49 has terminated. UPDATE |
|
@mawiesne for some reason the URL isn't working for me but I'm on a phone browser. Did this run get stuck too? |
|
50/51 did succeed. Onto evals are quite slow. Perhaps another investigstion target next time |
|
Looks like it passed. Squash and merge ok? |
Note
Stacked on #1072, #1073 and #1074. This branch includes their commits because it depends on
BertTokenizerfrom #1073. Once those merge, this PR will be rebased ontomainand the diff collapses to the OPENNLP-1838 changes only. Draft until then.What
SentenceVectorsDL,DocumentCategorizerDLandNameFinderDLnow tokenize withBertTokenizer(basic tokenization / normalization, then wordpiece) instead of rawWordpieceTokenizer.SentenceVectorsDLandDocumentCategorizerDL: true (the README-recommended models are uncased)NameFinderDL: false (recommended NER models such asdslim/bert-base-NERare cased; capitalization is a core signal for entity boundaries)InferenceOptions.setLowerCase(boolean)or the newSentenceVectorsDL(model, vocab, lowerCase)constructor. RoBERTa-style special-token detection is preserved.Why
See OPENNLP-1838 and OPENNLP-1837: without basic tokenization, uncased models receive
[UNK]for every capitalized or accented word, severely degrading results.Validation
CreateTokenizerTestcovers lower-casing vs. case-preserving tokenizers, RoBERTa special-token selection, and theInferenceOptionsdefault resolution.SentenceVectorsDLEvalpinned values still hold, and a capitalized variant of the eval sentence now produces vectors identical to the lowercase one (previously every capitalized word mapped to[UNK]). Added as an eval assertion.dslim/bert-base-NERvocabulary that allNameFinderDLEvalinput sentences tokenize identically before and after this change, so the NER eval expectations remain valid.Note for eval-data owners
DocumentCategorizerDLEvalexpectations will shift because lower casing changes the tokens sent to the uncased sentiment models (that is the intended fix). The pinned values need to be regenerated against the canonical eval-data ONNX files, which are not publicly available. Happy to update the pins if someone can run the eval and share the new values.