From 600152df2344d8ae04d3782534c5312ee535f43c Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Fri, 22 Mar 2024 20:55:11 +0100 Subject: [PATCH] fix(config): pass by config options, respect defaults (#1878) This bug had the unpleasant effect that it ignored defaults passed by the CLI. For instance threads could be changed only via model config file. --- core/config/application_config.go | 15 +++++++++++++++ core/config/backend_config.go | 25 ++++++++++++++----------- core/startup/startup.go | 6 ++++-- 3 files changed, 33 insertions(+), 13 deletions(-) diff --git a/core/config/application_config.go b/core/config/application_config.go index f25b4348..03242c3c 100644 --- a/core/config/application_config.go +++ b/core/config/application_config.go @@ -258,6 +258,21 @@ func WithApiKeys(apiKeys []string) AppOption { } } +// ToConfigLoaderOptions returns a slice of ConfigLoader Option. +// Some options defined at the application level are going to be passed as defaults for +// all the configuration for the models. +// This includes for instance the context size or the number of threads. +// If a model doesn't set configs directly to the config model file +// it will use the defaults defined here. +func (o *ApplicationConfig) ToConfigLoaderOptions() []ConfigLoaderOption { + return []ConfigLoaderOption{ + LoadOptionContextSize(o.ContextSize), + LoadOptionDebug(o.Debug), + LoadOptionF16(o.F16), + LoadOptionThreads(o.Threads), + } +} + // func WithMetrics(meter *metrics.Metrics) AppOption { // return func(o *StartupOptions) { // o.Metrics = meter diff --git a/core/config/backend_config.go b/core/config/backend_config.go index daaf0257..32e10a17 100644 --- a/core/config/backend_config.go +++ b/core/config/backend_config.go @@ -188,7 +188,14 @@ func (c *BackendConfig) FunctionToCall() string { return c.functionCallNameString } -func (cfg *BackendConfig) SetDefaults(debug bool, threads, ctx int, f16 bool) { +func (cfg *BackendConfig) SetDefaults(opts ...ConfigLoaderOption) { + lo := &LoadOptions{} + lo.Apply(opts...) + + ctx := lo.ctxSize + threads := lo.threads + f16 := lo.f16 + debug := lo.debug defaultTopP := 0.7 defaultTopK := 80 defaultTemp := 0.9 @@ -333,9 +340,6 @@ func (lo *LoadOptions) Apply(options ...ConfigLoaderOption) { // Load a config file for a model func (cl *BackendConfigLoader) LoadBackendConfigFileByName(modelName, modelPath string, opts ...ConfigLoaderOption) (*BackendConfig, error) { - lo := &LoadOptions{} - lo.Apply(opts...) - // Load a config file if present after the model name cfg := &BackendConfig{ PredictionOptions: schema.PredictionOptions{ @@ -350,7 +354,9 @@ func (cl *BackendConfigLoader) LoadBackendConfigFileByName(modelName, modelPath // Try loading a model config file modelConfig := filepath.Join(modelPath, modelName+".yaml") if _, err := os.Stat(modelConfig); err == nil { - if err := cl.LoadBackendConfig(modelConfig); err != nil { + if err := cl.LoadBackendConfig( + modelConfig, opts..., + ); err != nil { return nil, fmt.Errorf("failed loading model config (%s) %s", modelConfig, err.Error()) } cfgExisting, exists = cl.GetBackendConfig(modelName) @@ -360,7 +366,7 @@ func (cl *BackendConfigLoader) LoadBackendConfigFileByName(modelName, modelPath } } - cfg.SetDefaults(lo.debug, lo.threads, lo.ctxSize, lo.f16) + cfg.SetDefaults(opts...) return cfg, nil } @@ -371,9 +377,6 @@ func NewBackendConfigLoader() *BackendConfigLoader { } } func ReadBackendConfigFile(file string, opts ...ConfigLoaderOption) ([]*BackendConfig, error) { - lo := &LoadOptions{} - lo.Apply(opts...) - c := &[]*BackendConfig{} f, err := os.ReadFile(file) if err != nil { @@ -384,7 +387,7 @@ func ReadBackendConfigFile(file string, opts ...ConfigLoaderOption) ([]*BackendC } for _, cc := range *c { - cc.SetDefaults(lo.debug, lo.threads, lo.ctxSize, lo.f16) + cc.SetDefaults(opts...) } return *c, nil @@ -403,7 +406,7 @@ func ReadBackendConfig(file string, opts ...ConfigLoaderOption) (*BackendConfig, return nil, fmt.Errorf("cannot unmarshal config file: %w", err) } - c.SetDefaults(lo.debug, lo.threads, lo.ctxSize, lo.f16) + c.SetDefaults(opts...) return c, nil } diff --git a/core/startup/startup.go b/core/startup/startup.go index 43e6646d..828eb7a7 100644 --- a/core/startup/startup.go +++ b/core/startup/startup.go @@ -58,12 +58,14 @@ func Startup(opts ...config.AppOption) (*config.BackendConfigLoader, *model.Mode cl := config.NewBackendConfigLoader() ml := model.NewModelLoader(options.ModelPath) - if err := cl.LoadBackendConfigsFromPath(options.ModelPath); err != nil { + configLoaderOpts := options.ToConfigLoaderOptions() + + if err := cl.LoadBackendConfigsFromPath(options.ModelPath, configLoaderOpts...); err != nil { log.Error().Msgf("error loading config files: %s", err.Error()) } if options.ConfigFile != "" { - if err := cl.LoadBackendConfigFile(options.ConfigFile); err != nil { + if err := cl.LoadBackendConfigFile(options.ConfigFile, configLoaderOpts...); err != nil { log.Error().Msgf("error loading config file: %s", err.Error()) } }