Skip to content

Conversation

adegomme
Copy link
Contributor

Description

Lapack tests suffered same issue as BLAS tests which was reported in #204 and fixed in #251 . If a backend uses the same LAPACKE interface and symbols as the reference LAPACK, it will end up comparing only the first found library to itself.

This basically copies #251 solution by using dlopen each time the reference library symbols are needed instead of linking to the library directly.

In the process of testing this, I noticed that Lapack tests were not exactly working if only the lapack backend is built, as it depends on the BLAS library as well for accuracy checks. A second commit was added to make Lapack tests work alone, by checking that BLAS reference library can be found even if BLAS backend is not built. It also adds REF_LAPACK_ROOT in the search path for BLAS library, as in the reference design all blas/lapack libraries are in the same location (this avoids having to provide both REF_LAPACK_ROOT and REF_BLAS_ROOT in this case).

  • Do all unit tests pass locally? Attach a log.
    100% tests passed, 0 tests failed out of 796

@adegomme adegomme requested review from a team as code owners July 17, 2025 15:26
Copy link
Contributor

@andrewtbarker andrewtbarker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks good. Thanks for the changes!

Copy link
Contributor

@dnhsieh-intel dnhsieh-intel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes! I only have some minor comments.

One more thing is that the original inconsistency (reference_blas_templates.hpp in BLAS and lapack_reference_wrappers.hpp in LAPACK) now leads to:

  • reference_blas_templates.hpp includes reference_blas_wrappers.hpp in BLAS
  • lapack_reference_wrappers.hpp includes lapack_reference_templates.hpp in LAPACK

I will leave this for @andrewtbarker to decide if we need alignment.

@andrewtbarker
Copy link
Contributor

Thanks for the changes! I only have some minor comments.

One more thing is that the original inconsistency (reference_blas_templates.hpp in BLAS and lapack_reference_wrappers.hpp in LAPACK) now leads to:

  • reference_blas_templates.hpp includes reference_blas_wrappers.hpp in BLAS
  • lapack_reference_wrappers.hpp includes lapack_reference_templates.hpp in LAPACK

I will leave this for @andrewtbarker to decide if we need alignment.

I agree that the inconsistency is not ideal, but I don't feel strongly about the exact file names.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants