From 7253ec94394c0ff20023704caf756acd5c8faf16 Mon Sep 17 00:00:00 2001 From: Jakub Holy Date: Sun, 22 Sep 2024 01:37:48 +0200 Subject: [PATCH] Improve failed evaluation detection, reporting Issue: When we ask Wolfram to do something impossible, it typically returns the input form or $Failed, so it is hard to see what was wrong. However, when evaluated in a notebook, it does print an explanation. This is however ignored by the recommended `.waitForAnswer`. Fix: Follow the docs and add a packet listener, which captures the messages we care about in these cases (text, message) between we start eval and read the result. To make this easier, move the impl. of evaluation into a new protocol method `evaluate!` + add extracting captured messages from the listener + throw an error if the result seems to indicate a problem. Bonus: Upgrade Clojure to stable. --- README.md | 1 + deps.edn | 5 +- src/wolframite/base/evaluate.clj | 45 +++++------ src/wolframite/impl/jlink_proto_impl.clj | 98 ++++++++++++++++++++++-- src/wolframite/impl/protocols.clj | 3 + src/wolframite/tools/graphics.clj | 3 + 6 files changed, 124 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index 060eda8..e6149df 100644 --- a/README.md +++ b/README.md @@ -50,6 +50,7 @@ However, sometimes Wolframite may fail to find the correct path automatically an ```shell export WOLFRAM_INSTALL_PATH=/opt/mathematica/13.1 +export WOLFRAM_INSTALL_PATH="/Applications/Wolfram Engine.app/Contents/Resources/Wolfram Player.app/Contents" ``` ### Getting started diff --git a/deps.edn b/deps.edn index b69472f..2129b15 100644 --- a/deps.edn +++ b/deps.edn @@ -1,5 +1,5 @@ {:paths ["src" "resources"] - :deps {org.clojure/clojure {:mvn/version "1.12.0-beta1"} + :deps {org.clojure/clojure {:mvn/version "1.12.0"} org.clojure/tools.logging {:mvn/version "1.3.0"} org.scicloj/kindly {:mvn/version "4-beta5"} @@ -27,6 +27,9 @@ :main-opts ["--main" "cognitect.test-runner"] :exec-fn cognitect.test-runner.api/test} + :jlink-jar ; useful for IntelliJ sometimes + {:deps {wolfram/jlink {:local/root "./symlink-jlink.jar"}}} + :build ;; added by neil; 1) build with `clojure -T:build jar` then deploy with ;; `env CLOJARS_USERNAME= CLOJARS_PASSWORD= clojure -T:build deploy` {:deps {io.github.clojure/tools.build {:git/tag "v0.10.4" :git/sha "31388ff"} diff --git a/src/wolframite/base/evaluate.clj b/src/wolframite/base/evaluate.clj index 2cb03f0..e267331 100644 --- a/src/wolframite/base/evaluate.clj +++ b/src/wolframite/base/evaluate.clj @@ -34,28 +34,23 @@ {:pre [jlink-instance]} (assert (proto/expr? jlink-instance expr)) (assert (proto/kernel-link? jlink-instance)) - (let [link (proto/kernel-link jlink-instance)] - (if (options/flag?' (:flags opts) :serial) - (io! - (locking link - (doto link (.evaluate expr) (.waitForAnswer)) - ; When eval failed b/c it needs internet but offline, still (.error link) = 0, (.errorMessage link) = "No ... problem..." - (.getExpr link))) - (let [opts' (update opts :flags conj :serial) ;; FIXME: make sure this is supposed to be `:serial`, it's what I gather from previous version of the code - pid-expr (evaluate (convert/convert - (list 'Unique - ; Beware: technically, this is an invalid clj symbol due to the slashes: - (symbol "Wolframite/Concurrent/process")) opts') - opts)] - ;; FIXME: debug log: "pid-expr:" - (evaluate (convert/convert (list '= pid-expr (list 'ParallelSubmit expr)) opts') opts) - (evaluate (convert/convert '(QueueRun) opts') opts) - (loop [] - (let [[state result] (process-state pid-expr opts)] - (if (not= :finished state) - (do - (queue-run-or-wait opts) - (recur)) - (do - (evaluate (convert/convert (list 'Remove pid-expr) opts') opts) - result)))))))) + (if (options/flag?' (:flags opts) :serial) + (proto/evaluate! jlink-instance expr) + (let [opts' (update opts :flags conj :serial) ;; FIXME: make sure this is supposed to be `:serial`, it's what I gather from previous version of the code + pid-expr (evaluate (convert/convert + (list 'Unique + ; Beware: technically, this is an invalid clj symbol due to the slashes: + (symbol "Wolframite/Concurrent/process")) opts') + opts)] + ;; FIXME: debug log: "pid-expr:" + (evaluate (convert/convert (list '= pid-expr (list 'ParallelSubmit expr)) opts') opts) + (evaluate (convert/convert '(QueueRun) opts') opts) + (loop [] + (let [[state result] (process-state pid-expr opts)] + (if (not= :finished state) + (do + (queue-run-or-wait opts) + (recur)) + (do + (evaluate (convert/convert (list 'Remove pid-expr) opts') opts) + result))))))) diff --git a/src/wolframite/impl/jlink_proto_impl.clj b/src/wolframite/impl/jlink_proto_impl.clj index d0f334e..06fde5d 100644 --- a/src/wolframite/impl/jlink_proto_impl.clj +++ b/src/wolframite/impl/jlink_proto_impl.clj @@ -1,10 +1,11 @@ (ns wolframite.impl.jlink-proto-impl "The 'real' implementation of JLink, which does depend on JLink classes and thus cannot be loaded/required until JLink is on the classpath." - (:require [clojure.string :as str] + (:require [clojure.tools.logging :as log] [wolframite.impl.protocols :as proto]) (:import (clojure.lang BigInt) - [com.wolfram.jlink Expr KernelLink MathCanvas MathLinkException MathLinkFactory])) + [com.wolfram.jlink Expr KernelLink MathCanvas MathLink MathLinkException MathLinkFactory + PacketListener PacketArrivedEvent PacketPrinter])) (defn- array? [x] (some-> x class .isArray)) @@ -53,7 +54,89 @@ (type primitive-or-exprs)) :cause e}))))) -(defrecord JLinkImpl [opts kernel-link-atom] +(defrecord InfoPacketCaptureListener [capture] + ;; A packet listener that enables us to get hold of the normally ignored Print outputs + ;; and warning messages sent before a return packet. + PacketListener + (packetArrived [_this #_PacketArrivedEvent event] + (let [link (cast KernelLink (.getSource event))] + (some->> + (condp = (.getPktType event) ; note: `case` doesn't work 🤷 + MathLink/TEXTPKT + {:type :text :content (.getString link)} + + MathLink/MESSAGEPKT + (let [expr (.getExpr link)] + (when-not (.symbolQ expr) + ;; not sure why these are sent, not useful; e.g. Get when a Get call failed etc. + {:type :message :content expr})) + + nil) + (swap! capture conj))) + true)) + +(comment + (let [link (proto/kernel-link ((requiring-resolve 'wolframite.impl.jlink-instance/get)))] + ;(.removePacketListener link packet-listener) + (.addPacketListener link packet-listener) + ,) + ,) + +(defn install-packet-logger! + "Call this to help debug your program - it will print all incoming JLink packets (the units + of communication between JVM and Wolfram) to stdout. + + Ex.: + ```clj + (install-packet-logger! (proto/kernel-link (jlink-instance/get))) + ```" + [^KernelLink link] + (.addPacketListener link (PacketPrinter. System/out))) + +;; Wolfram sometimes indicates failure by returning the symbol $Failed +(defonce failed-expr (Expr. Expr/SYMBOL "$Failed")) + +(defn- evaluate! [^KernelLink link packet-capture-atom ^Expr expr] + (assert link "Kernel link not initialized?!") + (io! + (locking link + (reset! packet-capture-atom nil) + ;; NOTE: There is also evaluateToImage => byte[] of GIF for graphics-returning fns such as Plot + ;; NOTE 2: .waitForAnswer discard packets until ReturnPacket; our packet-listener collects those + (doto link (.evaluate expr) (.waitForAnswer)) + (let [res (.getExpr link) + messages (seq (first (reset-vals! packet-capture-atom nil))) + messages-text (mapv :content messages)] + (def M messages) + (cond + (and (seq messages) + (or (= res failed-expr) + (= res expr))) + ;; If input expr == output expr, this usually means the evaluation failed + ;; (or there was nothing to do); if there are also any extra text/message packets + ;; then it most likely has failed, and those messages explain what was wrong + (throw (ex-info (str "Evaluation seems to have failed. Result: " + res + " Details: " + (cond-> messages-text + (= 1 (count messages-text)) + first)) + {:expr expr + :messages messages + :result res})) + + (= res failed-expr) ; but no messages + (throw (ex-info (str "Evaluation has failed. Result: " + res + " No details available.") + {:expr expr :result res})) + + :else + (do (when (seq messages) + (log/info "Messages retrieved during evaluation:" messages-text)) + res)))))) + +(defrecord JLinkImpl [opts kernel-link-atom packet-listener] proto/JLink (create-kernel-link [_this kernel-link-opts] (loop [attempts 3, wait-ms 10, orig-err nil] @@ -63,6 +146,8 @@ (try (let [opts-array (into-array String kernel-link-opts) kernel-link (->> (doto (MathLinkFactory/createKernelLink ^"[Ljava.lang.String;" opts-array) + (.addPacketListener packet-listener) ; TBD doesn't get anything when link fails due to e.g. # kernels > license + ;; Note: The call below ensures we actually try to connect to the kernel (.discardAnswer)) (reset! kernel-link-atom))] ;(.getError kernel-link) (.getErrorMessage kernel-link) @@ -95,13 +180,15 @@ (.terminateKernel) (.close))) (reset! kernel-link-atom nil)) + (evaluate! [_this expr] + (evaluate! @kernel-link-atom (:capture packet-listener) expr)) (expr [_this primitive-or-exprs] (make-expr primitive-or-exprs)) (expr [_this type name] (Expr. ^int (case type :Expr/SYMBOL Expr/SYMBOL) ^String (apply str (replace {\/ \`} name)))) - (->expr [_this obj] + (->expr [_this obj] ; fallback for transforming anything we don't handle manually, via JLink itself (.getExpr (doto (MathLinkFactory/createLoopbackLink) (.put obj) @@ -153,4 +240,5 @@ (defn create [kernel-link-atom opts] (map->JLinkImpl {:opts opts - :kernel-link-atom kernel-link-atom})) + :kernel-link-atom kernel-link-atom + :packet-listener (->InfoPacketCaptureListener (atom nil))})) diff --git a/src/wolframite/impl/protocols.clj b/src/wolframite/impl/protocols.clj index ded297f..e6bb94f 100644 --- a/src/wolframite/impl/protocols.clj +++ b/src/wolframite/impl/protocols.clj @@ -10,6 +10,7 @@ from the existing code." (create-kernel-link [this kernel-link-opts]) (terminate-kernel! [this]) + (evaluate! [this expr] "Evaluate the given JLink Expr in the kernel") (expr [this] [this primitive-or-exprs] @@ -36,6 +37,8 @@ (throw (IllegalStateException. "JLink not loaded!"))) (terminate-kernel! [this] (throw (IllegalStateException. "JLink not loaded!"))) + (evaluate! [this expr] + (throw (IllegalStateException. "JLink not loaded!"))) (expr [this expr-coll] (throw (IllegalStateException. "JLink not loaded!"))) (expr [this type expr-coll] diff --git a/src/wolframite/tools/graphics.clj b/src/wolframite/tools/graphics.clj index 48b8335..75df755 100644 --- a/src/wolframite/tools/graphics.clj +++ b/src/wolframite/tools/graphics.clj @@ -1,5 +1,8 @@ (ns wolframite.tools.graphics "Displaying WL graphics with java.awt" + ;; Wolfram has You use MathCanvas when you want an AWT component and MathGraphicsJPanel when you + ;; want a Swing component - see https://reference.wolfram.com/language/JLink/tutorial/CallingJavaFromTheWolframLanguage.html#20608 + ;; Notice that KernelLink also has evaluateToImage() and evaluateToTypeset() methods (:require [wolframite.impl.jlink-instance :as jlink-instance] [wolframite.impl.protocols :as proto] [wolframite.core :as wl])