Fix scikit-learn build with Apple clang (#11496)

* Fix scikit-learn build with Apple clang

* Update compiler unit tests

* Fix unit tests

* Fix OpenMP dep

* recursive=True no longer necessary

* Add myself as a maintainer

* Specify which versions require OpenMP

* Typo fixes

* LLVM-OpenMP is the correct dependency, no OMPT

* Flake8 fix

* Undo Apple Clang OpenMP flag change, split into a separate PR

4 files changed, 111 insertions, 14 deletions

diff --git a/var/spack/repos/builtin/packages/llvm-openmp-ompt/package.py b/var/spack/repos/builtin/packages/llvm-openmp-ompt/package.py
index 2b7d94a6c5..3969da869d 100644
--- a/var/spack/repos/builtin/packages/llvm-openmp-ompt/package.py
+++ b/var/spack/repos/builtin/packages/llvm-openmp-ompt/package.py
@@ -50,15 +50,19 @@ class LlvmOpenmpOmpt(CMakePackage):
         # CMAKE rpath variable prevents standalone error
         # where this package wants the llvm tools path
         if '+standalone' in self.spec:
-                cmake_args.extend(
-                    ['-DLIBOMP_STANDALONE_BUILD=true',
-                     '-DCMAKE_BUILD_WITH_INSTALL_RPATH=true',
-                     '-DLIBOMP_USE_DEBUGGER=false'])
+            cmake_args.extend(
+                ['-DLIBOMP_STANDALONE_BUILD=true',
+                 '-DCMAKE_BUILD_WITH_INSTALL_RPATH=true',
+                 '-DLIBOMP_USE_DEBUGGER=false'])
 
         # Build llvm-openmp-ompt using the tr6_forwards branch
         # This requires the version to be 5.0 (50)
         if '@tr6_forwards' in self.spec:
-                cmake_args.extend(
-                    ['-DLIBOMP_OMP_VERSION=50'])
+            cmake_args.extend(
+                ['-DLIBOMP_OMP_VERSION=50'])
 
         return cmake_args
+
+    @property
+    def libs(self):
+        return find_libraries('libomp', root=self.prefix, recursive=True)
diff --git a/var/spack/repos/builtin/packages/llvm-openmp/package.py b/var/spack/repos/builtin/packages/llvm-openmp/package.py
new file mode 100644
index 0000000000..a11162b38d
--- /dev/null
+++ b/var/spack/repos/builtin/packages/llvm-openmp/package.py
@@ -0,0 +1,27 @@
+# Copyright 2013-2019 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+from spack import *
+
+
+class LlvmOpenmp(CMakePackage):
+    """The OpenMP subproject of LLVM contains the components required to build
+    an executable OpenMP program that are outside the compiler itself."""
+
+    homepage = "https://openmp.llvm.org/"
+    url      = "https://releases.llvm.org/8.0.0/openmp-8.0.0.src.tar.xz"
+
+    version('8.0.0', sha256='f7b1705d2f16c4fc23d6531f67d2dd6fb78a077dd346b02fed64f4b8df65c9d5')
+
+    depends_on('cmake@2.8:', type='build')
+
+    def cmake_args(self):
+        # Disable LIBOMP_INSTALL_ALIASES, otherwise the library is installed as
+        # libgomp alias which can conflict with GCC's libgomp.
+        return ['-DLIBOMP_INSTALL_ALIASES=OFF']
+
+    @property
+    def libs(self):
+        return find_libraries('libomp', root=self.prefix, recursive=True)
diff --git a/var/spack/repos/builtin/packages/ompt-openmp/package.py b/var/spack/repos/builtin/packages/ompt-openmp/package.py
index dd87bb7138..9b2018526c 100644
--- a/var/spack/repos/builtin/packages/ompt-openmp/package.py
+++ b/var/spack/repos/builtin/packages/ompt-openmp/package.py
@@ -23,3 +23,7 @@ class OmptOpenmp(CMakePackage):
     conflicts('%gcc@:4.7')
 
     root_cmakelists_dir = 'runtime'
+
+    @property
+    def libs(self):
+        return find_libraries('libomp', root=self.prefix, recursive=True)
diff --git a/var/spack/repos/builtin/packages/py-scikit-learn/package.py b/var/spack/repos/builtin/packages/py-scikit-learn/package.py
index 8559b57946..639279b6aa 100644
--- a/var/spack/repos/builtin/packages/py-scikit-learn/package.py
+++ b/var/spack/repos/builtin/packages/py-scikit-learn/package.py
@@ -10,8 +10,29 @@ class PyScikitLearn(PythonPackage):
     """A set of python modules for machine learning and data mining."""
 
     homepage = "https://pypi.python.org/pypi/scikit-learn"
-    url      = "https://pypi.io/packages/source/s/scikit-learn/scikit-learn-0.18.1.tar.gz"
+    url      = "https://pypi.io/packages/source/s/scikit-learn/scikit-learn-0.21.2.tar.gz"
 
+    maintainers = ['adamjstewart']
+    install_time_test_callbacks = ['install_test', 'import_module_test']
+
+    import_modules = [
+        'sklearn', 'sklearn.tree', 'sklearn.metrics', 'sklearn.ensemble',
+        'sklearn.experimental', 'sklearn.cluster',
+        'sklearn.feature_extraction', 'sklearn.__check_build',
+        'sklearn.semi_supervised', 'sklearn.gaussian_process',
+        'sklearn.compose', 'sklearn.datasets', 'sklearn.externals',
+        'sklearn.linear_model', 'sklearn.impute', 'sklearn.utils',
+        'sklearn.covariance', 'sklearn.neural_network',
+        'sklearn.feature_selection', 'sklearn.inspection', 'sklearn.svm',
+        'sklearn.manifold', 'sklearn.mixture', 'sklearn.preprocessing',
+        'sklearn.model_selection', 'sklearn._build_utils',
+        'sklearn.decomposition', 'sklearn.cross_decomposition',
+        'sklearn.neighbors', 'sklearn.metrics.cluster',
+        'sklearn.ensemble._hist_gradient_boosting'
+    ]
+
+    version('0.21.2', sha256='0aafc312a55ebf58073151b9308761a5fcfa45b7f7730cea4b1f066f824c72db')
+    version('0.21.1', sha256='228d0611e69e5250946f8cd7bbefec75347950f0ca426d0c518db8f06583f660')
     version('0.20.2', sha256='bc5bc7c7ee2572a1edcb51698a6caf11fae554194aaab9a38105d9ec419f29e6')
     version('0.20.0', sha256='97d1d971f8ec257011e64b7d655df68081dd3097322690afa1a71a1d755f8c18')
     version('0.19.1', 'b67143988c108862735a96cf2b1e827a')
@@ -21,12 +42,53 @@ class PyScikitLearn(PythonPackage):
     version('0.17.1', 'a2f8b877e6d99b1ed737144f5a478dfc')
     version('0.13.1', 'acba398e1d46274b8470f40d0926e6a4')
 
-    depends_on('python@2.6:2.8,3.3:', when='@:0.19.1')
-    depends_on('python@2.7:2.8,3.4:', when='@0.20.0:')
-    depends_on('py-numpy@1.6.1:', type=('build', 'run'), when='@:0.19.1')
-    depends_on('py-numpy@1.8.2:', type=('build', 'run'), when='@0.20.0:')
-    depends_on('py-scipy@0.9:', type=('build', 'run'), when='@:0.19.1')
-    depends_on('py-scipy@0.13.3:', type=('build', 'run'), when='@0.20.0:')
+    depends_on('python@2.6:2.8,3.3:', when='@:0.19')
+    depends_on('python@2.7:2.8,3.4:', when='@0.20.0:0.20.999')
+    depends_on('python@3.5:', when='@0.21:')
+    depends_on('py-numpy@1.6.1:', type=('build', 'run'), when='@:0.19')
+    depends_on('py-numpy@1.8.2:', type=('build', 'run'), when='@0.20.0:0.20.999')
+    depends_on('py-numpy@1.11.0:', type=('build', 'run'), when='@0.21:')
+    depends_on('py-scipy@0.9:', type=('build', 'run'), when='@:0.19')
+    depends_on('py-scipy@0.13.3:', type=('build', 'run'), when='@0.20.0:0.20.999')
+    depends_on('py-scipy@0.17.0:', type=('build', 'run'), when='@0.21:')
+    depends_on('py-joblib@0.11:', type=('build', 'run'))
     depends_on('py-cython@0.23:', type='build')
-    depends_on('py-test@3.3.0:', type='test')
+    depends_on('py-cython@0.28.5:', type='build', when='@0.21:')
+    depends_on('py-pytest@3.3.0:', type='test')
+    depends_on('py-pandas', type='test')
     depends_on('py-setuptools', type='build')
+    # Technically not correct, but currently no way to check if we
+    # are using Apple Clang or not.
+    depends_on('llvm-openmp', when='@0.21: %clang platform=darwin')
+
+    def setup_environment(self, spack_env, run_env):
+        # https://scikit-learn.org/stable/developers/advanced_installation.html#mac-osx
+        if self.spec.satisfies('@0.21: %clang platform=darwin'):
+            spack_env.append_flags(
+                'CPPFLAGS', '-Xpreprocessor -fopenmp')
+            spack_env.append_flags(
+                'CFLAGS', self.spec['llvm-openmp'].headers.include_flags)
+            spack_env.append_flags(
+                'CXXFLAGS',
+                self.spec['llvm-openmp'].headers.include_flags)
+            spack_env.append_flags(
+                'LDFLAGS', self.spec['llvm-openmp'].libs.ld_flags)
+            spack_env.append_flags(
+                'DYLD_LIBRARY_PATH',
+                self.spec['llvm-openmp'].libs.directories[0])
+
+            run_env.append_flags(
+                'DYLD_LIBRARY_PATH',
+                self.spec['llvm-openmp'].libs.directories[0])
+
+    def setup_dependent_environment(self, spack_env, run_env, dependent_spec):
+        if self.spec.satisfies('@0.21: %clang platform=darwin'):
+            spack_env.append_flags(
+                'DYLD_LIBRARY_PATH',
+                self.spec['llvm-openmp'].libs.directories[0])
+
+    def install_test(self):
+        # https://scikit-learn.org/stable/developers/advanced_installation.html#testing
+        with working_dir('spack-test', create=True):
+            pytest = which('pytest')
+            pytest(join_path(self.prefix, site_packages_dir, 'sklearn'))