fix(gallery): persist inference defaults where the loader reads them - #11232
fix(gallery): persist inference defaults where the loader reads them#11232jimmykarily wants to merge 1 commit into
Conversation
localai-org-maint-bot
left a comment
localai-org-maint-bot
left a comment
There was a problem hiding this comment.
The code change looks focused, but the commit trailer needs to follow this repository's AI-assistance policy before merge. Please replace:
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
with an Assisted-by: trailer in the format documented in .agents/ai-coding-assistants.md, for example:
Assisted-by: Claude:claude-opus-5
Keep your human Signed-off-by: trailer when amending the commit.
I also attempted go test ./core/gallery -run TestGallery -count=1; it is currently blocked before this package compiles by the missing generated pb.DetokenizeRequest / pb.DetokenizeResponse symbols that are also present on current master, so I am not attributing that failure to this PR.
45b6b10 to
91703ee
Compare
The recommended sampling parameters for a model family were applied at install and then never took effect. Two things went wrong on the way to disk. They were written as top level keys. ModelConfig embeds PredictionOptions under the "parameters" yaml key, so temperature, top_p, top_k, min_p, repeat_penalty and presence_penalty are only read from there. At the top level they parse without error and are then ignored for the life of the model. They were also merged in after the YAML had already been marshalled. The only re-marshal sat behind the artifact binding, which an entry carrying files: never reaches, so for those entries the defaults were computed and then dropped before anything was written. Neither failure was visible in normal use. ApplyInferenceDefaults runs again at load time and fills the same values from the same table, so the model ends up tuned correctly while the file on disk pins nothing. It surfaces when someone edits one of those values expecting it to win, or when a family is absent from inference_defaults.json and there is nothing to refill from. Both install paths are covered: an entry carrying files:, and one that binds a primary artifact instead. The empty base spec asserted that the authored parameters block landed verbatim. It now checks the authored keys individually, because the family defaults are merged into that same block. Assisted-by: Claude:claude-opus-5 Signed-off-by: Dimitris Karakasilis <dimitris@karakasilis.me>
91703ee to
13dc34f
Compare
localai-org-maint-bot
left a comment
localai-org-maint-bot
left a comment
There was a problem hiding this comment.
The amended commit now follows the repository AI-assistance policy, retains the human DCO sign-off, and remains focused with regression coverage for both install paths. git diff --check passes. The focused gallery test is currently blocked by missing generated UpscaleImage protobuf symbols, but the identical failure reproduces on exact current master, so it is not introduced here. Good to merge, @mudler.
2 participants
The recommended sampling parameters for a model family were applied at install and then never took effect. Two things went wrong on the way to disk.
They were written as top level keys. ModelConfig embeds PredictionOptions under the "parameters" yaml key, so temperature, top_p, top_k, min_p, repeat_penalty and presence_penalty are only read from there. At the top level they parse without error and are then ignored for the life of the model.
They were also merged in after the YAML had already been marshalled. The only re-marshal sat behind the artifact binding, which an entry carrying files: never reaches, so for those entries the defaults were computed and then dropped before anything was written.
Neither failure was visible in normal use. ApplyInferenceDefaults runs again at load time and fills the same values from the same table, so the model ends up tuned correctly while the file on disk pins nothing. It surfaces when someone edits one of those values expecting it to win, or when a family is absent from inference_defaults.json and there is nothing to refill from.
The empty base spec asserted that the authored parameters block landed verbatim. It now checks the authored keys individually, because the family defaults are merged into that same block.
Fixes #11230
Signed commits