CherryPick: Reconcile semantics for Suggestion Algorithms (#1633) (#1644)

* Reuse suggestions

* Fix tests

Author: johnugeorge

pkg/apis/manager/v1beta1/api.pb.go

@@ -290,6 +290,7 @@ message GetSuggestionsRequest {
     Experiment experiment = 1;
     repeated Trial trials = 2; // all completed trials owned by the experiment.
     int32 request_number = 3; ///The number of Suggestion you request at one time. When you set 3 to request_number, you can get three Suggestions at one time.
+    int32 total_request_number = 4;//The number of Suggestions requested till now
 }
 
 message GetSuggestionsReply {

pkg/apis/manager/v1beta1/api.proto

@@ -354,6 +354,7 @@ Discrete and Categorical type use List.
 | experiment | [Experiment](#api.v1.beta1.Experiment) |  |  |
 | trials | [Trial](#api.v1.beta1.Trial) | repeated | all completed trials owned by the experiment. |
 | request_number | [int32](#int32) |  | The number of Suggestion you request at one time. When you set 3 to request_number, you can get three Suggestions at one time. |
+| total_request_number | [int32](#int32) |  | The number of Suggestions requested till now |
 
 
 

pkg/apis/manager/v1beta1/gen-doc/api.md

@@ -975,6 +975,13 @@ <h3 id="api.v1.beta1.GetSuggestionsRequest">GetSuggestionsRequest</h3>
                   <td><p>The number of Suggestion you request at one time. When you set 3 to request_number, you can get three Suggestions at one time. </p></td>
                 </tr>
 
+                <tr>
+                  <td>total_request_number</td>
+                  <td><a href="#int32">int32</a></td>
+                  <td></td>
+                  <td><p>The number of Suggestions requested till now </p></td>
+                </tr>
+              
             </tbody>
           </table>
 

pkg/apis/manager/v1beta1/gen-doc/index.html

@@ -102,21 +102,21 @@ func (g *General) SyncAssignments(
 		instance.Status.AlgorithmSettings)
 
 	requestSuggestion := &suggestionapi.GetSuggestionsRequest{
-		Experiment:    g.ConvertExperiment(filledE),
-		Trials:        g.ConvertTrials(ts),
-		RequestNumber: int32(requestNum),
+		Experiment:         g.ConvertExperiment(filledE),
+		Trials:             g.ConvertTrials(ts),
+		RequestNumber:      int32(requestNum),
+		TotalRequestNumber: int32(instance.Spec.Requests),
 	}
 
 	// Get new suggestions
 	responseSuggestion, err := rpcClientSuggestion.GetSuggestions(ctx, requestSuggestion)
 	if err != nil {
 		return err
 	}
-	logger.Info("Getting suggestions", "endpoint", endpoint, "response", responseSuggestion,
-		"request", requestSuggestion)
+	logger.Info("Getting suggestions", "endpoint", endpoint, "Number of request parameters", requestNum, "Number of response parameters", len(responseSuggestion.ParameterAssignments))
 	if len(responseSuggestion.ParameterAssignments) != requestNum {
 		err := fmt.Errorf("The response contains unexpected trials")
-		logger.Error(err, "The response contains unexpected trials", "requestNum", requestNum, "response", responseSuggestion)
+		logger.Error(err, "The response contains unexpected trials")
 		return err
 	}
 

pkg/apis/manager/v1beta1/python/api_pb2.py

@@ -910,6 +910,7 @@ func newFakeRequest() *suggestionapi.GetSuggestionsRequest {
 				},
 			},
 		},
-		RequestNumber: 2,
+		RequestNumber:      2,
+		TotalRequestNumber: 6,
 	}
 }

pkg/controller.v1beta1/suggestion/suggestionclient/suggestionclient.go

@@ -103,12 +103,12 @@ def create_optimizer(self, algorithm_name):
             raise Exception(
                 '"Failed to create Chocolate optimizer for the algorithm: {}'.format(algorithm_name))
 
-    def getSuggestions(self, trials, request_number):
+    def getSuggestions(self, trials, request_number, total_request_number):
         """
         Get the new suggested trials with chocolate algorithm.
         """
         logger.info("-" * 100 + "\n")
-        logger.info("New GetSuggestions call\n")
+        logger.info("New GetSuggestions call with total requested {} and currently requesting {} \n".format(total_request_number, request_number))
         for _, trial in enumerate(trials):
             if trial.name not in self.recorded_trials_names:
                 loss_for_choco = float(trial.target_metric.value)
@@ -157,9 +157,25 @@ def getSuggestions(self, trials, request_number):
                     logger.info("New record in sqlite DB is updated")
                     logger.info("{}\n".format(
                         self.created_trials[new_trial_loss_idx]))
+        # Assuming that created_trials are already populated
+        # TODO: Handle Restart of algorithm pod
+        logger.info("{} Trials created in DB".format(len(self.created_trials)))
+        if total_request_number != len(self.created_trials) + request_number:
+            logger.info("Mismatch in generated trials with k8s suggestions trials")
+        new_actual_requested_no = total_request_number - len(self.created_trials)
+        prev_generated_no = request_number - new_actual_requested_no
+        logger.info("In this call, New {} Trials will be generated, {} Trials will be reused from previously generated".format(new_actual_requested_no, prev_generated_no))
 
         list_of_assignments = []
-        for i in range(request_number):
+        if prev_generated_no > 0:
+            for params in self.created_trials[-prev_generated_no:]:
+                if DB_FIELD_TRIAL_NAME in params:
+                    logger.error("Trial already updated in selected assignment {}".format(params))
+                new_assignment = BaseChocolateService.convert(
+                        self.search_space, params)
+                list_of_assignments.append(new_assignment)
+
+        for i in range(new_actual_requested_no):
             try:
                 token, chocolate_params = self.chocolate_optimizer.next()
                 new_assignment = BaseChocolateService.convert(
@@ -182,7 +198,7 @@ def getSuggestions(self, trials, request_number):
 
         if len(list_of_assignments) > 0:
             logger.info(
-                "GetSuggestions returns {} new Trials\n\n".format(request_number))
+                "GetSuggestions returns {} Trials from requested {} Trials\n\n".format(len(list_of_assignments),request_number))
 
         return list_of_assignments
 

pkg/controller.v1beta1/suggestion/suggestionclient/suggestionclient_test.go

@@ -84,7 +84,7 @@ def GetSuggestions(self, request, context):
 
         trials = Trial.convert(request.trials)
         new_assignments = self.base_service.getSuggestions(
-            trials, request.request_number)
+            trials, request.request_number, request.total_request_number)
         return api_pb2.GetSuggestionsReply(
             parameter_assignments=Assignment.generate(new_assignments)
         )

pkg/suggestion/v1beta1/chocolate/base_service.py

@@ -169,6 +169,7 @@ def test_get_suggestion(self):
             experiment=experiment,
             trials=trials,
             request_number=2,
+            total_request_number=2,
         )
 
         get_suggestion = self.test_server.invoke_unary_unary(