Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Unit tests] skip BLAS and LAPACK unit tests if Netlib is not found #577

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

mkrainiuk
Copy link
Contributor

@mkrainiuk mkrainiuk commented Oct 1, 2024

Description

This PR implements one more possible solution for the problem described in #557
In case Netlib libraries are not available it shouldn't block completely oneMKL Interfaces building, instead we can generate warning that BLAS and LAPACK unit tests will be skipped because of Netlib absence.

This PR also includes some pre-work for adding BLAS and LAPACK tests that won't require Netlib.

  • Example of the output users will get if Netlib is found
-- Found LAPACKE: /usr/lib64/liblapacke64.so
-- Found LAPACKE: /usr/lib64/liblapack64.so
-- Found LAPACKE: /usr/lib64/libcblas64.so
-- Found LAPACKE: /usr/lib64/libblas64.so
-- Found LAPACKE: /usr/include
...
-- Configuring done
-- Generating done
  • Example of the output users will get if Netlib is NOT found
-- Could NOT find LAPACKE (missing: LAPACKE64_file)
-- Could NOT find LAPACKE (missing: LAPACK64_file)
-- Could NOT find LAPACKE (missing: CBLAS64_file)
-- Could NOT find LAPACKE (missing: BLAS64_file)
-- Could NOT find LAPACKE (missing: LAPACKE_INCLUDE LAPACKE_LINK)
CMake Warning at tests/unit_tests/CMakeLists.txt:33 (message):
  Netlib LAPACKE headers or libraries are not found, LAPACK unit tests will
  be skipped
...
-- Configuring done
-- Generating done

Checklist

All Submissions

  • Do all unit tests pass locally? Attach a log. Both public and internal CIs were passed
  • Have you formatted the code using clang-format? N/A, all changes are CMake related only

tests/unit_tests/CMakeLists.txt Outdated Show resolved Hide resolved
tests/unit_tests/blas/batch/CMakeLists.txt Outdated Show resolved Hide resolved
@mkrainiuk mkrainiuk marked this pull request as ready for review October 15, 2024 00:27
@mkrainiuk mkrainiuk requested review from a team as code owners October 15, 2024 00:27
@Rbiessy
Copy link
Contributor

Rbiessy commented Oct 15, 2024

LGTM!

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.

Look good, thanks for the changes!

@@ -29,7 +36,7 @@ if(BUILD_SHARED_LIBS)
PUBLIC ${PROJECT_SOURCE_DIR}/include
PUBLIC ${PROJECT_SOURCE_DIR}/deps/googletest/include
PUBLIC ${CMAKE_BINARY_DIR}/bin
PUBLIC ${CBLAS_INCLUDE}
$<$<BOOL:${CBLAS_FOUND}>:${CBLAS_INCLUDE}>
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really follow this syntax, but are we changing this argument from PUBLIC to non-public?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

New logic helps us to drop CBLAS_INCLUDE from the compilation line when CBLAS header file is not found. Otherwise it might appear in the compilation line as -ICBLAS_INCLUDE_not_found and cause test compilation issue.

As for PUBLIC keyword since each BLAS test is a final binary, not library, we don't need to mark this dependency as needed to be propagated to other targets that depend on this target, because we don't have such targets.

@mkrainiuk mkrainiuk merged commit 6923d40 into oneapi-src:develop Oct 15, 2024
6 checks passed
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