Yggdrasil icon indicating copy to clipboard operation
Yggdrasil copied to clipboard

Add ReLAPACK in OpenBLAS

Open amontoison opened this issue 2 years ago • 9 comments

This pull request contains a new build recipe I built using the BinaryBuilder.jl wizard:

  • Package name: ReLAPACK

amontoison avatar Sep 15 '22 00:09 amontoison

@giordano @staticfloat I would like to add ReLAPACK in the artifact of OpenBLAS. One routine is very useful, it's gemmt and it is also provided by MKL. It will update one triangle of C such that C <- alpha * A * B + beta * C. It's relevant when C is hermitian and A = B' for instance. It also adds some variants of the LAPACK routines.

With this PR, the symbol gemmt will be added and the symbols of the other routines will have a prefix RELAPACK. I did this modification because the other routines are variants of the LAPACK routines with the same name. We will be also able to switch between the gemmt of OpenBLAS or MKL with LBT.

The current issue with the compilation is that ReLAPACK compiles .o files with the same names than LAPACK: https://dev.azure.com/JuliaPackaging/Yggdrasil/_build/results?buildId=22259&view=logs&j=ba03ae70-8d82-5361-1127-49c7c21d0cb8&t=d960f7f1-61ec-593e-1474-ee3577b9ed1d&l=22560 What is the simplest modification to not overwrite the .o of LAPACK? I suppose that we can just add the prefix RELAPACK for all .o files generated by ReLAPACK.

amontoison avatar Sep 16 '22 06:09 amontoison

OpenBLAS is a dependency of Julia and I have zero idea about the consequences of this change. I'm a bit worried about all those patches, they seem to change lots of code, and every time we want to upgrade OpenBLAS we'd need to refresh them, that's not exactly maintainable.

giordano avatar Sep 16 '22 18:09 giordano

In the past there has been huge resistance to adding more libraries to the Julia distribution (like for example when I tried to enabled the use of METIS for SuiteSparse). I think the best would be to compile relapack as a standalone library, which will be much easier to manage.

I had once looked into this briefly, but decided not to pursue, since the library doesn't seem like it is actively maintained: https://github.com/HPAC/ReLAPACK

ViralBShah avatar Sep 16 '22 18:09 ViralBShah

Mosè, this PR just adds few routines from ReLAPACK. If they merge my PR: https://github.com/xianyi/OpenBLAS/pull/3770, you will not need to update the patches related to ReLAPACK in the future. I highly doubt that it breaks things.

@ViralBShah ReLAPACK was added in OpenBLAS and they maintain it.

The main goal is to add the gemmt routine, which is also provided by MKL. I plan to add it in Julia and be able to switch the vendor with LBT. It's for this reason that I didn't compiled relapack as a standalone library.

amontoison avatar Sep 16 '22 19:09 amontoison

Ok, I'll review this closely over the weekend. But if we do want to merge this, we also will have to do the appropriate build changes in julia's source build of openblas too.

ViralBShah avatar Sep 16 '22 20:09 ViralBShah

I started to interface gemmt in Julia: https://github.com/JuliaLang/julia/commit/bfc85e30341045e73613e633f22cbb7abe8f4e3c OpenBLAS added it with the release 0.3.21: https://github.com/xianyi/OpenBLAS/issues/861 BLIS also provides this routine.

amontoison avatar Sep 16 '22 20:09 amontoison

I assume we still want to get this merged, right? Sorry, it just fell off the radar for me, but hope to come back to it shortly.

ViralBShah avatar Oct 14 '22 22:10 ViralBShah

Yes, it will be great! In a nutshell, we compile OpenBLAS with ReLAPACK to have the gemmt routine:

if [ ${version_patch} -ge 21 ]; then
  flags+=(BUILD_RELAPACK=1)
fi

but we don't want that the other routines of ReLAPACK replace the routine of LAPACK: #define INCLUDE_ALL 0 should be used instead of #define INCLUDE_ALL 1 in relapack/config.h.

The goal of the other patch is to avoid that the .o files of LAPACK are replaced by those of ReLAPACK because they have the same names. This issue will probably be fixed upstream: https://github.com/xianyi/OpenBLAS/pull/3770.

amontoison avatar Oct 15 '22 21:10 amontoison

I'd prefer to get this sorted out correctly upstream first.

giordano avatar Oct 15 '22 21:10 giordano

They added the gemmt routine in OpenBLAS v"3.22" (https://github.com/xianyi/OpenBLAS/pull/3796). We don't need to compile ReLAPACK! Mosè already opened a PR to compile the new version of OpenBLAS (https://github.com/JuliaPackaging/Yggdrasil/pull/6457).

amontoison avatar Mar 27 '23 05:03 amontoison