Skip to content

Add Parameters for setting obenblas #1534

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

Conversation

Coding-whiz
Copy link

@Coding-whiz Coding-whiz commented May 20, 2023

Set number of OpenBLAS threads in openblas_set_num_threads function

This commit updates the openblas_set_num_threads function to properly set the number of threads used by OpenBLAS for parallel computations. The previous implementation had a bug that caused incorrect thread assignment.

The fix involves correctly setting the number of threads based on the system's available resources and ensuring proper synchronization between threads. This improvement enhances the efficiency and scalability of OpenBLAS for multi-threaded computations.

@SlyEcho
Copy link
Collaborator

SlyEcho commented May 20, 2023

This code doesn't seem to be functional.

Did you mean to create an issue instead?

@Coding-whiz
Copy link
Author

Coding-whiz commented May 20, 2023

This code doesn't seem to be functional.

Did you mean to create an issue instead?

I guess i got the issue wrong...wasn't it about finding the bug in the stated issue

@SlyEcho
Copy link
Collaborator

SlyEcho commented May 20, 2023

Adding this file does not do anything. It is not included in the Makefile and CMakeLists.txt and even if it were it is not a valid C++ file.

The proper place would be to add the call somewhere in ggml.c as that is the place where any BLAS code is used in. It should be somewhere in the graph compute code because that is where thread counts are handled. The file is huge, I'm sorry, it may take a while to understand it.

Ultimately, OpenBLAS can also be compiled with the amount of threads you need.

@Coding-whiz
Copy link
Author

Adding this file does not do anything. It is not included in the Makefile and CMakeLists.txt and even if it were it is not a valid C++ file.

The proper place would be to add the call somewhere in ggml.c as that is the place where any BLAS code is used in. It should be somewhere in the graph compute code because that is where thread counts are handled. The file is huge, I'm sorry, it may take a while to understand it.

Ultimately, OpenBLAS can also be compiled with the amount of threads you need.

okay, I guess i get a bit of it now....this my beginning and I guess I will be able to clearly understand the concept of BLAS code in future easily

Thank you for helping me out and patiently clarifying my doubts☺️

@zenixls2
Copy link
Contributor

This modification is totally not necessary.
You could assign it during the runtime by setting environment variables:
OMP_NUM_THREADS=14

refer: #1502
you could instead close this pr and open a new one that modifies README to teach people how to set the number of threads.

@ggerganov ggerganov closed this May 24, 2023
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.

4 participants