diff --git a/core/gallery/empty_base_install_test.go b/core/gallery/empty_base_install_test.go index 6b104e233f02..b2a15c1ea5a0 100644 --- a/core/gallery/empty_base_install_test.go +++ b/core/gallery/empty_base_install_test.go @@ -236,8 +236,15 @@ var _ = Describe("InstallModelFromGallery with an empty base config", func() { Expect(install(e.Name, gallery.GalleryModel{})).To(Succeed()) cfg := installedConfig(e.Name) Expect(cfg["name"]).To(Equal(e.Name)) - // The catalog's own overrides, verbatim, laid over the empty base. - Expect(cfg["parameters"]).To(Equal(e.Overrides["parameters"])) + // The catalog's own overrides, laid over the empty base. parameters is + // checked key by key rather than as a whole map: the install also merges + // the model family's inference defaults into it, and what matters here is + // that the authored keys survive that. + authored, ok := e.Overrides["parameters"].(map[string]any) + Expect(ok).To(BeTrue()) + for key, want := range authored { + Expect(cfg["parameters"]).To(HaveKeyWithValue(key, want)) + } Expect(cfg["known_usecases"]).To(Equal(e.Overrides["known_usecases"])) }) }) diff --git a/core/gallery/inference_defaults_install_test.go b/core/gallery/inference_defaults_install_test.go new file mode 100644 index 000000000000..11f46bc42d68 --- /dev/null +++ b/core/gallery/inference_defaults_install_test.go @@ -0,0 +1,189 @@ +package gallery_test + +import ( + "context" + "fmt" + "maps" + "os" + "path/filepath" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "gopkg.in/yaml.v3" + + "github.com/mudler/LocalAI/core/config" + "github.com/mudler/LocalAI/core/gallery" + "github.com/mudler/LocalAI/pkg/modelartifacts" + "github.com/mudler/LocalAI/pkg/system" +) + +// The recommended sampling parameters for a model family are applied at install +// and persisted into the model YAML. Persisting them is only worth anything if +// they are written where the loader reads them back: PredictionOptions is nested +// under "parameters" in ModelConfig, so a top level "temperature" key parses +// without error and is then ignored for the life of the model. +// +// The expected values are read from the family table rather than written out +// here, so that retuning a family stays a one file change. +// +// Nothing here reaches the network. +var _ = Describe("Inference defaults persisted at install", func() { + var tempdir string + var galleries []config.Gallery + var systemState *system.SystemState + // The gallery listing is cached on the name and URL pair, so every spec + // needs a gallery of its own or it reads the previous spec's catalog. + galleryRevision := 0 + + // The name has to contain a pattern from inference_defaults.json, otherwise + // no defaults are applied and every assertion below passes vacuously. + const modelName = "qwen3.5-install-defaults" + + newGallery := func(entries ...gallery.GalleryModel) { + out, err := yaml.Marshal(entries) + Expect(err).ToNot(HaveOccurred()) + name := fmt.Sprintf("inference-defaults-%d", galleryRevision) + galleryRevision++ + galleryPath := filepath.Join(tempdir, name+".yaml") + Expect(os.WriteFile(galleryPath, out, 0600)).To(Succeed()) + galleries = []config.Gallery{{Name: name, URL: "file://" + galleryPath}} + } + + install := func(name string) error { + return gallery.InstallModelFromGallery( + context.TODO(), galleries, []config.Gallery{}, systemState, nil, + name, gallery.GalleryModel{}, func(string, string, string, float64) {}, false, false, false) + } + + installedConfig := func(name string) map[string]any { + dat, err := os.ReadFile(filepath.Join(tempdir, name+".yaml")) + Expect(err).ToNot(HaveOccurred()) + content := map[string]any{} + Expect(yaml.Unmarshal(dat, &content)).To(Succeed()) + return content + } + + // Seeding the weights keeps the install off the network: the downloader + // treats an already-present destination with no declared sha256 as fetched. + // extra goes into parameters:, so a spec can pin a value the defaults would + // otherwise supply. + seedGallery := func(extra map[string]any) { + Expect(os.WriteFile(filepath.Join(tempdir, "weights.gguf"), []byte("weights"), 0600)).To(Succeed()) + + params := map[string]any{"model": "weights.gguf"} + maps.Copy(params, extra) + + e := gallery.GalleryModel{Overrides: map[string]any{ + "backend": "llama-cpp", + "parameters": params, + }} + e.Name = modelName + e.AdditionalFiles = []gallery.File{{Filename: "weights.gguf", URI: "https://example.com/weights.gguf"}} + newGallery(e) + } + + // Guards the fixture itself. If the name stops matching a family the specs + // below would still pass while asserting nothing at all. + expectedFamily := func() map[string]float64 { + family := config.MatchModelFamily(modelName) + Expect(family).ToNot(BeEmpty(), "fixture name no longer matches a family in inference_defaults.json") + return family + } + + BeforeEach(func() { + var err error + tempdir, err = os.MkdirTemp("", "inference-defaults-install") + Expect(err).ToNot(HaveOccurred()) + DeferCleanup(func() { Expect(os.RemoveAll(tempdir)).To(Succeed()) }) + + systemState, err = system.GetSystemState(system.WithModelPath(tempdir)) + Expect(err).ToNot(HaveOccurred()) + }) + + It("writes them under parameters, where the loader reads them back", func() { + family := expectedFamily() + seedGallery(nil) + + Expect(install(modelName)).To(Succeed()) + + params, ok := installedConfig(modelName)["parameters"].(map[string]any) + Expect(ok).To(BeTrue(), "parameters should be a map") + + for key, want := range family { + Expect(params).To(HaveKey(key)) + Expect(params[key]).To(BeNumerically("==", want), "parameters.%s", key) + } + }) + + It("does not leave them at the top level, where they are ignored", func() { + family := expectedFamily() + seedGallery(nil) + + Expect(install(modelName)).To(Succeed()) + + cfg := installedConfig(modelName) + for key := range family { + Expect(cfg).ToNot(HaveKey(key), "%s at the top level is never read", key) + } + }) + + It("leaves a value the entry already sets alone", func() { + family := expectedFamily() + Expect(family).To(HaveKey("temperature")) + Expect(family["temperature"]).ToNot(BeNumerically("==", 0.05), "pick a value the family does not use") + + seedGallery(map[string]any{"temperature": 0.05}) + + Expect(install(modelName)).To(Succeed()) + + params, ok := installedConfig(modelName)["parameters"].(map[string]any) + Expect(ok).To(BeTrue(), "parameters should be a map") + Expect(params["temperature"]).To(BeNumerically("==", 0.05)) + }) + + // An entry that binds a primary artifact carries no files: of its own, so it + // takes the other branch of the install and none of the specs above reach it. + // It is also the one branch that already re-marshalled, which is why the + // defaults did land on disk there, at the top level where nothing reads them. + It("writes them under parameters on the artifact binding path too", func() { + family := expectedFamily() + + definition := &gallery.ModelConfig{ConfigFile: ` +backend: transformers +artifacts: + - name: model + target: model + source: + type: huggingface + repo: owner/repo +parameters: + model: owner/repo +`} + // Standing in for the materializer keeps the install off the network. + materializer := &fakeArtifactMaterializer{result: modelartifacts.Result{ + Spec: modelartifacts.Spec{ + Name: "model", Target: "model", + Source: modelartifacts.Source{Type: "huggingface", Repo: "owner/repo", Revision: "main"}, + Resolved: &modelartifacts.Resolved{ + Endpoint: "https://huggingface.co", + Revision: "0123456789abcdef0123456789abcdef01234567", + CacheKey: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + }, + }, + RelativePath: ".artifacts/huggingface/0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef/snapshot", + }} + + _, err := gallery.InstallModel(context.TODO(), systemState, modelName, definition, nil, nil, false, + gallery.WithArtifactMaterializer(materializer)) + Expect(err).ToNot(HaveOccurred()) + + cfg := installedConfig(modelName) + params, ok := cfg["parameters"].(map[string]any) + Expect(ok).To(BeTrue(), "parameters should be a map") + for key, want := range family { + Expect(params).To(HaveKey(key)) + Expect(params[key]).To(BeNumerically("==", want), "parameters.%s", key) + Expect(cfg).ToNot(HaveKey(key), "%s at the top level is never read", key) + } + }) +}) diff --git a/core/gallery/models.go b/core/gallery/models.go index f3184648b15e..190ed215943c 100644 --- a/core/gallery/models.go +++ b/core/gallery/models.go @@ -622,35 +622,51 @@ func InstallModel(ctx context.Context, systemState *system.SystemState, nameOver lconfig.ApplyInferenceDefaults(&modelConfig, name, modelConfig.Model) // Merge inference defaults into configMap so they are persisted without losing unknown fields. - if modelConfig.Temperature != nil { - if _, exists := configMap["temperature"]; !exists { - configMap["temperature"] = *modelConfig.Temperature + // They belong under "parameters": ModelConfig embeds PredictionOptions with + // that yaml key, so a top level "temperature" parses without error and is + // then ignored for the life of the model. + params, mergeable := configMap["parameters"].(map[string]any) + if configMap["parameters"] == nil { + params, mergeable = map[string]any{}, true + } + if mergeable { + // An entry that sets one of these keeps its own value. ApplyInferenceDefaults + // already skipped those fields; this keeps the write side symmetric. + setDefault := func(key string, value any) { + if _, exists := params[key]; !exists { + params[key] = value + } } - } - if modelConfig.TopP != nil { - if _, exists := configMap["top_p"]; !exists { - configMap["top_p"] = *modelConfig.TopP + if modelConfig.Temperature != nil { + setDefault("temperature", *modelConfig.Temperature) } - } - if modelConfig.TopK != nil { - if _, exists := configMap["top_k"]; !exists { - configMap["top_k"] = *modelConfig.TopK + if modelConfig.TopP != nil { + setDefault("top_p", *modelConfig.TopP) } - } - if modelConfig.MinP != nil { - if _, exists := configMap["min_p"]; !exists { - configMap["min_p"] = *modelConfig.MinP + if modelConfig.TopK != nil { + setDefault("top_k", *modelConfig.TopK) } - } - if modelConfig.RepeatPenalty != 0 { - if _, exists := configMap["repeat_penalty"]; !exists { - configMap["repeat_penalty"] = modelConfig.RepeatPenalty + if modelConfig.MinP != nil { + setDefault("min_p", *modelConfig.MinP) } - } - if modelConfig.PresencePenalty != 0 { - if _, exists := configMap["presence_penalty"]; !exists { - configMap["presence_penalty"] = modelConfig.PresencePenalty + if modelConfig.RepeatPenalty != 0 { + setDefault("repeat_penalty", modelConfig.RepeatPenalty) } + if modelConfig.PresencePenalty != 0 { + setDefault("presence_penalty", modelConfig.PresencePenalty) + } + if len(params) > 0 { + configMap["parameters"] = params + } + } + + // The marshal above predates this merge, and the only other re-marshal is + // behind the artifact binding below, which an entry carrying files: never + // reaches. Without this the defaults are computed and then dropped on the + // way to disk. + updatedConfigYAML, err = yaml.Marshal(configMap) + if err != nil { + return nil, fmt.Errorf("failed to marshal config with inference defaults: %v", err) } if valid, err := modelConfig.Validate(); !valid {