tech-pkg archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

BLAS fixes for 2021Q1



Hi,

there are two points that really should be fixed regarding BLAS support
before shipping 2021Q1. As this concerns packages far from the leaves,
I hope for some agreement here.

One is simple:

Index: Makefile
===================================================================
RCS file: /cvsroot/pkgsrc/math/lapack/Makefile,v
retrieving revision 1.41
diff -r1.41 Makefile
10c10
< 	-DBLAS_LIBRARIES=${BLAS_LIBS:Q} \
---
> 	-DBLAS_LIBRARIES=-lblas \


This is a clear error in the math/lapack package, which shall always
use the netlib BLAS library and not what mk/blas.buildlink3.mk says.
The latter isn't even included, resulting in -DBLAS_LIBRARIES= (empty),
anyway. It is safer to explicitly set the name of the BLAS library
there.

The other one is potentially even more sensitive: math/py-numpy. Yes,
that is quite the opposite of a leaf package. But it's also in a sad
state regarding usage of BLAS libraries. It sortof uses the framework,
but does not actually honour the choices it makes. The configuratio
code of numpy needs to be neutered not to overrule the pkgsrc choice. I
hope to eventually get that upstreamed, but for now hacking is needed.

Since many weeks now, there are necessary changes for it lurking in
wip/py-numpy:

Index: Makefile
===================================================================
RCS file: /cvsroot/pkgsrc/math/py-numpy/Makefile,v
retrieving revision 1.74
diff -r1.74 Makefile
20a21
> MAKE_ENV+=		BLAS_LIBS=${BLAS_LIBS:Q} LAPACK_LIBS=${LAPACK_LIBS:Q}
Index: buildlink3.mk
===================================================================
RCS file: /cvsroot/pkgsrc/math/py-numpy/buildlink3.mk,v
retrieving revision 1.7
diff -r1.7 buildlink3.mk
20a21,24
> # Dependend python packages re-use the BLAS_LIBS logic and
> # need this in the environment.
> MAKE_ENV+=	BLAS_LIBS=${BLAS_LIBS:Q} LAPACK_LIBS=${LAPACK_LIBS:Q}
> 
Index: distinfo
===================================================================
RCS file: /cvsroot/pkgsrc/math/py-numpy/distinfo,v
retrieving revision 1.56
diff -r1.56 distinfo
10c10
< SHA1 (patch-numpy_distutils_system__info.py) = 01879a0ad3c5eb0133fcce46ce10a52cdd3df7a4
---
> SHA1 (patch-numpy_distutils_system__info.py) = 21a7867e2fe1e080f51f5f8e7cc92958aafd1c62
Index: patches/patch-numpy_distutils_system__info.py
===================================================================
RCS file: /cvsroot/pkgsrc/math/py-numpy/patches/patch-numpy_distutils_system__info.py,v
retrieving revision 1.3
diff -r1.3 patch-numpy_distutils_system__info.py
5c5
< --- numpy/distutils/system_info.py.orig	2020-04-19 08:51:58.000000000 +0000
---
> --- numpy/distutils/system_info.py.orig	2020-06-02 05:24:58.000000000 +0000
7,8c7,8
< @@ -1644,14 +1644,6 @@ class lapack_opt_info(system_info):
<          return False
---
> @@ -1730,34 +1722,15 @@ class lapack_opt_info(system_info):
>          return getattr(self, '_calc_info_{}'.format(name))()
10,19c10,47
<      def _calc_info_openblas(self):
< -        info = get_info('openblas_lapack')
< -        if info:
< -            self.set_info(**info)
< -            return True
< -        info = get_info('openblas_clapack')
< -        if info:
< -            self.set_info(**info)
< -            return True
<          return False
---
>      def calc_info(self):
> -        user_order = os.environ.get(self.order_env_var_name, None)
> -        if user_order is None:
> -            lapack_order = self.lapack_order
> -        else:
> -            # the user has requested the order of the
> -            # check they are all in the available list, a COMMA SEPARATED list
> -            user_order = user_order.lower().split(',')
> -            non_existing = []
> -            lapack_order = []
> -            for order in user_order:
> -                if order in self.lapack_order:
> -                    lapack_order.append(order)
> -                elif len(order) > 0:
> -                    non_existing.append(order)
> -            if len(non_existing) > 0:
> -                raise ValueError("lapack_opt_info user defined "
> -                                 "LAPACK order has unacceptable "
> -                                 "values: {}".format(non_existing))
> -
> -        for lapack in lapack_order:
> -            if self._calc_info(lapack):
> -                return
> -
> -        if 'lapack' not in lapack_order:
> -            # Since the user may request *not* to use any library, we still need
> -            # to raise warnings to signal missing packages!
> -            warnings.warn(LapackNotFoundError.__doc__ or '', stacklevel=2)
> -            warnings.warn(LapackSrcNotFoundError.__doc__ or '', stacklevel=2)
> +        # Fixing usage of LAPACK specified in LAPACK_LIBS.
> +        # Existence of LAPACK_LIBS is mandatory. Things shall break early
> +        # if it is not set.
> +        info = {}
> +        info['language'] = 'f77'
> +        info['extra_link_args'] = os.environ['LAPACK_LIBS'].split()
> + 
> +        self.set_info(**info)
> +        return
21c49,95
<      def _calc_info_flame(self):
---
>  
>  class _ilp64_opt_info_mixin:
> @@ -1875,32 +1848,18 @@ class blas_opt_info(system_info):
>          return getattr(self, '_calc_info_{}'.format(name))()
>  
>      def calc_info(self):
> -        user_order = os.environ.get(self.order_env_var_name, None)
> -        if user_order is None:
> -            blas_order = self.blas_order
> -        else:
> -            # the user has requested the order of the
> -            # check they are all in the available list
> -            user_order = user_order.lower().split(',')
> -            non_existing = []
> -            blas_order = []
> -            for order in user_order:
> -                if order in self.blas_order:
> -                    blas_order.append(order)
> -                elif len(order) > 0:
> -                    non_existing.append(order)
> -            if len(non_existing) > 0:
> -                raise ValueError("blas_opt_info user defined BLAS order has unacceptable values: {}".format(non_existing))
> -
> -        for blas in blas_order:
> -            if self._calc_info(blas):
> -                return
> -
> -        if 'blas' not in blas_order:
> -            # Since the user may request *not* to use any library, we still need
> -            # to raise warnings to signal missing packages!
> -            warnings.warn(BlasNotFoundError.__doc__ or '', stacklevel=2)
> -            warnings.warn(BlasSrcNotFoundError.__doc__ or '', stacklevel=2)
> +        # Fixing usage of libcblas and the BLAS specified in BLAS_LIBS.
> +        # Existence of BLAS_LIBS is mandatory. Things shall break early
> +        # if it is not set.
> +        info = {}
> +        # I do not want to assume a language. It is potentially mixed anyway.
> +        #info['language'] = 'c'
> +        info['libraries'] = ['cblas']
> +        info['define_macros'] = [('HAVE_CBLAS', None)]
> +        info['extra_link_args'] = os.environ['BLAS_LIBS'].split()
> + 
> +        self.set_info(**info)
> +        return
>  
>  
>  class blas_ilp64_opt_info(blas_opt_info, _ilp64_opt_info_mixin):


This is a repeated iteration of the patch I use locally since some
pkgsrc releases. So far nobody dared or had the time to import it. The
freeze is not the best time to start, but it would be very unfortunate
to keep shipping math/py-numpy in a state where it claims to use
mk/blas.buildlink.mk, but really doesn't in the build process.

So … are these two fixes allowed for 2021Q1?


Alrighty then,

Thomas

-- 
Dr. Thomas Orgis
HPC @ Universität Hamburg


Home | Main Index | Thread Index | Old Index