devernay / cminpack

A C/C++ rewrite of the MINPACK software (originally in FORTRAN) for solving nonlinear equations and nonlinear least squares problems
http://devernay.free.fr/hacks/cminpack/
145 stars 63 forks source link

improvement: BLAS support on all CI configurations on Github workflows #73

Open luau-project opened 2 months ago

luau-project commented 2 months ago

What?

I have improved BLAS building/testing on all supported CI configurations on Github workflows (Ubuntu, MSYS2, MSVC).

Why?

It eases the process to check if future changes to the code is somewhat damaging the current state for BLAS support.

How?

[!NOTE]

In the MSVC CI rework, I had to remove the building/testing of x86 binaries.

devernay commented 2 months ago

My main comment is that if testing has to be done, it shouldn't be against the reference BLAS implementation, which is basically the same as the native minpack code, but against a modern implementation. I think OpenBLAS fits that purpose, as it's 10x faster than the reference implementation. It's also available for any platform, including MSVC (see https://vcpkg.link/ports/openblas).

Then, I don't think we should add a new dimension to the testing matrix for every feature we want to test. Only one configuration has to be tested with BLAS support. This is what I did for Linux, please do the same for Windows. The same would hold if/when we add LAPACK support to the cmake builds (it's easy, the code is there already)

luau-project commented 2 months ago

Update

I have followed your advices:

[!NOTE]

Since you voted for a compact build matrix, I took the choice to narrow the matrix for the most common build types on every workflow (Release, Debug).

luau-project commented 2 months ago

On a test branch (not committed here), I was able to also build / test cminpack (+ OpenBLAS) with clang-cl, which is clang with ABI-compatibility to MSVC toolchain. IMO, it is a nice addition to the MSVC CI.

Would you like to add it?