Closed djhoese closed 7 months ago
Is there a way we could activate openmp support at runtime? so that it's compiled with it, but if not present at runtime it issues a warning and runs without it? Otherwise I think 3 might be the best, if we then issue an error with a link on how to recompile without openmp when openmp isn't installed.
In https://github.com/bjlittle/geovista/issues/620 the pykdtree
runtime error occurred only when importing pykdtree.kdtree
. It does not occur if only pykdtree
is imported. Perhaps this means that a simple try..catch
block can be used in an __init__.py
file somewhere to, at the very least, convert this cryptic and broad error
ImportError: dlopen(/.../venv/lib/python3.12/site-packages/pykdtree/kdtree.cpython-312-darwin.so, 0x0002): symbol not found in flat namespace '___kmpc_for_static_fini'`
into a much more helpful error message which may point the user to a sensible solution? (e.g. msg = "install with brew or install with --no-binary"
, or msg = "visit pykdree/#106 for details"
)?
This way, although the runtime error is still present, at least the user can easily try one of the two solutions hinted from the error message, and resolve this issue very quickly and easily. Perhaps this isn't ideal, but at least the user will still be able to take advantage of the performance improvements with OpenMP
.
I had a similar thought. I was wondering if it would be possible at build time to build both versions of the extensions. We always build the one without OpenMP, but if we detect OpenMP we build that version too and have two possible extensions to import. Then we try/except on the OpenMP extension import and warn and fallback if we have to use the non-OpenMP version. If the OpenMP extension file(s) don't even exist then we know that the build process wasn't even able to find OpenMP so there is no reason to warn in that case...I think.
So I played around with my idea mentioned above with having two extensions, one compiled with OpenMP or at least attempted and one explicitly not including OpenMP. It all kind of worked except it seemed that Cython only compiled the cython module for a specific name once so when trying to import the second no-OpenMP version it gets confused by what the module name is.
Subject: [PATCH] Dual extensions with and without openmp
---
Index: setup.py
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/setup.py b/setup.py
--- a/setup.py (revision 75804c99cc873ccc74b19e47c3a90003fd6fce4b)
+++ b/setup.py (date 1705517886814)
@@ -20,8 +20,9 @@
import re
import numpy as np
-from Cython.Build import cythonize
-from setuptools import setup, Extension
+from Cython.Build import build_ext
+from Cython.Distutils import Extension
+from setuptools import setup
from setuptools.command.build_ext import build_ext
@@ -56,17 +57,20 @@
comp = self.compiler.compiler_type
omp_comp, omp_link = _omp_compile_link_args(comp)
if comp in ('unix', 'cygwin', 'mingw32'):
- extra_compile_args = ['-std=c99', '-O3'] + omp_comp
+ extra_compile_args = ['-std=c99', '-O3']
extra_link_args = omp_link
elif comp == 'msvc':
- extra_compile_args = ['/Ox'] + omp_comp
+ extra_compile_args = ['/Ox']
extra_link_args = omp_link
else:
# Add support for more compilers here
raise ValueError('Compiler flags undefined for %s. Please modify setup.py and add compiler flags'
% comp)
- self.extensions[0].extra_compile_args = extra_compile_args
+ # Only apply OpenMP specific flags to OpenMP
+ self.extensions[0].extra_compile_args = extra_compile_args + omp_comp
self.extensions[0].extra_link_args = extra_link_args
+ # No OpenMP version of the extension
+ self.extensions[1].extra_compile_args = extra_compile_args
build_ext.build_extensions(self)
@@ -186,12 +190,19 @@
readme = readme_file.read()
extensions = [
- Extension('pykdtree.kdtree', sources=['pykdtree/kdtree.pyx', 'pykdtree/_kdtree_core.c'],
+ Extension('pykdtree.kdtree_tryomp', sources=['pykdtree/kdtree.pyx', 'pykdtree/_kdtree_core.c'],
+ include_dirs=[np.get_include()],
+ define_macros=[("NPY_NO_DEPRECATED_API", "NPY_1_7_API_VERSION")],
+ cython_directives={"language_level": "3"},
+ ),
+ # NOTE: Compilation flags handled in build_ext_subclass (assumes second extension is noomp)
+ Extension('pykdtree.kdtree_noomp', sources=['pykdtree/kdtree.pyx', 'pykdtree/_kdtree_core.c'],
include_dirs=[np.get_include()],
define_macros=[("NPY_NO_DEPRECATED_API", "NPY_1_7_API_VERSION")],
- compiler_directions={"language_level": "3"},
+ cython_directives={"language_level": "3"},
),
]
+
setup(
name='pykdtree',
@@ -206,7 +217,7 @@
install_requires=['numpy'],
tests_require=['pytest'],
zip_safe=False,
- ext_modules=cythonize(extensions),
+ ext_modules=extensions,
cmdclass={'build_ext': build_ext_subclass},
classifiers=[
'Development Status :: 5 - Production/Stable',
Index: pykdtree/__init__.py
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/pykdtree/__init__.py b/pykdtree/__init__.py
--- a/pykdtree/__init__.py (revision 75804c99cc873ccc74b19e47c3a90003fd6fce4b)
+++ b/pykdtree/__init__.py (date 1705518033734)
@@ -1,0 +1,15 @@
+"""Simple wrapper around importing pykdtree for OpenMP use or not."""
+
+try:
+ from . import kdtree_tryomp as kdtree
+ raise ImportError("pykdtree")
+except ImportError:
+ import warnings
+ warnings.warn(
+ """Pykdtree failed to import its C extension. This usually means it
+was built with OpenMP (C-level parallelization library) support but could not
+find it on your system. Pykdtree will use a less performant version of the
+algorithm instead. To enable better performance OpenMP must be installed
+(ex. ``brew install omp`` on Mac with HomeBrew)."""
+ )
+ from . import kdtree_noomp as kdtree
\ No newline at end of file
$ python -c "from pykdtree import kdtree"
/home/davidh/repos/git/pykdtree/pykdtree/__init__.py:8: UserWarning: Pykdtree failed to import it's C extension. This usually means it
was built with OpenMP (C-level parellelization library) support but could not
find it on your system. Pykdtree will use a less performant version of the
algorithm instead. To enable better performance OpenMP must be installed
(ex. ``brew install omp`` on Mac with HomeBrew).
warnings.warn(
Traceback (most recent call last):
File "/home/davidh/repos/git/pykdtree/pykdtree/__init__.py", line 5, in <module>
raise ImportError("pykdtree")
ImportError: pykdtree
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/home/davidh/repos/git/pykdtree/pykdtree/__init__.py", line 15, in <module>
from . import kdtree_noomp as kdtree
ImportError: dynamic module does not define module export function (PyInit_kdtree_noomp)
Modern versions of pykdtree are released on PyPI as both binary wheels and a source distribution (sdist). Pykdtree includes C extensions that require compilation if installed from the sdist, but if installed from the binary wheel then no C compiler is required on the user's machine as the shared libraries (ex.
.so
files on linux) are bundled with the wheel. Pykdtree also optionally depends on OpenMP. If the extensions are built on a system with OpenMP installed then the extensions are linked to and therefore require OpenMP to exist at runtime. This means if pykdtree's wheels on PyPI were built with OpenMP then any user installing the wheels must also have OpenMP installed or they will get an error when importing pykdtree.This gets complicated in the case of MacOS where the build image used in pykdtree's CI environment includes OpenMP, but it only exists because it is a dependency of one of the other tools/libraries that GitHub Actions includes (via homebrew) in their runner image. Since this isn't a "standard" package when users do
pip install pykdtree
and get the wheel built with OpenMP, they get an error at import time. See https://github.com/bjlittle/geovista/issues/620 for one of these cases. The workarounds are to:brew install omp
I think is the command - I don't have a mac)pip install --force-reinstall --no-binary pykdtree pykdtree
)So our (as pykdtree maintainers) options are:
We could (and should) of course document all of these gotchas.