krickert commented on code in PR #1462:
URL: https://github.com/apache/opennlp/pull/1462#discussion_r4266407275
##########
opennlp-core/opennlp-ml/opennlp-dl-gpu/pom.xml:
##########
@@ -102,5 +109,32 @@
</plugins>
</build>
+ <!-- onnxruntime_gpu 1.30.0 ships native libraries for linux-x64 and win-x64
only, unlike the
Review Comment:
Changed to 1.31.0, dropped version from pom comment since it's redundant.
The manual just links to the requirements page instead of naming the versions.
##########
opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/AbstractDL.java:
##########
@@ -216,17 +217,44 @@ protected AbstractDL(final OrtEnvironment env, final
OrtSession session,
* @param inferenceOptions The inference options to read the GPU
configuration from.
* @return The configured session options.
*
- * @throws OrtException Thrown if the CUDA execution provider cannot be
added.
+ * @throws OrtException Thrown if the CUDA execution provider cannot be
added: the ONNX Runtime
+ * on the classpath has no CUDA support, its CUDA libraries cannot be
loaded, or no device
+ * answers to the configured device id. ONNX Runtime does not fall back
to the CPU. The
+ * session options created for the request are closed before the
exception is thrown.
* @throws IllegalArgumentException Thrown if {@code inferenceOptions} is
{@code null} or its
* split sizes are invalid, see {@link #validateSplitOptions(int, int)}.
*/
protected static OrtSession.SessionOptions sessionOptions(final
InferenceOptions inferenceOptions)
throws OrtException {
+ return sessionOptions(inferenceOptions, OrtSession.SessionOptions::new);
+ }
+
+ /**
+ * Builds ONNX session options as {@link #sessionOptions(InferenceOptions)}
does, from options
+ * that {@code factory} creates.
+ *
+ * @param inferenceOptions The inference options to read the GPU
configuration from.
+ * @param factory Creates the session options to configure.
+ * @return The configured session options.
+ *
+ * @throws OrtException Thrown if the CUDA execution provider cannot be
added.
+ * @throws IllegalArgumentException Thrown if {@code inferenceOptions} is
{@code null} or its
+ * split sizes are invalid.
+ */
+ static OrtSession.SessionOptions sessionOptions(final InferenceOptions
inferenceOptions,
Review Comment:
Inlined it into sessionOptions() and removed the test. Can't test the close
on failure without the seam since SessionOptions doesn't expose a closed state.
##########
opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/AbstractDL.java:
##########
@@ -216,17 +217,44 @@ protected AbstractDL(final OrtEnvironment env, final
OrtSession session,
* @param inferenceOptions The inference options to read the GPU
configuration from.
* @return The configured session options.
*
- * @throws OrtException Thrown if the CUDA execution provider cannot be
added.
+ * @throws OrtException Thrown if the CUDA execution provider cannot be
added: the ONNX Runtime
+ * on the classpath has no CUDA support, its CUDA libraries cannot be
loaded, or no device
+ * answers to the configured device id. ONNX Runtime does not fall back
to the CPU. The
+ * session options created for the request are closed before the
exception is thrown.
* @throws IllegalArgumentException Thrown if {@code inferenceOptions} is
{@code null} or its
* split sizes are invalid, see {@link #validateSplitOptions(int, int)}.
*/
protected static OrtSession.SessionOptions sessionOptions(final
InferenceOptions inferenceOptions)
throws OrtException {
+ return sessionOptions(inferenceOptions, OrtSession.SessionOptions::new);
+ }
+
+ /**
+ * Builds ONNX session options as {@link #sessionOptions(InferenceOptions)}
does, from options
+ * that {@code factory} creates.
+ *
+ * @param inferenceOptions The inference options to read the GPU
configuration from.
+ * @param factory Creates the session options to configure.
+ * @return The configured session options.
+ *
+ * @throws OrtException Thrown if the CUDA execution provider cannot be
added.
+ * @throws IllegalArgumentException Thrown if {@code inferenceOptions} is
{@code null} or its
+ * split sizes are invalid.
+ */
+ static OrtSession.SessionOptions sessionOptions(final InferenceOptions
inferenceOptions,
+ final Supplier<OrtSession.SessionOptions> factory) throws OrtException {
ParamChecks.requireNonNullArg(inferenceOptions, "inferenceOptions");
validateSplitOptions(inferenceOptions);
- final OrtSession.SessionOptions sessionOptions = new
OrtSession.SessionOptions();
+ final OrtSession.SessionOptions sessionOptions = factory.get();
if (inferenceOptions.isGpu()) {
- sessionOptions.addCUDA(inferenceOptions.getGpuDeviceId());
+ try {
+ // addCUDA needs ONNX Runtime's default logger, which exists once an
environment does.
+ OrtEnvironment.getEnvironment();
+ sessionOptions.addCUDA(inferenceOptions.getGpuDeviceId());
Review Comment:
Now rejects negative device ids early with ParamChecks.requireNonNegative.
Updated the @throws and the manual, and added tests for -1 and
Integer.MIN_VALUE that check the message.
##########
opennlp-docs/src/docbkx/machine-learning.xml:
##########
@@ -182,5 +182,25 @@ try (TextEmbedder embedder =
providers.select(spec).create(spec)) {
until another one is installed. Existing
<code>SentenceVectorsDL</code> constructors remain
available.
</para>
+ <para>
+ To compute the vectors on a GPU, depend on
<code>opennlp-dl-gpu</code> in place of
+ <code>opennlp-dl</code> and pass
<code>InferenceOptions</code> to
+ <code>SentenceVectorsDL</code>.
<code>opennlp-dl-gpu</code> supports linux-x64 and
+ win-x64 only, and the machine needs the CUDA and cuDNN
versions that the ONNX Runtime
+ release requires, see the
+ <ulink
url="https://onnxruntime.ai/docs/execution-providers/CUDA-ExecutionProvider.html#requirements";>CUDA
+ execution provider requirements</ulink>. The
<code>onnx</code> provider spec has no device
+ option yet, so an embedder created through it runs on
the CPU. If CUDA or the configured device is not available, the
Review Comment:
Rewrapped and removed "yet".
##########
opennlp-core/opennlp-ml/opennlp-dl/src/test/java/opennlp/dl/GpuRequestTest.java:
##########
@@ -0,0 +1,145 @@
+/*
+ * 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;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Supplier;
+
+import ai.onnxruntime.OrtException;
+import ai.onnxruntime.OrtSession;
+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.CsvSource;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import opennlp.dl.doccat.DocumentCategorizerDL;
+import opennlp.dl.doccat.scoring.AverageClassificationScoringStrategy;
+import opennlp.dl.namefinder.NameFinderDL;
+import opennlp.tools.sentdetect.NewlineSentenceDetector;
+
+import static opennlp.dl.OnnxTestSupport.UNUSABLE_DEVICE_ID;
+import static opennlp.dl.OnnxTestSupport.copy;
+import static opennlp.dl.OnnxTestSupport.gpu;
+import static opennlp.dl.vectors.TinyVectorModels.TINY_VECTORS;
+import static opennlp.dl.vectors.TinyVectorModels.model;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assumptions.abort;
+
+/**
+ * Tests how a GPU request in {@link InferenceOptions} reaches ONNX Runtime: a
request for a
+ * device that is not usable is rejected with an {@link OrtException}, never
served by the CPU,
+ * and a CPU request does not ask for any device.
+ *
+ * <p>In {@code opennlp-dl}, whose ONNX Runtime has no CUDA support, every GPU
request is
+ * rejected, so the tests there check that a request never falls back to the
CPU. The rejection
+ * of the device id itself is checked when {@code opennlp-dl-gpu} runs these
tests.</p>
+ */
+class GpuRequestTest {
+
+ private static final String PLAIN_VOCAB = "/opennlp/dl/vocab-plain.txt";
+
+ /** The device id is not read while {@code gpu} is off, so no provider is
requested. */
+ @ParameterizedTest
+ @ValueSource(ints = {-1, 0, UNUSABLE_DEVICE_ID})
+ void testSessionOptionsWithoutGpuIgnoreTheDeviceId(final int deviceId) {
+ final InferenceOptions options = new InferenceOptions();
+ options.setGpuDeviceId(deviceId);
+ assertDoesNotThrow(() -> {
+ try (OrtSession.SessionOptions sessionOptions =
AbstractDL.sessionOptions(options)) {
+ assertNotNull(sessionOptions);
+ }
+ });
+ }
+
+ /** A name finder on a device id that no machine has fails instead of
running on the CPU. */
+ @Test
+ void testNameFinderRejectsAnUnusableDevice(@TempDir final Path dir) throws
IOException {
+ final File model = model(dir, TINY_VECTORS);
+ final File vocab = copy(dir, PLAIN_VOCAB, "vocab.txt");
Review Comment:
Added VOCAB_FILE next to PLAIN_VOCAB.
##########
opennlp-core/opennlp-ml/opennlp-dl/src/test/java/opennlp/dl/vectors/SentenceVectorsDLEmbedderTest.java:
##########
@@ -60,7 +63,6 @@
*/
class SentenceVectorsDLEmbedderTest {
Review Comment:
Fixed, here and in the stacked #1468.
--
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]
