[CLIPPER-143] String/JSON Container Output #134

Corey-Zumar · 2017-04-26T06:06:44Z

No description provided.

AmplabJenkins · 2017-04-26T06:10:23Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/200/
Test FAILed.

AmplabJenkins · 2017-04-26T09:15:18Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/208/
Test FAILed.

AmplabJenkins · 2017-04-26T09:35:18Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/209/
Test FAILed.

Corey-Zumar · 2017-04-26T09:39:32Z

@dcrankshaw This is ready for review. Can you make sure that the tutorial still runs properly? I've verified that it works after rebuilding sklearn container, but I haven't tested with tensorflow due to preexisting import issues on my machine.

AmplabJenkins · 2017-04-26T09:51:27Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/210/
Test PASSed.

AmplabJenkins · 2017-04-26T10:04:16Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/211/
Test PASSed.

…xpect output yhat type string

Corey-Zumar

@dcrankshaw Merged the latest develop and addressed your comments. Will add the requested unit test later today

Corey-Zumar · 2017-04-27T10:38:40Z

containers/java/clipper-java-container/src/main/java/clipper/container/app/ModelContainer.java

+    // At minimum, the output contains an unsigned
+    // integer specifying the number of string
+    // outputs
+    int outputLenBytes = 4;


Corey-Zumar · 2017-04-27T10:41:38Z

containers/java/clipper-java-container/src/main/java/clipper/container/app/ModelContainer.java

+    // for storing string lengths. Advance past this segment
+    // for now
+    responseBuffer.position(baseStringLengthsPosition + (4 * numOutputs));
+    for (int i = 0; i < predictions.size(); i++) {


Corey-Zumar · 2017-04-27T10:44:08Z

...java/clipper-java-container/src/main/java/clipper/container/app/data/SerializableString.java

+   *
+   * @return The number of bytes written to the buffer
+   */
+  public int toBytes(ByteBuffer buffer) {


Done - renamed to encodeUTF8ToBuffer

Corey-Zumar · 2017-04-27T10:45:10Z

containers/python/noop_container.py

-    def predict_ints(self, inputs):
-        return np.array([np.sum(x) for x in inputs], dtype='float32')
+    def predict_ints(self, input_item):
+        return str(sum(input_item))


We UTF-8 encode them before serialization. See PredictionResponse#add_output in rpc.py.

Corey-Zumar · 2017-04-27T10:46:15Z

containers/python/rpc.py

+            total_string_length * MAXIMUM_UTF_8_CHAR_LENGTH_BYTES)
+        self.memview = memoryview(self.output_buffer)
+        struct.pack_into("<I", self.output_buffer, 0, num_outputs)
+        self.string_content_end_position = 4 + (4 * num_outputs)


Corey-Zumar · 2017-04-27T10:49:50Z

src/frontends/src/query_frontend.hpp

+      // If the string output is not JSON-formatted, include
+      // it as a raw string in the query response
+      clipper::json::add_string(json_response, PREDICTION_RESPONSE_KEY_OUTPUT,
+                                query_response.output_.y_hat_);


Ah, if I'm not mistaken, the json::add_string method should take care of this for us. I'll change the documentation to this effect.

AmplabJenkins · 2017-04-27T11:00:18Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/241/
Test FAILed.

AmplabJenkins · 2017-04-27T11:35:17Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/242/
Test FAILed.

AmplabJenkins · 2017-04-27T11:57:24Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/243/
Test PASSed.

AmplabJenkins · 2017-04-27T12:11:06Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/244/
Test PASSed.

AmplabJenkins · 2017-04-28T07:31:57Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/246/
Test PASSed.

…ning

AmplabJenkins · 2017-05-09T18:00:33Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/249/
Test FAILed.

AmplabJenkins · 2017-05-09T18:20:55Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/250/
Test FAILed.

dcrankshaw · 2017-05-09T18:26:33Z

That may be true, but this is one place where it's critical to the whole purpose of the system to enable these batch-optimizations. Otherwise Clipper is a non-starter for any models that need to run on the GPU or use BLAS libraries. I also don't think it's a huge added burden on the developer.

AmplabJenkins · 2017-05-09T18:33:18Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/251/
Test PASSed.

…d prediction

dcrankshaw

Okay this looks good to me. Add a comment about the batch-prediction interface and I'll wait for unit tests to pass then this is good to go.

dcrankshaw · 2017-05-09T18:06:05Z

containers/java/clipper-java-container/src/main/java/clipper/container/app/Model.java

+   * @return A JSON-formatted serializable string to be
+   * returned to Clipper as a prediction result
+   */
+  public abstract SerializableString predict(I inputVector);


@Corey-Zumar Note that Model.predict should actually take the full batch of input vectors at once, rather than having the RPC system call predict in a for-loop. This allows models to apply batch processing optimizations if they want. E.g. this is particularly critical for the tensorflow model when running on a GPU.

I fixed this in the Python code but I'm leaving it as-is for now in the Java container code. I have a fix for this as part of my re-org of the Java codebase in #133.

dcrankshaw · 2017-05-09T18:20:48Z

containers/java/clipper-java-container/src/main/java/clipper/container/app/RPCTestModel.java

-    List<Float> eventCodes = new ArrayList<>();
-    for (int i = 0; i < eventHistory.length; i++) {
+    // Begin building a JSON array
+    StringBuilder eventCodeJson = new StringBuilder("[");


For future reference, don't manually construct JSON. Use a json serialization library.

dcrankshaw · 2017-05-09T18:29:20Z

containers/python/TensorFlowCifarDockerfile

@@ -2,7 +2,7 @@ FROM clipper/py-rpc:latest

 MAINTAINER Dan Crankshaw <dscrankshaw@gmail.com>

-RUN pip install tensorflow
+RUN pip install tensorflow==1.0


This is to make sure that the tensorflow version is compatible with our pre-trained model

AmplabJenkins · 2017-05-09T19:12:39Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/252/
Test PASSed.

Corey-Zumar added the status: in progress label Apr 26, 2017

Corey-Zumar requested a review from dcrankshaw April 26, 2017 09:38

Corey-Zumar added status: needs review and removed status: in progress labels Apr 26, 2017

Corey-Zumar mentioned this pull request Apr 26, 2017

JSON response from prediction #93

Closed

Corey-Zumar added 19 commits April 26, 2017 04:19

Skeleton

a2652e6

Progress

26da365

Update java container to render string predictions, change infra to e…

03881e4

…xpect output yhat type string

Update clipper manager for string output

c3531b4

Java container string outputs functional

fea1207

Fix json string truncation issue, C++ end-to-end bench

a9756e3

Remove unnecessary json library

f1a82d5

Fix broken tests

66361f4

More test fixes

c548846

Python string progress

08cb49c

String outputs for Python containers

fba6def

Reduce noop operation overhead

5fceb56

Remove extraneous print

b03c0c2

Update tf container

007a3f9

Update container docs with prediction response serialization

f1d006b

Update container_implementation.md

6dbaada

Format code

4376cdd

Rpc fix

7c61074

doc

92ef4d6

format code

6d7c04f

Corey-Zumar commented Apr 27, 2017

View reviewed changes

Fix tests

3e6eaaa

Corey-Zumar added 3 commits April 27, 2017 04:42

Fix rpc protocol test

10046ca

Format code

d5fc1b4

Docs

45e0fea

Corey-Zumar added 3 commits April 28, 2017 00:02

New unit tests

0a3c7ed

Finish unit tests

30d823d

Format code

5fde315

dcrankshaw added the status: needs review label May 8, 2017

dcrankshaw added 2 commits May 9, 2017 10:56

made python rpc handle prediction batches correctly and fix tf versio…

dedbcd4

…ning

Merge remote-tracking branch 'ucbrise/develop' into model_json

0018e0f

dcrankshaw added 2 commits May 9, 2017 11:13

format code

16ca015

fixed rpc test

0bba5bb

fixed some bugs introduced by moving python containers back to batche…

8cc8cd7

…d prediction

dcrankshaw approved these changes May 9, 2017

View reviewed changes

dcrankshaw removed the status: needs review label May 9, 2017

dcrankshaw mentioned this pull request May 9, 2017

Add options to run unittests separately #131

Merged

dcrankshaw merged commit ea5647d into ucbrise:develop May 9, 2017

dcrankshaw mentioned this pull request May 9, 2017

Duplicate models_used_ field. #73

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CLIPPER-143] String/JSON Container Output #134

[CLIPPER-143] String/JSON Container Output #134

Corey-Zumar commented Apr 26, 2017

AmplabJenkins commented Apr 26, 2017

AmplabJenkins commented Apr 26, 2017

AmplabJenkins commented Apr 26, 2017

Corey-Zumar commented Apr 26, 2017

AmplabJenkins commented Apr 26, 2017

AmplabJenkins commented Apr 26, 2017

Corey-Zumar left a comment

Corey-Zumar Apr 27, 2017

Corey-Zumar Apr 27, 2017

Corey-Zumar Apr 27, 2017

Corey-Zumar Apr 27, 2017

Corey-Zumar Apr 27, 2017

Corey-Zumar Apr 27, 2017

AmplabJenkins commented Apr 27, 2017

AmplabJenkins commented Apr 27, 2017

AmplabJenkins commented Apr 27, 2017

AmplabJenkins commented Apr 27, 2017

AmplabJenkins commented Apr 28, 2017

AmplabJenkins commented May 9, 2017

AmplabJenkins commented May 9, 2017

dcrankshaw commented May 9, 2017

AmplabJenkins commented May 9, 2017

dcrankshaw left a comment

dcrankshaw May 9, 2017

dcrankshaw May 9, 2017

dcrankshaw May 9, 2017

AmplabJenkins commented May 9, 2017

[CLIPPER-143] String/JSON Container Output #134

[CLIPPER-143] String/JSON Container Output #134

Conversation

Corey-Zumar commented Apr 26, 2017

AmplabJenkins commented Apr 26, 2017

AmplabJenkins commented Apr 26, 2017

AmplabJenkins commented Apr 26, 2017

Corey-Zumar commented Apr 26, 2017

AmplabJenkins commented Apr 26, 2017

AmplabJenkins commented Apr 26, 2017

Corey-Zumar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AmplabJenkins commented Apr 27, 2017

AmplabJenkins commented Apr 27, 2017

AmplabJenkins commented Apr 27, 2017

AmplabJenkins commented Apr 27, 2017

AmplabJenkins commented Apr 28, 2017

AmplabJenkins commented May 9, 2017

AmplabJenkins commented May 9, 2017

dcrankshaw commented May 9, 2017

AmplabJenkins commented May 9, 2017

dcrankshaw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AmplabJenkins commented May 9, 2017