LucianoCirino / efficiency-nodes-comfyui

A collection of ComfyUI custom nodes. ⚠️ WARNING: This repo is no longer maintained.
https://civitai.com/models/32342
GNU General Public License v3.0
601 stars 154 forks source link

Loader not working with SDXL #63

Open cavit99 opened 1 year ago

cavit99 commented 1 year ago
Screenshot 2023-07-28 at 10 42 07 Screenshot 2023-07-28 at 10 41 58
lhovav commented 1 year ago

Same with me

LucianoCirino commented 1 year ago

Will investigate

LucianoCirino commented 1 year ago

This should now be fixed by comit https://github.com/LucianoCirino/efficiency-nodes-comfyui/commit/1ed9e0e1a2293a57930ea055f98f306589759317. Please test and close if good :)

image

lhovav commented 1 year ago

Hi, @LucianoCirino thanks for the quick fix! although I didn't open the ticket I can confirm it does fix the issue, But I'm getting this error when trying to use the refiner in a 2nd pass: image

image

Probably something that I'm doing wrong..

Pat-Petereit commented 1 year ago

Can confirm, i have the same error. It doesnt matter if i use a efficent or default sampler in the second step. No problem if i use a defualt ksampler as first step and efficient ksampler as second Bugjson.zip

Attached a workflow to recreate (needs to change path of ckpt). Error only happens with refiner, no problem with second pass of base

cavit99 commented 1 year ago

This should now be fixed by comit 1ed9e0e. Please test and close if good :)

image

Thanks so much for attending to it. Any chance refiner can also be fixed on 2nd pass so we can close? Love your work

tkoenig89 commented 1 year ago

Hi, @LucianoCirino thanks for the quick fix! although I didn't open the ticket I can confirm it does fix the issue, But I'm getting this error when trying to use the refiner in a 2nd pass: image

image

Probably something that I'm doing wrong..

@lhovav I had this issue already, i figured out that you cannot use the conditioning from the base model, with the refiner sampler. You need to encode the prompts with the clip from the refiner to make the refiner-sampler work. Hope this helps.

tkoenig89 commented 1 year ago

fixedjson.zip image

Here is your workflox, but fixed. But as i understand this is not really what the 'original' SDXL-comfy workflow does. It somehow leaves noise in the image generated from the base model, and letting the refiner finish it off. Here we just finish something with the base, than add new noise before refining. But i guess this is not a topic for this bug, as the issue itself is solved - i guess.

lhovav commented 1 year ago

Hi @tkoenig89 Thanks for the help! Can confirm that it works the way you suggested, I still don't know if its something that can be fixed (or should) - it feels more efficient to continue the conditions from one efficient loader, But obviously its easy to say than done... Thanks again!

LucianoCirino commented 1 year ago

I was unable to replicate error. Maybe I fixed it accidently with one of my recent commits. Unless I am doing something wrong?

image tsc_test1.zip

lhovav commented 1 year ago

Hi @LucianoCirino , Thanks for taking care of it. In your example you are using 3 Ksamplers, in mine, I used only 2. Full work flow: image

If you use this work flow you will likely get this error: image

Thanks!

tkoenig89 commented 1 year ago

@LucianoCirino i dont't think you fixed it, because it might not be a bug, but how SDXL works. Your workflow seems (and i'm not really a pro here :) ) flawed in the regards, that you use the base model in your refiner sampler (flows through the first sampler to the refiner sampler). Thats the reason why you don't get the 'mat and mat2' error. If you would use the loaded refiner model, you will get the same error, as the encoded prompts (CONDITIONINGS) will not work with the refiner. I guess there are same matrixes (mat1, mat2) that have different dimension for base and refiner. I find it hard to explain, as this is something i did not find anywhere else and just 'made' up in my own mind :D.

I tried to explain this in one of my post above. If you want to use the refiner the 'SDXL' way, you may need to extend your EfficientSampler so you can enable 'leftover noise' in the base, and disable 'add noise' on the refiner sampler (see comfy examples for SDXL: https://comfyanonymous.github.io/ComfyUI_examples/sdxl/).

One thing i found bad about this SDXL workflow at the moment: you need to provide the prompts twice for the correct encodings to happen.

lhovav commented 1 year ago

did he use the refiner? the models in @LucianoCirino workflow is only the base model...

tkoenig89 commented 1 year ago

you are right. My brain tricked me in seeing the refiner checkpoint in the top loader, for some reason. Maybe because i had the refiner in there for my workflow. But there is still the point about, how to do the SDXL workflow with the efficient nodes.

LucianoCirino commented 1 year ago

@LucianoCirino i dont't think you fixed it, because it might not be a bug, but how SDXL works. Your workflow seems (and i'm not really a pro here :) ) flawed in the regards, that you use the base model in your refiner sampler (flows through the first sampler to the refiner sampler). Thats the reason why you don't get the 'mat and mat2' error. If you would use the loaded refiner model, you will get the same error, as the encoded prompts (CONDITIONINGS) will not work with the refiner. I guess there are same matrixes (mat1, mat2) that have different dimension for base and refiner. I find it hard to explain, as this is something i did not find anywhere else and just 'made' up in my own mind :D.

I tried to explain this in one of my post above. If you want to use the refiner the 'SDXL' way, you may need to extend your EfficientSampler so you can enable 'leftover noise' in the base, and disable 'add noise' on the refiner sampler (see comfy examples for SDXL: https://comfyanonymous.github.io/ComfyUI_examples/sdxl/).

One thing i found bad about this SDXL workflow at the moment: you need to provide the prompts twice for the correct encodings to happen.

I apologize for the misunderstanding and now see the issue.

image

Summary: To have the refiner ksampler step to work, you you must encode your text conditioning to the refiner using the refiner's clip.

I am now closing this topic.

tkoenig89 commented 1 year ago

@LucianoCirino not sure if this is just me, but after i reinstalled comfy ui on a different disk and installed efficient nodes, i get the pooled_output error again. This seems related to the loader, as using the normal comfy nodes just works fine with SDXL checkpoints. Maybe the fix, that worked before has been overriden by a different change/fix?

LucianoCirino commented 1 year ago

Updated repo, Efficiency nodes now support a simpliefied SDXL workflow. XY Plot compatible aswell.