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

Ability to split model components to different backend devices #461

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

softcookiepp
Copy link

The ability to specify separate backend device indexes for CLIP, Unet, and VAE was implemented. This works for all backends that accept a device index as an initialization parameter (CUDA, Vulkan, and SYCL.) The aim of this was to support splitting the parts of the model across multiple GPUs.

}

}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove whitespace

if (model_backend_index == -1) {
// default behavior, last device selected
for (int device = 0; device < ggml_backend_vk_get_device_count(); ++device) {
backend = ggml_backend_vk_init(device);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are initializing EVERY vk device here (and leak it)

Copy link
Author

@softcookiepp softcookiepp Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied this from the original code verbatim in order to ensure that behavior prior to my changes was preserved. I just fixed it though. Instead of iterating over all device indexes, initializing all of them, and only keeping the last one, it instead simply picks the last index and initializes the corresponding backend device.

bool keep_vae_on_cpu,
int model_backend_index = -1,
int clip_backend_index = -1,
int vae_backend_index = -1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C can not have default params

Removed whitespace, changed the default vulkan device initialization to use the last device without initializing all other previous devices.
@softcookiepp
Copy link
Author

Everything should be fixed now

@@ -5,6 +5,7 @@
#include <random>
#include <string>
#include <vector>
#include <stdexcept>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no exceptions used.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anything else that needs changing?

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

Successfully merging this pull request may close these issues.

2 participants