You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opennlp.apache.org by "krickert (via GitHub)" <gi...@apache.org> on 2026/10/11 16:14:29 UTC
[PR] GH-1392: Add padding strategies for SentenceVectorsDL.embedAll and pad on a GPU by default (opennlp)
krickert opened a new pull request, #1474:
URL: https://github.com/apache/opennlp/pull/1474
Draft until #1468 merges; this branch is stacked on it.
SentenceVectorsDL.embedAll groups texts by exact tokenized length. On natural text almost every length differs, so most inferences hold one or two texts, and on a GPU a call of mixed lengths is no faster than calling embed in a loop.
PaddingStrategy sets how embedAll batches texts of different lengths:
- EXACT_LENGTH does not pad and runs one inference per tokenized length. It is the default on the CPU.
- LONGEST orders texts by length and pads each batch to its longest text. It is the default on a GPU when the vocabulary has a padding token.
- MAX_LENGTH pads every text to the maximum length, for models or accelerators that need a fixed shape.
A new constructor takes the strategy, and the onnx provider takes a padding option. The padding id is read from the vocabulary ([PAD], or <pad> for RoBERTa). Padded positions are 0 in the attention mask and mean pooling skips them, so the vectors match up to floating-point rounding when the model honors the mask. Every strategy stays within 16384 token positions per inference. The machine learning chapter documents the option and the GPU default.
SentenceVectorsDLBenchmark on all-MiniLM-L6-v2, RTX 4080 SUPER, texts of 4 to 40 words, embedAll time per call with EXACT_LENGTH and with the GPU default:
| Texts | EXACT_LENGTH | LONGEST |
| ---: | ---: | ---: |
| 8 | 3603 us | 862 us |
| 32 | 11431 us | 2152 us |
| 128 | 23815 us | 7443 us |
Tests: batching under each strategy, the token position cap with padding, the padding id from the vocabulary, the device default, and SentenceVectorsDLPaddingEdgesTest for the cap boundaries under every strategy, input order with interleaved lengths, a RoBERTa vocabulary, padding id 0, CLS pooling and unit length scaling, the pooled-output model, single embed under MAX_LENGTH, and CUDA against the CPU (skipped without a CUDA provider or device). tiny-masked.onnx, generated by gen_tiny_masked_model.py next to it, sums the ids times the attention mask, so a test can tell masked padding from unmasked.
Fixes #1392
--
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: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-1392: Add padding strategies for SentenceVectorsDL.embedAll and pad on a GPU by default (opennlp)
Posted by "krickert (via GitHub)" <gi...@apache.org>.
krickert commented on code in PR #1474:
URL: https://github.com/apache/opennlp/pull/1474#discussion_r4385583216
##########
opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/AbstractDL.java:
##########
@@ -418,13 +422,41 @@ static WordpieceEncoder createWordpieceEncoder(
}
/**
- * The classification, separator and unknown tokens a tokenizer adds or emits.
+ * {@return the vocabulary id of the padding token} The padding token is {@code <pad>} for a
+ * RoBERTa vocabulary and {@code [PAD]} otherwise.
+ *
+ * @param vocab The vocabulary map.
+ * @throws IllegalArgumentException Thrown if the vocabulary has no padding token, or if it has
+ * the RoBERTa classification and separator tokens but no unknown token.
+ */
+ protected static long padTokenId(final Map<String, Integer> vocab) {
Review Comment:
Done. Both helpers are now instance methods that read `vocab`, are marked `@Internal(since = "3.0.0")`, and document their `IllegalArgumentException` conditions.
--
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: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-1392: Add padding strategies for SentenceVectorsDL.embedAll and pad on a GPU by default (opennlp)
Posted by "krickert (via GitHub) via commits" <co...@opennlp.apache.org>.
krickert commented on code in PR #1474:
URL: https://github.com/apache/opennlp/pull/1474#discussion_r4442248045
##########
opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/vectors/SentenceVectorsDL.java:
##########
@@ -244,6 +296,18 @@ public SentenceVectorsDL(final File model, final File vocabulary, final boolean
}
+ /**
+ * {@return the padding of a session built without a padding strategy:
+ * {@link #DEFAULT_GPU_PADDING} on a GPU for a token output if the vocabulary has a padding
+ * token, {@link #DEFAULT_CPU_PADDING} otherwise} A model with a pooled output is not padded by
+ * default because it may ignore the attention mask, in which case padding changes the vectors.
+ *
+ * @param gpu {@code true} if the session runs on a GPU.
+ */
+ PaddingStrategy devicePadding(final boolean gpu) {
+ return gpu && !pooledOutput && hasPadToken() ? DEFAULT_GPU_PADDING : DEFAULT_CPU_PADDING;
Review Comment:
Added a release note in the README migration section and documented this in `InferenceOptions.setGpu`. Both explain the GPU default, possible floating-point differences, and how to retain unpadded batches with `EXACT_LENGTH`.
--
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: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-1392: Add padding strategies for SentenceVectorsDL.embedAll and pad on a GPU by default (opennlp)
Posted by "krickert (via GitHub) via commits" <co...@opennlp.apache.org>.
krickert commented on code in PR #1474:
URL: https://github.com/apache/opennlp/pull/1474#discussion_r4442313302
##########
opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/vectors/SentenceVectorsDL.java:
##########
@@ -216,20 +235,53 @@ public SentenceVectorsDL(final File model, final File vocabulary, final boolean
final InferenceOptions inferenceOptions)
Review Comment:
Removed the seven-argument overload and updated its callers to use `SentenceVectorsOptions` and `InferenceOptions`. The older convenience constructors remain.
--
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: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-1392: Add padding strategies for SentenceVectorsDL.embedAll and pad on a GPU by default (opennlp)
Posted by "rzo1 (via GitHub) via commits" <co...@opennlp.apache.org>.
rzo1 commented on code in PR #1474:
URL: https://github.com/apache/opennlp/pull/1474#discussion_r4440810208
##########
opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/vectors/SentenceVectorsDL.java:
##########
@@ -244,6 +296,18 @@ public SentenceVectorsDL(final File model, final File vocabulary, final boolean
}
+ /**
+ * {@return the padding of a session built without a padding strategy:
+ * {@link #DEFAULT_GPU_PADDING} on a GPU for a token output if the vocabulary has a padding
+ * token, {@link #DEFAULT_CPU_PADDING} otherwise} A model with a pooled output is not padded by
+ * default because it may ignore the attention mask, in which case padding changes the vectors.
+ *
+ * @param gpu {@code true} if the session runs on a GPU.
+ */
+ PaddingStrategy devicePadding(final boolean gpu) {
+ return gpu && !pooledOutput && hasPadToken() ? DEFAULT_GPU_PADDING : DEFAULT_CPU_PADDING;
Review Comment:
Existing GPU users now get LONGEST without asking, so vectors change slightly. Needs a release note, and a mention in InferenceOptions#setGpu.
##########
opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/vectors/SentenceVectorsDL.java:
##########
@@ -442,10 +555,29 @@ private float[][] run(final Tokens[] batch) throws OrtException {
}
/**
- * Pools the token vectors of one input into its sentence vector.
+ * {@return the row width of one inference} It is the maximum length under
+ * {@link PaddingStrategy#MAX_LENGTH}, otherwise the longest row of the batch.
+ *
+ * @param batch The encodings of one inference, at least one.
+ */
+ private int width(final Tokens[] batch) {
+ if (padding == PaddingStrategy.MAX_LENGTH) {
Review Comment:
With MAX_LENGTH the dimension probe also runs at maxLength. If maxLength is above the model's position limit, this fails where EXACT_LENGTH works. One line in the Javadoc.
##########
opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/vectors/SentenceVectorsOptions.java:
##########
@@ -0,0 +1,60 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.dl.vectors;
+
+import opennlp.tools.util.ParamChecks;
+
+/**
+ * Pooling, normalization, truncation and padding settings for {@link SentenceVectorsDL}.
+ *
+ * @param pooling How token vectors are pooled. Not used for a model with a
+ * {@code sentence_embedding} output. Must not be {@code null}.
+ * @param normalize {@code true} to scale every vector to unit length.
+ * @param maxLength The maximum number of tokens per input, including the classification and
+ * separator tokens. Longer input is truncated. Must be at least {@code 2}.
+ * @param padding The padding strategy, or {@code null} for the device and model default:
+ * {@link SentenceVectorsDL#DEFAULT_GPU_PADDING} on a GPU for a token output if the
+ * vocabulary has a padding token, {@link SentenceVectorsDL#DEFAULT_CPU_PADDING} otherwise.
+ * An explicit padded strategy requires a padding token in the vocabulary.
+ *
+ * @since 3.0.0
+ */
+public record SentenceVectorsOptions(Pooling pooling, boolean normalize, int maxLength,
+ PaddingStrategy padding) {
+
+ /**
+ * Creates vector settings after validating the pooling mode and maximum length.
+ *
+ * @throws IllegalArgumentException Thrown if {@code pooling} is {@code null} or
+ * {@code maxLength} is less than {@code 2}.
+ */
+ public SentenceVectorsOptions {
+ ParamChecks.requireNonNullArg(pooling, "pooling");
+ if (maxLength < 2) {
Review Comment:
This was MIN_LENGTH in SentenceVectorsDL before, keep the constant.
##########
opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/vectors/SentenceVectorsOptions.java:
##########
@@ -0,0 +1,60 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.dl.vectors;
+
+import opennlp.tools.util.ParamChecks;
+
+/**
+ * Pooling, normalization, truncation and padding settings for {@link SentenceVectorsDL}.
+ *
+ * @param pooling How token vectors are pooled. Not used for a model with a
+ * {@code sentence_embedding} output. Must not be {@code null}.
+ * @param normalize {@code true} to scale every vector to unit length.
+ * @param maxLength The maximum number of tokens per input, including the classification and
+ * separator tokens. Longer input is truncated. Must be at least {@code 2}.
+ * @param padding The padding strategy, or {@code null} for the device and model default:
Review Comment:
null meaning "device default" is easy to miss. An explicit AUTO, or a withDefaults() factory, would be clearer.
##########
opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/vectors/SentenceVectorsDL.java:
##########
@@ -216,20 +235,53 @@ public SentenceVectorsDL(final File model, final File vocabulary, final boolean
final InferenceOptions inferenceOptions)
Review Comment:
3.0.0 isn't released yet, do we need this overload next to the options one?
##########
opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/vectors/OnnxTextEmbedderProvider.java:
##########
@@ -159,6 +170,30 @@ private Pooling poolingOption(final ProviderSpec spec) {
throw new IllegalArgumentException(POOLING_OPTION + " must be " + MEAN + " or " + CLS);
}
+ /**
+ * Reads the {@value #PADDING_OPTION} option.
+ *
+ * @param spec The spec to read.
+ * @return The padding strategy, {@link PaddingStrategy#EXACT_LENGTH} if the option is not set.
+ * @throws IllegalArgumentException Thrown if the value is not the lower case name of a
+ * {@link PaddingStrategy}.
+ */
+ private PaddingStrategy paddingOption(final ProviderSpec spec) {
+ final String value = spec.option(PADDING_OPTION, null);
+ if (value == null) {
+ return SentenceVectorsDL.DEFAULT_CPU_PADDING;
+ }
+ final List<String> names = new ArrayList<>();
+ for (final PaddingStrategy padding : PaddingStrategy.values()) {
+ final String name = StringUtil.toLowerCase(padding.name());
+ if (name.equals(value)) {
Review Comment:
Case-sensitive, padding=LONGEST fails. Lowercase the value first?
--
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: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-1392: Add padding strategies for SentenceVectorsDL.embedAll and pad on a GPU by default (opennlp)
Posted by "krickert (via GitHub)" <gi...@apache.org>.
krickert commented on code in PR #1474:
URL: https://github.com/apache/opennlp/pull/1474#discussion_r4385613555
##########
opennlp-docs/src/docbkx/machine-learning.xml:
##########
@@ -176,10 +176,17 @@ try (TextEmbedder embedder = providers.select(spec).create(spec)) {
<code>lowerCase=false</code>, <code>pooling=mean</code> (default) or
<code>pooling=cls</code>, <code>normalize=true</code> (default) or
<code>normalize=false</code>, and <code>maxLength</code>, the maximum number of tokens
- per input (512 by default); longer input is truncated. The defaults match
- sentence-transformers models such as all-MiniLM-L6-v2. A model with a
- <code>sentence_embedding</code> output is used as it is, without pooling. A spec with
- another option is not supported. The key
+ per input (512 by default); longer input is truncated. The option <code>padding</code>
+ sets how <code>embedAll</code> batches texts of different lengths:
+ <code>exact_length</code> (default) does not pad and runs one inference per tokenized
+ length, <code>longest</code> pads each batch to its longest text, and
+ <code>max_length</code> pads every text to <code>maxLength</code>. The vectors match up
+ to floating-point rounding either way, provided the model honors the attention mask; a
Review Comment:
done
--
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: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-1392: Add padding strategies for SentenceVectorsDL.embedAll and pad on a GPU by default (opennlp)
Posted by "krickert (via GitHub)" <gi...@apache.org>.
krickert commented on code in PR #1474:
URL: https://github.com/apache/opennlp/pull/1474#discussion_r4385591551
##########
opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/vectors/SentenceVectorsDL.java:
##########
@@ -244,6 +331,17 @@ public SentenceVectorsDL(final File model, final File vocabulary, final boolean
}
+ /**
+ * {@return the padding of a session built without a padding strategy: {@link #DEFAULT_PADDING}
+ * on the CPU, {@link #DEFAULT_GPU_PADDING} on a GPU if the vocabulary has a padding token}
+ *
+ * @param gpu {@code true} if the session runs on a GPU.
+ * @param vocab The vocabulary of the session.
+ */
+ static PaddingStrategy devicePadding(final boolean gpu, final Map<String, Integer> vocab) {
Review Comment:
No problem. Models with a pooled output now keep `EXACT_LENGTH` by default on the GPU too.
Got it so an explicit strategy can still override this. The manual and Javadoc explain that padding can change the vectors when the model ignores the attention mask.
--
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: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-1392: Add padding strategies for SentenceVectorsDL.embedAll and pad on a GPU by default (opennlp)
Posted by "krickert (via GitHub)" <gi...@apache.org>.
krickert commented on code in PR #1474:
URL: https://github.com/apache/opennlp/pull/1474#discussion_r4385604619
##########
opennlp-core/opennlp-ml/opennlp-dl/src/test/java/opennlp/dl/vectors/SentenceVectorsDLEmbedderTest.java:
##########
@@ -240,6 +269,177 @@ void testEmbedAllMatchesSingleEmbedsExactly(@TempDir final Path dir) throws Exce
}
}
+ /**
+ * {@return the number of rows of each batch}
+ *
+ * @param batches The batches.
+ */
+ private static List<Integer> sizes(final List<List<Integer>> batches) {
Review Comment:
Done. The batch-size extraction is shared through `TinyVectorModels.sizes`, the model references use `TINY_POOLED` and `TINY_MASKED`, and both strategy loops are now parameterized tests.
--
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: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-1392: Add padding strategies for SentenceVectorsDL.embedAll and pad on a GPU by default (opennlp)
Posted by "krickert (via GitHub)" <gi...@apache.org>.
krickert commented on code in PR #1474:
URL: https://github.com/apache/opennlp/pull/1474#discussion_r4385602706
##########
opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/vectors/SentenceVectorsDL.java:
##########
@@ -216,6 +233,74 @@ public SentenceVectorsDL(final File model, final File vocabulary, final boolean
final InferenceOptions inferenceOptions)
throws OrtException, IOException {
+ this(model, vocabulary, lowerCase, pooling, normalize, maxLength, inferenceOptions, null);
+
+ }
+
+ /**
+ * Instantiates a {@link SentenceVectorsDL sentence vector generator} using ONNX models, with
+ * the {@link PaddingStrategy} that {@link #embedAll(List)} batches by and the
+ * {@link InferenceOptions} the session is built from.
+ *
+ * @param model The file name of a sentence vectors ONNX model.
+ * @param vocabulary The file name of the vocabulary file for the model.
+ * @param lowerCase {@code true} for uncased models (lower casing and accent
+ * stripping during tokenization), {@code false} for cased models. A lower case setting in
+ * {@code inferenceOptions} takes precedence.
+ * @param pooling How token vectors are pooled. Not used for a model with a
+ * {@code sentence_embedding} output. Must not be {@code null}.
+ * @param normalize {@code true} to scale every vector to unit length.
+ * @param maxLength The maximum number of tokens per input, {@code [CLS]} and {@code [SEP]}
+ * included; longer input is truncated. Must be at least {@code 2}.
+ * @param padding How {@link #embedAll(List)} batches texts of different lengths. Must not be
+ * {@code null}. Every strategy but {@link PaddingStrategy#EXACT_LENGTH} needs a
+ * {@code [PAD]} token in the vocabulary, or {@code <pad>} for a RoBERTa vocabulary.
+ * @param inferenceOptions The {@link InferenceOptions}, of which the GPU settings and the lower
+ * case setting are used. Must not be {@code null}.
+ *
+ * @throws IllegalArgumentException Thrown if {@code pooling}, {@code padding} or
+ * {@code inferenceOptions} is {@code null}, if {@code inferenceOptions} contains invalid
+ * split options, if {@code maxLength} is less than {@code 2}, if the model has no output of
+ * shape {@code [batch, hidden]} or {@code [batch, tokens, hidden]}, or if {@code padding}
+ * pads and the vocabulary has no padding token.
+ * @throws OrtException Thrown if the {@code model} cannot be loaded, or if a GPU was requested
+ * and ONNX Runtime cannot provide it. ONNX Runtime does not fall back to the CPU.
+ * @throws IOException Thrown if errors occurred loading the {@code model} or {@code vocabulary}.
+ *
+ * @since 3.0.0
+ */
+ public SentenceVectorsDL(final File model, final File vocabulary, final boolean lowerCase,
Review Comment:
Renamed the constant to `DEFAULT_CPU_PADDING`.
I know what you mean regarding the the constructor count. I didn't change anything yet here.
A small vectors-specific options object seems cleaner than adding more overloads. I'd keep padding there and leave `InferenceOptions` for session configuration.
WDYT?
--
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: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-1392: Add padding strategies for SentenceVectorsDL.embedAll and pad on a GPU by default (opennlp)
Posted by "krickert (via GitHub) via commits" <co...@opennlp.apache.org>.
krickert commented on code in PR #1474:
URL: https://github.com/apache/opennlp/pull/1474#discussion_r4442317106
##########
opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/vectors/SentenceVectorsOptions.java:
##########
@@ -0,0 +1,60 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.dl.vectors;
+
+import opennlp.tools.util.ParamChecks;
+
+/**
+ * Pooling, normalization, truncation and padding settings for {@link SentenceVectorsDL}.
+ *
+ * @param pooling How token vectors are pooled. Not used for a model with a
+ * {@code sentence_embedding} output. Must not be {@code null}.
+ * @param normalize {@code true} to scale every vector to unit length.
+ * @param maxLength The maximum number of tokens per input, including the classification and
+ * separator tokens. Longer input is truncated. Must be at least {@code 2}.
+ * @param padding The padding strategy, or {@code null} for the device and model default:
+ * {@link SentenceVectorsDL#DEFAULT_GPU_PADDING} on a GPU for a token output if the
+ * vocabulary has a padding token, {@link SentenceVectorsDL#DEFAULT_CPU_PADDING} otherwise.
+ * An explicit padded strategy requires a padding token in the vocabulary.
+ *
+ * @since 3.0.0
+ */
+public record SentenceVectorsOptions(Pooling pooling, boolean normalize, int maxLength,
+ PaddingStrategy padding) {
+
+ /**
+ * Creates vector settings after validating the pooling mode and maximum length.
+ *
+ * @throws IllegalArgumentException Thrown if {@code pooling} is {@code null} or
+ * {@code maxLength} is less than {@code 2}.
+ */
+ public SentenceVectorsOptions {
+ ParamChecks.requireNonNullArg(pooling, "pooling");
+ if (maxLength < 2) {
Review Comment:
Restored `MIN_LENGTH` in `SentenceVectorsOptions`, where the validation now lives. The check and error message both use it.
--
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: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-1392: Add padding strategies for SentenceVectorsDL.embedAll and pad on a GPU by default (opennlp)
Posted by "mawiesne (via GitHub) via commits" <co...@opennlp.apache.org>.
mawiesne merged PR #1474:
URL: https://github.com/apache/opennlp/pull/1474
--
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: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-1392: Add padding strategies for SentenceVectorsDL.embedAll and pad on a GPU by default (opennlp)
Posted by "krickert (via GitHub) via commits" <co...@opennlp.apache.org>.
krickert commented on code in PR #1474:
URL: https://github.com/apache/opennlp/pull/1474#discussion_r4442256849
##########
opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/vectors/SentenceVectorsDL.java:
##########
@@ -442,10 +555,29 @@ private float[][] run(final Tokens[] batch) throws OrtException {
}
/**
- * Pools the token vectors of one input into its sentence vector.
+ * {@return the row width of one inference} It is the maximum length under
+ * {@link PaddingStrategy#MAX_LENGTH}, otherwise the longest row of the batch.
+ *
+ * @param batch The encodings of one inference, at least one.
+ */
+ private int width(final Tokens[] batch) {
+ if (padding == PaddingStrategy.MAX_LENGTH) {
Review Comment:
Added this to the constructor and `MAX_LENGTH` Javadoc: when the output dimension needs probing, that inference also uses `maxLength` and can fail if it exceeds the model's position limit.
--
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: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-1392: Add padding strategies for SentenceVectorsDL.embedAll and pad on a GPU by default (opennlp)
Posted by "krickert (via GitHub) via commits" <co...@opennlp.apache.org>.
krickert commented on code in PR #1474:
URL: https://github.com/apache/opennlp/pull/1474#discussion_r4442305469
##########
opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/vectors/SentenceVectorsOptions.java:
##########
@@ -0,0 +1,60 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.dl.vectors;
+
+import opennlp.tools.util.ParamChecks;
+
+/**
+ * Pooling, normalization, truncation and padding settings for {@link SentenceVectorsDL}.
+ *
+ * @param pooling How token vectors are pooled. Not used for a model with a
+ * {@code sentence_embedding} output. Must not be {@code null}.
+ * @param normalize {@code true} to scale every vector to unit length.
+ * @param maxLength The maximum number of tokens per input, including the classification and
+ * separator tokens. Longer input is truncated. Must be at least {@code 2}.
+ * @param padding The padding strategy, or {@code null} for the device and model default:
Review Comment:
Added `PaddingStrategy.AUTO` and made it the default. It resolves to the device and model strategy during construction. Null padding is now rejected, and the provider and manual use `AUTO` explicitly.
--
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: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-1392: Add padding strategies for SentenceVectorsDL.embedAll and pad on a GPU by default (opennlp)
Posted by "krickert (via GitHub) via commits" <co...@opennlp.apache.org>.
krickert commented on code in PR #1474:
URL: https://github.com/apache/opennlp/pull/1474#discussion_r4442324402
##########
opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/vectors/OnnxTextEmbedderProvider.java:
##########
@@ -159,6 +170,30 @@ private Pooling poolingOption(final ProviderSpec spec) {
throw new IllegalArgumentException(POOLING_OPTION + " must be " + MEAN + " or " + CLS);
}
+ /**
+ * Reads the {@value #PADDING_OPTION} option.
+ *
+ * @param spec The spec to read.
+ * @return The padding strategy, {@link PaddingStrategy#EXACT_LENGTH} if the option is not set.
+ * @throws IllegalArgumentException Thrown if the value is not the lower case name of a
+ * {@link PaddingStrategy}.
+ */
+ private PaddingStrategy paddingOption(final ProviderSpec spec) {
+ final String value = spec.option(PADDING_OPTION, null);
+ if (value == null) {
+ return SentenceVectorsDL.DEFAULT_CPU_PADDING;
+ }
+ final List<String> names = new ArrayList<>();
+ for (final PaddingStrategy padding : PaddingStrategy.values()) {
+ final String name = StringUtil.toLowerCase(padding.name());
+ if (name.equals(value)) {
Review Comment:
Done. The value is normalized with `StringUtil.toLowerCase` before matching. Added parameterized tests for lowercase, uppercase and mixed-case values, including `AUTO`.
--
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: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-1392: Add padding strategies for SentenceVectorsDL.embedAll and pad on a GPU by default (opennlp)
Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on code in PR #1474:
URL: https://github.com/apache/opennlp/pull/1474#discussion_r4371978745
##########
opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/AbstractDL.java:
##########
@@ -418,13 +422,41 @@ static WordpieceEncoder createWordpieceEncoder(
}
/**
- * The classification, separator and unknown tokens a tokenizer adds or emits.
+ * {@return the vocabulary id of the padding token} The padding token is {@code <pad>} for a
+ * RoBERTa vocabulary and {@code [PAD]} otherwise.
+ *
+ * @param vocab The vocabulary map.
+ * @throws IllegalArgumentException Thrown if the vocabulary has no padding token, or if it has
+ * the RoBERTa classification and separator tokens but no unknown token.
+ */
+ protected static long padTokenId(final Map<String, Integer> vocab) {
Review Comment:
`padTokenId` and `hasPadToken` would become protected API in 3.0.0. Please mark them `@Internal` like the other new helpers, and read the `vocab` field instead of making them static. `hasPadToken` also lacks the `@throws`.
##########
opennlp-core/opennlp-ml/opennlp-dl/src/test/java/opennlp/dl/vectors/SentenceVectorsDLEmbedderTest.java:
##########
@@ -240,6 +269,177 @@ void testEmbedAllMatchesSingleEmbedsExactly(@TempDir final Path dir) throws Exce
}
}
+ /**
+ * {@return the number of rows of each batch}
+ *
+ * @param batches The batches.
+ */
+ private static List<Integer> sizes(final List<List<Integer>> batches) {
Review Comment:
`sizes(...)` exists three times (also `TinyVectorModels` and `SentenceVectorsDLBatchLimitTest`). The literals `"tiny-pooled.onnx"`/`"tiny-masked.onnx"` duplicate `TINY_POOLED`/`TINY_MASKED`, and the two loops over strategies could be a `@ParameterizedTest`.
##########
opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/vectors/SentenceVectorsDL.java:
##########
@@ -244,6 +331,17 @@ public SentenceVectorsDL(final File model, final File vocabulary, final boolean
}
+ /**
+ * {@return the padding of a session built without a padding strategy: {@link #DEFAULT_PADDING}
+ * on the CPU, {@link #DEFAULT_GPU_PADDING} on a GPU if the vocabulary has a padding token}
+ *
+ * @param gpu {@code true} if the session runs on a GPU.
+ * @param vocab The vocabulary of the session.
+ */
+ static PaddingStrategy devicePadding(final boolean gpu, final Map<String, Integer> vocab) {
Review Comment:
On a GPU, `LONGEST` is also chosen for models with a pooled output, where we cannot tell whether the mask is honoured (`testPooledOutputModelSumsThePadding` shows the difference). I would keep `EXACT_LENGTH` for those, or at least say it in the manual.
##########
opennlp-docs/src/docbkx/machine-learning.xml:
##########
@@ -176,10 +176,17 @@ try (TextEmbedder embedder = providers.select(spec).create(spec)) {
<code>lowerCase=false</code>, <code>pooling=mean</code> (default) or
<code>pooling=cls</code>, <code>normalize=true</code> (default) or
<code>normalize=false</code>, and <code>maxLength</code>, the maximum number of tokens
- per input (512 by default); longer input is truncated. The defaults match
- sentence-transformers models such as all-MiniLM-L6-v2. A model with a
- <code>sentence_embedding</code> output is used as it is, without pooling. A spec with
- another option is not supported. The key
+ per input (512 by default); longer input is truncated. The option <code>padding</code>
+ sets how <code>embedAll</code> batches texts of different lengths:
+ <code>exact_length</code> (default) does not pad and runs one inference per tokenized
+ length, <code>longest</code> pads each batch to its longest text, and
+ <code>max_length</code> pads every text to <code>maxLength</code>. The vectors match up
+ to floating-point rounding either way, provided the model honors the attention mask; a
Review Comment:
"either way" reads a bit informal ("under every strategy"). Please also mention that `max_length` pads single `embed` calls as well.
##########
opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/vectors/SentenceVectorsDL.java:
##########
@@ -216,6 +233,74 @@ public SentenceVectorsDL(final File model, final File vocabulary, final boolean
final InferenceOptions inferenceOptions)
throws OrtException, IOException {
+ this(model, vocabulary, lowerCase, pooling, normalize, maxLength, inferenceOptions, null);
+
+ }
+
+ /**
+ * Instantiates a {@link SentenceVectorsDL sentence vector generator} using ONNX models, with
+ * the {@link PaddingStrategy} that {@link #embedAll(List)} batches by and the
+ * {@link InferenceOptions} the session is built from.
+ *
+ * @param model The file name of a sentence vectors ONNX model.
+ * @param vocabulary The file name of the vocabulary file for the model.
+ * @param lowerCase {@code true} for uncased models (lower casing and accent
+ * stripping during tokenization), {@code false} for cased models. A lower case setting in
+ * {@code inferenceOptions} takes precedence.
+ * @param pooling How token vectors are pooled. Not used for a model with a
+ * {@code sentence_embedding} output. Must not be {@code null}.
+ * @param normalize {@code true} to scale every vector to unit length.
+ * @param maxLength The maximum number of tokens per input, {@code [CLS]} and {@code [SEP]}
+ * included; longer input is truncated. Must be at least {@code 2}.
+ * @param padding How {@link #embedAll(List)} batches texts of different lengths. Must not be
+ * {@code null}. Every strategy but {@link PaddingStrategy#EXACT_LENGTH} needs a
+ * {@code [PAD]} token in the vocabulary, or {@code <pad>} for a RoBERTa vocabulary.
+ * @param inferenceOptions The {@link InferenceOptions}, of which the GPU settings and the lower
+ * case setting are used. Must not be {@code null}.
+ *
+ * @throws IllegalArgumentException Thrown if {@code pooling}, {@code padding} or
+ * {@code inferenceOptions} is {@code null}, if {@code inferenceOptions} contains invalid
+ * split options, if {@code maxLength} is less than {@code 2}, if the model has no output of
+ * shape {@code [batch, hidden]} or {@code [batch, tokens, hidden]}, or if {@code padding}
+ * pads and the vocabulary has no padding token.
+ * @throws OrtException Thrown if the {@code model} cannot be loaded, or if a GPU was requested
+ * and ONNX Runtime cannot provide it. ONNX Runtime does not fall back to the CPU.
+ * @throws IOException Thrown if errors occurred loading the {@code model} or {@code vocabulary}.
+ *
+ * @since 3.0.0
+ */
+ public SentenceVectorsDL(final File model, final File vocabulary, final boolean lowerCase,
Review Comment:
Five public constructors plus a private one with swapped parameters feels like too much. Do we need the new one, or can the strategy go into a small options object for the vectors? `DEFAULT_PADDING` is the CPU default, so maybe `DEFAULT_CPU_PADDING`.
##########
opennlp-core/opennlp-ml/opennlp-dl/src/test/java/opennlp/dl/vectors/SentenceVectorsDLPaddingEdgesTest.java:
##########
@@ -0,0 +1,327 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package opennlp.dl.vectors;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Stream;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.EnumSource;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import opennlp.dl.InferenceOptions;
+import opennlp.dl.Tokens;
+
+import static opennlp.dl.vectors.TinyVectorModels.DELTA;
+import static opennlp.dl.vectors.TinyVectorModels.TINY_MASKED;
+import static opennlp.dl.vectors.TinyVectorModels.TINY_POOLED;
+import static opennlp.dl.vectors.TinyVectorModels.TINY_VECTORS;
+import static opennlp.dl.vectors.TinyVectorModels.UNIT_VECTOR;
+import static opennlp.dl.vectors.TinyVectorModels.assertSameEmbeddings;
+import static opennlp.dl.vectors.TinyVectorModels.gpuOrAbort;
+import static opennlp.dl.vectors.TinyVectorModels.model;
+import static opennlp.dl.vectors.TinyVectorModels.scale;
+import static opennlp.dl.vectors.TinyVectorModels.sizes;
+import static opennlp.dl.vectors.TinyVectorModels.vocab;
+import static opennlp.dl.vectors.TinyVectorModels.vocabPad6;
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+
+/**
+ * Edge cases of the padding strategies of {@link SentenceVectorsDL#embedAll(List)}: the limit of
+ * token positions per inference under padding, other vocabularies, pooling and normalization of
+ * padded rows, a model that ignores the mask, and a GPU.
+ *
+ * <p>With the test vocabulary "hello" encodes to 3 tokens and "hello world" to 4.</p>
+ */
+class SentenceVectorsDLPaddingEdgesTest {
+
+ private static final int LIMIT = SentenceVectorsDL.MAX_BATCH_TOKEN_POSITIONS;
+
+ private static File write(final Path dir, final String name, final List<String> tokens)
+ throws IOException {
+ final Path file = dir.resolve(name);
+ Files.write(file, tokens);
+ return file.toFile();
+ }
+
+ private static SentenceVectorsDL embedder(final File model, final File vocabulary,
+ final Pooling pooling, final boolean normalize, final int maxLength,
+ final PaddingStrategy padding) throws Exception {
+ return new SentenceVectorsDL(model, vocabulary, true, pooling, normalize, maxLength, padding,
+ new InferenceOptions());
+ }
+
+ private static Tokens[] tokens(final int length, final int count) {
+ return Collections.nCopies(count, TinyVectorModels.tokens(length)).toArray(new Tokens[0]);
+ }
+
+ // The plan tests do not run a text, so one embedder per strategy is enough.
+ private static SentenceVectorsDL planner(final Path dir, final PaddingStrategy padding,
+ final int maxLength) throws Exception {
+ return embedder(model(dir, TINY_VECTORS), vocab(dir), Pooling.MEAN, false, maxLength,
+ padding);
+ }
+
+ /**
+ * LONGEST fills an inference to exactly the limit, splits at one row more, and runs a row wider
+ * than the limit on its own.
+ */
+ @Test
+ void testLongestAtTheLimitAndOverIt(@TempDir final Path dir) throws Exception {
+ try (SentenceVectorsDL longest = planner(dir, PaddingStrategy.LONGEST, 8)) {
+ assertEquals(List.of(LIMIT / 4), sizes(longest.batches(tokens(4, LIMIT / 4))));
+ assertEquals(List.of(LIMIT / 4, 1), sizes(longest.batches(tokens(4, LIMIT / 4 + 1))));
+ assertEquals(List.of(LIMIT / 2, 1), sizes(longest.batches(tokens(2, LIMIT / 2 + 1))));
+ assertEquals(List.of(List.of(0)), longest.batches(tokens(LIMIT + 1, 1)));
+ final Tokens[] mixed = {tokens(LIMIT + 1, 1)[0], tokens(3, 1)[0], tokens(LIMIT + 1, 1)[0]};
+ assertEquals(List.of(List.of(1), List.of(0), List.of(2)), longest.batches(mixed));
+ }
+ }
+
+ /** MAX_LENGTH with a length above the limit runs one row per inference. */
+ @Test
+ void testMaxLengthAboveTheLimitRunsOneRowEach(@TempDir final Path dir) throws Exception {
+ try (SentenceVectorsDL fixed = planner(dir, PaddingStrategy.MAX_LENGTH, LIMIT + 5)) {
+ assertEquals(List.of(1, 1, 1), sizes(fixed.batches(tokens(3, 3))));
+ }
+ }
+
+ /** MAX_LENGTH with lengths that divide the limit evenly, or equal it. */
+ @ParameterizedTest
+ @MethodSource("maxLengthPlans")
+ void testMaxLengthThatDividesTheLimit(final int maxLength, final int count,
+ final List<Integer> expected, @TempDir final Path dir) throws Exception {
+ try (SentenceVectorsDL fixed = planner(dir, PaddingStrategy.MAX_LENGTH, maxLength)) {
+ assertEquals(expected, sizes(fixed.batches(tokens(3, count))));
+ }
+ }
+
+ static Stream<Arguments> maxLengthPlans() {
+ return Stream.of(
+ Arguments.of(8, 2048, List.of(2048)),
+ Arguments.of(8, 2049, List.of(2048, 1)),
+ Arguments.of(8, 4097, List.of(2048, 2048, 1)),
+ Arguments.of(LIMIT, 2, List.of(1, 1)),
+ Arguments.of(LIMIT / 2, 3, List.of(2, 1)));
+ }
+
+ /**
+ * An input as long as a MAX_LENGTH wider than the limit still embeds, in call order, next to
+ * short inputs.
+ */
+ @Test
+ void testMaxLengthAboveTheLimitEmbeds(@TempDir final Path dir) throws Exception {
+ try (SentenceVectorsDL fixed = embedder(model(dir, TINY_MASKED), vocabPad6(dir),
+ Pooling.MEAN, false, LIMIT + 5, PaddingStrategy.MAX_LENGTH)) {
+ final float[][] batch = fixed.embedAll(List.of("hello world", "hello", "world"));
+ assertArrayEquals(scale(19), batch[0], DELTA);
+ assertArrayEquals(scale(14), batch[1], DELTA);
+ assertArrayEquals(scale(15), batch[2], DELTA);
+ }
+ }
+
+ /** Alternating lengths with duplicates come back in input order after LONGEST sorts them. */
+ @Test
+ void testInterleavedLengthsKeepInputOrder(@TempDir final Path dir) throws Exception {
+ try (SentenceVectorsDL longest = embedder(model(dir, TINY_MASKED), vocabPad6(dir),
+ Pooling.MEAN, false, 8, PaddingStrategy.LONGEST)) {
+ final List<String> texts = new ArrayList<>();
+ for (int i = 0; i < LIMIT / 4 + 1; i++) {
+ texts.add("hello world");
+ texts.add("hello");
+ texts.add("hello world");
+ texts.add("world");
+ }
+ final float[][] batch = longest.embedAll(texts);
+ assertEquals(texts.size(), batch.length);
+ for (int i = 0; i < batch.length; i++) {
+ final String text = texts.get(i);
+ final float sum = "hello world".equals(text) ? 19 : "hello".equals(text) ? 14 : 15;
+ assertArrayEquals(scale(sum), batch[i], DELTA, "row " + i);
+ }
+ }
+ }
+
+ /**
+ * A RoBERTa vocabulary with {@code <pad>} pads with that token: {@code tiny-masked.onnx} sums
+ * the ids times the mask, so padded rows give the sum of their own ids.
+ */
+ @ParameterizedTest
+ @EnumSource(value = PaddingStrategy.class, names = {"LONGEST", "MAX_LENGTH"})
+ void testRobertaVocabularyPads(final PaddingStrategy padding, @TempDir final Path dir)
+ throws Exception {
+ // <s>=0 <pad>=1 </s>=2 <unk>=3 hello=4 world=5
+ final File roberta = write(dir, "roberta.txt",
+ List.of("<s>", "<pad>", "</s>", "<unk>", "hello", "world"));
+ try (SentenceVectorsDL vectors = embedder(model(dir, TINY_MASKED), roberta,
+ Pooling.MEAN, false, 8, padding)) {
+ final float[][] batch = vectors.embedAll(List.of("hello world", "hello"));
+ assertArrayEquals(scale(11), batch[0], DELTA);
+ assertArrayEquals(scale(6), batch[1], DELTA);
+ }
+ }
+
+ /** A padding id of 0, the usual case, gives the same masked sums as the id 6 fixture. */
+ @ParameterizedTest
+ @EnumSource(value = PaddingStrategy.class, names = {"LONGEST", "MAX_LENGTH"})
+ void testPaddingIdZero(final PaddingStrategy padding, @TempDir final Path dir)
+ throws Exception {
+ try (SentenceVectorsDL vectors = embedder(model(dir, TINY_MASKED), vocab(dir),
+ Pooling.MEAN, false, 8, padding)) {
+ final float[][] batch = vectors.embedAll(List.of("hello world", "hello", "world", ""));
+ assertArrayEquals(scale(19), batch[0], DELTA);
+ assertArrayEquals(scale(14), batch[1], DELTA);
+ assertArrayEquals(scale(15), batch[2], DELTA);
+ assertArrayEquals(scale(10), batch[3], DELTA);
+ }
+ }
+
+ static Stream<Arguments> poolingCases() {
+ final List<Arguments> cases = new ArrayList<>();
+ for (final PaddingStrategy padding : List.of(PaddingStrategy.LONGEST,
+ PaddingStrategy.MAX_LENGTH)) {
+ cases.add(Arguments.of(padding, Pooling.CLS, false, scale(7), scale(7)));
+ cases.add(Arguments.of(padding, Pooling.CLS, true, UNIT_VECTOR, UNIT_VECTOR));
+ cases.add(Arguments.of(padding, Pooling.MEAN, true, UNIT_VECTOR, UNIT_VECTOR));
+ }
+ return cases.stream();
+ }
+
+ /**
+ * CLS pooling reads the first position, which padding never moves, and unit length scaling
+ * applies per row, for padded rows too.
+ */
+ @ParameterizedTest
+ @MethodSource("poolingCases")
+ void testClsAndNormalizeUnderPadding(final PaddingStrategy padding, final Pooling pooling,
+ final boolean normalize, final float[] helloWorld, final float[] hello,
+ @TempDir final Path dir) throws Exception {
+ try (SentenceVectorsDL vectors = embedder(model(dir, TINY_VECTORS), vocabPad6(dir),
+ pooling, normalize, 8, padding)) {
+ final float[][] batch = vectors.embedAll(List.of("hello world", "hello", "hello world"));
+ assertArrayEquals(helloWorld, batch[0], DELTA);
+ assertArrayEquals(hello, batch[1], DELTA);
+ assertArrayEquals(helloWorld, batch[2], DELTA);
+ }
+ }
+
+ /**
+ * The pooled-output model ignores the mask: a padded row also sums the padding id, so a batch
+ * differs from the single-input result under LONGEST, and every call pads to the maximum length
+ * under MAX_LENGTH, single calls included. This pins the documented caveat.
+ */
+ @Test
+ void testPooledOutputModelSumsThePadding(@TempDir final Path dir) throws Exception {
+ final File pooled = model(dir, TINY_POOLED);
+ final File vocabulary = vocabPad6(dir);
+ try (SentenceVectorsDL longest = embedder(pooled, vocabulary, Pooling.MEAN, false, 8,
+ PaddingStrategy.LONGEST);
+ SentenceVectorsDL fixed = embedder(pooled, vocabulary, Pooling.MEAN, false, 8,
+ PaddingStrategy.MAX_LENGTH)) {
+ final float[][] batch = longest.embedAll(List.of("hello world", "hello"));
+ assertArrayEquals(scale(19), batch[0], DELTA);
+ assertArrayEquals(scale(14 + 6), batch[1], DELTA);
+ assertFalse(Arrays.equals(longest.embed("hello"), batch[1]));
+ assertArrayEquals(scale(14), longest.embed("hello"), DELTA);
+ assertArrayEquals(scale(14 + 5 * 6), fixed.embed("hello"), DELTA);
+ assertArrayEquals(scale(19 + 4 * 6), fixed.embed("hello world"), DELTA);
+ }
+ }
+
+ /**
+ * A single {@code embed} under MAX_LENGTH pads to the maximum length: a model that ignores the
+ * mask shows the padding ids, a model that reads it gives the unpadded sum.
+ */
+ @Test
+ void testSingleEmbedUnderMaxLengthIsPadded(@TempDir final Path dir) throws Exception {
+ try (SentenceVectorsDL ignoring = embedder(model(dir, TINY_POOLED), vocabPad6(dir),
+ Pooling.MEAN, false, 6, PaddingStrategy.MAX_LENGTH);
+ SentenceVectorsDL masked = embedder(model(dir, TINY_MASKED), vocabPad6(dir),
+ Pooling.MEAN, false, 6, PaddingStrategy.MAX_LENGTH)) {
+ assertArrayEquals(scale(14 + 3 * 6), ignoring.embed("hello"), DELTA);
+ assertArrayEquals(scale(14), masked.embed("hello"), DELTA);
+ assertEquals(3, masked.dimension());
+ }
+ }
+
+ static Stream<Arguments> boundaryPlans() {
+ final List<Arguments> cases = new ArrayList<>();
+ for (final PaddingStrategy padding : PaddingStrategy.values()) {
+ cases.add(Arguments.of(padding, 4, LIMIT / 4, List.of(LIMIT / 4)));
+ cases.add(Arguments.of(padding, 4, LIMIT / 4 + 1, List.of(LIMIT / 4, 1)));
+ cases.add(Arguments.of(padding, 2, LIMIT / 2, List.of(LIMIT / 2)));
+ cases.add(Arguments.of(padding, 2, 0, List.of()));
+ }
+ return cases.stream();
+ }
+
+ /**
+ * The row count boundaries of GH-1391 hold under every strategy, with the maximum length set
Review Comment:
Please describe the invariant instead of referencing GH-1391.
--
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: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-1392: Add padding strategies for SentenceVectorsDL.embedAll and pad on a GPU by default (opennlp)
Posted by "krickert (via GitHub)" <gi...@apache.org>.
krickert commented on code in PR #1474:
URL: https://github.com/apache/opennlp/pull/1474#discussion_r4385602706
##########
opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/vectors/SentenceVectorsDL.java:
##########
@@ -216,6 +233,74 @@ public SentenceVectorsDL(final File model, final File vocabulary, final boolean
final InferenceOptions inferenceOptions)
throws OrtException, IOException {
+ this(model, vocabulary, lowerCase, pooling, normalize, maxLength, inferenceOptions, null);
+
+ }
+
+ /**
+ * Instantiates a {@link SentenceVectorsDL sentence vector generator} using ONNX models, with
+ * the {@link PaddingStrategy} that {@link #embedAll(List)} batches by and the
+ * {@link InferenceOptions} the session is built from.
+ *
+ * @param model The file name of a sentence vectors ONNX model.
+ * @param vocabulary The file name of the vocabulary file for the model.
+ * @param lowerCase {@code true} for uncased models (lower casing and accent
+ * stripping during tokenization), {@code false} for cased models. A lower case setting in
+ * {@code inferenceOptions} takes precedence.
+ * @param pooling How token vectors are pooled. Not used for a model with a
+ * {@code sentence_embedding} output. Must not be {@code null}.
+ * @param normalize {@code true} to scale every vector to unit length.
+ * @param maxLength The maximum number of tokens per input, {@code [CLS]} and {@code [SEP]}
+ * included; longer input is truncated. Must be at least {@code 2}.
+ * @param padding How {@link #embedAll(List)} batches texts of different lengths. Must not be
+ * {@code null}. Every strategy but {@link PaddingStrategy#EXACT_LENGTH} needs a
+ * {@code [PAD]} token in the vocabulary, or {@code <pad>} for a RoBERTa vocabulary.
+ * @param inferenceOptions The {@link InferenceOptions}, of which the GPU settings and the lower
+ * case setting are used. Must not be {@code null}.
+ *
+ * @throws IllegalArgumentException Thrown if {@code pooling}, {@code padding} or
+ * {@code inferenceOptions} is {@code null}, if {@code inferenceOptions} contains invalid
+ * split options, if {@code maxLength} is less than {@code 2}, if the model has no output of
+ * shape {@code [batch, hidden]} or {@code [batch, tokens, hidden]}, or if {@code padding}
+ * pads and the vocabulary has no padding token.
+ * @throws OrtException Thrown if the {@code model} cannot be loaded, or if a GPU was requested
+ * and ONNX Runtime cannot provide it. ONNX Runtime does not fall back to the CPU.
+ * @throws IOException Thrown if errors occurred loading the {@code model} or {@code vocabulary}.
+ *
+ * @since 3.0.0
+ */
+ public SentenceVectorsDL(final File model, final File vocabulary, final boolean lowerCase,
Review Comment:
Done. Added `SentenceVectorsOptions` for pooling, normalization, maximum length and padding, replacing the eight-argument overload. The existing constructors delegate to it. Removed the private constructor with swapped parameters. `InferenceOptions` remains separate for session settings.
Also renamed the constant to `DEFAULT_CPU_PADDING` and updated the manual example.
--
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: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org