cheald / sd-webui-loractl

An Automatic1111 extension for dynamically controlling the weights of LoRAs during image generation
MIT License
239 stars 10 forks source link

`--medvram`: hires fix being enabled causes incorrect image to be generated, often causes NaN in VAE #12

Closed catboxanon closed 1 year ago

catboxanon commented 1 year ago

Basically as the title describes. I tested this on both 1.5.1 and the latest commit on the dev branch as of submitting this (https://github.com/AUTOMATIC1111/stable-diffusion-webui/commit/45be87afc6b173ebda94f427b8d396a2842ddf3c). This only happens with --medvram.

The first part of this issue is when hires fix is enabled, it causes the image generated to be vastly different, even with no denoising. The live preview makes it pretty obvious when you test this.

Without hires fix ![with](https://github.com/cheald/sd-webui-loractl/assets/122327233/b6b94c7c-5329-4ba8-bf94-290e6a6a5c1a)
With hires fix ![with2](https://github.com/cheald/sd-webui-loractl/assets/122327233/1dad2068-ac6c-48ca-9bcd-88e4dea96d0b)

The NaN issue is not always consistent and depends on seed, but it does seem to happen a lot. It can also occur when interrupting the first pass before hires fix.

Also notable, when enabling the option to plot the weights and hires fix is enabled, the value for a LoRA entered as <lora:name:1> instead shows a weight of 0. But when using something like a style LoRA, it's still pretty obvious it's being applied by the preview.

tmppcr_37d9

Model and LoRA used isn't relevant as this affects any of either. This was also tested with all other extensions disabled (besides the built-in).

cheald commented 1 year ago

Very interesting. My first inclination is that this is likely a bug in the webui itself. It looks like what's happening is that ExtraNetworkLora.activate is being called multiple times, but when medvram is set, rather than the parsed params string, we're instead getting the parameters fed back in that were set during the previous activate() pass. This causes loractl to discard the values it had parsed, and instead causes it to just set "0" (as that was the dummy value set to avoid breaking the superclass parser). This breaks idempotency for activate().

I haven't been able to reproduce the NaN issue, but it seems like it's likely related.

I've got a local fix which sets the dummy value to -1337, then doesn't reparse the weights if we get that as the first positional parameter. It seems to work, and should cause obvious errors if there are other cases where the dummy value is being used rather than the loractl overrides. Give af4bd51 a go and let me know if that seems to fix it.

catboxanon commented 1 year ago

That solved it -- thanks for looking into it!

I'm going to ping @KohakuBlueleaf on this because they might be able to look into this more in regards to the webui itself, but since this fixes the issue for this extension I'll close this now.