Re: BLAS fixes for 2021Q1

[Thomas Klausner <wiz%NetBSD.org@localhost>]

On Wed, Mar 24, 2021 at 01:35:50AM +0100, Dr. Thomas Orgis wrote:
> 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?

Please commit these fixes, and bump PKGREVISION for both since at
least in some cases they will change the binary package.

Thanks,
 Thomas