uncomplicate / neanderthal

Fast Clojure Matrix Library
http://neanderthal.uncomplicate.org
Eclipse Public License 1.0
1.06k stars 56 forks source link

Memory leak? #62

Closed tuh8888 closed 5 years ago

tuh8888 commented 5 years ago

When using this package to perform computations over large numbers of matrices, the JVM keeps crashing due to running out of memory. I see that you recommend using the uncomplicate.commons.core/with-release method to release memory. But that doesn't seem to fix the problem. I've been using the native-double factory. The code I've been using is below:

(ns linear-algebra
  (:require [uncomplicate.commons.core :as uncomplicate]
            [uncomplicate.neanderthal.core :as thal]
            [uncomplicate.neanderthal.real :as thal-real]))

(defn unit-vec
  [factory v]
  (uncomplicate/let-release [v (thal/vctr factory v)
                             alpha-1 (thal/nrm2 v)
                             alpha (/ alpha-1)
                             result (thal/scal alpha v)]
    (seq result)))

(defn cosine-sim
  [factory v1 v2]
  (uncomplicate/let-release [v1 (unit-vec factory v1)
                             v2 (unit-vec factory v2)
                             result (thal/dot v1 v2)]
    result))

(defn unit-vec-sum
  [factory & vectors]
  (let [vectors (keep seq vectors)]
    (if (seq vectors)
      (->> vectors
           (reduce (fn [v1 v2]
                     (if v1
                       (uncomplicate/let-release [v1 (thal/vctr factory v1)
                                                  v2 (thal/vctr factory v2)
                                                  sum-v (thal/xpy v1 v2)]
                         (seq sum-v))
                       v2))
                   nil)
           (unit-vec factory)))))

(defn vectors->matrix
  [{:keys [factory]} vectors]
  (let [d (some #(count %) vectors)]
    (->> vectors
         (mapcat seq)
         (thal/ge factory d (count vectors)))))

(defn mdot
  [{:keys [vector-fn] :as params} s1 s2]
  (let [v1 (map vector-fn s1)
        v2 (map vector-fn s2)]
    (uncomplicate/let-release [s1-mat (vectors->matrix params v1)
                               s1-mat-trans (thal/trans s1-mat)
                               s2-mat (vectors->matrix params v2)]
      #_(log/info #_(thal/mrows s1) #_(thal/ncols s1) #_(thal/mrows s2) #_(thal/ncols s2))
      (thal/mm s1-mat-trans s2-mat))))

(defn best-in-row-from-matrix
  [score-mat init i s]
  (->> s
       (map-indexed vector)
       (reduce
         (fn [{:keys [score] :as best} [j s]]
           (uncomplicate/let-release [new-score (thal-real/entry score-mat i j)]
             (if (< score new-score)
               {:match s :score (float new-score)}
               best)))
         init)))

(defn find-best-match
  [params s1 s2]
  (uncomplicate/let-release [score-mat (mdot params s1 s2)]
    (when score-mat
      (->> s1
           (map-indexed vector)
           (reduce
             (fn [best [i sample]]
               (-> (best-in-row-from-matrix score-mat best i s2)
                   (assoc :sample sample)))
             {:score 0})))))

(defn find-best-row-matches
  [params s1 s2]
  (uncomplicate/let-release [score-mat (mdot params s1 s2)]
    (when score-mat
      (->> s1
           (map-indexed (fn [i sample]
                          (-> score-mat
                              (best-in-row-from-matrix {:score 0} i s2)
                              (assoc :sample sample))))
           (doall)))))
blueberry commented 5 years ago

Hi tuh,

As I can see, you didn't call with-release anywhere in your code. Thus, the immediate cause of your memory leaks is obvious: you are creating many heavy objects, and you are not releasing them anywhere.

You seem to use let-release which only releases objects if there is an exception in its scope.

However, there is another issue with your code: You are creating heavy objects inside loops (units-vec-sum for example), calling a simple operation on them, and then immediately releasing them (assuming that you fix the let-release/with-release confusion). That would not give you much of neanderthal's acceleration... Another issue that I see is that you are using vectors, and then looping over them. That is an indication that you should use matrix operations instead (if possible), but is a question of how you approach your algorithm implementation in any library, and not an issue specific to neanderthal.

Sorry that I can't debug a 50 line program, these are just some things that were obvious at a glance.

tuh8888 commented 5 years ago

I'm including this here as it may be related. When including the result in the with-release clause, the result comes out wrong. Specifically, the first entry is usually 0 or a very small number.

From the first example from "Dense Triangular Matrices", get the correct result.

(uncomplicate/with-release [a (uncomplicate.neanderthal.native/dtr 3 (range 1 7) {:layout :row :diag :unit :uplo :lower})
                            b (uncomplicate.neanderthal.native/dge 3 4 (range 1 13))]
              (thal/mm a b))
=> #RealGEMatrix[double, mxn:3x4, layout:column, offset:0]
   ▥       ↓       ↓       ↓       ↓       ┓    
   →       1.00    4.00    7.00   10.00         
   →       2.00    5.00    8.00   11.00         
   →       3.00    6.00    9.00   12.00         
   ┗                                       ┛   

But, when applying seq to the result and releasing the multiplied result:

(uncomplicate/with-release [a (uncomplicate.neanderthal.native/dtr 3 (range 1 7) {:layout :row :diag :unit :uplo :lower})
                            b (uncomplicate.neanderthal.native/dge 3 4 (range 1 13))
                            result (thal/mm a b)]
              (seq result))
=> ((0.0 2.0 3.0) (4.0 5.0 6.0) (7.0 8.0 9.0) (10.0 11.0 12.0))

I tried wrapping it a bit more and the first two entries were wrong:

(let [result (uncomplicate/with-release [a (uncomplicate.neanderthal.native/dtr 3 (range 1 7) {:layout :row :diag :unit :uplo :lower})
                                         b (uncomplicate.neanderthal.native/dge 3 4 (range 1 13))]
               (thal/mm a b))
      actual-result (seq result)]
  (when (uncomplicate/release result)
    actual-result))
=> ((6.91308495732296E-310 6.9130843064705E-310 3.0) (4.0 5.0 6.0) (7.0 8.0 9.0) (10.0 11.0 12.0))
blueberry commented 5 years ago

This is because seq is lazy, and its elements are realized when REPL tries to print it out, after the result has been released. Use eager seq (doall).

(EDIT: this works as intended)

BTW. You'll get the same garbage printed out even without seq if you try it with larger matrices, since REPL will try to print out a matrix that has already been released. With smaller matrices, you'll sometimes still see the old numbers since the memory is plenty and everything happens so quickly that the memory is still not physically destroyed...

tuh8888 commented 5 years ago

@blueberry Thanks for your explanations. I was using with-release before, and was having memory issues, so I switched to let-release but misunderstood what it did. Thank you for your quick replies