-
-
Notifications
You must be signed in to change notification settings - Fork 326
Feature/model parameter overrides #1109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Feature/model parameter overrides #1109
Conversation
…oint); Add lora preset capability;
Fix where preset without a description leads to substantial whitespace between the preset title and settings
| function applyOnePreset(preset) { | ||
| function applyOnePreset(preset, isNestedPreset = false) { | ||
| // Collect LoRA updates before applying to handle merging | ||
| let presetLoras = preset.param_map.loras ? preset.param_map.loras.split(',').map(l => l.trim()).filter(l => l.length > 0) : []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imo any changes to how loras are handled in presets is out-of-scope and should be a separate followup. This is already a very big PR, try to keep it focused to the core function.
| * @param {string} itemName - The full model/LoRA name (path or filename) | ||
| * @returns {string} The base filename without path or extension | ||
| */ | ||
| function extractItemKeyNameFromPath(itemName) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get rid of this, no. bad.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What specifically are you concerned about here?
My concern was that as I reorganize my models, I don't want them to lose their presets just because they moved to a new folder path. That wasn't an issue when it was custom metadata, but as a user setting, need a durable key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maintaining model list vs user list sync is... an open question to be discussed, but the solution is definitely not this.
| public string ImageShiftingCycles = "true"; | ||
|
|
||
| [ConfigComment("When a model with a linked preset is selected, should the preset parameters be applied immediately? By default, the preset is just selected for review.")] | ||
| public bool AutoApplyModelPresets = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd probably make more sense for this to be a setting on each Preset rather than a global user setting. Some presets should autoapply, others shouldn't. The functionality would probably be beneficial for all presets rather than specific to model. This should aaalso probably be a separate PR.
As discussed in Discord, this allows users to select a preset for any model or LoRA. The preset is added when the model/LoRA are selected. These are stored as user settings.
For models, the preset is unselected (if not applied already) if the user clicks another model that has a different preset.
Documentation is included.
This adds two optional user settings as well:
This is my first deep trip into the code, so please let me know if I've accidentally re-implemented utility functions, etc. that already exist. I've tried to follow the conventions and style of this repo as much as I can, and to minimize the "blast radius" of the new code.