WASasquatch / was-node-suite-comfyui

An extensive node suite for ComfyUI with over 210 new nodes
MIT License
1.19k stars 178 forks source link

bug: number operations with greater-than or equals throws error #85

Closed dereekb closed 1 year ago

dereekb commented 1 year ago

When using the output as a boolean, the following error is thrown in console:

Traceback (most recent call last):
  File "/stable-diffusion/execution.py", line 184, in execute
    executed += recursive_execute(self.server, prompt, self.outputs, x, extra_data)
  File "/stable-diffusion/execution.py", line 60, in recursive_execute
    executed += recursive_execute(server, prompt, outputs, input_unique_id, extra_data)
  File "/stable-diffusion/execution.py", line 70, in recursive_execute
    if "ui" in outputs[unique_id]:
TypeError: argument of type 'float' is not iterable

Input types don't matter. Doesn't throw an error when using greater-than.

Also, there's a typo for this option. (equels)

error

Example:

greaterthanerror.zip

Workaround:

Use greater than and adjust the inputs accordingly.

WASasquatch commented 1 year ago

Oh yep, that's the issue. "equels", so it was defaulting to output number_a, but number_a wasn't being output as a tuple. Thanks for catching that honestly lol. Should be patched now.

dereekb commented 1 year ago

Great, thanks for patching that quickly! I was making a tangled mess of nodes earlier and suddenly got that error.

I think there's another bug with the Number Input Condition Switch too and the values it outputs when using greater-than. Instead of return true/false as 0/1 it was returning the greater of the value or something like that. I'll reproduce it when I get a chance and post it here.

WASasquatch commented 1 year ago

Hmm, yeah let me know if you see something. I don't see any issues with it at a glance

        if comparison:
            if comparison == 'greater-than':
                result = number_a if number_a > number_b else number_b
            elif comparison == 'greater-than or equals':
                result = number_a if number_a >= number_b else number_b
            elif comparison == 'less-than':
                result = number_a if number_a < number_b else number_b
            elif comparison == 'less-than or equals':
                result = number_a if number_a <= number_b else number_b
            elif comparison == 'equals':
                result = number_a if number_a == number_b else number_b
            elif comparison == 'does not equal':
                result = number_a if number_a != number_b else number_b
            elif comparison == 'divisible by':
                result = number_a if number_b % number_a == 0 else number_b
            elif comparison == 'if A odd':
                result = number_a if number_a % 2 != 0 else number_b
            elif comparison == 'if A even':
                result = number_a if number_a % 2 == 0 else number_b
            elif comparison == 'if A prime':
                result = number_a if self.is_prime(number_a) else number_b
            elif comparison == 'factor of':
                result = number_a if number_b % number_a == 0 else number_b
            else:
                result = number_a

Edit: Oh no, wait, it's the same issue. Should be patched.

WASasquatch commented 1 year ago

Has it been corrected on your end?

dereekb commented 1 year ago

I didn't know it was by design, but I originally expected the Number Input Condition to return a boolean value instead of the larger of the two values (Max Value functionality).

I guess the Number Input Condition functions all work like that though where either A or B is returned based on the code above. Aside from my initial confusion yep it works great, thanks! I've been running through most of the nodes in your library and if I find any more bugs I'll make a new issue.

WASasquatch commented 1 year ago

I didn't know it was by design, but I originally expected the Number Input Condition to return a boolean value instead of the larger of the two values (Max Value functionality).

I guess the Number Input Condition functions all work like that though where either A or B is returned based on the code above. Aside from my initial confusion yep it works great, thanks! I've been running through most of the nodes in your library and if I find any more bugs I'll make a new issue.

That is a good point though, so: https://github.com/WASasquatch/was-node-suite-comfyui/commit/391adf9b24b0ce8f78a612729a3cfb55c3c7bb54

dereekb commented 1 year ago

Thanks, that's helpful! Some other functions I might suggest is AND / OR / etc, with any non-zero number acting as true. Right now I've gotta pipe some different nodes together to achieve some of those and sometimes it isn't as clear as a single input that read "if A and B" or "if A or B"

WASasquatch commented 1 year ago

I'm not sure how that'd work? ComfyUI always requires an output. So can't really do a "A or B" cause they both would contain something and be "True". Additionally "A and B" assumes a computation that A or B is resulting in True with?

dereekb commented 1 year ago

Something like:

if comparison == 'and':
                result = number_a if number_a != 0 and number_b != 0 else number_b
elif comparison == 'or':
                result = number_a if number_a != 0 or number_b != 0 else number_b

I am not brushed up on python syntax so that might not be right. Here's what I'm thinking in JS:

if (comparison === 'if A and B') {
   return (a !== 0 && b !== 0) ? a : b;
} else if (comparison === 'if A or B') {
   return (a !== 0 || b !== 0 ) ? a : b;
} else if (comparison === 'if A nor B') {
   return (a === 0 && b === 0) ? a : b;
}

Of course also taking respect to the "return a boolean" result.

Right now to achieve an AND you need two booleans inputs (instead of any value) and can multiple those two booleans together (if any are 0 then it returns 0). OR is a bit more challenging, and neither handles cases where you just want to use non-negative numbers. The AND and OR functions above would help the Number Input Condition Switch cover those cases with a single node.

Here's a specific example where it would be useful. The first Landscape Enabled A and connecting nodes are essentially NOR, and Landscape Enabled is an AND. With "if A and B" and "if A nor B" I could replace the below with just 2 nodes.

and