ltdrdata / ComfyUI-Impact-Pack

Custom nodes pack for ComfyUI This custom node helps to conveniently enhance images through Detector, Detailer, Upscaler, Pipe, and more.
GNU General Public License v3.0
1.9k stars 186 forks source link

request: config model path with extra_model_path #478

Closed xingren23 closed 9 months ago

ltdrdata commented 9 months ago

https://github.com/ltdrdata/ComfyUI-Manager/issues/420#issuecomment-1958646418

xingren23 commented 9 months ago

I suggest we split these two topics for discussion. I'd like to request support for users to configure model paths through extra_model_paths in the ComfyUI-Impact-Pack.

ltdrdata commented 9 months ago

I suggest we split these two topics for discussion. I'd like to request support for users to configure model paths through extra_model_paths in the ComfyUI-Impact-Pack.

Are you talking about setting the paths for the currently used ComfyUI-Impact-Pack/models/ultralytics and ComfyUI-Impact-Pack/models/sams?

I haven’t tested it, but it seems like it would apply if you add ultralytics_bbox, ultralytics_segm, ultralytics, sam to extra_model_path.yaml.

xingren23 commented 9 months ago

I suggest we split these two topics for discussion. I'd like to request support for users to configure model paths through extra_model_paths in the ComfyUI-Impact-Pack.

Are you talking about setting the paths for the currently used ComfyUI-Impact-Pack/models/ultralytics and ComfyUI-Impact-Pack/models/sams?

I haven’t tested it, but it seems like it would apply if you add ultralytics_bbox, ultralytics_segm, ultralytics, sam to extra_model_path.yaml.

That won't work. The code doesn't validate the configuration of folder_names_and_paths; instead, it uses models_dir directly.

ltdrdata commented 9 months ago

I suggest we split these two topics for discussion. I'd like to request support for users to configure model paths through extra_model_paths in the ComfyUI-Impact-Pack.

Are you talking about setting the paths for the currently used ComfyUI-Impact-Pack/models/ultralytics and ComfyUI-Impact-Pack/models/sams? I haven’t tested it, but it seems like it would apply if you add ultralytics_bbox, ultralytics_segm, ultralytics, sam to extra_model_path.yaml.

That won't work. The code doesn't validate the configuration of folder_names_and_paths; instead, it uses models_dir directly.

The extra_model_paths.yaml is not a replacement, but an addition. This also applies to my code for adding model paths. Even if the Impact Pack has internally added the basic model path, it should be normal to be able to read from the path if it is additionally registered in the extra_model_paths.yaml.

xingren23 commented 9 months ago

I suggest we split these two topics for discussion. I'd like to request support for users to configure model paths through extra_model_paths in the ComfyUI-Impact-Pack.

Are you talking about setting the paths for the currently used ComfyUI-Impact-Pack/models/ultralytics and ComfyUI-Impact-Pack/models/sams? I haven’t tested it, but it seems like it would apply if you add ultralytics_bbox, ultralytics_segm, ultralytics, sam to extra_model_path.yaml.

That won't work. The code doesn't validate the configuration of folder_names_and_paths; instead, it uses models_dir directly.

The extra_model_paths.yaml is not a replacement, but an addition. This also applies to my code for adding model paths. Even if the Impact Pack has internally added the basic model path, it should be normal to be able to read from the path if it is additionally registered in the extra_model_paths.yaml.

The configurations I previously set up had issues, but using ultralytics, ultralytics_bbox, and ultralytics_segm works fine. Thank you for patiently explaining.

raonsol commented 4 months ago

I used sams instead of sam to work with extra_model_paths.yaml. Other dirs works fine.