search

SeargeDP / ComfyUI_Searge_LLM

Custom nodes for ComfyUI that utilize a language model to generate text-to-image prompts
MIT License
47 stars 5 forks source link

Handle llama_cpp_python installation based on gpu & cpu availability #15

Closed ashish-aesthisia closed 1 week ago

ashish-aesthisia commented 2 months ago

We have been using this with cuda enabled machine but when installing this node, it always installs llama_cpp_python, which is cpu version.

For us when we changed to llama_cpp_python_cuda, it works faster & LLM models are able to load quickly.

Closes #6 #3

Test: After this changes, it automatically picks up the llama cpp installation based on CPU or GPU availability.

Let me know if any further changes are needed.

SeargeDP commented 2 months ago

Could you explain how the change in this PR is doing anything different from what the requirements.txt did before? You seem to do the same checks for system and python version and are selecting the same wheels in case they match. The main change is that the requirements.txt file installed both versions of llama-cpp before (cpu and cuda) and with this PR only one of them would be installed.

It's perfectly fine for both packages to be installed, I explicitly have code in the node: See code here

try:
    Llama = importlib.import_module("llama_cpp_cuda").Llama
except ImportError:
    Llama = importlib.import_module("llama_cpp").Llama

that tries to load the cuda version first and only falls back to using the cpu version if that failed.

ashish-aesthisia commented 2 months ago

Could you explain how the change in this PR is doing anything different from what the requirements.txt did before? You seem to do the same checks for system and python version and are selecting the same wheels in case they match. The main change is that the requirements.txt file installed both versions of llama-cpp before (cpu and cuda) and with this PR only one of them would be installed.

It's perfectly fine for both packages to be installed, I explicitly have code in the node: See code here

try:
    Llama = importlib.import_module("llama_cpp_cuda").Llama
except ImportError:
    Llama = importlib.import_module("llama_cpp").Llama

that tries to load the cuda version first and only falls back to using the cpu version if that failed.

Yes You are right I think, This might be a problem on our end as requirements are not persistent. I installed the node & saw both cpu & gpu llama python. I think we don't need to do this change.

Key problem is that our session doesn't persist the requirements. So Let me once try on our end, this PR might not be needed. Thanks for the clarification.

SeargeDP commented 2 months ago

Key problem is that our session doesn't persist the requirements. So Let me once try on our end, this PR might not be needed. Thanks for the clarification.

OK, that makes sense. And if you don't persist installed packages, you will also see similar problems with all kinds of other comfyui nodes. Let me know if you encounter any more problems.