Re: [Buildroot] [PATCH v5 3/3] package/armadillo: allow to select between lapack or openblas

[Thomas Petazzoni <thomas.petazzoni@bootlin.com>]

Hello,

On Mon, 26 Jul 2021 08:24:37 +0200
Arnout Vandecappelle <arnout@mind.be> wrote:

> > fortran is only required if lapack has to be used: if lapack is said as optional this is
> > maybe not required ?  
> 
>  Oh, I intended to insert a comment there but apparently forgot: We need either
> openblas (which doesn't require fortran) or lapack (which does require fortran).
> Therefore, on architectures which do support openblas, we should have a comment
> that just says "C++", while on architectures that don't support openblas (but do
> support lapack) we should have a comment that says "C++ and Fortran".

... but this is exactly what your patch does:

comment "armadillo needs a toolchain w/ C++"
        depends on BR2_PACKAGE_OPENBLAS_ARCH_SUPPORTS
        depends on !BR2_INSTALL_LIBSTDCPP

comment "armadillo needs a toolchain w/ fortran, C++"
        depends on !BR2_PACKAGE_OPENBLAS_ARCH_SUPPORTS # otherwise, see comment above
        depends on BR2_PACKAGE_LAPACK_ARCH_SUPPORTS
        depends on !BR2_TOOLCHAIN_HAS_FORTRAN || !BR2_INSTALL_LIBSTDCPP

If we're on an arch supported by openblas, "needs a toolchain w/ C++"
is displayed.

If we're on an arch not supported by openblas, but supported by lapack,
"needs a toolchain w/ fortran, C++" is displayed.

So it all looks good to me.


> > It's an open question:
> > until now armadillo was always built with blas and lapack support provided by
> > clapack. To avoid breaking current behaviour (lapack support) why not
> > considering selecting lapack package unconditionaly and let user choise for
> > blas support? I know the main issue is the dependency to fortran...  
> 
>  What you're basically saying is that in the choice between lapack and openblas,
> lapack should be the default, right?
> 
>  Can you maybe explain a bit what your original reason for this patch was?
> Armadillo basically hides the BLAS implementation, so unless it's buggy, you
> shouldn't even notice which implementation is behind. Also, the lapack package
> itself says that you should not use its BLAS implementation but you should use a
> better one instead.

I was also confused by Gwenhael's comment, including with his reply to
your question. So at this point, I've applied your patch as-is, we can
always improve/fix later on if needed.

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot