Overhaul xgboost packages (#21661)

* Overhaul xgboost packages
* Don't deprecate xgboost 0.90
* Add py-xgboost@0.90, but deprecate it
* extras_require added in 1.0.0
* Deprecate py-dask-xgboost
* Simplify setting cuda arch

3 files changed, 96 insertions, 18 deletions

diff --git a/var/spack/repos/builtin/packages/py-dask-xgboost/package.py b/var/spack/repos/builtin/packages/py-dask-xgboost/package.py
index 4bbe21486f..3428d49d1e 100644
--- a/var/spack/repos/builtin/packages/py-dask-xgboost/package.py
+++ b/var/spack/repos/builtin/packages/py-dask-xgboost/package.py
@@ -7,12 +7,15 @@ from spack import *
 
 
 class PyDaskXgboost(PythonPackage):
-    """Distributed training with XGBoost and Dask.distributed."""
+    """Distributed training with XGBoost and Dask.distributed.
+
+    Deprecated: use `py-xgboost+dask` instead."""
 
     homepage = "https://github.com/dask/dask-xgboost/"
     pypi = "dask-xgboost/dask-xgboost-0.1.11.tar.gz"
 
-    version('0.1.11', sha256='3fbe1bf4344dc74edfbe9f928c7e3e6acc26dc57cefd8da8ae56a15469c6941c')
+    # Deprecated, see https://github.com/dask/dask-xgboost/issues/80
+    version('0.1.11', sha256='3fbe1bf4344dc74edfbe9f928c7e3e6acc26dc57cefd8da8ae56a15469c6941c', deprecated=True)
 
     variant('sparse', default=False, description='Add sparse support')
 
diff --git a/var/spack/repos/builtin/packages/py-xgboost/package.py b/var/spack/repos/builtin/packages/py-xgboost/package.py
index 5a12966afe..746d58a1f4 100644
--- a/var/spack/repos/builtin/packages/py-xgboost/package.py
+++ b/var/spack/repos/builtin/packages/py-xgboost/package.py
@@ -3,6 +3,7 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
+import os
 
 from spack import *
 
@@ -14,15 +15,25 @@ class PyXgboost(PythonPackage):
     homepage  = 'https://xgboost.ai/'
     pypi = 'xgboost/xgboost-1.3.3.tar.gz'
 
+    maintainers = ['adamjstewart']
+    import_modules = ['xgboost']
+
     version('1.3.3', sha256='397051647bb837915f3ff24afc7d49f7fca57630ffd00fb5ef66ae2a0881fb43')
+    version('0.90',  sha256='d69f90d61a63e8889fd39a31ad00c629bac1ca627f8406b9b6d4594c9e29ab84', deprecated=True)
 
     variant('pandas',       default=False, description='Enable Pandas extensions for training.')
     variant('scikit-learn', default=False, description='Enable scikit-learn extensions for training.')
     variant('dask',         default=False, description='Enables Dask extensions for distributed training.')
     variant('plotting',     default=False, description='Enables tree and importance plotting.')
 
-    depends_on('cmake',         type='build')
-    depends_on('python@3.6:',   type=('build', 'run'))
+    for ver in ['1.3.3']:
+        depends_on('xgboost@' + ver, when='@' + ver)
+
+    depends_on('cmake@3.12:', when='@1.0:1.2.999', type='build')
+    depends_on('llvm-openmp', when='@:1.2.999 %apple-clang')
+    depends_on('python@3.6:', when='@1.2:', type=('build', 'run'))
+    depends_on('python@3.5:', when='@1.0:', type=('build', 'run'))
+    depends_on('python@3.4:',   type=('build', 'run'))
     depends_on('py-setuptools', type=('build'))
     depends_on('py-numpy',      type=('build', 'run'))
     depends_on('py-scipy',      type=('build', 'run'))
@@ -37,3 +48,43 @@ class PyXgboost(PythonPackage):
 
     depends_on('py-graphviz',   when='+plotting', type=('build', 'run'))
     depends_on('py-matplotlib', when='+plotting', type=('build', 'run'))
+
+    conflicts('+pandas', when='@:0.999')
+    conflicts('+scikit-learn', when='@:0.999')
+    conflicts('+dask', when='@:0.999')
+    conflicts('+plotting', when='@:0.999')
+
+    # `--use-system-libxgboost` is only valid for the 'install' phase, but we want to
+    # skip building of the C++ library and rely on an external dependency
+    phases = ['install']
+
+    @when('@:0.90')
+    def patch(self):
+        # Fix OpenMP support on macOS
+        filter_file("OPENMP_FLAGS = -fopenmp",
+                    "OPENMP_FLAGS = {0}".format(self.compiler.openmp_flag),
+                    os.path.join("xgboost", "Makefile"), string=True)
+
+    @when('@1.3:')
+    def patch(self):
+        # https://github.com/dmlc/xgboost/issues/6706
+        # 'setup.py' is hard-coded to search in Python installation prefix
+        filter_file("lib_path = os.path.join(sys.prefix, 'lib')",
+                    "lib_path = '{0}'".format(self.spec['xgboost'].libs.directories[0]),
+                    "setup.py", string=True)
+
+        # Same for run-time search
+        filter_file("os.path.join(curr_path, 'lib'),",
+                    "'{0}',".format(self.spec['xgboost'].libs.directories[0]),
+                    os.path.join('xgboost', 'libpath.py'), string=True)
+
+    @when('@1.3:')
+    def install_args(self, spec, prefix):
+        args = super(PyXgboost, self).install_args(spec, prefix)
+        args.append('--use-system-libxgboost')
+        return args
+
+    # Tests need to be re-added since `phases` was overridden
+    run_after('install')(
+        PythonPackage._run_default_install_time_test_callbacks)
+    run_after('install')(PythonPackage.sanity_check_prefix)
diff --git a/var/spack/repos/builtin/packages/xgboost/package.py b/var/spack/repos/builtin/packages/xgboost/package.py
index 374e89fbdb..5b9476faf6 100644
--- a/var/spack/repos/builtin/packages/xgboost/package.py
+++ b/var/spack/repos/builtin/packages/xgboost/package.py
@@ -7,25 +7,49 @@ from spack import *
 
 
 class Xgboost(CMakePackage, CudaPackage):
-    """Scalable, Portable and Distributed Gradient Boosting (GBDT, GBRT or GBM)
-       Library, for Python, R, Java, Scala, C++ and more. Runs on single
-       machine, Hadoop, Spark, Flink and DataFlow"""
+    """XGBoost is an optimized distributed gradient boosting library designed to be
+    highly efficient, flexible and portable. It implements machine learning algorithms
+    under the Gradient Boosting framework. XGBoost provides a parallel tree boosting
+    (also known as GBDT, GBM) that solve many data science problems in a fast and
+    accurate way. The same code runs on major distributed environment (Hadoop, SGE, MPI)
+    and can solve problems beyond billions of examples."""
 
     homepage = "https://xgboost.ai/"
-    url      = "https://github.com/dmlc/xgboost/releases/download/v0.81/xgboost-0.81.tar.bz2"
     git      = "https://github.com/dmlc/xgboost.git"
 
-    version('0.90', tag='v0.90', submodules=True)
-    version('0.81', sha256='9d8ff161699111d45c96bd15229aa6d80eb1cab7cbbef7e8eaa60ccfb5a4f806')
+    maintainers = ['adamjstewart']
+
+    version('master', branch='master', submodules=True)
+    version('1.3.3', tag='v1.3.3', submodules=True)
+    version('0.90', tag='v0.90', submodules=True, deprecated=True)
+    version('0.81', tag='v0.81', submodules=True, deprecated=True)
+
+    variant('nccl', default=False, description='Build with NCCL to enable distributed GPU support')
+    variant('openmp', default=True, description='Build with OpenMP support')
+
+    depends_on('cmake@3.13:', type='build')
+    depends_on('cmake@3.16:', when='platform=darwin', type='build')
+    depends_on('ninja', type='build')
+    depends_on('cuda@10:', when='+cuda')
+    depends_on('nccl', when='+nccl')
+    depends_on('llvm-openmp', when='%apple-clang +openmp')
+
+    conflicts('%gcc@:4.999', msg='GCC version must be at least 5.0!')
+    conflicts('+nccl', when='~cuda', msg='NCCL requires CUDA')
+    conflicts('+cuda', when='~openmp', msg='CUDA requires OpenMP')
+
+    generator = 'Ninja'
 
     def cmake_args(self):
-        return [
-            '-DUSE_CUDA={0}'.format('YES' if '+cuda' in self.spec else 'NO')
+        # https://xgboost.readthedocs.io/en/latest/build.html
+        args = [
+            self.define_from_variant('USE_CUDA', 'cuda'),
+            self.define_from_variant('USE_NCCL', 'nccl'),
+            self.define_from_variant('USE_OPENMP', 'openmp'),
         ]
 
-    def install(self, spec, prefix):
-        install_tree(str(self.stage.source_path), prefix)
-        # create a bin directory for executable "xgboost" which is possibly
-        # used in functional testing of the compilation target "libxgboost"
-        mkdirp(prefix.bin)
-        install('xgboost', prefix.bin)
+        if '+cuda' in self.spec and 'cuda_arch=none' not in self.spec:
+            args.append(self.define(
+                'GPU_COMPUTE_VER', self.spec.variants['cuda_arch'].value))
+
+        return args