Skip to content

Commit

Permalink
Improve failed evaluation detection, reporting
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
holyjak committed Sep 22, 2024
1 parent 5fbe077 commit 5c0f226
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 31 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion deps.edn
Original file line number Diff line number Diff line change
@@ -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"}
Expand Down Expand Up @@ -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=<tbd> CLOJARS_PASSWORD=<clojars-token> clojure -T:build deploy`
{:deps {io.github.clojure/tools.build {:git/tag "v0.10.4" :git/sha "31388ff"}
Expand Down
45 changes: 20 additions & 25 deletions src/wolframite/base/evaluate.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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)))))))
90 changes: 85 additions & 5 deletions src/wolframite/impl/jlink_proto_impl.clj
Original file line number Diff line number Diff line change
@@ -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))
Expand Down Expand Up @@ -53,7 +54,81 @@
(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 {:type :message :content (.toString (.getExpr link))}
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)]
(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]
Expand All @@ -63,6 +138,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)
Expand Down Expand Up @@ -95,13 +172,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)
Expand Down Expand Up @@ -153,4 +232,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))}))
3 changes: 3 additions & 0 deletions src/wolframite/impl/protocols.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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]
Expand Down

0 comments on commit 5c0f226

Please sign in to comment.