rzo1 commented on code in PR #1512:
URL: https://github.com/apache/opennlp/pull/1512#discussion_r4440809443
##########
opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/AbstractDL.java:
##########
@@ -231,12 +237,16 @@ protected static OrtSession.SessionOptions
sessionOptions(final InferenceOptions
if (inferenceOptions.isGpu()) {
ParamChecks.requireNonNegative(inferenceOptions.getGpuDeviceId(),
"gpuDeviceId");
}
- final OrtSession.SessionOptions sessionOptions = new
OrtSession.SessionOptions();
+ final OrtSession.SessionOptions sessionOptions =
+ initializeNative(OrtSession.SessionOptions::new);
if (inferenceOptions.isGpu()) {
try {
// addCUDA needs ONNX Runtime's default logger, which exists once an
environment does.
OrtEnvironment.getEnvironment();
sessionOptions.addCUDA(inferenceOptions.getGpuDeviceId());
+ } catch (UnsatisfiedLinkError e) {
Review Comment:
If this one fires, the core runtime loaded but the CUDA provider didn't.
"Could not load ONNX Runtime native library" points to the wrong fix. Mention
CUDA/cuDNN here instead.
##########
opennlp-core/opennlp-ml/opennlp-dl/src/main/java/opennlp/dl/AbstractDL.java:
##########
@@ -245,6 +255,32 @@ protected static OrtSession.SessionOptions
sessionOptions(final InferenceOptions
return sessionOptions;
}
+ private static <T> T initializeNative(final Supplier<T> nativeOperation) {
+ return initializeNative(nativeOperation,
+ System.getProperty("os.name"), System.getProperty("os.arch"));
+ }
+
+ static <T> T initializeNative(final Supplier<T> nativeOperation, final
String osName,
+ final String architecture) {
+ try {
+ return nativeOperation.get();
+ } catch (UnsatisfiedLinkError e) {
Review Comment:
The native lib is loaded in a static initializer, so you only get the
UnsatisfiedLinkError on the first try. After that it's NoClassDefFoundError
"Could not initialize class ai.onnxruntime...", and that isn't wrapped. Can you
cover that too?
##########
opennlp-docs/src/docbkx/machine-learning.xml:
##########
@@ -188,15 +188,62 @@ try (TextEmbedder embedder =
providers.select(spec).create(spec)) {
<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 NVIDIA GPUs on
- linux-x64 and win-x64 only. On macOS, use
<code>opennlp-dl</code>. 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, so an embedder created through it runs on the
CPU. If CUDA or the configured
- device is not available, the constructor throws an
<code>OrtException</code>; ONNX Runtime
- does not fall back to the CPU. A negative device id is
rejected with an
- <code>IllegalArgumentException</code>. A lower case
setting in the options takes
+ <code>SentenceVectorsDL</code>. The modules use ONNX
Runtime 1.31.0, whose Java artifacts
Review Comment:
#1515 bumps this to 1.31.1. Drop the patch version, or this goes stale on
every bump.
##########
opennlp-core/opennlp-ml/opennlp-dl/src/test/java/opennlp/dl/AbstractDLNativeLibraryErrorTest.java:
##########
@@ -0,0 +1,43 @@
+/*
+ * 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 org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class AbstractDLNativeLibraryErrorTest {
+
+ @Test
+ void reportsPlatformAndCpuAlternativeWhenNativeLibraryCannotLoad() {
+ final UnsatisfiedLinkError original = new UnsatisfiedLinkError("synthetic
loader failure");
+
+ final UnsatisfiedLinkError wrapped =
assertThrows(UnsatisfiedLinkError.class,
+ () -> AbstractDL.initializeNative(() -> {
+ throw original;
+ }, "macOS", "aarch64"));
+
+ assertTrue(wrapped.getMessage().contains("macOS/aarch64"));
+ assertTrue(wrapped.getMessage().contains("opennlp-dl-gpu"));
+ assertTrue(wrapped.getMessage().contains("opennlp-dl"));
Review Comment:
Already covered by the "opennlp-dl-gpu" check. Just assertEquals the full
message.
--
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]
