Skip to content

Commit

Permalink
Remove OpenMP thread support from convolution.convolve()
Browse files Browse the repository at this point in the history
See #7971

Signed-off-by: James Noss <jnoss@stsci.edu>
  • Loading branch information
jamienoss committed Oct 26, 2018
1 parent 47072a2 commit c1c6a70
Show file tree
Hide file tree
Showing 7 changed files with 8 additions and 72 deletions.
2 changes: 1 addition & 1 deletion .travis.yml
Expand Up @@ -73,7 +73,7 @@ matrix:
- os: osx
stage: Cron tests
env: SETUP_CMD='test --remote-data=astropy'
CONDA_DEPENDENCIES="$CONDA_ALL_DEPENDENCIES clang llvm-openmp"
CONDA_DEPENDENCIES="$CONDA_ALL_DEPENDENCIES clang"
PIP_DEPENDENCIES='jplephem'
CCOMPILER=clang
EVENT_TYPE='cron'
Expand Down
18 changes: 0 additions & 18 deletions CHANGES.rst
Expand Up @@ -15,7 +15,6 @@ astropy.convolution

- ``convolve`` now accepts any array-like input, not just ``numpy.ndarray`` or
lists. [#7303]
- Thread with OpenMP support [#7293]

astropy.coordinates
^^^^^^^^^^^^^^^^^^^
Expand Down Expand Up @@ -270,11 +269,6 @@ astropy.convolution

- ``kernel`` can now be a tuple. [#7561]

- Add ``n_threads`` parameter to control number of threads used in C computation.
An exception is raised for negative values.
A warning is issued if ``n_threads`` > total number of CPUs reported by the OS.
A warning is raised if ``n_threads`` > 1 and Astropy was NOT built with OpenMP support. [#7293]

- Not technically an API changes, however, the doc string indicated that ``boundary=None``
was the default when actually it is ``boundary='fill'``. The doc string has been corrected,
however, someone may interpret this as an API change not realising that nothing has actually
Expand Down Expand Up @@ -455,12 +449,6 @@ astropy.convolution
hoisting, and vectorization, etc. Compiler optimization level ``-O2`` required for
hoisting and ``-O3`` for vectorization. [#7293]

- ``convolve()``: Core computation now threaded using OpenMP. However, the root
setup disables this by default. To build the package with OpenMP support
instantiate the environment variable ``ASTROPY_SETUP_WITH_OPENMP=1``.
E.g. ``ASTROPY_SETUP_WITH_OPENMP=1 pip install astropy --no-cache-dir`` or
``ASTROPY_SETUP_WITH_OPENMP=1 ./setup.py build``. [#7293]

- ``convolve()``: ``nan_treatment=‘interpolate’`` was slow to compute irrespective of
whether any NaN values exist within the array. The input array is now
checked for NaN values and interpolation is disabled if non are found. This is a
Expand Down Expand Up @@ -623,12 +611,6 @@ Other Changes and Additions
leading to some speed improvements, and setting the stage for allowing
overrides with ``__array_ufunc__``. [#7502]

- Building with OpenMP support is disabled by default. To build the package with
OpenMP support (currently only used in ``convolution.convolve``) create and
set the environment variable ``ASTROPY_SETUP_WITH_OPENMP=1``.
E.g. ``ASTROPY_SETUP_WITH_OPENMP=1 pip install astropy --no-cache-dir`` or
``ASTROPY_SETUP_WITH_OPENMP=1 ./setup.py build``. [#7293]



3.0.6 (unreleased)
Expand Down
13 changes: 4 additions & 9 deletions astropy/convolution/convolve.py
Expand Up @@ -19,7 +19,6 @@
from ..modeling.core import _make_arithmetic_operator, BINARY_OPERATORS
from ..modeling.core import _CompoundModelMeta
from .utils import KernelSizeError, has_even_axis, raise_even_kernel_exception
from ..utils.openmp import handle_n_threads_usage

# Turn the faulthandler ON to help catch any signals, e.g. segfaults
# or asserts. This doesn't, currently, work with Jupyter Notebook.
Expand Down Expand Up @@ -71,7 +70,7 @@
@support_nddata(data='array')
def convolve(array, kernel, boundary='fill', fill_value=0.,
nan_treatment='interpolate', normalize_kernel=True, mask=None,
preserve_nan=False, normalization_zero_tol=1e-8, n_threads=0):
preserve_nan=False, normalization_zero_tol=1e-8):
'''
Convolve an array with a kernel.
Expand Down Expand Up @@ -130,12 +129,6 @@ def convolve(array, kernel, boundary='fill', fill_value=0.,
The absolute tolerance on whether the kernel is different than zero.
If the kernel sums to zero to within this precision, it cannot be
normalized. Default is "1e-8".
n_threads : int
The number of threads to use (default 0). 0 => use all. There is no
limit, be cautious if over commmiting resources.
Note that an exception is raised for negative values. A warning is issued
if n_threads>1 and astropy was NOT built with OpenMP support.
With DASK, it is more performant to set ``n_threads=1``.
Returns
-------
Expand All @@ -159,7 +152,9 @@ def convolve(array, kernel, boundary='fill', fill_value=0.,
if nan_treatment not in ('interpolate', 'fill'):
raise ValueError("nan_treatment must be one of 'interpolate','fill'")

n_threads = handle_n_threads_usage(n_threads)
# OpenMP support is disabled at the C src code level, changing this will have
# no effect.
n_threads = 1

# Keep refs to originals
passed_kernel = kernel
Expand Down
3 changes: 0 additions & 3 deletions astropy/convolution/setup_package.py
Expand Up @@ -3,7 +3,6 @@
import os
import sys
from distutils.extension import Extension
from astropy_helpers.openmp_helpers import add_openmp_flags_if_available

C_CONVOLVE_PKGDIR = os.path.relpath(os.path.dirname(__file__))

Expand All @@ -23,6 +22,4 @@ def get_extensions():
include_dirs=["numpy"],
language='c')

add_openmp_flags_if_available(lib_convolve_ext)

return [lib_convolve_ext]
3 changes: 3 additions & 0 deletions astropy/convolution/src/convolve.h
Expand Up @@ -3,6 +3,9 @@

#include <stddef.h>

// Forcibly disable OpenMP support at the src level
#undef _OPENMP

#if defined(_MSC_VER)

#define FORCE_INLINE __forceinline
Expand Down
35 changes: 0 additions & 35 deletions astropy/utils/openmp.py

This file was deleted.

6 changes: 0 additions & 6 deletions setup.py
Expand Up @@ -22,7 +22,6 @@
from astropy_helpers.distutils_helpers import is_distutils_display_option
from astropy_helpers.git_helpers import get_git_devstr
from astropy_helpers.version_helpers import generate_version_py
from astropy_helpers.openmp_helpers import generate_openmp_enabled_py

import astropy

Expand All @@ -46,11 +45,6 @@
generate_version_py(NAME, VERSION, RELEASE, get_debug_option(NAME),
uses_git=not RELEASE)

# Generate function to check OpenMP support at runtime
# The current default is to NOT build with OpenMP support, i.e. ``disable_openmp = True``
disable_openmp = os.environ.get('ASTROPY_SETUP_WITH_OPENMP', '0') == '0'
generate_openmp_enabled_py(NAME, disable_openmp=disable_openmp)

# Get configuration information from all of the various subpackages.
# See the docstring for setup_helpers.update_package_files for more
# details.
Expand Down

0 comments on commit c1c6a70

Please sign in to comment.