review: remove condition to verify algorithmName for early stopping

kubeflow · Nov 9, 2021 · cae15c6 · cae15c6
1 parent 543ad58
commit cae15c6
Show file tree

Hide file tree

Showing 4 changed files with 7 additions and 8 deletions.
diff --git 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 <suggestion-name>-<suggestion-algorithm>
-	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
@@ -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
@@ -84,8 +84,8 @@ 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):
-                        return False, "min_trials_required must be greater or equal than zero (>=0)"
+                    if not (int(setting.value) > 0):
+                        return False, "min_trials_required must be greater than zero (>0)"
                 elif setting.name == "start_step":
                     if not (int(setting.value) >= 1):
                         return False, "start_step must be greater or equal than one (>=1)"

diff --git a/test/unit/v1beta1/earlystopping/test_medianstop_service.py b/test/unit/v1beta1/earlystopping/test_medianstop_service.py
@@ -87,14 +87,14 @@ def test_validate_early_stopping_settings(self):
             algorithm_settings=[
                 api_pb2.EarlyStoppingSetting(
                     name="min_trials_required",
-                    value="-1",
+                    value="0",
                 ),
             ],
         )
 
         _, _, code, details = utils.call_validate(self.test_server, early_stopping)
         self.assertEqual(code, grpc.StatusCode.INVALID_ARGUMENT)
-        self.assertEqual(details, "min_trials_required must be greater or equal than zero (>=0)")
+        self.assertEqual(details, "min_trials_required must be greater than zero (>0)")
 
         # Wrong start_step
         early_stopping = api_pb2.EarlyStoppingSpec(