Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

[Clojure] Add resource scope to clojure package #13993

Merged
merged 12 commits into from
Feb 5, 2019
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@
[org.apache.clojure-mxnet.kvstore :as kvstore]
[org.apache.clojure-mxnet.kvstore-server :as kvstore-server]
[org.apache.clojure-mxnet.optimizer :as optimizer]
[org.apache.clojure-mxnet.eval-metric :as eval-metric])
[org.apache.clojure-mxnet.eval-metric :as eval-metric]
[org.apache.clojure-mxnet.resource-scope :as resource-scope])
(:gen-class))

(def data-dir "data/") ;; the data directory to store the mnist data
Expand All @@ -51,28 +52,6 @@
(when-not (.exists (io/file (str data-dir "train-images-idx3-ubyte")))
(sh "../../scripts/get_mnist_data.sh"))

;;; Load the MNIST datasets
(defonce train-data (mx-io/mnist-iter {:image (str data-dir "train-images-idx3-ubyte")
:label (str data-dir "train-labels-idx1-ubyte")
:label-name "softmax_label"
:input-shape [784]
:batch-size batch-size
:shuffle true
:flat true
:silent false
:seed 10
:num-parts num-workers
:part-index 0}))

(defonce test-data (mx-io/mnist-iter {:image (str data-dir "t10k-images-idx3-ubyte")
:label (str data-dir "t10k-labels-idx1-ubyte")
:input-shape [784]
:batch-size batch-size
:flat true
:silent false
:num-parts num-workers
:part-index 0}))

(defn get-symbol []
(as-> (sym/variable "data") data
(sym/fully-connected "fc1" {:data data :num-hidden 128})
Expand All @@ -82,7 +61,7 @@
(sym/fully-connected "fc3" {:data data :num-hidden 10})
(sym/softmax-output "softmax" {:data data})))

(defn start
(defn start
([devs] (start devs num-epoch))
([devs _num-epoch]
(when scheduler-host
Expand All @@ -96,18 +75,33 @@
(do
(println "Starting Training of MNIST ....")
(println "Running with context devices of" devs)
(let [_mod (m/module (get-symbol) {:contexts devs})]
(m/fit _mod {:train-data train-data
:eval-data test-data
(resource-scope/with-let [_mod (m/module (get-symbol) {:contexts devs})]
(-> _mod
(m/fit {:train-data (mx-io/mnist-iter {:image (str data-dir "train-images-idx3-ubyte")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just for my understanding, does resource-scope not work when the train-data and eval-data are defined outside the with-let block?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will work if they are defs outside, but you will still get the undisposed ndarray warning. I refactored to move it out to functions which does work and looks better too :)

:label (str data-dir "train-labels-idx1-ubyte")
:label-name "softmax_label"
:input-shape [784]
:batch-size batch-size
:shuffle true
:flat true
:silent false
:seed 10
:num-parts num-workers
:part-index 0})
:eval-data (mx-io/mnist-iter {:image (str data-dir "t10k-images-idx3-ubyte")
:label (str data-dir "t10k-labels-idx1-ubyte")
:input-shape [784]
:batch-size batch-size
:flat true
:silent false
:num-parts num-workers
:part-index 0})
:num-epoch _num-epoch
:fit-params (m/fit-params {:kvstore kvstore
:optimizer optimizer
:eval-metric eval-metric})})
(println "Finish fit")
_mod
)

))))
(m/save-checkpoint {:prefix "target/test" :epoch _num-epoch}))
(println "Finish fit"))))))

(defn -main [& args]
(let [[dev dev-num] args
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
;;

(ns imclassification.train-mnist-test
(:require
(:require
[clojure.test :refer :all]
[clojure.java.io :as io]
[clojure.string :as s]
Expand All @@ -26,14 +26,15 @@

(defn- file-to-filtered-seq [file]
(->>
file
file
(io/file)
(io/reader)
(line-seq)
(filter #(not (s/includes? % "mxnet_version")))))

(deftest mnist-two-epochs-test
(module/save-checkpoint (mnist/start [(context/cpu)] 2) {:prefix "target/test" :epoch 2})
(is (=
(file-to-filtered-seq "test/test-symbol.json.ref")
(file-to-filtered-seq "target/test-symbol.json"))))
(do
(mnist/start [(context/cpu)] 2)
(is (=
(file-to-filtered-seq "test/test-symbol.json.ref")
(file-to-filtered-seq "target/test-symbol.json")))))
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
;;
;; 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.
;;

(ns org.apache.clojure-mxnet.resource-scope
(:require [org.apache.clojure-mxnet.util :as util])
(:import (org.apache.mxnet ResourceScope)))

(defmacro
using
"Uses a Resource Scope for all forms. This is a way to manage all Native Resources like NDArray and Symbol - it will deallocate all Native Resources by calling close on them automatically. It will not call close on Native Resources returned from the form.
Example:
(resource-scope/using
(let [temp-x (ndarray/ones [3 1])
temp-y (ndarray/ones [3 1])]
(ndarray/+ temp-x temp-y))) "
[& forms]
`(ResourceScope/using (new ResourceScope) (util/forms->scala-fn ~@forms)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this may be a stupid question but is there any value in reusing a previously defined ResourceScope object? why does the scala API allow a scope to be passed in?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good question. From looking at the Scala api I would say that it would enable finer grained control - you could manually add or remove native resources from a scope and then manually call close on it. But I don't think that we need to expose that on the Clojure side, unless you have a different view on it.

Copy link
Contributor

@kedarbellare kedarbellare Jan 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't see a big need for it -- at least for now -- but i was wondering whether the motivation was something like tf.variable_scope (e.g. reusing symbols).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know - maybe @nswamy knows more about the context behind the design? ^



(defmacro
with-do
"Alias for a do within a resource scope using.
Example:
(resource-scope/with-do
(ndarray/ones [3 1])
:all-cleaned-up)
"
[& forms]
`(using (do ~@forms)))

(defmacro
with-let
"Alias for a let within a resource scope using.
Example:
(resource-scope/with-let [temp-x (ndarray/ones [3 1])
temp-y (ndarray/ones [3 1])]
(ndarray/+ temp-x temp-y))"
[& forms]
`(using (let ~@forms)))
Original file line number Diff line number Diff line change
Expand Up @@ -239,3 +239,9 @@
(apply $/immutable-list))
;; pass-through
map-or-tuple-seq)))

(defmacro forms->scala-fn
"Creates a scala fn of zero args from forms"
[& forms]
`($/fn []
(do ~@forms)))
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
;;
;; 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.
;;

(ns org.apache.clojure-mxnet.resource-scope-test
(:require [org.apache.clojure-mxnet.ndarray :as ndarray]
[org.apache.clojure-mxnet.symbol :as sym]
[org.apache.clojure-mxnet.resource-scope :as resource-scope]
[clojure.test :refer :all]))


(deftest test-resource-scope-with-ndarray
(let [native-resources (atom {})
x (ndarray/ones [2 2])
return-val (resource-scope/using
gigasquid marked this conversation as resolved.
Show resolved Hide resolved
(let [temp-x (ndarray/ones [3 1])
temp-y (ndarray/ones [3 1])]
(swap! native-resources assoc :temp-x temp-x)
(swap! native-resources assoc :temp-y temp-y)
(ndarray/+ temp-x 1)))]
(is (true? (ndarray/is-disposed (:temp-x @native-resources))))
(is (true? (ndarray/is-disposed (:temp-y @native-resources))))
(is (false? (ndarray/is-disposed return-val)))
(is (false? (ndarray/is-disposed x)))
(is (= [2.0 2.0 2.0] (ndarray/->vec return-val)))))

(deftest test-nested-resource-scope-with-ndarray
(let [native-resources (atom {})
x (ndarray/ones [2 2])
return-val (resource-scope/using
(let [temp-x (ndarray/ones [3 1])]
(swap! native-resources assoc :temp-x temp-x)
(resource-scope/using
(let [temp-y (ndarray/ones [3 1])]
(swap! native-resources assoc :temp-y temp-y)))))]
(is (true? (ndarray/is-disposed (:temp-y @native-resources))))
(is (true? (ndarray/is-disposed (:temp-x @native-resources))))
(is (false? (ndarray/is-disposed x)))))

(deftest test-resource-scope-with-sym
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you also add a unit test for nested with sym?

Copy link
Contributor

@kedarbellare kedarbellare Jan 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one more unit test idea (similar in spirit to scala api): many ndarray/ones within a vector and only the first one is returned after a transformation (e.g. (+ (first v) 1))

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting the result here - I put test cases in, but if a whole vector has been assigned to a let binding and you return the first, the whole vector is not disposed. You have to call copy on the first or not assign the whole vector to a let to have it disposed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some edge cases still I think there - but on the whole, it's an improvement for mem management of native resources.

(let [native-resources (atom {})
x (sym/ones [2 2])
return-val (resource-scope/using
(let [temp-x (sym/ones [3 1])
temp-y (sym/ones [3 1])]
(swap! native-resources assoc :temp-x temp-x)
(swap! native-resources assoc :temp-y temp-y)
(sym/+ temp-x 1)))]
(is (true? (sym/is-disposed (:temp-x @native-resources))))
(is (true? (sym/is-disposed (:temp-y @native-resources))))
(is (false? (sym/is-disposed return-val)))
(is (false? (sym/is-disposed x)))))

(deftest test-nested-resource-scope-with-ndarray
(let [native-resources (atom {})
x (ndarray/ones [2 2])
return-val (resource-scope/using
(let [temp-x (ndarray/ones [3 1])]
(swap! native-resources assoc :temp-x temp-x)
(resource-scope/using
(let [temp-y (ndarray/ones [3 1])]
(swap! native-resources assoc :temp-y temp-y)))))]
(is (true? (ndarray/is-disposed (:temp-y @native-resources))))
(is (true? (ndarray/is-disposed (:temp-x @native-resources))))
(is (false? (ndarray/is-disposed x)))))

(deftest test-nested-resource-scope-with-sym
(let [native-resources (atom {})
x (sym/ones [2 2])
return-val (resource-scope/using
(let [temp-x (sym/ones [3 1])]
(swap! native-resources assoc :temp-x temp-x)
(resource-scope/using
(let [temp-y (sym/ones [3 1])]
(swap! native-resources assoc :temp-y temp-y)))))]
(is (true? (sym/is-disposed (:temp-y @native-resources))))
(is (true? (sym/is-disposed (:temp-x @native-resources))))
(is (false? (sym/is-disposed x)))))

;;; Note that if first is returned the rest of the collection ndarrays will
;;; NOT be disposed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update comment?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yes - the curse of the leftover misleading comments :)

(deftest test-list-creation-with-returning-first
(let [native-resources (atom {})
return-val (resource-scope/using
(let [temp-ndarrays (mapv (constantly (ndarray/ones [3 1])) (range 5))
_ (swap! native-resources assoc :temp-ndarrays temp-ndarrays)]
(first temp-ndarrays)))]
(is (false? (ndarray/is-disposed return-val)))
(is (every? false? (mapv ndarray/is-disposed (:temp-ndarrays @native-resources))))))

;;; To have the rest of the collection disposed you need to call copy on the result
(deftest test-list-creation-with-return-a-copy-of-first
(let [native-resources (atom {})
return-val (resource-scope/using
(let [temp-ndarrays (repeatedly 3 #(ndarray/ones [3 1]))
_ (swap! native-resources assoc :temp-ndarrays temp-ndarrays)]
(ndarray/copy (first temp-ndarrays))))]
(is (false? (ndarray/is-disposed return-val)))
(is (every? true? (mapv ndarray/is-disposed (:temp-ndarrays @native-resources))))))

;;; If you don't assign them all to a let vector binding and just return the first
;; then they are disposed as usual
(deftest test-list-creation-with-return-a-copy-of-first
(let [native-resources (atom [])
return-val (resource-scope/using
(first (repeatedly 5 #(do
(let [x (ndarray/ones [3 1])]
(swap! native-resources conj x)
x)))))]
(is (false? (ndarray/is-disposed return-val)))
(is (every? true? (mapv ndarray/is-disposed (:temp-ndarrays @native-resources))))))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@native-resources is a vector. just check (mapv ndarray/is-disposed @native-resources)? (also i didn't know that (:keyword vector) didn't raise an error)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If fixed some other problems with laziness in the tests with repeatedly. Evidently the weird behavior in the dispose of the first of the collection was just my fault on the tests. Now everything works as expected 🎆


(deftest test-with-let
(let [native-resources (atom {})
x (ndarray/ones [2 2])
return-val (resource-scope/with-let [temp-x (ndarray/ones [3 1])
temp-y (ndarray/ones [3 1])]
(swap! native-resources assoc :temp-x temp-x)
(swap! native-resources assoc :temp-y temp-y)
(ndarray/+ temp-x 1))]
(is (true? (ndarray/is-disposed (:temp-x @native-resources))))
(is (true? (ndarray/is-disposed (:temp-y @native-resources))))
(is (false? (ndarray/is-disposed return-val)))
(is (false? (ndarray/is-disposed x)))
(is (= [2.0 2.0 2.0] (ndarray/->vec return-val)))))

(deftest test-with-do
(let [native-resources (atom {})
x (ndarray/ones [2 2])
return-val (resource-scope/with-do
(swap! native-resources assoc :temp-x (ndarray/ones [3 1]))
(swap! native-resources assoc :temp-y (ndarray/ones [3 1]))
(ndarray/ones [3 1]))]
(is (true? (ndarray/is-disposed (:temp-x @native-resources))))
(is (true? (ndarray/is-disposed (:temp-y @native-resources))))
(is (false? (ndarray/is-disposed return-val)))
(is (false? (ndarray/is-disposed x)))
(is (= [1.0 1.0 1.0] (ndarray/->vec return-val)))))
Original file line number Diff line number Diff line change
Expand Up @@ -226,3 +226,10 @@
(let [nda (util/map->scala-tuple-seq {:a-b (ndarray/ones [1 2])})]
(is (= "a_b" (._1 (.head nda))))
(is (= [1.0 1.0] (ndarray/->vec (._2 (.head nda)))))))

(deftest test-forms->scala-fn
(let [scala-fn (util/forms->scala-fn
(def x 1)
(def y 2)
{:x x :y y})]
(is (= {:x 1 :y 2} (.apply scala-fn)))))