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,31 @@
(sym/fully-connected "fc3" {:data data :num-hidden 10})
(sym/softmax-output "softmax" {:data data})))

(defn start

(defn 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}))

(defn 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}))

(defn start
([devs] (start devs num-epoch))
([devs _num-epoch]
(when scheduler-host
Expand All @@ -96,18 +99,16 @@
(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 (train-data)
:eval-data (eval-data)
: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,149 @@
;;
;; 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 (doall (repeatedly 3 #(ndarray/ones [3 1])))
_ (reset! native-resources temp-ndarrays)]
(first temp-ndarrays)))]
(is (false? (ndarray/is-disposed return-val)))
(is (= [false true true] (mapv ndarray/is-disposed @native-resources)))))

;;; collections not referenced are disposed
(deftest test-list-creation
(let [native-resources (atom [])
return-val (resource-scope/using
(let [temp-ndarrays (doall (repeatedly 3 #(ndarray/ones [3 1])))
_ (reset! native-resources temp-ndarrays)]
(ndarray/ones [3 1])))]
(is (false? (ndarray/is-disposed return-val)))
(is (= [true true true] (mapv ndarray/is-disposed @native-resources)))))

(deftest test-list-creation-without-let
(let [native-resources (atom [])
return-val (resource-scope/using
(first (doall (repeatedly 3 #(do
(let [x (ndarray/ones [3 1])]
(swap! native-resources conj x)
x))))))]
(is (false? (ndarray/is-disposed return-val)))
(is (= [false true true] (mapv ndarray/is-disposed @native-resources)))))

(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)))))