diff --git a/.travis.yml b/.travis.yml index f2a7f1cfe3f..aaf26d7bbe0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -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' diff --git a/CHANGES.rst b/CHANGES.rst index 789991e3c7a..a744dab1514 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -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 ^^^^^^^^^^^^^^^^^^^ @@ -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 @@ -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 @@ -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) diff --git a/astropy/convolution/convolve.py b/astropy/convolution/convolve.py index 242b1af19b9..c638608aebc 100644 --- a/astropy/convolution/convolve.py +++ b/astropy/convolution/convolve.py @@ -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. @@ -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. @@ -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 ------- @@ -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 diff --git a/astropy/convolution/setup_package.py b/astropy/convolution/setup_package.py index 198f444eefa..e5674023a15 100644 --- a/astropy/convolution/setup_package.py +++ b/astropy/convolution/setup_package.py @@ -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__)) @@ -23,6 +22,4 @@ def get_extensions(): include_dirs=["numpy"], language='c') - add_openmp_flags_if_available(lib_convolve_ext) - return [lib_convolve_ext] diff --git a/astropy/convolution/src/convolve.h b/astropy/convolution/src/convolve.h index 5b8aa6c8ce0..37c549135d7 100644 --- a/astropy/convolution/src/convolve.h +++ b/astropy/convolution/src/convolve.h @@ -3,6 +3,9 @@ #include +// Forcibly disable OpenMP support at the src level +#undef _OPENMP + #if defined(_MSC_VER) #define FORCE_INLINE __forceinline diff --git a/astropy/utils/openmp.py b/astropy/utils/openmp.py deleted file mode 100644 index 135a9bbd610..00000000000 --- a/astropy/utils/openmp.py +++ /dev/null @@ -1,35 +0,0 @@ -# Licensed under a 3-clause BSD style license - see LICENSE.rst - -import warnings -from multiprocessing import cpu_count -from ..openmp_enabled import is_openmp_enabled -from .exceptions import AstropyUserWarning - -def handle_n_threads_usage(n_threads): - ''' - Helper function to check n_thread usage and correct where possible. - ''' - - if not isinstance(n_threads, int) or n_threads < 0: - raise ValueError("n_threads must be a positive integer") - - total_cpus = cpu_count() - if n_threads > 1 and not is_openmp_enabled(): - warnings.warn("n_threads={0} used yet Astropy was NOT built " - "with OpenMP support. " - "Running single threaded only.".format(n_threads), - AstropyUserWarning) - n_threads = 1 - elif n_threads > total_cpus: - warnings.warn("n_threads is greater than the total number " - "of CPUs: {0}. Over commiting threads is unlikely " - "to boost performance and may lock-up your machine. " - "However, if mainly IO intensive the rule of thumb " - "is to use 1.5*total. NOTE: Check hyperthreading status, " - "if ON, using the total number of CPUs will already be over commiting.".format(total_cpus), - AstropyUserWarning) - - if n_threads == 0: - n_threads = total_cpus - - return n_threads diff --git a/setup.py b/setup.py index 6728a66cc26..b844b3d27bb 100755 --- a/setup.py +++ b/setup.py @@ -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 @@ -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.