Feedback and ideas

[VykosX]

Feel free to leave any feedback and ideas! This is still very much a WIP.

[christian-byrne]

Is the Data Monitor node supporting formulas when the parameters are tensors? Things like ~%aux1% ^ %aux2% > %aux3 where aux1 and aux2 are masks and aux3 is an image. The AST parser seems to be converting tensors to strings? Can you explain how it works?

[VykosX]

Sorry, I wrote many comments before refreshing and seeing your replies.

I can definitely PR something. Can you explain briefly the __MEM__STORAGE__KEYS__ system to help me?

Btw if you'd like to make this collaboration a little easier and/or get ahold of me for more questions outside of the feedback posts, feel free to send a message over at Discord, my username is the same as on github (vykosx). I'm also on the official ComfyUI and L2 discord servers. =)

Looking forward to your PR, it's wonderful seeing interest in the pack, I think there's still a lot that can be done in the pack to support all kinds of dynamic workflow systems, but it's just been me trying to figure things out so far.

[christian-byrne]

Thanks for all the info.

The string casting seems necessary based on what ast.parse expects. In _eval there is already logic for including local vars in eval scope. The solution seems to be to add a isinstance(aux, torch.Tensor) in replace_vars, and if the aux represents a tensor, just pass the reference to safe_eval like safe_eval(expr, variables={aux: tensor_reference}), instead of doing direct interpolation with repr. WDYT?

Not sure if there's a way to do it while maintaining the "safe" aspect entirely.

[VykosX]

That seems like a good solution to me. It's kind of what I attempted directly inside safe_eval initially but I missed the forced typecasting in replace_vars and just assumed the tensors had an implicit string conversion. I think perhaps check inside replace_vars() for the isinstance of Tensor and skip processing passthrough and auxs when it is in the same way None is being skipped?

Then you can call safe_eval just as you suggested for the FORMULA case, and assuming tensors are being properly handled on that it should work. I'm unsure if safe_eval can natively support tensor operations as is or if AST will throw a fit, but as long as the operators are being overloaded correctly and the tensors are modified in-place rather than resulting in copies I think it should work.

I'm also assuming tensors are inherently "safe" (i mean they're called .safetensors they're supposed to be free of pickles) so it's probably okay to allow them through directly. Hell I was willing to let them through to regular eval() as an explicit exception but even that didnt fix things due to that pesky string conversion hehe. This actually explains so much I feel silly not having noticed it sooner.

[christian-byrne]

Good point. Unsafe image tensors are not a realistic concern because uploading them isn't really possible and model tensors are serialized safely.

[VykosX]

Good point. Unsafe image tensors are not a realistic concern because uploading them isn't really possible and model tensors are serialized safely.

Heya. How goes your PR to add tensor manipulation functionality? In the meantime I did some dirty patches in Data Monitor to just remove the ellipses from the tensor reprs so at least it doesn't crash safe_eval so that I could reenable being able to reference the inputs of Loop nodes within their conditions. I didn't want to mess with tensors until you've had your chance to get that added, so it's just a small addition.

[noahaust2]

Would it be possible to get some example workflows, especially for the cycle nodes? I think these nodes would be a lifesaver for a project I'm working on but I can't figure out the documentation

[VykosX]

There are many workflows in the Workflow folder included with the nodes, several of which showcase the usage of cycles.