InvokeAI
InvokeAI copied to clipboard
feat(nodes): skip on duplicate loras instead of erroring
Summary
The LoRA and SDXL LoRA nodes would error if it duplicated LoRAs. To make the nodes more resilient, they now skip dupes and log a warning to the console instead.
Also added a warning for the LoRA Collection Loader nodes. These already skipped but didn't log a warning.
Related Issues / Discussions
https://discord.com/channels/1020123559063990373/1130288930319761428/1266235074647691335
QA Instructions
This workflow has 4 test flows, each with 3 LoRA loaders:
- SDXL LoRA chain
- SDXL LoRA collection
- SD1.5 LoRA chain
- SD1.5 LoRA collection
For each flow, set two of the loaders to the same LoRA and the 3rd to a different. The output for each flow should have only two LoRAs, and you should have warnings in the console for each.
If you test this on main, we expect errors for both the chain flows.
Merge Plan
n/a
Checklist
- [x] The PR has a short but descriptive title, suitable for a changelog
- [ ] Tests added / updated (if applicable)
- [ ] Documentation added / updated (if applicable)
Discussed with @StAlKeR7779 - in some cases, skipping duplicates may not be correct - for example, we may want to let the last instance of a LoRA be used, or sum the weights.
Maybe we add an enum field - on_conflict or duplicate_handling, something like that - to describe how to handle duplicate LoRAs. Valid options could be 'skip' | 'replace' | 'error' | 'sum_weights'... 'max' | 'min' | 'average'... Probably overkill.
Also, 'skip' and 'replace' are ambiguous for the collection loader, because we don't know the order in which the collect node does its collecting. It's not clear which would be taken of duplicates provided to the collect.
Of all the possible duplicate handling methods, error still makes the most sense to me as the default.
Maybe we start with min, max, error? That would handle most cases I can think of.
Some context from the problem report. They were using this node:
It outputs unet and clip fields with the loras baked in.
I feel that most cases will be closed by:
error- error if lora applied beforeskip- use already applied weightreplace/override- use new lora weightsum/merge- sum old and new weight
For example case with metadata node should be closed by replace/override option, as it allow to select weight of details lora independent of what used for original image generation.
For default logic I feel that should be sum/merge as it do exactly what applying lora means - patch what provided, or error to notify user about double use of lora.
About skip - I not sure in which cases it should fit.