Skip to content

whisper: use vulkan as gpu backend when available #2302

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

Merged
merged 2 commits into from
Jul 16, 2024

Conversation

mstephenson6
Copy link
Contributor

  • When built with Vulkan support, attempt to initialize the ggml vk backend as a gpu

@mstephenson6 mstephenson6 changed the title ggml: use vulkan as gpu backend when available whisper: use vulkan as gpu backend when available Jul 14, 2024
@ggerganov
Copy link
Member

Hi, have you tested that it works correctly when Vulkan is enabled?

@mstephenson6
Copy link
Contributor Author

@ggerganov - yes, so far just on an Apple M1Pro running Fedora, with the "Honeykrisp" Vulkan driver.

I'm excited about this working, it's the very first GPU-accelerated ML I've been able to do on Linux + M1.

Happy to test other GPU/OS combos too.

VK_LOG_DEBUG("ggml_vk_init(" << ctx->name << ", " << idx << ")");
ggml_vk_instance_init();
GGML_ASSERT(idx < vk_instance.device_indices.size());
Copy link
Member

Choose a reason for hiding this comment

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

Is this change necessary? The original code works in https://github.com/ggerganov/llama.cpp, not sure why we would need to change it here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it so ggml_vk_instance_init() would populate device_indices right before the assertion, and it would avoid another vk-specific call in whisper.cpp. Without this, the assertion failed every time, no other code had attempted to initialize devices.

I didn't look at how llama.cpp did it, but I can check it out.

Copy link
Member

Choose a reason for hiding this comment

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

I see, seems the change is good. cc @0cc4m to confirm that it's OK to move the assert after the init call

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, that's fine. Makes more sense that way, too.

@ggerganov ggerganov merged commit f68298c into ggml-org:master Jul 16, 2024
45 of 46 checks passed
iThalay pushed a commit to iThalay/whisper.cpp that referenced this pull request Sep 23, 2024
* ggml: use vulkan as gpu backend when available

Signed-off-by: Matt Stephenson <[email protected]>

* whisper: enable using vk as default buffer type

Signed-off-by: Matt Stephenson <[email protected]>

---------

Signed-off-by: Matt Stephenson <[email protected]>
iThalay pushed a commit to iThalay/whisper.cpp that referenced this pull request Sep 23, 2024
* ggml: use vulkan as gpu backend when available

Signed-off-by: Matt Stephenson <[email protected]>

* whisper: enable using vk as default buffer type

Signed-off-by: Matt Stephenson <[email protected]>

---------

Signed-off-by: Matt Stephenson <[email protected]>
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.

3 participants