Rework lapacke CMake module

[tpadioleau]

Fixes #936

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.90%. Comparing base (f1fef3e) to head (5668cf3).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1083   +/-   ##
=======================================
  Coverage   94.90%   94.90%           
=======================================
  Files          64       64           
  Lines        2844     2844           
  Branches      893      893           
=======================================
  Hits         2699     2699           
  Misses         95       95           
  Partials       50       50           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[EmilyBourne]

Did you work out why this was the case? Does CMAKE_PREFIX_PATH always contain an include directory?

[tpadioleau]

Did you work out why this was the case?

Good question, I wonder if it is necessary. Claude added this comment but I am not sure I agree. If the _ROOT variables are not set, nor CMAKE_PREFIX_PATH, CMake should not even find the .so library.

Does CMAKE_PREFIX_PATH always contain an include directory?

I don't think so, in the case of a Spack installation of OpenBLAS it exists.

[EmilyBourne]

This may search in slightly different directories. IF we can determine the difference then we could probably avoid this step

[tpadioleau]

Adding some tests:

• https://github.com/tpadioleau/tests-lapacke/tree/new-findlapacke
• https://github.com/tpadioleau/tests-lapacke/tree/old-findlapacke

It essentially improves the situation for Spack when openblas is installed out of the system paths.

[tpadioleau]

@EmilyBourne I did further cleaning and tests. If you look at the master branch of https://github.com/tpadioleau/tests-lapacke, we can see that the new implementation now works with openblas on macOS and out of the box with Spack (because Spack fills CMAKE_PREFIX_PATH).

The tests check different configurations, from multiple OS, platforms and package managers. The failing tests on the new implementation are related to a problem of installation of atlas.

What do you think ? Should we merge ?

[EmilyBourne]

@EmilyBourne I did further cleaning and tests. If you look at the master branch of https://github.com/tpadioleau/tests-lapacke, we can see that the new implementation now works with openblas on macOS and out of the box with Spack (because Spack fills CMAKE_PREFIX_PATH).

The tests check different configurations, from multiple OS, platforms and package managers. The failing tests on the new implementation are related to a problem of installation of atlas.

What do you think ? Should we merge ?

If everything that is failing now was also failing before then is definitely an improvement