Skip to content

Reconcile semantics for Suggestion Algorithms #1633

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 24, 2021
Merged

Reconcile semantics for Suggestion Algorithms #1633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ce12a89
Reuse suggestions
johnugeorge Aug 24, 2021
acc67db
Fix tests
johnugeorge Aug 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
237 changes: 123 additions & 114 deletions pkg/apis/manager/v1beta1/api.pb.go
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions pkg/apis/manager/v1beta1/api.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/manager/v1beta1/gen-doc/api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 |



Expand Down
7 changes: 7 additions & 0 deletions pkg/apis/manager/v1beta1/gen-doc/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down
67 changes: 37 additions & 30 deletions pkg/apis/manager/v1beta1/python/api_pb2.py
View file
Open in desktop

Large diffs are not rendered by default.

12 changes: 6 additions & 6 deletions pkg/controller.v1beta1/suggestion/suggestionclient/suggestionclient.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/controller.v1beta1/suggestion/suggestionclient/suggestionclient_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -910,6 +910,7 @@ func newFakeRequest() *suggestionapi.GetSuggestionsRequest {
},
},
},
RequestNumber: 2,
RequestNumber: 2,
TotalRequestNumber: 6,
}
}
24 changes: 20 additions & 4 deletions pkg/suggestion/v1beta1/chocolate/base_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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))
Comment on lines +165 to +167
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the first call, new_actual_requested_no = 0 and prev_generated_no = 3, when request_number = 3.
Is that correct ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is the other way around.
In the normal case,
total_request_number == len(self.created_trials) + request_number where self.created_trials correspond to the number of previously created trials. So, prev_generated_no will be 0 in this case

When there is a difference, it means that some of the suggestions in self.created_trials(same as in DB) are not recorded in K8s Suggestions resource. So, prev_generated_no will be greater than 0 in this case


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(
Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion pkg/suggestion/v1beta1/chocolate/service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
)
Expand Down
1 change: 1 addition & 0 deletions test/suggestion/v1beta1/test_chocolate_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down