[Clojure] package infer tweaks #13864
Conversation
- add tests for base classifier and with-ndarray as well
- add test for plain predict
|
@kedarbellare Can you please take a look 😸 |
...b/clojure-package/examples/infer/imageclassifier/test/infer/imageclassifier_example_test.clj
Outdated
Show resolved
Hide resolved
...b/clojure-package/examples/infer/imageclassifier/test/infer/imageclassifier_example_test.clj
Show resolved
Hide resolved
...rib/clojure-package/examples/infer/objectdetector/test/infer/objectdetector_example_test.clj
Show resolved
Hide resolved
| (map first predictions))))) | ||
| (is (string? class)) | ||
| (is (< 0.8 prob)) | ||
| (every? #(< 0 % 1) [x-min x-max y-min y-max]) |
|
|
||
| (s/def ::wrapped-predictor (s/keys :req-un [::predictor])) | ||
| (s/def ::wrapped-classifier (s/keys :req-un [::classifier])) | ||
| (s/def ::wrapped-image-classifier (s/keys :req-un [::image-classifier])) | ||
| (s/def ::wrapped-detector (s/keys :req-un [::object-detector])) | ||
|
|
||
| (defn- format-detection-predictions [predictions] |
There was a problem hiding this comment.
so much cleaner! thanks for refactoring 😃
| ([image input-shape-vec dtype] | ||
| (util/validate! ::image image "Invalid image") | ||
| (util/validate! (s/coll-of int?) input-shape-vec "Invalid shape vector") | ||
| (ImageClassifier/bufferedImageToPixels image (shape/->shape input-shape-vec) dtype/FLOAT32))) |
There was a problem hiding this comment.
use dtype here instead of dtype/FLOAT32?
There was a problem hiding this comment.
nice catch again - thanks :)
|
Thanks so much @kedarbellare for your review. Great feedback 😄 |
There was a problem hiding this comment.
another thing to check is the captcha example in which i used the infer package. there might be a slight tweak needed there (i.e. removing first): https://github.com/apache/incubator-mxnet/blob/master/contrib/clojure-package/examples/captcha/src/captcha/infer_ocr.clj#L53. unfortunately i couldn't add a unit test for the captcha inference part since it requires a model.
lgtm. please check the captcha inference before merging. and thanks for the changes! 👍
| (* (aget prob-and-bounds 3) width) | ||
| (* (aget prob-and-bounds 4) height)))) | ||
| (doseq [p predictions] | ||
| (println p)) |
There was a problem hiding this comment.
minor: one thing this does change is that the bounds (e.g. x-min, y-min) are not scaled by the width and height of the image. is that fine? maybe this should be added in the docstring of the detection method?
There was a problem hiding this comment.
oh right - that was a miss on my part - I'll add that back it (without the aget)
| @@ -54,23 +54,16 @@ | |||
| "Print image detector predictions for the given input file" | |||
| [predictions width height] | |||
There was a problem hiding this comment.
width and height aren't used anymore. remove?
There was a problem hiding this comment.
If I add the w and h back into the print method it should be good and resolve it
|
Oh good point about the captcha - will do 👍 |
| (->> (.predict (:predictor wrapped-predictor) | ||
| (util/vec->indexed-seq [(float-array (first inputs))])) | ||
| (util/coerce-return-recursive) | ||
| (first) |
There was a problem hiding this comment.
oh! i just remembered that this is changing semantics of predict API. instead of taking a vector of [image1 image2 ...] and returning [predictions1 predictions2 ...] this is just returning predictions for the first image. is that ok? a workaround may be to just take one input or input-array instead of a vector of those.
There was a problem hiding this comment.
good point. I'm thinking maybe that should be map float array on the inputs instead
I'll add a test for it to be sure. Thanks for catching that :)
| (* x-min width) | ||
| (* y-min height) | ||
| (* x-max width) | ||
| (* y-max height)))) |
|
@kedarbellare I removed the |
This reverts commit c2110ad.
* change object detection prediction to be a map * change predictions to a map for image-classifiers * change return types of the classifiers to be a map - add tests for base classifier and with-ndarray as well * tweak return types and inputs for predict - add test for plain predict * updated infer-classify examples * adjust the infer/object detections tests * tweak predictor test * Feedback from @kedarbellare review * put scaling back in * put back predict so it can handle multiple inputs * restore original functions signatures (remove first)
Description
Makes tweaks to the infer package to make it more friendly by using Clojure data structures for inputs and outputs to functions. Also add tests to cover all the infer functions.
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
Comments
Object detection now returns a map structure ex:
{:class car, :prob 0.99847263, :x-min 0.6097917, :y-min 0.1406818, :x-max 0.8906532, :y-max 0.29426125}Image classification also returns a map structure
{:class n02123159 tiger cat, :prob 0.30337515}