Skip to content

Commit 171d37e

Browse files
tenzen-ytenzen-y
committed
fix error messages
1 parent 45ad4e8 commit 171d37e

File tree

7 files changed

+37
-169
lines changed

7 files changed

+37
-169
lines changed

‎manifests/v1beta1/components/controller/katib-config.yaml‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,6 @@ data:
6262
early-stopping: |-
6363
{
6464
"medianstop": {
65-
"image": "docker.io/kubeflowkatib/earlystopping-medianstop:latest",
66-
"algorithmSettings": {
67-
"min_trials_required": "int",
68-
"start_step": "int"
69-
}
65+
"image": "docker.io/kubeflowkatib/earlystopping-medianstop:latest"
7066
}
7167
}

‎pkg/controller.v1beta1/experiment/manifest/generator.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ type Generator interface {
3838
GetTrialTemplate(instance *experimentsv1beta1.Experiment) (string, error)
3939
GetRunSpecWithHyperParameters(experiment *experimentsv1beta1.Experiment, trialName, trialNamespace string, assignments []commonapiv1beta1.ParameterAssignment) (*unstructured.Unstructured, error)
4040
GetSuggestionConfigData(algorithmName string) (katibconfig.SuggestionConfig, error)
41-
GetEarlyStoppingConfigData(earlyStoppingSpec *commonapiv1beta1.EarlyStoppingSpec) (katibconfig.EarlyStoppingConfig, error)
41+
GetEarlyStoppingConfigData(algorithmName string) (katibconfig.EarlyStoppingConfig, error)
4242
GetMetricsCollectorConfigData(cKind commonapiv1beta1.CollectorKind) (katibconfig.MetricsCollectorConfig, error)
4343
}
4444

@@ -70,8 +70,8 @@ func (g *DefaultGenerator) GetSuggestionConfigData(algorithmName string) (katibc
7070
}
7171

7272
// GetEarlyStoppingConfigData returns early stopping configuration for a given algorithm.
73-
func (g *DefaultGenerator) GetEarlyStoppingConfigData(earlyStoppingSpec *commonapiv1beta1.EarlyStoppingSpec) (katibconfig.EarlyStoppingConfig, error) {
74-
return katibconfig.GetEarlyStoppingConfigData(earlyStoppingSpec, g.client.GetClient())
73+
func (g *DefaultGenerator) GetEarlyStoppingConfigData(algorithmName string) (katibconfig.EarlyStoppingConfig, error) {
74+
return katibconfig.GetEarlyStoppingConfigData(algorithmName, g.client.GetClient())
7575
}
7676

7777
// GetRunSpecWithHyperParameters returns the specification for trial with hyperparameters.

‎pkg/controller.v1beta1/suggestion/composer/composer.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ func (g *General) DesiredDeployment(s *suggestionsv1beta1.Suggestion) (*appsv1.D
8080
// If early stopping is used, get the config data.
8181
earlyStoppingConfigData := katibconfig.EarlyStoppingConfig{}
8282
if s.Spec.EarlyStopping != nil && s.Spec.EarlyStopping.AlgorithmName != "" {
83-
earlyStoppingConfigData, err = katibconfig.GetEarlyStoppingConfigData(s.Spec.EarlyStopping, g.Client)
83+
earlyStoppingConfigData, err = katibconfig.GetEarlyStoppingConfigData(s.Spec.EarlyStopping.AlgorithmName, g.Client)
8484
if err != nil {
8585
return nil, err
8686
}

‎pkg/mock/v1beta1/experiment/manifest/generator.go‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/util/v1beta1/katibconfig/config.go‎

Lines changed: 6 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"context"
2121
"encoding/json"
2222
"fmt"
23-
"strconv"
2423
"strings"
2524

2625
corev1 "k8s.io/api/core/v1"
@@ -47,17 +46,10 @@ type SuggestionConfig struct {
4746

4847
// EarlyStoppingConfig is the JSON early stopping structure in Katib config.
4948
type EarlyStoppingConfig struct {
50-
Image string `json:"image"`
51-
ImagePullPolicy corev1.PullPolicy `json:"imagePullPolicy,omitempty"`
52-
AlgorithmSettings map[string]EarlyStoppingAlgorithmSettingValueType `json:"algorithmSettings,omitempty"`
49+
Image string `json:"image"`
50+
ImagePullPolicy corev1.PullPolicy `json:"imagePullPolicy,omitempty"`
5351
}
5452

55-
type EarlyStoppingAlgorithmSettingValueType string
56-
57-
const (
58-
ValueTypeInt EarlyStoppingAlgorithmSettingValueType = "int"
59-
)
60-
6153
// MetricsCollectorConfig is the JSON metrics collector structure in Katib config.
6254
type MetricsCollectorConfig struct {
6355
Image string `json:"image"`
@@ -147,7 +139,7 @@ func GetSuggestionConfigData(algorithmName string, client client.Client) (Sugges
147139
}
148140

149141
// GetEarlyStoppingConfigData gets the config data for the given early stopping algorithm name.
150-
func GetEarlyStoppingConfigData(earlyStoppingSpec *common.EarlyStoppingSpec, client client.Client) (EarlyStoppingConfig, error) {
142+
func GetEarlyStoppingConfigData(algorithmName string, client client.Client) (EarlyStoppingConfig, error) {
151143
configMap := &corev1.ConfigMap{}
152144
earlyStoppingConfigData := EarlyStoppingConfig{}
153145
err := client.Get(
@@ -171,47 +163,20 @@ func GetEarlyStoppingConfigData(earlyStoppingSpec *common.EarlyStoppingSpec, cli
171163
}
172164

173165
// Try to find EarlyStoppingConfig for the algorithm.
174-
earlyStoppingConfigData, ok = earlyStoppingsConfig[earlyStoppingSpec.AlgorithmName]
166+
earlyStoppingConfigData, ok = earlyStoppingsConfig[algorithmName]
175167
if !ok {
176-
return EarlyStoppingConfig{}, fmt.Errorf("failed to find early stopping config for algorithm: %s in ConfigMap: %s", earlyStoppingSpec.AlgorithmName, consts.KatibConfigMapName)
168+
return EarlyStoppingConfig{}, fmt.Errorf("failed to find early stopping config for algorithm: %s in ConfigMap: %s", algorithmName, consts.KatibConfigMapName)
177169
}
178170

179171
// Get image from config.
180172
image := earlyStoppingConfigData.Image
181173
if strings.TrimSpace(image) == "" {
182-
return EarlyStoppingConfig{}, fmt.Errorf("required value for image configuration of algorithm name: %s", earlyStoppingSpec.AlgorithmName)
174+
return EarlyStoppingConfig{}, fmt.Errorf("required value for image configuration of algorithm name: %s", algorithmName)
183175
}
184176

185177
// Set Image Pull Policy.
186178
earlyStoppingConfigData.ImagePullPolicy = setImagePullPolicy(earlyStoppingConfigData.ImagePullPolicy)
187179

188-
// Early stopping service add default values.
189-
if earlyStoppingSpec.AlgorithmSettings == nil || earlyStoppingConfigData.AlgorithmSettings == nil {
190-
return earlyStoppingConfigData, nil
191-
}
192-
193-
// Get algorithmSettings.
194-
algorithmSettingsConfigData := earlyStoppingConfigData.AlgorithmSettings
195-
for _, inputSetting := range earlyStoppingSpec.AlgorithmSettings {
196-
if inputSetting.Name == "" || inputSetting.Value == "" {
197-
return EarlyStoppingConfig{}, fmt.Errorf("required value for early stopping algprithm settings of algorithm name: %s", earlyStoppingSpec.AlgorithmName)
198-
}
199-
200-
valueTypeConfigData, exist := algorithmSettingsConfigData[inputSetting.Name]
201-
if !exist {
202-
return EarlyStoppingConfig{}, fmt.Errorf("failed to find early stopping config for algorithm settings: %s in ConfigMap: %s", inputSetting.Name, consts.KatibConfigMapName)
203-
}
204-
205-
switch valueTypeConfigData {
206-
case ValueTypeInt:
207-
if _, err = strconv.Atoi(inputSetting.Value); err != nil {
208-
return EarlyStoppingConfig{}, fmt.Errorf("%s must be integer value for early stopping algorithm name: %s", inputSetting.Value, inputSetting.Name)
209-
}
210-
default:
211-
return EarlyStoppingConfig{}, fmt.Errorf("failed to find early stopping algorithm setings value type: %s in ConfigMap: %s", valueTypeConfigData, consts.KatibConfigMapName)
212-
}
213-
}
214-
215180
return earlyStoppingConfigData, nil
216181
}
217182

‎pkg/util/v1beta1/katibconfig/config_test.go‎

Lines changed: 24 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -153,34 +153,30 @@ func TestGetSuggestionConfigData(t *testing.T) {
153153

154154
}
155155

156-
const (
157-
testEarlyStoppingAlgorithmName = "test-early-stopping-algorithm1"
158-
testEarlyStoppingAlgorithmSettingName = "test-early-stopping-setting1"
159-
)
160-
161156
func TestGetEarlyStoppingConfigData(t *testing.T) {
157+
const testAlgorithmName = "test-early-stopping"
162158

163159
tests := []struct {
164-
testDescription string
165-
katibConfig *katibConfig
166-
expected *EarlyStoppingConfig
167-
inputEarlyStoppingSpec *commonv1beta1.EarlyStoppingSpec
168-
err bool
160+
testDescription string
161+
katibConfig *katibConfig
162+
expected *EarlyStoppingConfig
163+
inputAlgorithmName string
164+
err bool
169165
}{
170166
{
171167
testDescription: "All parameters correctly are specified",
172168
katibConfig: func() *katibConfig {
173-
kc := &katibConfig{earlyStopping: map[string]*EarlyStoppingConfig{testEarlyStoppingAlgorithmName: newFakeEarlyStoppingConfig()}}
174-
kc.earlyStopping[testEarlyStoppingAlgorithmName].ImagePullPolicy = corev1.PullIfNotPresent
169+
kc := &katibConfig{earlyStopping: map[string]*EarlyStoppingConfig{testAlgorithmName: newFakeEarlyStoppingConfig()}}
170+
kc.earlyStopping[testAlgorithmName].ImagePullPolicy = corev1.PullIfNotPresent
175171
return kc
176172
}(),
177173
expected: func() *EarlyStoppingConfig {
178174
c := newFakeEarlyStoppingConfig()
179175
c.ImagePullPolicy = corev1.PullIfNotPresent
180176
return c
181177
}(),
182-
inputEarlyStoppingSpec: newFakeEarlyStoppingSpec(),
183-
err: false,
178+
inputAlgorithmName: testAlgorithmName,
179+
err: false,
184180
},
185181
{
186182
testDescription: "There is not katib-config.",
@@ -193,112 +189,38 @@ func TestGetEarlyStoppingConfigData(t *testing.T) {
193189
err: true,
194190
},
195191
{
196-
testDescription: "There is not the AlgorithmName",
197-
katibConfig: &katibConfig{earlyStopping: map[string]*EarlyStoppingConfig{testEarlyStoppingAlgorithmName: newFakeEarlyStoppingConfig()}},
198-
inputEarlyStoppingSpec: func() *commonv1beta1.EarlyStoppingSpec {
199-
es := newFakeEarlyStoppingSpec()
200-
es.AlgorithmName = "invalid-algorithm-name"
201-
return es
202-
}(),
203-
err: true,
192+
testDescription: "There is not the AlgorithmName",
193+
katibConfig: &katibConfig{earlyStopping: map[string]*EarlyStoppingConfig{testAlgorithmName: newFakeEarlyStoppingConfig()}},
194+
inputAlgorithmName: "invalid-algorithm-name",
195+
err: true,
204196
},
205197
{
206198
testDescription: "Image filed is empty in katib-config configMap",
207199
katibConfig: func() *katibConfig {
208-
kc := &katibConfig{earlyStopping: map[string]*EarlyStoppingConfig{testEarlyStoppingAlgorithmName: newFakeEarlyStoppingConfig()}}
209-
kc.earlyStopping[testEarlyStoppingAlgorithmName].Image = ""
200+
kc := &katibConfig{earlyStopping: map[string]*EarlyStoppingConfig{testAlgorithmName: newFakeEarlyStoppingConfig()}}
201+
kc.earlyStopping[testAlgorithmName].Image = ""
210202
return kc
211203
}(),
212-
inputEarlyStoppingSpec: newFakeEarlyStoppingSpec(),
213-
err: true,
204+
inputAlgorithmName: testAlgorithmName,
205+
err: true,
214206
},
215207
{
216208
testDescription: fmt.Sprintf("GetEarlyStoppingConfigData sets %s to imagePullPolicy", consts.DefaultImagePullPolicy),
217209
katibConfig: func() *katibConfig {
218-
kc := &katibConfig{earlyStopping: map[string]*EarlyStoppingConfig{testEarlyStoppingAlgorithmName: newFakeEarlyStoppingConfig()}}
219-
kc.earlyStopping[testEarlyStoppingAlgorithmName].ImagePullPolicy = ""
220-
return kc
221-
}(),
222-
expected: newFakeEarlyStoppingConfig(),
223-
inputEarlyStoppingSpec: newFakeEarlyStoppingSpec(),
224-
err: false,
225-
},
226-
{
227-
testDescription: "There is not the EarlyStoppingSettings in Experiment resource",
228-
katibConfig: &katibConfig{earlyStopping: map[string]*EarlyStoppingConfig{testEarlyStoppingAlgorithmName: newFakeEarlyStoppingConfig()}},
229-
inputEarlyStoppingSpec: func() *commonv1beta1.EarlyStoppingSpec {
230-
es := newFakeEarlyStoppingSpec()
231-
es.AlgorithmSettings = nil
232-
return es
233-
}(),
234-
err: false,
235-
},
236-
{
237-
testDescription: "There is not the algorithmSettings field in katib-config ConfigMap",
238-
katibConfig: func() *katibConfig {
239-
kc := &katibConfig{earlyStopping: map[string]*EarlyStoppingConfig{testEarlyStoppingAlgorithmName: newFakeEarlyStoppingConfig()}}
240-
kc.earlyStopping[testEarlyStoppingAlgorithmName].AlgorithmSettings = nil
210+
kc := &katibConfig{earlyStopping: map[string]*EarlyStoppingConfig{testAlgorithmName: newFakeEarlyStoppingConfig()}}
211+
kc.earlyStopping[testAlgorithmName].ImagePullPolicy = ""
241212
return kc
242213
}(),
243-
inputEarlyStoppingSpec: newFakeEarlyStoppingSpec(),
244-
err: false,
245-
},
246-
{
247-
testDescription: "EarlyStoppingSettings name field is empty in Experiment resource",
248-
katibConfig: &katibConfig{earlyStopping: map[string]*EarlyStoppingConfig{testEarlyStoppingAlgorithmName: newFakeEarlyStoppingConfig()}},
249-
inputEarlyStoppingSpec: func() *commonv1beta1.EarlyStoppingSpec {
250-
es := newFakeEarlyStoppingSpec()
251-
es.AlgorithmSettings[0].Name = ""
252-
return es
253-
}(),
254-
err: true,
255-
},
256-
{
257-
testDescription: "EarlyStoppingSettings value field is empty in Experiment resource",
258-
katibConfig: &katibConfig{earlyStopping: map[string]*EarlyStoppingConfig{testEarlyStoppingAlgorithmName: newFakeEarlyStoppingConfig()}},
259-
inputEarlyStoppingSpec: func() *commonv1beta1.EarlyStoppingSpec {
260-
es := newFakeEarlyStoppingSpec()
261-
es.AlgorithmSettings[0].Value = ""
262-
return es
263-
}(),
264-
err: true,
265-
},
266-
{
267-
testDescription: "Set invalid algorithm setting name for early stopping in Experiment resource",
268-
katibConfig: &katibConfig{earlyStopping: map[string]*EarlyStoppingConfig{testEarlyStoppingAlgorithmName: newFakeEarlyStoppingConfig()}},
269-
inputEarlyStoppingSpec: func() *commonv1beta1.EarlyStoppingSpec {
270-
es := newFakeEarlyStoppingSpec()
271-
es.AlgorithmSettings[0].Name = "invalid-algorithm-setting-name"
272-
return es
273-
}(),
274-
err: true,
275-
},
276-
{
277-
testDescription: "Set invalid algorithm setting value for early stopping in Experiment resource",
278-
katibConfig: &katibConfig{earlyStopping: map[string]*EarlyStoppingConfig{testEarlyStoppingAlgorithmName: newFakeEarlyStoppingConfig()}},
279-
inputEarlyStoppingSpec: func() *commonv1beta1.EarlyStoppingSpec {
280-
es := newFakeEarlyStoppingSpec()
281-
es.AlgorithmSettings[0].Value = "invalid-value-type"
282-
return es
283-
}(),
284-
err: true,
285-
},
286-
{
287-
testDescription: "Set invalid algorithm setting value for early stopping in katib-config ConfigMap",
288-
katibConfig: func() *katibConfig {
289-
kc := &katibConfig{earlyStopping: map[string]*EarlyStoppingConfig{testEarlyStoppingAlgorithmName: newFakeEarlyStoppingConfig()}}
290-
kc.earlyStopping[testEarlyStoppingAlgorithmName].AlgorithmSettings[testEarlyStoppingAlgorithmSettingName] = "invalid-type"
291-
return kc
292-
}(),
293-
inputEarlyStoppingSpec: newFakeEarlyStoppingSpec(),
294-
err: true,
214+
expected: newFakeEarlyStoppingConfig(),
215+
inputAlgorithmName: testAlgorithmName,
216+
err: false,
295217
},
296218
}
297219

298220
for _, tt := range tests {
299221
t.Run(tt.testDescription, func(t *testing.T) {
300222
fakeKubeClient := newFakeKubeClient(newFakeKatibConfigMap(tt.katibConfig))
301-
actual, err := GetEarlyStoppingConfigData(tt.inputEarlyStoppingSpec, fakeKubeClient)
223+
actual, err := GetEarlyStoppingConfigData(tt.inputAlgorithmName, fakeKubeClient)
302224
if (err != nil) != tt.err {
303225
t.Errorf("want error: %v, actual: %v", tt.err, err)
304226
} else if tt.expected != nil {
@@ -488,21 +410,6 @@ func newFakeEarlyStoppingConfig() *EarlyStoppingConfig {
488410
return &EarlyStoppingConfig{
489411
Image: "early-stopping-image",
490412
ImagePullPolicy: consts.DefaultImagePullPolicy,
491-
AlgorithmSettings: map[string]EarlyStoppingAlgorithmSettingValueType{
492-
testEarlyStoppingAlgorithmSettingName: ValueTypeInt,
493-
},
494-
}
495-
}
496-
497-
func newFakeEarlyStoppingSpec() *commonv1beta1.EarlyStoppingSpec {
498-
return &commonv1beta1.EarlyStoppingSpec{
499-
AlgorithmName: testEarlyStoppingAlgorithmName,
500-
AlgorithmSettings: []commonv1beta1.EarlyStoppingSetting{
501-
{
502-
Name: testEarlyStoppingAlgorithmSettingName,
503-
Value: "2",
504-
},
505-
},
506413
}
507414
}
508415

‎pkg/webhook/v1beta1/experiment/validator/validator.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ func (g *DefaultValidator) validateEarlyStopping(es *commonapiv1beta1.EarlyStopp
183183
return fmt.Errorf("no spec.earlyStopping.algorithm.algorithmName specified")
184184
}
185185

186-
if _, err := g.GetEarlyStoppingConfigData(es); err != nil {
186+
if _, err := g.GetEarlyStoppingConfigData(es.AlgorithmName); err != nil {
187187
return fmt.Errorf("unable to get EarlyStopping config data for algorithm %s: %v", es.AlgorithmName, err)
188188
}
189189

0 commit comments

Comments
 (0)