better handling of compiler options

.gitignore

@@ -17,6 +17,10 @@ hpfrec.egg-info/
     *.*
 
 *.so
+*.o
+*.out
+*.exe
+*.dylib
 
 hpfrec/
 	*.so

README.md

@@ -38,24 +38,21 @@ Package is available on PyPI, can be installed with:
 pip install hpfrec
 ```
 
-As it contains Cython code, it requires a C compiler. In Windows, this usually means it requires a Visual Studio Build Tools installation (with MSVC140 component for `conda`) (or MinGW + GCC), and if using Anaconda, might also require configuring it to use said Visual Studio instead of MinGW, otherwise the installation from `pip` might fail. For more details see this guide:
-[Cython Extensions On Windows](https://github.com/cython/cython/wiki/CythonExtensionsOnWindows)
-
-On Python 2.7 on Windows, it might additionally require installing extra Visual Basic modules (untested).
-
-On Linux, the `pip` install should work out-of-the-box, as long as the system has `gcc`.
-
-**Note for macOS users:** on macOS, the Python version of this package will compile **without** multi-threading capabilities. This is due to default apple's redistribution of clang not providing OpenMP modules, and aliasing it to gcc which causes confusions in build scripts. If you have a non-apple version of clang with the OpenMP modules, or if you have gcc installed, you can compile this package with multi-threading enabled by setting up an environment variable `ENABLE_OMP=1`:
+Or if that fails:
 ```
-export ENABLE_OMP=1
-pip install hpfrec
+pip install --no-use-pep517 hpfrec
 ```
-(Alternatively, can also pass argument `enable-omp` to the setup.py file: `python setup.py install enable-omp`)
 
-**Note2:** the setup script uses a PEP517 environment, which means it will create an isolated virtual environment, install its build dependencies there, compile, and then copy to the actual environment. This can causes issues - for example, if one has NumPy<1.20 and the build environment installs NumPy>=1.20, there will be a binary incompatibility which will make the package fail to import. To avoid PEP517, install with:
+**Note for macOS users:** on macOS, the Python version of this package might compile **without** multi-threading capabilities. In order to enable multi-threading support, first install OpenMP:
 ```
-pip install --no-use-pep517 hpfrec
+brew install libomp
 ```
+And then reinstall this package: `pip install --force-reinstall hpfrec`.
+
+
+As it contains Cython code, it requires a C compiler. In Windows, this usually means it requires a Visual Studio Build Tools installation (with MSVC140 component for `conda`) (or MinGW + GCC), and if using Anaconda, might also require configuring it to use said Visual Studio instead of MinGW, otherwise the installation from `pip` might fail. For more details see this guide:
+[Cython Extensions On Windows](https://github.com/cython/cython/wiki/CythonExtensionsOnWindows)
+
 
 ## Sample usage
 

hpfrec/__init__.py

@@ -1,6 +1,6 @@
 import pandas as pd, numpy as np
 import multiprocessing, os, warnings
-from . import cython_loops_float, cython_loops_double
+from . import cython_loops_float, cython_loops_double, _check_openmp
 import ctypes, types, inspect
 from scipy.sparse import coo_matrix, csr_matrix
 pd.options.mode.chained_assignment = None
@@ -245,6 +245,13 @@ def __init__(self, k=30, a=0.3, a_prime=0.3, b_prime=1.0,
 		assert ncores>0
 		assert isinstance(ncores, int)
 
+		if (ncores > 1) and not (_check_openmp.get()):
+			msg_omp  = "Attempting to use more than 1 thread, but "
+			msg_omp += "package was built without multi-threading "
+			msg_omp += "support - see the project's GitHub page for "
+			msg_omp += "more information."
+			warnings.warn(msg_omp)
+
 		if random_seed is not None:
 			assert isinstance(random_seed, int)
 

hpfrec/return0.pyx

@@ -0,0 +1,2 @@
+def get():
+    return 0

hpfrec/return1.pyx

@@ -0,0 +1,2 @@
+def get():
+    return 1

setup.py

@@ -7,46 +7,96 @@
 	from distutils.extension import Extension
 from Cython.Distutils import build_ext
 import numpy
-import sys, os
+import sys, os, subprocess, warnings, re
+
+found_omp = True
+def set_omp_false():
+	global found_omp
+	found_omp = False
 
 ## https://stackoverflow.com/questions/724664/python-distutils-how-to-get-a-compiler-that-is-going-to-be-used
 class build_ext_subclass( build_ext ):
 	def build_extensions(self):
-		c = self.compiler.compiler_type
-		if c == 'msvc': # visual studio
+		if self.compiler.compiler_type == 'msvc':
 			for e in self.extensions:
 				e.extra_compile_args = ['/openmp', '/O2']
-		else: # gcc and clang
-			for e in self.extensions:
-				e.extra_compile_args = ['-fopenmp', '-O2', '-march=native', '-std=c99']
-				e.extra_link_args = ['-fopenmp']
-				### Comment: -Ofast gives worse speed than -O2 or -O3
+		else:
+			self.add_march_native()
+			self.add_openmp_linkage()
 
-		## Note: apple will by default alias 'gcc' to 'clang', and will ship its own "special"
-		## 'clang' which has no OMP support and nowadays will purposefully fail to compile when passed
-		## '-fopenmp' flags. If you are using mac, and have an OMP-capable compiler,
-		## comment out the code below, or set 'use_omp' to 'True'.
-		if not use_omp:
 			for e in self.extensions:
-				e.extra_compile_args = [arg for arg in e.extra_compile_args if arg != '-fopenmp']
-				e.extra_link_args    = [arg for arg in e.extra_link_args    if arg != '-fopenmp']
+				# e.extra_compile_args = ['-fopenmp', '-O2', '-march=native', '-std=c99']
+				# e.extra_link_args = ['-fopenmp']
+				### Comment: -Ofast gives worse speed than -O2 or -O3
+				e.extra_compile_args = ['-O2', '-std=c99']
 
 		build_ext.build_extensions(self)
 
+	def add_march_native(self):
+		arg_march_native = "-march=native"
+		arg_mcpu_native = "-mcpu=native"
+		if self.test_supports_compile_arg(arg_march_native):
+			for e in self.extensions:
+				e.extra_compile_args.append(arg_march_native)
+		elif self.test_supports_compile_arg(arg_mcpu_native):
+			for e in self.extensions:
+				e.extra_compile_args.append(arg_mcpu_native)
+
+	def add_openmp_linkage(self):
+		arg_omp1 = "-fopenmp"
+		arg_omp2 = "-qopenmp"
+		arg_omp3 = "-xopenmp"
+		args_apple_omp = ["-Xclang", "-fopenmp", "-lomp"]
+		if self.test_supports_compile_arg(arg_omp1):
+			for e in self.extensions:
+				e.extra_compile_args.append(arg_omp1)
+				e.extra_link_args.append(arg_omp1)
+		elif (sys.platform[:3].lower() == "dar") and self.test_supports_compile_arg(args_apple_omp):
+			for e in self.extensions:
+				e.extra_compile_args += ["-Xclang", "-fopenmp"]
+				e.extra_link_args += ["-lomp"]
+		elif self.test_supports_compile_arg(arg_omp2):
+			for e in self.extensions:
+				e.extra_compile_args.append(arg_omp2)
+				e.extra_link_args.append(arg_omp2)
+		elif self.test_supports_compile_arg(arg_omp3):
+			for e in self.extensions:
+				e.extra_compile_args.append(arg_omp3)
+				e.extra_link_args.append(arg_omp3)
+		else:
+			set_omp_false()
+			for e in self.extensions:
+				e.sources = [re.sub(r"^(.*)return1\.pyx$", r"\1return0.pyx", s) for s in e.sources]
 
-use_omp = (("enable-omp" in sys.argv)
-		   or ("-enable-omp" in sys.argv)
-		   or ("--enable-omp" in sys.argv))
-if use_omp:
-	sys.argv = [a for a in sys.argv if a not in ("enable-omp", "-enable-omp", "--enable-omp")]
-if os.environ.get('ENABLE_OMP') is not None:
-	use_omp = True
-if sys.platform[:3] != "dar":
-	use_omp = True
+	def test_supports_compile_arg(self, comm):
+		is_supported = False
+		try:
+			if not hasattr(self.compiler, "compiler"):
+				return False
+			if not isinstance(comm, list):
+				comm = [comm]
+			print("--- Checking compiler support for option '%s'" % " ".join(comm))
+			fname = "hpfrec_compiler_testing.c"
+			with open(fname, "w") as ftest:
+				ftest.write(u"int main(int argc, char**argv) {return 0;}\n")
+			try:
+				cmd = [self.compiler.compiler[0]]
+			except:
+				cmd = list(self.compiler.compiler)
+			val_good = subprocess.call(cmd + [fname])
+			try:
+				val = subprocess.call(cmd + comm + [fname])
+				is_supported = (val == val_good)
+			except:
+				is_supported = False
+		except:
+			pass
+		try:
+			os.remove(fname)
+		except:
+			pass
+		return is_supported
 
-### Shorthand for apple computer:
-### uncomment line below
-# use_omp = True
 
 setup(
 	name = 'hpfrec',
@@ -57,7 +107,7 @@ def build_extensions(self):
 	 'scipy',
 	 'cython'
 ],
-	version = '0.2.4',
+	version = '0.2.5',
 	description = 'Hierarchical Poisson matrix factorization for recommender systems',
 	author = 'David Cortes',
 	author_email = 'david.cortes.rivera@gmail.com',
@@ -66,15 +116,30 @@ def build_extensions(self):
 	classifiers = [],
 
 	cmdclass = {'build_ext': build_ext_subclass},
-	ext_modules = [ Extension("hpfrec.cython_loops_float", sources=["hpfrec/cython_float.pyx"], include_dirs=[numpy.get_include()]),
-					Extension("hpfrec.cython_loops_double", sources=["hpfrec/cython_double.pyx"], include_dirs=[numpy.get_include()])]
+	ext_modules = [ Extension(
+						"hpfrec.cython_loops_float",
+						sources=["hpfrec/cython_float.pyx"],
+						include_dirs=[numpy.get_include()]
+					),
+					Extension(
+						"hpfrec.cython_loops_double",
+						sources=["hpfrec/cython_double.pyx"],
+						include_dirs=[numpy.get_include()]
+					),
+					Extension(
+						"hpfrec._check_openmp",
+						sources=["hpfrec/return1.pyx"],
+						include_dirs=[numpy.get_include()]
+					) ]
 )
 
-if not use_omp:
-	import warnings
-	apple_msg  = "\n\n\nMacOS detected. Package will be built without multi-threading capabilities, "
-	apple_msg += "due to Apple's lack of OpenMP support in default clang installs. In order to enable it, "
-	apple_msg += "install the package directly from GitHub: https://www.github.com/david-cortes/hpfrec\n"
-	apple_msg += "Using 'python setup.py install enable-omp'. "
-	apple_msg += "You'll also need an OpenMP-capable compiler.\n\n\n"
-	warnings.warn(apple_msg)
+if not found_omp:
+	omp_msg  = "\n\n\nCould not detect OpenMP. Package will be built without multi-threading capabilities. "
+	omp_msg += " To enable multi-threading, first install OpenMP"
+	if (sys.platform[:3] == "dar"):
+		omp_msg += " - for macOS: 'brew install libomp'\n"
+	else:
+		omp_msg += " modules for your compiler. "
+
+	omp_msg += "Then reinstall this package from scratch: 'pip install --force-reinstall hpfrec'.\n"
+	warnings.warn(omp_msg)