Batch predict by tatevikstep · Pull Request #321 · ucbrise/clipper

tatevikstep · 2017-11-20T22:53:02Z

Added support for input of type: { "input_batch" := [[double] | [int] | [byte] | [float] | string] }

Source code changes:

query_frontend.hpp
json_util.cpp/hpp
query_frontend_tests.cpp

Added testing for:

Batch inputs of type: [[double] | [int] | [float] | string]
Expected error message on malformed batch inputs (non-matching types)

Questions/Still to-do:

No testing for:
- Input batch of type [[byte]]. There's no testing for type [byte] either, I think because EXPECT_EQ() doesn't take binary inputs
- Return batch predictions only after all queries are processed
Some error messages/comments aren't updated to show the acceptable input formats

Merging with develop branch after batch input testing

AmplabJenkins · 2017-11-20T22:55:03Z

Can one of the admins verify this patch?

dcrankshaw · 2017-11-21T20:09:07Z

jenkins ok to test

AmplabJenkins · 2017-11-21T20:15:48Z

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

AmplabJenkins · 2017-11-21T23:47: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/788/
Test FAILed.

AmplabJenkins · 2017-11-27T18:42:51Z

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

dcrankshaw

This looks good. There are a couple things that you should add:

Add an integration test that uses the batch prediction interface for end-to-end queries. Just add another test like this one that sends batch predictions and checks to make sure the right number of predictions are returned.
Add an example of querying Clipper using the batch prediction interface to the examples directory and add a subsection to the Clipper documentation here that explains the batch prediction interface.

dcrankshaw · 2017-11-27T18:00:13Z

src/benchmarks/src/bench_utils.hpp

-*/
+ * Returns the value corresponding to `key` in `config` as a long
+ */
 long get_long(const std::string &key,


Revert these changes (to keep the commit history clean).

dcrankshaw · 2017-11-27T18:00:20Z

src/container/include/container/container_rpc.hpp

- */
+   * @return `true` if the received heartbeat is a request for container
+   * metadata. `false` otherwise.
+   */


dcrankshaw · 2017-11-27T18:05:00Z

src/frontends/src/query_frontend.hpp

+                  final_content += content + "\n";
+                } catch (const std::exception& e) {
+                  // case: returned a response before all predictions in the
+                  // batch were ready


Log this error

dcrankshaw · 2017-11-27T18:06:11Z

src/frontends/src/query_frontend.hpp

+        predictions
+            .then([response,
+                   app_metrics](std::vector<folly::Try<Response>> tries) {
+              std::string final_content;


Use a std::stringstream to accumulate the full response.

dcrankshaw · 2017-11-27T18:06:54Z

src/frontends/src/query_frontend.hpp

+                  app_metrics.throughput_->mark(1);
+
+                  std::string content = get_prediction_response_content(r);
+                  final_content += content + "\n";


This will turn into a stringstream appends (e.g. final_content << content << "\n";)

dcrankshaw · 2017-11-27T18:09:22Z

src/frontends/src/query_frontend.hpp

+      auto prediction = query_processor_.predict(
+          Query{name, uid, input, latency_slo_micros, policy, models});
+      predictions.push_back(std::move(prediction));
+    } else {  // d.HasMember("input_batch") instead


Does it make sense to explicitly check for d.hasMember("input_batch") here, rather than leaving it as an else statement?

dcrankshaw · 2017-11-27T18:13:49Z

src/libclipper/include/clipper/redis.hpp

@@ -41,7 +41,7 @@ bool contains_prohibited_chars_for_group(std::string value);
 /**
 * Issues a command to Redis and checks return code.
 * \return Returns true if the command was successful.


revert all these whitespace changes

dcrankshaw · 2017-11-27T18:14:03Z

src/libclipper/include/clipper/rpc_service.hpp

+   * \param `msg`: A vector of individual messages to send to this container.
+   * The messages will be sent as a single, multi-part ZeroMQ message so
+   * it is very efficient.
+   */


AmplabJenkins · 2017-11-30T23:50:40Z

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

merge develop branch

AmplabJenkins · 2017-12-01T01:20:35Z

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

AmplabJenkins · 2017-12-01T07:00:48Z

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

AmplabJenkins · 2017-12-01T07:50:35Z

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

AmplabJenkins · 2017-12-11T19:38:05Z

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

…atch_predict pulling new formatting

AmplabJenkins · 2017-12-11T21:15:08Z

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

AmplabJenkins · 2017-12-11T21:20:39Z

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

AmplabJenkins · 2018-01-03T22:55: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/821/
Test PASSed.

AmplabJenkins · 2018-01-03T23:28:27Z

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

dcrankshaw

LGTM

AmplabJenkins · 2018-01-04T01:26:14Z

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

Tatevik Stepanyan added 12 commits September 16, 2017 16:41

parsing both json formats (untested), not handling final return format

1292ae6

reverting to develop code

b2cc750

decode_and_handle_predict changes only

0e5f5b5

initial changes to add_application

9e37507

code compiles, not tested

a0fd8d2

batch prediction working for type double inputs

5a4a287

supporting floats and ints

0d42e43

byte and string types

1b144d7

old frontend tests now compatible with new decode_and_handle_predict

8c02958

added tests (passing) for int, double and string types

f43114c

added test for wrong input type error handling

7a9cfd3

Merge branch 'develop' into batch_predict

94e2e5a

Merging with develop branch after batch input testing

reverted some docker files, scripts back to versions on develop branch

b3ec5c2

dcrankshaw self-requested a review November 21, 2017 20:10

dcrankshaw added the status: needs review label Nov 21, 2017

ran ./bin/format_code.sh, passing ./bin/check_format.sh

51ecf46

dcrankshaw added 2 commits November 27, 2017 09:51

formatted code with correct version of clang-format

4404547

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

3f5a431

dcrankshaw requested changes Nov 27, 2017

View reviewed changes

dcrankshaw added status: needs revision and removed status: needs review labels Nov 27, 2017

Tatevik Stepanyan added 4 commits November 30, 2017 11:35

revert files to develop branch

fe5fb5d

revert more files to develop

0d9ca9f

back to fe5fb5d

efd2d49

back to remote version

ddb9c9f

refactored code, not formatted

08191c8

Tatevik Stepanyan added 2 commits November 30, 2017 17:12

revert redis.hpp

6c923be

Merge branch 'develop' into batch_predict

6eaa7f4

merge develop branch

Tatevik Stepanyan added 2 commits November 30, 2017 20:51

single json response, array in output field

d59f0fd

batch prediction integration test

3461df5

typo fix

9d23155

format code

bed6827

dcrankshaw added status: needs review and removed status: needs revision labels Dec 11, 2017

Tatevik Stepanyan added 3 commits December 11, 2017 12:01

new model and app name for extra test

3f06542

Merge branch 'batch_predict' of github.com:tatevikstep/clipper into b…

9a29ef1

…atch_predict pulling new formatting

batch query example

c35f793

dcrankshaw added 2 commits January 3, 2018 13:33

Merge branch 'develop' into batch_predict

d31c9fa

format code

7aa122c

dcrankshaw approved these changes Jan 4, 2018

View reviewed changes

Merge branch 'develop' into batch_predict

0db4c2b

dcrankshaw added status: accepted and removed status: needs review labels Jan 4, 2018

dcrankshaw merged commit 193cf8b into ucbrise:develop Jan 4, 2018

Corey-Zumar mentioned this pull request Jan 18, 2018

Add batch prediction interface #292

Closed

Conversation

tatevikstep commented Nov 20, 2017

Uh oh!

AmplabJenkins commented Nov 20, 2017

Uh oh!

dcrankshaw commented Nov 21, 2017

Uh oh!

AmplabJenkins commented Nov 21, 2017

Uh oh!

AmplabJenkins commented Nov 21, 2017

Uh oh!

AmplabJenkins commented Nov 27, 2017

Uh oh!

dcrankshaw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AmplabJenkins commented Nov 30, 2017

Uh oh!

AmplabJenkins commented Dec 1, 2017

Uh oh!

AmplabJenkins commented Dec 1, 2017

Uh oh!

AmplabJenkins commented Dec 1, 2017

Uh oh!

AmplabJenkins commented Dec 11, 2017

Uh oh!

AmplabJenkins commented Dec 11, 2017

Uh oh!

AmplabJenkins commented Dec 11, 2017

Uh oh!

AmplabJenkins commented Jan 3, 2018

Uh oh!

AmplabJenkins commented Jan 3, 2018

Uh oh!

dcrankshaw left a comment

Choose a reason for hiding this comment

Uh oh!

AmplabJenkins commented Jan 4, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants