Make OpenBLAS's usage of OpenMP respect openblas_set_num_threads()#5808
Conversation
|
Would be nice to get a fix merged, if someone has time to review. |
|
@frbg - @itamarst is working with me and @kumaraditya303 on improving support for free-threading in Python packages. He's identified the issue this PR in particular fixes as the cause of substantial CPU oversubscription in scikit-learn using the threading joblib backend. We'd appreciate your eyes on this and the original issue. |
|
Can you split this please ? openblas_set_num_threads() immediately getting reset to the OMP maximum is clearly unintended (though obviously it should be bounded by the OMP maximum). Regarding the other other things in this PR - openblas_set_num_threads_local is intended to match mkl_set_num_threads_local (but may not yet be complete) - so nothing to do with forcing OMP parallel jobs inside an OMP_PARALLEL region (which IIRC would be invoking undefined behavior at best). |
…ixes OpenMathLib#5806. Until now, the code in `num_cpu_avail()`, if (blas_cpu_number != openmp_nthreads) { goto_set_num_threads(openmp_nthreads); } would just always set the threads back to OpenMP's thread count.
nh2
force-pushed
the
issue-5806-openblas_set_num_threads-openmp
branch
from
d18385b to
f8674a7
Compare
Done. I moved the rest to PR #5907. |
|
Thank you |
4 participants
Fixes #5806.
Also
openblas_set_num_threads_local()does, see Add BLAS extension openblas_set_num_threads_local() #4425 (comment)num_cpu_avail()I complained about inopenblas_set_num_threads()is silently overridden when built withUSE_OPENMP#5806 (comment)Reviewing by commit is recommended.