Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
alessandroperilli opened this issue Nov 15, 2023 · 6 comments
Closed

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

alessandroperilli opened this issue Nov 15, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@alessandroperilli
Copy link

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 alessandroperilli added the bug Something isn't working label Nov 15, 2023
@alessandroperilli
Copy link
Author

Nevermind. This is not related to SD Parameter Generator:
comfyanonymous/ComfyUI#1933

@alessandroperilli alessandroperilli changed the title [BUG] - Incompatibility with new LCM sampler? [FEATURE REQUEST] - Incompatibility with new LCM sampler? Nov 16, 2023
@alessandroperilli alessandroperilli changed the title [FEATURE REQUEST] - Incompatibility with new LCM sampler? [FEATURE REQUEST] - Support for new LCM sampler Nov 16, 2023
@alessandroperilli
Copy link
Author

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
Copy link
Owner

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.

@receyuki receyuki reopened this Nov 17, 2023
@receyuki receyuki added enhancement New feature or request and removed bug Something isn't working labels Nov 17, 2023
@receyuki
Copy link
Owner

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants