Skip to content

Commit eed4e1d

Browse files
irar2irar2
authored
Improved support for empty command line arguments (#80)
* Don't add empty strings to arg values, support empty flags, tests Signed-off-by: Ira <IRAR@il.ibm.com> * Fixed lint error Signed-off-by: Ira <IRAR@il.ibm.com> --------- Signed-off-by: Ira <IRAR@il.ibm.com>
1 parent 05d8c15 commit eed4e1d

File tree

3 files changed

+60
-14
lines changed

3 files changed

+60
-14
lines changed

‎pkg/llm-d-inference-sim/config.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ func (c *configuration) validate() error {
125125
// Upstream vLLM behaviour: when --served-model-name is not provided,
126126
// it falls back to using the value of --model as the single public name
127127
// returned by the API and exposed in Prometheus metrics.
128-
if len(c.ServedModelNames) == 0 || c.ServedModelNames[0] == "" {
128+
if len(c.ServedModelNames) == 0 {
129129
c.ServedModelNames = []string{c.Model}
130130
}
131131

‎pkg/llm-d-inference-sim/config_test.go‎

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ import (
2424
"k8s.io/klog/v2"
2525
)
2626

27+
const qwenModelName = "Qwen/Qwen2-0.5B"
28+
2729
func createSimConfig(args []string) (*configuration, error) {
2830
oldArgs := os.Args
2931
defer func() {
@@ -66,7 +68,7 @@ var _ = Describe("Simulator configuration", func() {
6668
// Config from config.yaml file
6769
c = newConfig()
6870
c.Port = 8001
69-
c.Model = "Qwen/Qwen2-0.5B"
71+
c.Model = qwenModelName
7072
c.ServedModelNames = []string{"model1", "model2"}
7173
c.MaxLoras = 2
7274
c.MaxCPULoras = 5
@@ -128,7 +130,7 @@ var _ = Describe("Simulator configuration", func() {
128130
"{\"name\":\"lora3\",\"path\":\"/path/to/lora3\"}",
129131
}
130132
test = testCase{
131-
name: "config file with command line args",
133+
name: "config file with command line args with different format",
132134
args: []string{"cmd", "--model", model, "--config", "../../manifests/config.yaml", "--port", "8002",
133135
"--served-model-name",
134136
"--lora-modules={\"name\":\"lora3\",\"path\":\"/path/to/lora3\"}",
@@ -153,7 +155,7 @@ var _ = Describe("Simulator configuration", func() {
153155
"{\"name\":\"lora3\",\"path\":\"/path/to/lora3\"}",
154156
}
155157
test = testCase{
156-
name: "config file with command line args",
158+
name: "config file with command line args with empty string",
157159
args: []string{"cmd", "--model", model, "--config", "../../manifests/config.yaml", "--port", "8002",
158160
"--served-model-name", "",
159161
"--lora-modules", "{\"name\":\"lora3\",\"path\":\"/path/to/lora3\"}",
@@ -162,6 +164,44 @@ var _ = Describe("Simulator configuration", func() {
162164
}
163165
tests = append(tests, test)
164166

167+
// Config from config.yaml file plus command line args with empty string for loras
168+
c = newConfig()
169+
c.Port = 8001
170+
c.Model = qwenModelName
171+
c.ServedModelNames = []string{"model1", "model2"}
172+
c.MaxLoras = 2
173+
c.MaxCPULoras = 5
174+
c.MaxNumSeqs = 5
175+
c.TimeToFirstToken = 2
176+
c.InterTokenLatency = 1
177+
c.LoraModules = []loraModule{}
178+
c.LoraModulesString = []string{}
179+
test = testCase{
180+
name: "config file with command line args with empty string for loras",
181+
args: []string{"cmd", "--config", "../../manifests/config.yaml", "--lora-modules", ""},
182+
expectedConfig: c,
183+
}
184+
tests = append(tests, test)
185+
186+
// Config from config.yaml file plus command line args with empty parameter for loras
187+
c = newConfig()
188+
c.Port = 8001
189+
c.Model = qwenModelName
190+
c.ServedModelNames = []string{"model1", "model2"}
191+
c.MaxLoras = 2
192+
c.MaxCPULoras = 5
193+
c.MaxNumSeqs = 5
194+
c.TimeToFirstToken = 2
195+
c.InterTokenLatency = 1
196+
c.LoraModules = []loraModule{}
197+
c.LoraModulesString = []string{}
198+
test = testCase{
199+
name: "config file with command line args with empty parameter for loras",
200+
args: []string{"cmd", "--config", "../../manifests/config.yaml", "--lora-modules"},
201+
expectedConfig: c,
202+
}
203+
tests = append(tests, test)
204+
165205
// Invalid configurations
166206
test = testCase{
167207
name: "invalid model",
@@ -205,17 +245,19 @@ var _ = Describe("Simulator configuration", func() {
205245
Entry(tests[2].name, tests[2].args, tests[2].expectedConfig),
206246
Entry(tests[3].name, tests[3].args, tests[3].expectedConfig),
207247
Entry(tests[4].name, tests[4].args, tests[4].expectedConfig),
248+
Entry(tests[5].name, tests[5].args, tests[5].expectedConfig),
249+
Entry(tests[6].name, tests[6].args, tests[6].expectedConfig),
208250
)
209251

210252
DescribeTable("invalid configurations",
211253
func(args []string) {
212254
_, err := createSimConfig(args)
213255
Expect(err).To(HaveOccurred())
214256
},
215-
Entry(tests[5].name, tests[5].args),
216-
Entry(tests[6].name, tests[6].args),
217257
Entry(tests[7].name, tests[7].args),
218258
Entry(tests[8].name, tests[8].args),
219259
Entry(tests[9].name, tests[9].args),
260+
Entry(tests[10].name, tests[10].args),
261+
Entry(tests[11].name, tests[11].args),
220262
)
221263
})

‎pkg/llm-d-inference-sim/simulator.go‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ func (s *VllmSimulator) parseCommandParamsAndLoadConfig() error {
143143
servedModelNames := getParamValueFromArgs("served-model-name")
144144
loraModuleNames := getParamValueFromArgs("lora-modules")
145145

146-
f := pflag.NewFlagSet("llm-d-inference-sim flags", pflag.ExitOnError)
146+
f := pflag.NewFlagSet("llm-d-inference-sim flags", pflag.ContinueOnError)
147147

148148
f.IntVar(&config.Port, "port", config.Port, "Port")
149149
f.StringVar(&config.Model, "model", config.Model, "Currently 'loaded' model")
@@ -157,12 +157,14 @@ func (s *VllmSimulator) parseCommandParamsAndLoadConfig() error {
157157
f.Int64Var(&config.Seed, "seed", config.Seed, "Random seed for operations (if not set, current Unix time in nanoseconds is used)")
158158

159159
// These values were manually parsed above in getParamValueFromArgs, we leave this in order to get these flags in --help
160-
var servedModelNameStrings multiString
161-
f.Var(&servedModelNameStrings, "served-model-name", "Model names exposed by the API (a list of space-separated strings)")
162-
var configFile string
163-
f.StringVar(&configFile, "config", "", "The configuration file")
164-
var loras multiString
165-
f.Var(&loras, "lora-modules", "List of LoRA adapters (a list of space-separated JSON strings)")
160+
var dummyString string
161+
f.StringVar(&dummyString, "config", "", "The configuration file")
162+
var dummyMultiString multiString
163+
f.Var(&dummyMultiString, "served-model-name", "Model names exposed by the API (a list of space-separated strings)")
164+
f.Var(&dummyMultiString, "lora-modules", "List of LoRA adapters (a list of space-separated JSON strings)")
165+
// In order to allow empty arguments, we set a dummy NoOptDefVal for these flags
166+
f.Lookup("served-model-name").NoOptDefVal = "dummy"
167+
f.Lookup("lora-modules").NoOptDefVal = "dummy"
166168

167169
flagSet := flag.NewFlagSet("simFlagSet", flag.ExitOnError)
168170
klog.InitFlags(flagSet)
@@ -208,7 +210,9 @@ func getParamValueFromArgs(param string) []string {
208210
if strings.HasPrefix(arg, "--") {
209211
break
210212
}
211-
values = append(values, arg)
213+
if arg != "" {
214+
values = append(values, arg)
215+
}
212216
} else {
213217
if arg == "--"+param {
214218
readValues = true

0 commit comments

Comments
 (0)