From 564e93a0844d3dd236f3026d285505122daada60 Mon Sep 17 00:00:00 2001 From: tenzen-y Date: Wed, 10 Nov 2021 07:19:04 +0900 Subject: [PATCH] review: remove condition to verify algorithmName for early stopping --- pkg/controller.v1beta1/suggestion/suggestion_controller.go | 5 ++--- .../suggestion/suggestionclient/suggestionclient.go | 2 +- pkg/earlystopping/v1beta1/medianstop/service.py | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/pkg/controller.v1beta1/suggestion/suggestion_controller.go b/pkg/controller.v1beta1/suggestion/suggestion_controller.go index 60c6057deb5..fec2cb44546 100644 --- a/pkg/controller.v1beta1/suggestion/suggestion_controller.go +++ b/pkg/controller.v1beta1/suggestion/suggestion_controller.go @@ -223,8 +223,7 @@ func (r *ReconcileSuggestion) ReconcileSuggestion(instance *suggestionsv1beta1.S // If early stopping is used, create RBAC. // If controller should reconcile RBAC, // ServiceAccount name must be equal to - - if instance.Spec.EarlyStopping != nil && instance.Spec.EarlyStopping.AlgorithmName != "" && - deploy.Spec.Template.Spec.ServiceAccountName == util.GetSuggestionRBACName(instance) { + if instance.Spec.EarlyStopping != nil && deploy.Spec.Template.Spec.ServiceAccountName == util.GetSuggestionRBACName(instance) { serviceAccount, role, roleBinding, err := r.DesiredRBAC(instance) if err != nil { @@ -275,7 +274,7 @@ func (r *ReconcileSuggestion) ReconcileSuggestion(instance *suggestionsv1beta1.S // return nil since it is a terminal condition return nil } - if instance.Spec.EarlyStopping != nil && instance.Spec.EarlyStopping.AlgorithmName != "" { + if instance.Spec.EarlyStopping != nil { if err = r.ValidateEarlyStoppingSettings(instance, experiment); err != nil { logger.Error(err, "Marking suggestion failed as early stopping settings validation failed") msg := fmt.Sprintf("Validation failed: %v", err) diff --git a/pkg/controller.v1beta1/suggestion/suggestionclient/suggestionclient.go b/pkg/controller.v1beta1/suggestion/suggestionclient/suggestionclient.go index 9e79f50d675..de161f023f0 100644 --- a/pkg/controller.v1beta1/suggestion/suggestionclient/suggestionclient.go +++ b/pkg/controller.v1beta1/suggestion/suggestionclient/suggestionclient.go @@ -130,7 +130,7 @@ func (g *General) SyncAssignments( earlyStoppingRules := []commonapiv1beta1.EarlyStoppingRule{} // If early stopping is set, call GetEarlyStoppingRules after GetSuggestions. - if instance.Spec.EarlyStopping != nil && instance.Spec.EarlyStopping.AlgorithmName != "" { + if instance.Spec.EarlyStopping != nil { endpoint = util.GetEarlyStoppingEndpoint(instance) connEarlyStopping, err := grpc.Dial(endpoint, grpc.WithInsecure()) if err != nil { diff --git a/pkg/earlystopping/v1beta1/medianstop/service.py b/pkg/earlystopping/v1beta1/medianstop/service.py index 83d3d306e83..6cdfe826d92 100644 --- a/pkg/earlystopping/v1beta1/medianstop/service.py +++ b/pkg/earlystopping/v1beta1/medianstop/service.py @@ -84,7 +84,7 @@ def validate_medianstop_setting(early_stopping_settings): for setting in early_stopping_settings: try: if setting.name == "min_trials_required": - if not (int(setting.value) >= 0): + if not (int(setting.value) > 0): return False, "min_trials_required must be greater or equal than zero (>=0)" elif setting.name == "start_step": if not (int(setting.value) >= 1):