Conversation
|
This needs to wait after M6. |
OPENNLP-1937 (apache#1290) proposes this interface for opennlp-api without the experimental marker. This branch had an earlier copy that still used it, so the integration build hit an add/add conflict on the file and could resolve it either way. Take the upstream file as is. No behaviour changes: an annotation, an import, and a javadoc paragraph.
|
Putting this in draft. We should separate ONNX and CLI from the embedding library. ONNX can remain the default through an SPI, while downstream projects can exclude it and supply another implementation. Any native integration currently pulls in both unnecessarily. This would fix that concern. I’ll include the split in this ticket. It should be a focused packaging change that preserves the default runtime behavior and makes add-ons easier to use. It's not a lot of work to do. |
Select model factories by provider ID, retain ONNX as the default, and allow independently owned models through the closeable TextEmbedder API. Keep model initialization out of provider discovery and preserve the existing SentenceVectorsDL inference path. Tests cover multiple providers, missing and duplicate IDs, independent model lifetimes, and ONNX output parity. The root model-path regression first failed with NullPointerException; it now rejects the invalid path. The SPI tests initially failed to compile before the API was introduced. Validation: targeted Maven verify with dependencies, apache-rat:check, and final path-validation regression tests passed.
|
The discovery half of this is fine — Where the binding sitsThree places, all in
The docs say it out loud: "The API has no ONNX or CLI dependency" … "the default remains There is a second-order problem with keying on Also, the loop propagates Proposal: CDI's resolution model, in plain Java SECDI discovers portable extensions through
In public interface TextEmbedderProvider {
String name(); // identity: pinning + diagnostics, not the routing key
default int priority() { return 0; } // @Priority analogue, larger wins
default boolean isAvailable() { return true; } // native runtime actually loadable
boolean supports(Path model, Map<String, String> options);
TextEmbedder load(Path model, Map<String, String> options) throws IOException;
}public final class TextEmbedders {
public static final String PROVIDER_PROPERTY = "opennlp.embedder.provider";
// no DEFAULT_PROVIDER constant
public static TextEmbedderProvider select(Path model, Map<String, String> options,
ClassLoader loader) {
String pinned = System.getProperty(PROVIDER_PROPERTY);
if (pinned != null && !pinned.isBlank()) {
return byName(pinned, loader);
}
List<TextEmbedderProvider> candidates = installed(loader).stream()
.filter(TextEmbedderProvider::isAvailable)
.filter(p -> p.supports(model, options))
.toList();
// highest priority wins; only a genuine tie at the top is ambiguous, and that
// message tells the caller to set PROVIDER_PROPERTY
...
}
}
@Override
public boolean supports(Path model, Map<String, String> options) {
return model != null && model.getFileName().toString().endsWith(".onnx");
}ONNX stays the de facto default because it is the only provider in the default distribution — a packaging fact, not an API constant. A future Alternatives considered
Two more things worth deciding on this PR
The minimal version is small: drop |
…nstead of a fixed name The API no longer names a backend. TextEmbedderProviders drops the DEFAULT_PROVIDER constant and getDefault(); select(model, options) picks among the installed providers that are available and support the request, and the highest priority wins. Two supporting providers with the same priority are reported as ambiguous with their class names and the system property opennlp.embedder.provider, which pins a provider by name. get(name) ranks providers of one name by priority as well, so a module can replace another module's provider under the same name. installed() lists providers through ServiceLoader.stream() and skips a registration that cannot be loaded or instantiated, with a warning, so one broken jar does not hide the other providers. TextEmbedderProvider adds priority(), isAvailable() and supports(model, options). OnnxTextEmbedderProvider supports a model file named *.onnx with its own options and is available when the ONNX Runtime classes load; the checks do not initialize the runtime. Tests: TextEmbedderProvidersTest covers name lookup, priority replacement, same-priority ambiguity, unavailable and failing availability checks, capability routing, the pin property, skipped broken registrations and a blank name, 15 tests, none naming a backend. OnnxTextEmbedderProviderTest covers the capability rule and the registration in opennlp-dl. SentenceVectorsDLEmbedderTest selects the provider for the model instead of asking for a default. The manual section describes selection, the pin property, and that discovery uses the class path, since OpenNLP ships no module descriptors.
bc1ef85 to
1378f64
Compare
|
Thanks, both points taken. Pushed in 1378f64 on top of the rebase onto main:
On the two decisions: I would keep |
…ce-loaded components A contract-neutral registration and selection model for optional implementations, the shape the text embedder review chose: a provider names the contract it creates, its name, a priority, whether it is available in this process, and whether it supports a request. A request is a ComponentSpec, a model location plus options, so a provider can claim a request without opening anything. Providers register once per jar under META-INF/services/opennlp.tools.util.ext.ComponentProvider and are looked up by contract: installed(), get(name) ranked by priority under one name, supporting(spec) as the ranked list for callers that want every matching component, select(spec) as the single best one with a tie reported as ambiguous, and a per-contract pin property opennlp.provider.<Contract>. A registration that cannot be loaded is skipped with a warning. No reflection and no regular expressions in the lookup path. Tests: ComponentProvidersTest covers per-contract listing, independent creation, name lookup, priority replacement, ties, unavailable and throwing availability checks, capability routing and the ranked list, the pin property, skipped registrations, blank names and a missing contract, 13 tests. ComponentSpecTest covers copying, ordering, null rejection, option checks, case-insensitive location suffixes, withOption and equality, 14 tests. The introduction chapter adds a section on component providers. (cherry picked from commit 005541f)
StemmerAnnotatorProvider in opennlp-runtime is the first ComponentProvider registration: contract DocumentAnnotator, name stemmer, supporting a request without a location with algorithm as the single option, absent or porter for the Porter stemmer and any Snowball algorithm name otherwise, compared through StringUtil case mapping. The provider is listed in the module's META-INF/services file. StemmerAnnotatorProviderTest covers the identity, the supported and rejected requests, the annotators it creates for porter and english over a tokenized document, and the registration through ComponentProviders, 10 tests. The introduction section names it as the example. (cherry picked from commit d60943ec05c915e613972d11cb896c4f987d7aa1)
|
Pushed two more commits, 1bf8cbb and ca15862, with the contract-neutral form of the same model: |
Based on main. Adds
opennlp.tools.embeddings.TextEmbedderandTextEmbedderProviderto opennlp-api, makesSentenceVectorsDLimplement the embedder, and registers it as theonnxprovider in opennlp-dl throughMETA-INF/services.Core had no contract for "text in, vector out".
WordVectorTablelooks up a stored vector for one word, andSentenceVectorsDLexposes vectors only through the concrete class, so no API could accept an arbitrary embedder, and an add-on can only implement a contract present in a released core version.The interface
TextEmbedder:embed(CharSequence), a defaultedembedAll(List)for implementations that batch, anddimension(). InSentenceVectorsDL,embedadaptsgetVectors(which now rejects null input),embedAllbuckets inputs by token count and runs one session per bucket so a batch has no padding and each result matches the single-input path, anddimension()reads the model output metadata with a cached measurement as fallback.Discovery and selection
TextEmbedderProviderhasname(),priority()(default 0),isAvailable()(default true),supports(model, options)andload(model, options). Providers register inMETA-INF/services/opennlp.tools.embeddings.TextEmbedderProvider; construction must not load models or initialize a native runtime.TextEmbedderProvidersin opennlp-api names no backend:select(model, options)considers the installed providers that are available and support the request, and returns the one with the highest priority. A tie is anIllegalStateExceptionnaming the classes and the pin property. No candidate is anIllegalArgumentExceptionnaming the installed providers.opennlp.embedder.providerpins a provider by name;get(name)does the same in code. Several providers of one name are ranked by priority, so a module such as a future opennlp-dl-gpu provider can replace the ONNX one under the same name with a larger priority, without an API change.installed()lists providers throughServiceLoader.stream()and skips a registration that cannot be loaded or instantiated, with a warning, so one broken jar does not hide the others.OnnxTextEmbedderProvidersupports a model file named*.onnxwith its own options (vocabulary, required, resolved against the model's directory when relative;lowerCase, default true) and is available when the ONNX Runtime classes load. ONNX is the default only as a packaging fact: it is the one provider in the default distribution.Tests
TextEmbedderProvidersTest, 15 tests over providers registered in a temporary class loader: name lookup and independent model ownership, priority replacement under one name, same-priority ambiguity, unavailable and throwing availability checks, capability routing, the pin property including a missing pinned name, skipped broken registrations (wrong type, no public constructor, missing class), a blank name, and no provider at all. None of them names a backend.OnnxTextEmbedderProviderTest: the capability rule for file names and options, availability, and the registration in opennlp-dl.SentenceVectorsDLEmbedderTestruns a real ONNX session. The bundled graph is 373 bytes and computesoutput[b][t] = float(input_ids[b][t]) * [0.5, -1, 2], so expected vectors follow from the ids; the generator script is committed next to it.rat-excludesand.gitignoregain entries for the binary, since*.onnxis ignored repo-wide.opennlp-api and opennlp-dl with checkstyle and forbiddenapis,
-Dopennlp.forkCount=1.Manual
machine-learning.xml, section "Text embedding providers": the contract, how selection ranks providers, the pin property, the ONNX options, and that discovery uses the class path because OpenNLP ships no module descriptors.Component providers, the contract-neutral form
The last two commits add the same registration model for any contract, in
opennlp.tools.util.ext, so an add-on can ship an annotator, a gazetteer, a stemmer or an embedder and register it the same way:ComponentProvider<T>:type()(the contract, for exampleDocumentAnnotator.class),name(),priority(),isAvailable(),supports(ComponentSpec),create(ComponentSpec).ComponentSpec: the request, an immutable location (file or directory, or none) plus ordered string options, withhasOnlyOptions,locationEndsWithandoptionfor capability checks. This is the spec type from the review comment.ComponentProviders:installed(type),get(type, name)ranked by priority under one name,supporting(type, spec)as the ranked list for callers that want every matching component,select(type, spec)as the single best with a tie reported as ambiguous, and one pin property per contract,opennlp.provider.<Contract>. OneMETA-INF/services/opennlp.tools.util.ext.ComponentProviderfile per jar; a broken registration is skipped with a warning. No reflection and no regular expressions in the lookup path.StemmerAnnotatorProviderin opennlp-runtime, namestemmer, contractDocumentAnnotator, optionalgorithm(porteror a Snowball name).TextEmbedderProviderkeeps itsPathplusMapsignature in this PR so the embedder review stays readable; converging it onComponentProvider<TextEmbedder>is a mechanical follow-up.Tests:
ComponentProvidersTest(13),ComponentSpecTest(14),StemmerAnnotatorProviderTest(10). The introduction chapter adds the section "Component providers".Open points
META-INF/servicesfiles in the repository and there is nomodule-info.java. The manual states the class path is the supported configuration; aprovidesclause is a decision for when module descriptors are introduced.TextEmbedderProvidershould extendComponentProvider<TextEmbedder>in this PR or in the follow-up.OPENNLP-1937