pamparamm / ComfyUI-ppm

Fixed AttentionCouple/NegPip(negative weights in prompts), more CFG++ samplers, etc.
GNU Affero General Public License v3.0
51 stars 4 forks source link

CLIPNegPip Incompatible with Alternative Prompt Weight Interpretation Methods #1

Closed setothegreat closed 1 week ago

setothegreat commented 1 month ago

Hey there, and thanks for the great node additions! They've been a massive improvement over previous solutions in my limited testing so far, and will be super helpful for my workflows!

That being said, I have encountered an error when using Advanced CLIP Text Encode by BlenderNeko, and I would imagine the same issue would appear in other nodes that use the same re-weighting functions:

Error occurred when executing BNK_CLIPTextEncodeAdvanced:
Sizes of tensors must match except in dimension 0. Expected size 154 but got size 77 for tensor number 1 in the list.

This occurs when using any weight interpretation method other than Comfy. It doesn't occur with any of the token normalization methods so long as Comfy is the selected weight interpretation. Being that Comfy++ tends to perform better than normal Comfy with longer prompts, it would be appreciated if this could be remedied.

This is the full error:

Error occurred when executing BNK_CLIPTextEncodeAdvanced:
Sizes of tensors must match except in dimension 0. Expected size 154 but got size 77 for tensor number 1 in the list.

  File "F:\Diffusion\ComfyUI\ComfyUI\execution.py", line 151, in recursive_execute
    output_data, output_ui = get_output_data(obj, input_data_all)
  File "F:\Diffusion\ComfyUI\ComfyUI\execution.py", line 81, in get_output_data
    return_values = map_node_over_list(obj, input_data_all, obj.FUNCTION, allow_interrupt=True)
  File "F:\Diffusion\ComfyUI\ComfyUI\execution.py", line 74, in map_node_over_list
    results.append(getattr(obj, func)(**slice_dict(input_data_all, i)))
  File "F:\Diffusion\ComfyUI\ComfyUI\custom_nodes\ComfyUI_ADV_CLIP_emb\nodes.py", line 21, in encode
    embeddings_final, pooled = advanced_encode(clip, text, token_normalization, weight_interpretation, w_max=1.0, apply_to_pooled=affect_pooled=='enable')
  File "F:\Diffusion\ComfyUI\ComfyUI\custom_nodes\ComfyUI_ADV_CLIP_emb\adv_encode.py", line 250, in advanced_encode
    embs_l, _ = advanced_encode_from_tokens(tokenized['l'],
  File "F:\Diffusion\ComfyUI\ComfyUI\custom_nodes\ComfyUI_ADV_CLIP_emb\adv_encode.py", line 200, in advanced_encode_from_tokens
    weighted_emb, tokens_down, _ = down_weight(unweighted_tokens, weights, word_ids, base_emb, length, encode_func)
  File "F:\Diffusion\ComfyUI\ComfyUI\custom_nodes\ComfyUI_ADV_CLIP_emb\adv_encode.py", line 138, in down_weight
    embs = torch.cat([base_emb, embs])
pamparamm commented 1 month ago

Yeah, I've encountered the same issue before, but it's not so simple to fix. It requires lots of patching in adv_encode.py. I'll probably fix that later by duplicating Advanced CLIP nodes into my extension.

setothegreat commented 1 month ago

Yeah, I've encountered the same issue before, but it's not so simple to fix. It requires lots of patching in adv_encode.py. I'll probably fix that later by duplicating Advanced CLIP nodes into my extension.

No worries, and take your time. Because your NegPip, AttentionCouplePPM and GuidanceLimiter nodes improve multi-subject images so much it's still a drastic improvement over just using Comfy++ on it's own

pamparamm commented 1 month ago

I've added support for all weight interpretations. It requires Advanced CLIP Text Encode extension to be installed under default name in custom_nodes/ComfyUI_ADV_CLIP_emb.

setothegreat commented 1 month ago

I've added support for all weight interpretations. It requires Advanced CLIP Text Encode extension to be installed under default name in custom_nodes/ComfyUI_ADV_CLIP_emb.

Still getting the same error. Updated PPM and reinstalled ComfyUI_ADV_CLIP_emb with a new workflow just to confirm.

pamparamm commented 4 weeks ago

Thanks for report! Selecting weight interpretations from ADV_CLIP by using STYLE keyword from ComfyUI prompt control extension worked correctly, so I didn't bother to check nodes from just ADV_CLIP - my mistake :( . There was a bug in hijacking process causing some troubles with nodes from ADVCLIP (with `BNK` prefix) - I've pushed a fix, they should work now.

pamparamm commented 1 week ago

I'll mark this issue as a completed. If the problem still occurs on your side - feel free to re-open it.