receyuki / comfyui-prompt-reader-node

The ultimate solution for managing image metadata and multi-tool compatibility. ComfyUI node version of the SD Prompt Reader
MIT License
289 stars 22 forks source link

[FEATURE REQUEST] - Support for new LCM sampler #20

Closed alessandroperilli closed 1 year ago

alessandroperilli commented 1 year ago

Description

I just released AP Workflow 6.0 with the SD Parameter Generator. Everything works fine, except when I try to test the new LCM sampler with the LCM models:

Screenshot 2023-11-15 at 15 06 52

This configuration generates the following error:

Error occurred when executing SDParameterGenerator:

'model.diffusion_model.input_blocks.0.0.weight'

File "xyz/ComfyUI/execution.py", line 153, in recursive_execute output_data, output_ui = get_output_data(obj, input_data_all) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "xyz/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 "xyz/ComfyUI/execution.py", line 76, in map_node_over_list results.append(getattr(obj, func)(**slice_dict(input_data_all, i))) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "xyz/ComfyUI/custom_nodes/comfyui-prompt-reader-node/nodes.py", line 603, in generate_parameter checkpoint = comfy.sd.load_checkpoint_guess_config( ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "xyz/ComfyUI/comfy/sd.py", line 431, in load_checkpoint_guess_config model_config = model_detection.model_config_from_unet(sd, "model.diffusion_model.", unet_dtype) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "xyz/ComfyUI/comfy/model_detection.py", line 141, in model_config_from_unet unet_config = detect_unet_config(state_dict, unet_key_prefix, dtype) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "xyz/ComfyUI/comfy/model_detection.py", line 49, in detect_unet_config model_channels = state_dict['{}input_blocks.0.0.weight'.format(key_prefix)].shape[0]

From what I read, LCM is fully compatible with Apple MPS, I wonder if it's an issue with the node itself.

Reproduction steps

No response

Image file

No response

Version

1.0.0

alessandroperilli commented 1 year ago

Nevermind. This is not related to SD Parameter Generator: https://github.com/comfyanonymous/ComfyUI/issues/1933

alessandroperilli commented 1 year ago

I'd like to add some additional details to this issue, just for transparency:

After reading the link I posted above, and some testing, I saw that the LCM sampler generates the error simply because the LCM and LCM-LoRA models come with the diffusers format.

To load them, you need one or two nodes, as I quickly implemented here:

Screenshot 2023-11-16 at 20 07 42

So, perhaps, the SD Parameter Generator could do this.

On the other hand, I'm concerned that it might stretch the scope of this node too much and force you to implement too many things. Also, personally, the quality of the images I get with LCM is very low (maybe I'm doing something wrong), so I'm not 100% sure it's worth the effort.

To me, it's a nice-to-have, not a must-have, and definitely much lower priority than the other things I submitted so far.

receyuki commented 1 year ago

I’ve recently been busy with other things, so could you tell me which of these 6 feature requests you’ve mentioned are more urgent, so I can add them asap, while the rest may need to wait for a while. About the LCM sampler, I may need to do some search before I can make decisions, since it is new knowledge to me.

alessandroperilli commented 1 year ago

In order of priority, I'd say:

  1. 8

  2. 6

  3. 15

  4. 16

  5. 20 (if you decide that it's worth doing it)

  6. 17

But it's entirely up to you. I'd go for the easiest to implement, to have a series of quick wins :)

Thank you, as usual.

receyuki commented 1 year ago

I have decided not to include LCM-related features for now.

In terms of development difficulty, this would be a simple copy-paste job as official support for LCM is already in place, thus the problem doesn't lie here.

Based on my understanding, LCM is not currently mainstream, and the number of LCM models is very limited. I believe users who currently use LCM have the capability to figure out how to load LCM models through other nodes. Since LCM models are UNET models, users would still need to load regular checkpoints to provide CLIP models, and they would also need additional specific parameters, which would significantly increase the complexity of the Generator.

I have completed all the features except for LCM support in the dev branch. My current plan is to try writing batch support as mentioned in #13. If this takes some time, I will merge the completed parts into the main branch first.

alessandroperilli commented 1 year ago

I think it's a good decision. Given that LCM-LoRA can be applied transparently to existing workflows, it's possible that very few people will look for the actual LCM models. For them, I'll simply show how to patch AP Workflow.