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

elf setting thread limits with side effects to other libraries/tasks. #75

Closed
k-dominik opened this issue Jul 13, 2023 · 2 comments · Fixed by #77
Closed

elf setting thread limits with side effects to other libraries/tasks. #75

k-dominik opened this issue Jul 13, 2023 · 2 comments · Fixed by #77

Comments

@k-dominik
Copy link
Contributor

Currently, elf might set various env variables and set the number of threads for mkl by triggering some of the imports.

relevant code:

elf/elf/util.py

Lines 206 to 235 in bed5a57

def set_numpy_threads(n_threads):
""" Set the number of threads numpy exposes to its
underlying linalg library.
This needs to be called BEFORE the numpy import and sets the number
of threads statically.
Based on answers in https://github.com/numpy/numpy/issues/11826.
"""
# set number of threads for mkl if it is used
try:
import mkl
mkl.set_num_threaads(n_threads)
except Exception:
pass
for name in ['libmkl_rt.so', 'libmkl_rt.dylib', 'mkl_Rt.dll']:
try:
mkl_rt = ctypes.CDLL(name)
mkl_rt.mkl_set_num_threads(ctypes.byref(ctypes.c_int(n_threads)))
except Exception:
pass
# set number of threads in all possibly relevant environment variables
os.environ['OMP_NUM_THREADS'] = str(n_threads)
os.environ['OPENBLAS_NUM_THREADS'] = str(n_threads)
os.environ['MKL_NUM_THREADS'] = str(n_threads)
os.environ['VECLIB_NUM_THREADS'] = str(n_threads)
os.environ['NUMEXPR_NUM_THREADS'] = str(n_threads)

This has side effects, where e.g. in ilastik, those settings will limit the number of threads, even for subprocesses. Using these functions in a script is probably fine (but who knows what users want to do downstream), but especially in our application context, where a user could do something completely different after some workflow involving elf, with detrimental effects to the performance (seen in the neural network classification workflow, which would only predict on a single thread).

I think it's worth investigating threadpoolctl for scoped limiting of threads for specific functions.

k-dominik added a commit to k-dominik/ilastik that referenced this issue Jul 13, 2023
elf limits threads on import, which has side effects even for the
tiktorch process (via env variables).
In order to get proper performance out of neural networks on CPU, we
unset these variables before starting the process.

xref: constantinpape/elf#75
k-dominik added a commit to ilastik/ilastik that referenced this issue Jul 13, 2023
elf limits threads on import, which has side effects even for the
tiktorch process (via env variables).
In order to get proper performance out of neural networks on CPU, we
unset these variables before starting the process.

xref: constantinpape/elf#75
@constantinpape
Copy link
Owner

@k-dominik : the easiest way to check is this:

from elf.util import set_numpy_threads  
set_numpy_threads(1)
import time
import numpy as np
  
  
x = np.random.rand(2048, 2048)
N = 10
  
ts = []
print("Start")
for _ in range(N):
      t0 = time.time()
      x @ x 
      ts.append(time.time() - t0) 
print(min(ts))

This takes ca. 0.1 sec when I run it without set_numpy_threads and 0.4 sec with. (On my laptop with 4 cores).
So if you see the same slowdown with threadpoolctl it means that it works.

@constantinpape
Copy link
Owner

Hi @k-dominik ,
since this issue also affects my code I went ahead and fixed it. (I checked, and treadpoolctl works as expected).
I will draft a new release soon.

k-dominik added a commit to k-dominik/ilastik that referenced this issue Nov 9, 2023
elf limits threads on import, which has side effects even for the
tiktorch process (via env variables).
In order to get proper performance out of neural networks on CPU, we
unset these variables before starting the process.

xref: constantinpape/elf#75
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 a pull request may close this issue.

2 participants