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

Set OMP_PROC_BIND=false before calling omp_get_max_threads #1241

Merged
merged 2 commits into from
Aug 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ New Features
Performance Improvements
------------------------

- Work around an OMP bug that disables multiprocessing on some systems when omp_get_max_threads
is called. (#1241)


Bug Fixes
Expand Down
17 changes: 17 additions & 0 deletions galsim/utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -1842,6 +1842,11 @@ def set_omp_threads(num_threads, logger=None):
# Tell OpenMP to use this many threads
if logger:
logger.debug('Telling OpenMP to use %d threads',num_threads)

# Cf. comment in get_omp_threads. Do it here too.
var = "OMP_PROC_BIND"
if var not in os.environ:
os.environ[var] = "false"
num_threads = _galsim.SetOMPThreads(num_threads)

# Report back appropriately.
Expand All @@ -1860,6 +1865,18 @@ def get_omp_threads():

:returns: num_threads
"""
# Some OMP implemenations have a bug where if omp_get_max_threads() is called
# (which is what this function does), it sets something called thread affinity.
# The upshot of that is that multiprocessing (i.e. not even just omp threading) is confined
# to a single hardware thread. Yeah, it's idiotic, but that seems to be the case.
# The only solution found by Eli, who looked into it pretty hard, is to set the env
# variable OMP_PROC_BIND to "false". This seems to stop the bad behavior.
# So we do it here always before calling GetOMPThreads.
# If this breaks someone valid use of this variable, let us know and we can try to
# come up with another solution, but without this lots of multiprocessing breaks.
var = "OMP_PROC_BIND"
if var not in os.environ:
os.environ[var] = "false"
return _galsim.GetOMPThreads()

class single_threaded:
Expand Down
11 changes: 11 additions & 0 deletions tests/test_sensor.py
Original file line number Diff line number Diff line change
Expand Up @@ -1208,6 +1208,17 @@ def test_omp():
assert "OpenMP reports that it will use 1 threads" in cl.output
assert "Unable to use multiple threads" in cl.output

# This is really just for coverage. Check that OMP_PROC_BIND gets set properly.
with mock.patch('os.environ', {}):
assert os.environ.get('OMP_PROC_BIND') is None
galsim.get_omp_threads()
assert os.environ.get('OMP_PROC_BIND') == 'false'

with mock.patch('os.environ', {}):
assert os.environ.get('OMP_PROC_BIND') is None
galsim.set_omp_threads(4)
assert os.environ.get('OMP_PROC_BIND') == 'false'


@timer
def test_big_then_small():
Expand Down
Loading