krickert commented on PR #1152: URL: https://github.com/apache/opennlp/pull/1152#issuecomment-4939437080
Thanks. Addressed in 9dd6ff7d, one pushback on the marker question: Blocking: - TensorInfo: the shape is now copied on construction and on access, equals/hashCode compare by value, and elementCount() rejects overflow from crafted headers via exact multiplication. Tests pin the defensive copies, the value semantics, and the overflow. - Experimental marker: not applied, deliberately. The public surface is five types with a small, verified contract: the pooling formula is pinned against the reference implementations, the safetensors reader implements a frozen file format, and the listed follow-ups are additive consumers (an ANN index slots behind mostSimilar, the gRPC provider is a caller), not surface reshapes. WordVectorTable's marker reflected an unfinished feature; this one is finished. If the release manager prefers the annotation anyway it is a two-line change. - Distribution wiring: opennlp-embeddings added to the distr dependencies and its apidocs fileSet to the assembly, matching the other extension modules. - WordPieceVocabulary: dedicated test class added covering the line-number ids, duplicate rejection (naming both lines), the -1 sentinel, and token(int) bounds, which now fail loud with IllegalArgumentException instead of leaking an IndexOutOfBoundsException. Minor: - Unicode escapes now require four hex digits; signed input fails loud. Tested. - skipValue holds numbers in skipped fields to the JSON grammar; malformed tokens fail loud, well-formed scientific notation still passes. Tested both ways. - elementCount overflow: fixed with the TensorInfo change above. - Checked IOException: agreed, the load and read entry points now declare IOException like the rest of the project's loaders; UncheckedIOException is gone from the module. - Boolean flags: replaced with Casing (UNCASED/CASED) and Normalization (L2/NONE) enums, so the switches cannot be swapped silently. Manual, README, tests, and benchmark updated. - The redundant punctuation pass lives in the reused opennlp-api tokenizers, so the fix belongs there; noted as a follow-up rather than worked around here. - Fixture writer: extracted to a shared test utility used by the tests; the JMH benchmark keeps its own copy because it compiles in a separate source set. - Stale javadoc reference removed. - The element-count versus byte-range guard has its negative test. - else/catch placement aligned with project style across the module. - model2vec-rs: no code was translated or ported. The implementation was written against the safetensors format specification and the documented Model2Vec pooling semantics; the Rust and Python implementations were used as behavioral oracles (outputs compared for numerical parity), not as source. No attribution entry is required. -- 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]
