fix: allow changing clip_skip back to its default value #687
Conversation
|
I don't think this fix is correct. It's breaking some models now. |
|
@LostRuins , could you please check if those models work with clip_skip explicitly set to 2? I did some additional tests, and I suspect this change ended up setting the default clip_skip value to 1 for all models. All SD 1.5 models I've tested seem OK, but some SDXL models do produce black, or pure noise images with clip_skip=1, even before this change (that seems related to their base models: CyberRealistic_Pony and CyberIllustrious are broken, while CyberRealisticXL works fine). I don't know if making all SDXL models default to clip_skip 2 is "right", but that'd be outside the scope of a fix anyway; I'll change the PR. |
|
I've left a comment on the KCPP PR, but as @LostRuins mentions I have experienced issues with the changes outlined. Manually setting it to 2 works, but I am unsure if this is the correct behaviour to default to - I know the existing logic used to generate images consistently with the SDXL models I use, but beyond that I'm afraid I don't have much more information. |
|
Reopened at #697 . |
CLIPTextModel ignored setting clip_skip back to -1, instead retaining whatever value was previously defined.
This is not relevant to command-line sd, since it's currently not possible to change clip_skip between generations, but could affect any frontend that reuses model instances for multiple images.