Forge was updated today - Errors on inference when Model Mixer isn't enabled

CCpt5 commented 2 months ago

Hey!

Forge lllyasviel was very kind and provided a much needed update to Forge today (including upgrading GRadio to 4.0). I installed MMixer from the installer and w/o even enabling it I've noticed errors in the console:

https://github.com/lllyasviel/stable-diffusion-webui-forge/commits/main/

---
To load target model SDXL
Begin to load 1 model
Reuse 1 loaded models
[Memory Management] Current Free GPU Memory (MB) =  15968.17822265625
[Memory Management] Model Memory (MB) =  0.0
[Memory Management] Minimal Inference Memory (MB) =  1024.0
[Memory Management] Estimated Remaining GPU Memory (MB) =  14944.17822265625
Moving model(s) has taken 0.02 seconds
100%|████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 22/22 [00:05<00:00,  3.97it/s]
*** Error running before_process: D:\stable-diffusion-webui-forge\extensions\sd-webui-model-mixer\scripts\model_mixer.py                                                                                                                           | 132/56100 [00:37<3:54:53,  3.97it/s]
    Traceback (most recent call last):
      File "D:\stable-diffusion-webui-forge\modules\scripts.py", line 836, in before_process
        script.before_process(p, *script_args)
    TypeError: ModelMixerScript.before_process() missing 6 required positional arguments: 'enabled', 'model_a', 'base_model', 'mm_max_models', 'mm_finetune', and 'mm_states'

wkpark commented 2 months ago

several gradio4 related bugs fixed. but the current forge seems too unstable.

There is a change that causes compatibility issues with the A1111 webui. It's better to wait until it's more stable and the compatibility issues have been resolved.

[ ] low level gradio component Generate button hooking will not work. - for example, AutoMerger does not work with the latest forge.

CCpt5 commented 2 months ago

Ok great, thanks for working on it! I'm sure it'll settle in at some point, i'll use A1111 or any merging at this point. Can close if you want or leave open. Thx.

CCpt5 commented 1 month ago

Developer lllyasviel (Forge) has been on an incredible coding run and it seems like he's getting it to the point of stable.

Curious about your thoughts on Flux merging? Should it be possible with 64(+) system ram?

wkpark commented 1 month ago

Developer lllyasviel (Forge) has been on an incredible coding run and it seems like he's getting it to the point of stable.

Curious about your thoughts on Flux merging? Should it be possible with 64(+) system ram?

current Forge system is incompatible with the original webui. (~~it looks like forge use newly adapted the comfyui's backend engine~~) so, model-mixer does not support Flux merging, currently.

SD1.x, SD2.x and SDXL is basically the same structure but SD3 and flux are totally different. so, webui specific features will not work.

The basic principle of merging models is simple, but getting the model mixer to work with flux seems to be not a simple task,

[ ] disable some webui specific features for flux or refactor to support all features.
- [ ] disable block level merge
- [x] or make another/modular flux specific block level merge routines.
- [ ] disable partial merge feature ... and so on.
- [x] or refactor to support partial merge

and about system memory,

SD1.x, SD2.x - read whole weights of Model-A into RAM, and also read model-B, C, D... into RAM sequantially.
for SDXL, whole model-A weights into RAM and open model-B,C,D ... using safetensors.torch.safe_open().

for FLUX, we can use safe_open() always to reduce ram usage.

inflatebot commented 1 month ago

Yeah, I wouldn't hold my breath WRT Forge becoming compatible with A1111 again, at least not for a long time. lllyasviel's been rewriting the whole thing, moving the codebase into a proper inheritance tree and yeeting the old LDM library into the stratosphere. I had to port NegPiP by hand. While it didn't take that long, it was tedious, because despite Forge currently having moved to the new backend already, there's not actually any documentation on it, and it's way more complex than I'm comfortable with. I basically had to print out the relevant objects' members one at a time, restarting Forge and trying a generation, until I could drill down to the parameters that matched the old ones what needed replacing. I have no idea why he didn't just provide a temporary mapping so that the extensions that survived the Gradio 4 transition didn't spontaneously explode. Or, y'know, do that work in a branch?

Anyways, if you're actually any good at Python unlike me, it shouldn't be that big of a problem. Don't mind me having a complain in your repo.

(They're not using ComfyUI as a backend by the way, that's a misconception.)

wkpark commented 1 month ago

yes. forge's backend not compatible with comyui at all. but many parts of forge engine seems based on comfy's work

comfy's ops.py -> forge's operations.py - naming is similar, overall structure are very similar but operations.py use @contextlib.contextmanager monkey patch method
comfy's memory_management.py -> forge's memory_management.py - looks like copy and pasted but modified a lot.
(originally I said, "it looks like" forge use "newly adapted" the comfyui's backend engine. its not the same "comfyui's engine" but I admit It could be misunderstood)

and thanks for your comments!

inflatebot commented 4 weeks ago

(originally I said, "it looks like" forge use "newly adapted" the comfyui's backend engine. its not the same "comfyui's engine" but I admit It could be misunderstood)

I see, it was me that misunderstood you, then. Apologies for that!

wkpark / sd-webui-model-mixer

Forge was updated today - Errors on inference when Model Mixer isn't enabled #161