extracting metric value in multiple ways (kubeflow#1140)

* migrate this PR to v1beta1 * fix * fix * fix * add example for metric strategies * modify MetricStrategy specification * fix
sperlingxx · Jun 29, 2020 · 1a8759d · 1a8759d
1 parent c43c1e1
commit 1a8759d
Show file tree

Hide file tree

Showing 14 changed files with 430 additions and 77 deletions.
diff --git a/examples/v1beta1/metric-strategy-example.yaml b/examples/v1beta1/metric-strategy-example.yaml
@@ -0,0 +1,65 @@
+apiVersion: "kubeflow.org/v1beta1"
+kind: Experiment
+metadata:
+  namespace: kubeflow
+  labels:
+    controller-tools.k8s.io: "1.0"
+  name: metric-strategy-example
+spec:
+  objective:
+    type: maximize
+    goal: 0.99
+    objectiveMetricName: Validation-accuracy
+    additionalMetricNames: [Train-accuracy]
+    metricStrategies:
+      - name: Train-accuracy
+        value: "latest"
+      - name: Validation-accuracy
+        value: "max"
+  algorithm:
+    algorithmName: tpe
+  parallelTrialCount: 3
+  maxTrialCount: 12
+  maxFailedTrialCount: 3
+  parameters:
+    - name: --lr
+      parameterType: double
+      feasibleSpace:
+        min: "0.01"
+        max: "0.03"
+    - name: --num-layers
+      parameterType: int
+      feasibleSpace:
+        min: "2"
+        max: "5"
+    - name: --optimizer
+      parameterType: categorical
+      feasibleSpace:
+        list:
+          - sgd
+          - adam
+          - ftrl
+  trialTemplate:
+    goTemplate:
+      rawTemplate: |-
+        apiVersion: batch/v1
+        kind: Job
+        metadata:
+          name: {{.Trial}}
+          namespace: {{.NameSpace}}
+        spec:
+          template:
+            spec:
+              containers:
+              - name: {{.Trial}}
+                image: docker.io/kubeflowkatib/mxnet-mnist
+                command:
+                - "python3"
+                - "/opt/mxnet-mnist/mnist.py"
+                - "--batch-size=64"
+                {{- with .HyperParameters}}
+                {{- range .}}
+                - "{{.Name}}={{.Value}}"
+                {{- end}}
+                {{- end}}
+              restartPolicy: Never
diff --git a/pkg/apis/controller/common/v1beta1/common_types.go b/pkg/apis/controller/common/v1beta1/common_types.go
@@ -51,6 +51,8 @@ type ObjectiveSpec struct {
 	// This can be empty if we only care about the objective metric.
 	// Note: If we adopt a push instead of pull mechanism, this can be omitted completely.
 	AdditionalMetricNames []string `json:"additionalMetricNames,omitempty"`
+	// This field is allowed to missing, experiment defaulter (webhook) will fill it.
+	MetricStrategies []MetricStrategy `json:"metricStrategies,omitempty"`
 }
 
 type ObjectiveType string
@@ -66,9 +68,25 @@ type ParameterAssignment struct {
 	Value string `json:"value,omitempty"`
 }
 
+// ObjectiveExtractType describes the various approaches to extract objective value from metrics.
+type MetricStrategyType string
+
+const (
+	ExtractByMin    MetricStrategyType = "min"
+	ExtractByMax    MetricStrategyType = "max"
+	ExtractByLatest MetricStrategyType = "latest"
+)
+
+type MetricStrategy struct {
+	Name  string             `json:"name,omitempty"`
+	Value MetricStrategyType `json:"value,omitempty"`
+}
+
 type Metric struct {
-	Name  string `json:"name,omitempty"`
-	Value string `json:"value,omitempty"`
+	Name   string  `json:"name,omitempty"`
+	Min    float64 `json:"min,omitempty"`
+	Max    float64 `json:"max,omitempty"`
+	Latest string  `json:"latest,omitempty"`
 }
 
 // +k8s:deepcopy-gen=true

diff --git a/pkg/apis/controller/common/v1beta1/zz_generated.deepcopy.go b/pkg/apis/controller/common/v1beta1/zz_generated.deepcopy.go
diff --git a/pkg/apis/controller/experiments/v1beta1/experiment_defaults.go b/pkg/apis/controller/experiments/v1beta1/experiment_defaults.go
diff --git a/pkg/apis/v1beta1/openapi_generated.go b/pkg/apis/v1beta1/openapi_generated.go
diff --git a/pkg/apis/v1beta1/swagger.json b/pkg/apis/v1beta1/swagger.json
@@ -662,10 +662,18 @@
     },
     "v1beta1.Metric": {
       "properties": {
-        "name": {
+        "latest": {
           "type": "string"
         },
-        "value": {
+        "max": {
+          "type": "number",
+          "format": "double"
+        },
+        "min": {
+          "type": "number",
+          "format": "double"
+        },
+        "name": {
           "type": "string"
         }
       }
@@ -707,6 +715,13 @@
           "type": "number",
           "format": "double"
         },
+        "metricStrategies": {
+          "description": "This field is allowed to missing, experiment defaulter (webhook) will fill it.",
+          "type": "object",
+          "additionalProperties": {
+            "type": "string"
+          }
+        },
         "objectiveMetricName": {
           "type": "string"
         },

diff --git a/pkg/controller.v1beta1/experiment/util/status_util.go b/pkg/controller.v1beta1/experiment/util/status_util.go
@@ -16,9 +16,10 @@ limitations under the License.
 package util
 
 import (
-	"strconv"
-
+	"fmt"
+	"math"
 	logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
+	"strconv"
 
 	commonv1beta1 "github.com/kubeflow/katib/pkg/apis/controller/common/v1beta1"
 	experimentsv1beta1 "github.com/kubeflow/katib/pkg/apis/controller/experiments/v1beta1"
@@ -63,7 +64,6 @@ func updateTrialsSummary(instance *experimentsv1beta1.Experiment, trials *trials
 		objectiveValueGoal = *instance.Spec.Objective.Goal
 	}
 	objectiveType := instance.Spec.Objective.Type
-	objectiveMetricName := instance.Spec.Objective.ObjectiveMetricName
 
 	for index, trial := range trials.Items {
 		sts.Trials++
@@ -79,7 +79,7 @@ func updateTrialsSummary(instance *experimentsv1beta1.Experiment, trials *trials
 			sts.PendingTrialList = append(sts.PendingTrialList, trial.Name)
 		}
 
-		objectiveMetricValueStr := getObjectiveMetricValue(trial, objectiveMetricName)
+		objectiveMetricValueStr := getObjectiveMetricValue(trial)
 		if objectiveMetricValueStr == nil {
 			continue
 		}
@@ -140,13 +140,38 @@ func updateTrialsSummary(instance *experimentsv1beta1.Experiment, trials *trials
 	return isObjectiveGoalReached
 }
 
-func getObjectiveMetricValue(trial trialsv1beta1.Trial, objectiveMetricName string) *string {
+func getObjectiveMetricValue(trial trialsv1beta1.Trial) *string {
 	if trial.Status.Observation == nil {
 		return nil
 	}
+	var objectiveStrategy commonv1beta1.MetricStrategyType
+	objectiveMetricName := trial.Spec.Objective.ObjectiveMetricName
+	for _, strategy := range trial.Spec.Objective.MetricStrategies {
+		if strategy.Name == objectiveMetricName {
+			objectiveStrategy = strategy.Value
+			break
+		}
+	}
 	for _, metric := range trial.Status.Observation.Metrics {
 		if objectiveMetricName == metric.Name {
-			return &metric.Value
+			switch objectiveStrategy {
+			case commonv1beta1.ExtractByMin:
+				if math.IsNaN(metric.Min) {
+					return &metric.Latest
+				}
+				value := fmt.Sprintf("%f", metric.Min)
+				return &value
+			case commonv1beta1.ExtractByMax:
+				if math.IsNaN(metric.Max) {
+					return &metric.Latest
+				}
+				value := fmt.Sprintf("%f", metric.Max)
+				return &value
+			case commonv1beta1.ExtractByLatest:
+				return &metric.Latest
+			default:
+				return nil
+			}
 		}
 	}
 	return nil