OPENNLP-1885: Pure-Java SentencePiece inference with exact original-text spans (opennlp-subword) - #1165
OPENNLP-1885: Pure-Java SentencePiece inference with exact original-text spans (opennlp-subword)#1165krickert wants to merge 17 commits into
Conversation
krickert
changed the title
krickert
changed the title
|
Single-thread throughput measurement after 3fb8b6b, for the record. Machine: AMD Ryzen 9 9950X3D (16 cores, one thread used), Linux, OpenJDK 25.0.3. Workload: 100,100 short texts (77 distinct lines cycled), t5-small unigram model, 32k vocabulary, 1.17M pieces total, 3 warmup passes over the corpus, then 5 timed passes.
That is 1.42x the reference on the same corpus and model, measured call for call from a host language, so the binding's per-call overhead is included in the reference number; the raw C++ core inside a batch loop is faster than that number. The Java side also does more work per token, since it maps every piece back to a UTF-16 span of the original input, which the reference does not produce against the original string. Output is unchanged: the bundled parity fixtures and the T5-small and ALBERT real-model fixtures assert identical pieces, ids, spans, and normalized forms against the reference before and after the optimization commit. |
|
This is now dependent on the embeddings to land. Marking ready for review |
|
If it depends on #1152 , it should still be "draft" state (since the base PR is also draft) |
|
@rzo1 Correction, my earlier comment had the dependency backwards: this PR is standalone on main, and #1152 is the one that stacks on it (its base is this branch). Nothing in opennlp-subword references the embeddings module, and CI is green on the full matrix with no #1152 code involved. Marking it ready again, sorry for the churn. |
cc74b82 to
6d40bc0
Compare
rzo1
left a comment
rzo1
left a comment
There was a problem hiding this comment.
Thanks for the extensive work here. Reviewed file-by-file — no blocking issues; the public API validates at the boundary with IAE, serialVersionUIDs are in order, the module is wired in, and the BertTokenizer removal is fine for a pre-release milestone. A handful of minor items to clean up before merge:
Javadoc — missing @throws
WordpieceEncoder.java:75/:87— the 1-arg and 2-arg public constructors delegate to the throwing map constructor (null/duplicate vocab, missing special token) but lack the@throws IllegalArgumentExceptionclause the 5-arg (:106) and map (:126) constructors already carry. Please add it, matching the 5-arg wording.PieceTrie.java:95—build()propagates IAE on a duplicate piece but omits the@throwsbothBuildermethods carry. Add@throws IllegalArgumentException Thrown if a piece is defined more than once.DoubleArrayTrie.java:57— constructor throws IAE whenlengthis non-positive or not a multiple of four; add@throws IllegalArgumentException Thrown if length is not a positive multiple of four.DoubleArrayTrie.java:79—longestPrefixMatch()maps an out-of-range unit reference (corrupt data) to IAE with no@throws; please document it.
Constants
ModelProtoReader.java:100— the tag field-number shift (>>> 3) and wire-type mask (& 7) are raw literals repeated ~14×, whileWIRE_*/FIELD_*are already named constants. Please declareTAG_FIELD_SHIFT = 3andTAG_WIRE_MASK = 7(orfieldOf(tag)/wireTypeOf(tag)helpers) and use them at every tag-decomposition site.PieceTrie.java:70— the 256-entry dispatch-table width is an unnamed literal at:70and:76; onlyDIRECT_THRESHOLDis named. Please addDIRECT_TABLE_SIZE = 256.
Duplication
IntBuilder.javavsByteBuilder.java— the 1.5× growth (data.length + (data.length >> 1)), theMath.max(capacity, 16)floor, andtruncate()validation are duplicated byte-for-byte. Please name the shared16floor and growth policy so the two copies stay in sync. The int/byte split itself is fine as primitive specialization.
Comments
SentencePieceNormalizer.java:150/:193— "heading whitespace" / "heading spaces" should read "leading" in both comments.
Exception convention (discussion)
SentencePieceTokenizer.java:224/:240—load(Path)/load(InputStream)surface a malformed.modelas the uncheckedIllegalArgumentException, which diverges from the other OpenNLP model loaders that throw the checkedInvalidFormatExceptionfor bad model content. Would you considerInvalidFormatExceptionfor content errors (keeping IAE for null-arg guards)? The class currently uses IAE uniformly by design, so flagging for discussion rather than as a fix.
Process
BertTokenizershipped in theopennlp-3.0.0-M4tag, so describing its removal as "unreleased" is slightly imprecise. Not blocking for a milestone, but please correct the wording and add a one-line migration pointer for formerBertTokenizerusers →WordpieceEncoder.
One note on coverage: I reviewed the parity/fixture tests for form, not by re-running them. The piece-for-piece reference assertions look right structurally, but I have not executed the suite as part of this pass.
|
This will be projected to OpenNLP 3.0.0 (M6) - not M5 |
…ENNLP-1895 recorded Restate the map against apache main a864230, cut as 3.0.0-M5 on 2026-07-24. apache#1177 (OPENNLP-1870) merged upstream and moves into the merged box, apache#1190 and apache#1191 are marked ready for review, and OPENNLP-1895 (quantized embedding tables) joins the diagram in its own colour: filed in JIRA with the pull request deliberately held until apache#1165 and apache#1152 move. Statuses now carry the measured GitHub draft flag and how far each head sits behind main, which surfaces three things the old text did not: apache#1182 is a draft again, apache#1167 is based on main rather than on apache#1155 and carries the seam and isBlank commits as copies, and apache#1152 reports conflicts only because its apache-hosted sentencepiece base has diverged from the refreshed head.
|
This PR removes stuff which was added in a previous milestone release of opennlp. Please add some rational so reviewers get an idea why sth was dropped. In addition, it needs to be carefully checked, which classes belong into API and which can go into a custom submoduel. |
|
@rzo1 the removals are intentional; I'll provide the full rational in a few hours. I should've provided a changelog at the top of the review. It'll go over every detail. Moving to draft until that detailed explanation lands. |
|
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. The new contract is Example: 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 returnedThe old pipeline is kept as
|
|
Nothing in your argumentation force the removal: those are separable. EncoderTokenizer already is the compat shim, it's just package-private and in opennlp-dl. Three things I'd like to change:
If you'd rather see the class gone in this PR, the minimum I'd want is the adapter promoted to public API in opennlp-api plus a migration note, so Tokenizer t = new BertTokenizer(vocab, lowerCase) has a one-line replacement. But it's ~30 lines of delegation and it buys back both the source API and the binary compatibility, so I'd rather deprecate now and remove in 3.1, after it has been deprecated through one stable release. The encoder itself and the span mapping look good - this is only about how we retire the old entry point. |
|
No problem! On it now. |
|
All three are in.
The compatibility check before deleting the old pipeline surfaced a real divergence: the encoder kept U+2028 and U+2029 inside words while the old One deliberate drift to follow convention, documented in the |
…with exact original-text spans New opennlp-extensions module implementing SentencePiece model inference without native code: the ModelProto reader, the model-embedded normalizer (precompiled character map over a Darts-clone double-array trie, whitespace collapsing and escaping, the dummy word-boundary marker), unigram best-path segmentation, BPE agenda merging, byte fallback, and user-defined symbol handling. The public contract is SubwordTokenizer/SubwordPiece; every piece reports the exact UTF-16 span of the caller's original text it came from, and the model normalizer is also exposed as an OffsetAwareNormalizer producing AlignedText. Parity with the reference implementation is asserted, not assumed: five tiny bundled models (unigram, unigram with byte fallback, BPE, identity normalization, whitespace-as-suffix) carry fixtures generated by the sentencepiece Python package over 40 inputs each, checked piece for piece, id for id, span for span, plus each model's embedded self-test samples. An opt-in test (-Dopennlp.subword.eval.dir) runs the same assertions against real downloaded models; T5-small and ALBERT-base-v2 pass exactly, including mixed scripts, emoji ZWJ sequences, BOM, and CRLF inputs.
…step The vocabulary trie dispatches wide nodes (the root and first level of a real vocabulary) through a 256-entry direct table, one load per byte, and scans narrow nodes' short label slices linearly instead of binary searching; a randomized differential test holds both layouts against a map-backed reference, and moving the duplicate-piece detection into the counting pass fixes the index error it previously produced. Non-unknown segments reuse the vocabulary's piece string instead of decoding their bytes, since the trie match means the bytes are identical. The normalizer precomputes, per possible first byte, whether any character-map rule or user-defined symbol starts with it; a clear bit proves the prefix machinery would pass the byte through raw, so plain ASCII text skips it entirely. The per-chunk record became a per-call scratch, the input view keeps its oversized buffers with an explicit length instead of trimming (pure-ASCII text gets an identity offset map and no map array at all), the Viterbi scratch is one interleaved array with scores as raw float bits, and the character-map trie walk relies on the JVM's own bounds checks with the fail-loud translation on the cold path. All 37 bundled parity tests and the T5-small and ALBERT real-model fixtures pass byte-identically. Single-thread throughput on the T5-small vocabulary goes from 2.83M to 6.47M pieces per second, from 0.62x to 1.42x of the reference implementation measured through its Python binding.
SubwordTokenizer and SubwordPiece move to opennlp.tools.tokenize, next to Tokenizer and WordpieceTokenizer, matching where every other seam of this round lives. The opennlp-subword module keeps only the SentencePiece implementation.
…zer into it WordpieceEncoder in opennlp-api runs the full BERT tokenization pipeline as a SubwordTokenizer: every piece carries its vocabulary id and the span of the original text, surviving the normalization steps that change, insert, and remove characters. Content is computed with the same library calls the previous pipeline made; offsets come from a per-code-point rerun, with contextual case mappings (Greek final sigma) falling back to word-wide spans that widen but never misplace. List and map constructors cover line-number and explicit-id vocabularies. BertTokenizer, unreleased and superseded, is removed. The dl tokenizer creation builds on the encoder behind the existing Tokenizer plumbing via a package-private adapter with unchanged special-token selection, and a vocabulary missing its special tokens now fails at construction instead of at the first id mapping, pinned by a test. Parity is enforced twice: a differential suite against the reference pipeline (kept test-only as ReferenceBertPipeline) over a curated corpus plus 800 randomized inputs, and the removed class's reference token sequences ported case for case. WordpieceTokenizer is untouched.
…verrides Applies the review conventions from the OPENNLP-1869 review: class javadoc states the contracts instead of design narrative, every override carries inheritDoc with its null contract, and the private helpers are documented.
The tokenizer is Serializable through the OffsetAwareNormalizer contract but declared no serialVersionUID, which the compiler warns about. Added the serialver-computed value so it matches the convention used across the normalizer classes.
Adds a Subword Tokenization section to the Tokenizer chapter: the SubwordTokenizer contract and its original-text span guarantee, loading and using a SentencePiece model including the OffsetAwareNormalizer face, and the WordpieceEncoder pipeline with its vocab.txt construction and special-token framing.
…s, name the format constants, document every helper
…bjectInputFilter SentencePieceTokenizer gains serialize(OutputStream) and deserialize(InputStream) methods. Reads are filtered through an ObjectInputFilter that allow-lists only the classes reachable from a legitimate tokenizer graph and bounds graph depth, references, and array length; foreign payloads are rejected with InvalidClassException before being materialised. Limits are adjustable through a DeserializationLimits record for unusually large vocabularies; the allow-list is not configurable. The serialVersionUID is recomputed for the new public methods.
Add SentencePieceUsageExampleTest asserting the load-and-encode workflow and point the tokenizer manual section at it.
…ormed models, tag helpers, javadoc throws
…ixtures, thread safety wording - Normalize the argument validation messages to the project style, naming the offending parameter and dropping the leading article and the trailing period, in WordpieceEncoder, SentencePieceTokenizer, ModelProtoReader, BpeEncoder, UnigramEncoder and the SubwordPiece compact constructor. - Stop promising thread safety in the SubwordTokenizer contract and state that it is implementation specific instead; the manual now records that both shipped implementations are immutable and therefore safe for concurrent use. - Drop the @throws IllegalArgumentException tags that only repeated the inherited contract on the normalize and normalizeAligned overrides, leaving a plain {@inheritdoc} as the rest of the class does. - Remove commentary about release history rather than about the code: the pointer to the BertTokenizer class of the 3.0.0 milestone builds in WordpieceTokenizer, and the "frozen" qualifier on the ReferenceBertPipeline baseline. - Move the bundled model loading and the fixture file reading out of SentencePieceParityTest into SentencePieceFixtures, so the alignment, validation and serialization tests no longer reach into another test class for a tokenizer. - Extract MODEL_SUFFIX and FIXTURES_SUFFIX constants on SentencePieceFixtures and use them in SentencePieceRealModelEvalTest when deriving a fixture path from a model path, instead of repeating the two literals. - Fold the five duplicated @valuesource model lists into a single SentencePieceFixtures#models @MethodSource, so adding a bundled model stays a one line change. - Correct the parity test javadoc, which credited a nonexistent gen_fixtures.tsv sibling script instead of the gen_fixtures.py script in the test resources. - Pin accessors that had no coverage: every score is finite and out of range ids are rejected, byte pieces occur only in byte fallback models and always render in the <0x..> form, and isByte rejects negative ids. - Assert SubwordPiece.span() next to start and end in WordpieceEncoderTest so the derived span stays covered by the piece assertions. - Document the IOException of the serialized helper in the serialization test and fully qualify the OutputStream javadoc link now that the import is gone.
… per review Applies the review: the old entry point stays through one stable release instead of being removed, and the DL extension point keeps its descriptor. - Recreate BertTokenizer in opennlp-api as a thin shim, deprecated since 3.0.0 forRemoval, with the original three Set based constructors and the original tokenizePos message. tokenize delegates to encodeToPieces; ids are synthesized from the set order because the tokenize path never reads them. Null contract follows this branch's reviewed convention, IllegalArgumentException, documented in the throws clauses. - Delete the package-private EncoderTokenizer; the adapter now lives in opennlp-api where downstream code can reach it. AbstractDL's protected createTokenizer returns BertTokenizer again, restoring the override descriptor so an already compiled subclass keeps overriding at runtime, and createPipelineTokenizer hands back the shim. - Delete ReferenceBertPipeline and point the curated and randomized differential tests in WordpieceEncoderTest at the shim, pinning shim and encoder to one sequence. Add BertTokenizerTest covering each constructor's argument validation, the default special token chain, and the exact tokenizePos message. Independent expected sequences continue to live in WordpieceEncoderReferenceSequencesTest. - Fix a real divergence the compatibility check surfaced: the encoder kept U+2028 and U+2029 inside words while the old pipeline split on them, so a word carrying a line or paragraph separator became the unknown piece. cleanAndIsolateCjk now maps Zl and Zp to a space, with a span asserting regression test. - Manual: the WordPiece section describes the deprecation and the migration, including the Set to List vocabulary change.
3 participants
Adds a new opennlp-extensions/opennlp-subword module implementing SentencePiece model inference purely in Java: the model file reader (hand-written protobuf wire parsing, no new dependency), the model-embedded normalizer (precompiled character map over the double-array trie format, whitespace collapsing and escaping, word-boundary marker), unigram best-path segmentation, BPE agenda merging, byte fallback, and user-defined symbol handling. The public contract is SubwordTokenizer/SubwordPiece; every piece reports the exact UTF-16 span of the caller's original text it came from, and the model normalizer is also exposed as an OffsetAwareNormalizer producing AlignedText.
Parity with the reference implementation is asserted, not assumed. Five tiny trained models are bundled with fixtures generated by the sentencepiece Python package (unigram, unigram with byte fallback, BPE, identity normalization, whitespace-as-suffix), checked piece for piece, id for id, span for span, plus each model's embedded self-test samples. An opt-in test (-Dopennlp.subword.eval.dir) runs the same assertions against real downloaded models; t5-small (32k vocabulary) and albert-base-v2 (30k) pass exactly, including mixed scripts, emoji ZWJ sequences, BOM, and CRLF inputs. Fixture regeneration scripts live in the test resources.
Measured single-thread throughput on the t5-small vocabulary is about 2.8M pieces/s; the native reference is about 1.6x faster. The value here is zero native dependencies, one shareable thread-safe instance, and exact original-text offsets. A non-breaking performance follow-up with identified wins is planned separately.