FizzleDorf / ComfyUI_FizzNodes

Custom Nodes for Comfyui
MIT License
399 stars 60 forks source link

FizzNode's addWeighted()'s zero-padding seem wrong #46

Closed gavinliukaiber closed 10 months ago

gavinliukaiber commented 12 months ago

These 2 lines pads zero as the token embedding when the second prompt is longer.

However, when the embedding for the token output by clip is non-zero, as shown by the sum of its 786 dimensions being around -86 (the following output was obtained by turning debugger on at the above lines) :

t0.shape
torch.Size([1, 154, 768])

t0.sum(2)   # only the first 3 tokens of t0 are actual text. everything else are padding.
tensor([[-80.5575, -84.9922, -86.5110, -89.2761, -88.5500, -87.9479, -87.4741,
         -87.3303, -87.5545, -87.9854, -88.1384, -88.2550, -88.3028, -88.2968,
         -88.2977, -88.2566, -88.2179, -88.1763, -88.1114, -88.0569, -87.9825,
         -87.9123, -87.8530, -87.7786, -87.7417, -87.6993, -87.6719, -87.6595,
         -87.6190, -87.5953, -87.5536, -87.5174, -87.5034, -87.4509, -87.4293,
         -87.4009, -87.3871, -87.4080, -87.3753, -87.3691, -87.3749, -87.3128,
         -87.2906, -87.2220, -87.1898, -87.1448, -87.1336, -87.1117, -87.1122,
         -87.1306, -87.1080, -87.1134, -87.0603, -87.0418, -86.9933, -86.9530,
         -86.9266, -86.8840, -86.8813, -86.8558, -86.8131, -86.7775, -86.7166,
         -86.7124, -86.6881, -86.6805, -86.6832, -86.6497, -86.6852, -86.6795,
         -86.7144, -86.6964, -86.6483, -86.6575, -86.5954, -86.6392, -86.5924,
           0.0000,   0.0000,   0.0000,   0.0000,   0.0000,   0.0000,   0.0000,
           0.0000,   0.0000,   0.0000,   0.0000,   0.0000,   0.0000,   0.0000,
           0.0000,   0.0000,   0.0000,   0.0000,   0.0000,   0.0000,   0.0000,
           0.0000,   0.0000,   0.0000,   0.0000,   0.0000,   0.0000,   0.0000,
           0.0000,   0.0000,   0.0000,   0.0000,   0.0000,   0.0000,   0.0000,
           0.0000,   0.0000,   0.0000,   0.0000,   0.0000,   0.0000,   0.0000,
           0.0000,   0.0000,   0.0000,   0.0000,   0.0000,   0.0000,   0.0000,
           0.0000,   0.0000,   0.0000,   0.0000,   0.0000,   0.0000,   0.0000,
           0.0000,   0.0000,   0.0000,   0.0000,   0.0000,   0.0000,   0.0000,
           0.0000,   0.0000,   0.0000,   0.0000,   0.0000,   0.0000,   0.0000,
           0.0000,   0.0000,   0.0000,   0.0000,   0.0000,   0.0000,   0.0000]])

Therefore this padding seems incorrect. I think the correct way should be padding the original string with token used in CLIP, but I am not sure how to do that. thoughts?

gavinliukaiber commented 12 months ago

actually we can probably use clip.cond_stage_model.clip_l.special_tokens which has the index for the padding token: {'start': 49406, 'end': 49407, 'pad': 49407}

gavinliukaiber commented 12 months ago

A separate thing --- this line silently truncates the longer prompt, which also seem incorrect?

FizzleDorf commented 11 months ago

I've made some changes yesterday with your suggestion. The error is no longer present but I'll keep this open while people try it out and if there is any issues, they can report here and I'll have a look at it. Sorry it's been a while looking into this, it's been a busy week.

OliviaOliveiira commented 11 months ago

Hey there! Went through several github issues here, and must say, the issue is still there. Updated both comfy and custom nodes, everything's latest. ERROR:root:!!! Exception during processing !!! ERROR:root:Traceback (most recent call last): File "C:\ComfyUI_BLYAT\ComfyUI_windows_portable_nvidia_cu121_or_cpu\ComfyUI_windows_portable\ComfyUI\execution.py", line 153, in recursive_execute output_data, output_ui = get_output_data(obj, input_data_all) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "C:\ComfyUI_BLYAT\ComfyUI_windows_portable_nvidia_cu121_or_cpu\ComfyUI_windows_portable\ComfyUI\execution.py", line 83, in get_output_data return_values = map_node_over_list(obj, input_data_all, obj.FUNCTION, allow_interrupt=True) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "C:\ComfyUI_BLYAT\ComfyUI_windows_portable_nvidia_cu121_or_cpu\ComfyUI_windows_portable\ComfyUI\execution.py", line 76, in map_node_over_list results.append(getattr(obj, func)(**slice_dict(input_data_all, i))) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "C:\ComfyUI_BLYAT\ComfyUI_windows_portable_nvidia_cu121_or_cpu\ComfyUI_windows_portable\ComfyUI\custom_nodes\ComfyUI_FizzNodes\ScheduledNodes.py", line 126, in animate pc = BatchPoolAnimConditioning( pos_cur_prompt, pos_nxt_prompt, weight, clip,) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "C:\ComfyUI_BLYAT\ComfyUI_windows_portable_nvidia_cu121_or_cpu\ComfyUI_windows_portable\ComfyUI\custom_nodes\ComfyUI_FizzNodes\BatchFuncs.py", line 208, in BatchPoolAnimConditioning final_conditioning = torch.cat(cond_out, dim=0) ^^^^^^^^^^^^^^^^^^^^^^^^^^ RuntimeError: Sizes of tensors must match except in dimension 0. Expected size 154 but got size 77 for tensor number 25 in the list. image (prompt not of my own, so hid it) Although it went through when there were 9-10 frame nubmers total, not sure what was the exact number.

Grihail commented 11 months ago

Yes! I ran into that problem too! A problem without a cause. image

itera-chen commented 10 months ago

I am encounter the same issue here. I notice that when I modify the prompt by removing repeating vocabularies between pre/app text and schedule prompt can sometime resolve the error.

FizzleDorf commented 10 months ago

this issue was solved in the latest commit. Thanks for your patience