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

Add findBLAS support to CMakeLists.txt #844

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open
70 changes: 69 additions & 1 deletion .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,22 +60,24 @@ jobs:
compiler: ${{ matrix.toolchain.compiler }}
version: ${{ matrix.toolchain.version }}

# Build and test with built-in BLAS and LAPACK
- name: Configure with CMake
if: ${{ contains(matrix.build, 'cmake') }}
run: >-
cmake -Wdev -G Ninja
-DCMAKE_BUILD_TYPE=Release
-DCMAKE_MAXIMUM_RANK:String=4
-DCMAKE_INSTALL_PREFIX=$PWD/_dist
-DFIND_BLAS:STRING=FALSE
-S . -B ${{ env.BUILD_DIR }}

- name: Build and compile
if: ${{ contains(matrix.build, 'cmake') }}
run: cmake --build ${{ env.BUILD_DIR }} --parallel

- name: catch build fail
run: cmake --build ${{ env.BUILD_DIR }} --verbose --parallel 1
if: ${{ failure() && contains(matrix.build, 'cmake') }}
run: cmake --build ${{ env.BUILD_DIR }} --verbose --parallel 1

- name: test
if: ${{ contains(matrix.build, 'cmake') }}
Expand All @@ -89,3 +91,69 @@ jobs:
- name: Install project
if: ${{ contains(matrix.build, 'cmake') }}
run: cmake --install ${{ env.BUILD_DIR }}

Build-with-MKL:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
toolchain:
- {compiler: intel, version: '2024.1'}
build: [cmake]
env:
BUILD_DIR: ${{ matrix.build == 'cmake' && 'build' || '.' }}
APT_PACKAGES: >-
intel-oneapi-mkl
intel-oneapi-mkl-devel
steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Set up Python 3.x
uses: actions/setup-python@v5 # Use pip to install latest CMake, & FORD/Jin2For, etc.
with:
python-version: 3.x

- name: Install fypp
run: pip install --upgrade fypp ninja

- name: Setup Fortran compiler
uses: fortran-lang/[email protected]
id: setup-fortran
with:
compiler: ${{ matrix.toolchain.compiler }}
version: ${{ matrix.toolchain.version }}

- name: Install Intel oneAPI MKL
run: |
sudo apt-get install ${APT_PACKAGES}
source /opt/intel/oneapi/mkl/latest/env/vars.sh
printenv >> $GITHUB_ENV

# Build and test with external BLAS and LAPACK (MKL on Ubuntu with Intel compilers)
- name: Configure with CMake and MKL
run: >-
cmake -Wdev -G Ninja
-DCMAKE_BUILD_TYPE=Release
-DCMAKE_MAXIMUM_RANK:String=4
-DCMAKE_INSTALL_PREFIX=$PWD/_dist
-DFIND_BLAS:STRING=TRUE
-S . -B ${{ env.BUILD_DIR }}

- name: Build and compile with MKL
run: cmake --build ${{ env.BUILD_DIR }} --parallel

- name: catch build fail with MKL
if: failure()
run: cmake --build ${{ env.BUILD_DIR }} --verbose --parallel 1

- name: test with MKL
run: >-
ctest
--test-dir ${{ env.BUILD_DIR }}
--parallel
--output-on-failure
--no-tests=error

- name: Install project with MKL
run: cmake --install ${{ env.BUILD_DIR }}
72 changes: 71 additions & 1 deletion .github/workflows/ci_windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ jobs:

- name: Install fypp
run: pip install fypp


# Build and test with built-in BLAS and LAPACK
- run: >-
PATH=$PATH:/mingw64/bin/ cmake
-Wdev
Expand All @@ -65,6 +66,7 @@ jobs:
-DCMAKE_Fortran_FLAGS_DEBUG="-Wall -Wextra -Wimplicit-interface -fPIC -g -fcheck=all -fbacktrace"
-DCMAKE_MAXIMUM_RANK:String=4
-DCMAKE_INSTALL_PREFIX=$PWD/_dist
-DFIND_BLAS:STRING=FALSE
env:
FC: gfortran
CC: gcc
Expand All @@ -88,3 +90,71 @@ jobs:

- name: Install project
run: PATH=$PATH:/mingw64/bin/ cmake --install build

msys2-build-with-OpenBLAS:
runs-on: windows-latest
strategy:
fail-fast: false
matrix:
include: [
{ msystem: MINGW64, arch: x86_64 }
]
defaults:
run:
shell: msys2 {0}
steps:
- uses: actions/checkout@v2

- name: Setup MinGW native environment
uses: msys2/setup-msys2@v2
with:
msystem: ${{ matrix.msystem }}
update: false
install: >-
git
mingw-w64-${{ matrix.arch }}-gcc
mingw-w64-${{ matrix.arch }}-gcc-fortran
mingw-w64-${{ matrix.arch }}-python
mingw-w64-${{ matrix.arch }}-python-pip
mingw-w64-${{ matrix.arch }}-python-setuptools
mingw-w64-${{ matrix.arch }}-cmake
mingw-w64-${{ matrix.arch }}-ninja
mingw-w64-${{ matrix.arch }}-openblas

- name: Install fypp
run: pip install fypp

# Build and test with external BLAS and LAPACK (OpenBLAS on MINGW64)
- name: Configure with CMake and OpenBLAS
run: >-
PATH=$PATH:/mingw64/bin/ cmake
-Wdev
-B build
-DCMAKE_BUILD_TYPE=Debug
-DCMAKE_Fortran_FLAGS_DEBUG="-Wall -Wextra -Wimplicit-interface -fPIC -g -fcheck=all -fbacktrace"
-DCMAKE_MAXIMUM_RANK:String=4
-DCMAKE_INSTALL_PREFIX=$PWD/_dist
-DFIND_BLAS:STRING=TRUE
env:
FC: gfortran
CC: gcc
CXX: g++

- name: CMake build with OpenBLAS
run: PATH=$PATH:/mingw64/bin/ cmake --build build --parallel

- name: catch build fail
if: failure()
run: PATH=$PATH:/mingw64/bin/ cmake --build build --verbose --parallel 1

- name: CTest with OpenBLAS
run: PATH=$PATH:/mingw64/bin/ ctest --test-dir build --output-on-failure --parallel -V -LE quadruple_precision

- uses: actions/upload-artifact@v1
if: failure()
with:
name: WindowsCMakeTestlog_openblas
path: build/Testing/Temporary/LastTest.log

- name: Install project with OpenBLAS
run: PATH=$PATH:/mingw64/bin/ cmake --install build
26 changes: 26 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,32 @@ if(NOT DEFINED CMAKE_MAXIMUM_RANK)
set(CMAKE_MAXIMUM_RANK 4 CACHE STRING "Maximum array rank for generated procedures")
endif()

option(FIND_BLAS "Find external BLAS and LAPACK" ON)

# --- find BLAS and LAPACK
if(FIND_BLAS)
if(NOT BLAS_FOUND)
#Required for MKL
if(DEFINED ENV{MKLROOT} OR "${BLA_VENDOR}" MATCHES "^Intel")
enable_language("C")
endif()
find_package("BLAS")
endif()
if(BLAS_FOUND)
add_compile_definitions(STDLIB_EXTERNAL_BLAS)
endif()
if(NOT LAPACK_FOUND)
#Required for MKL
if(DEFINED ENV{MKLROOT} OR "${BLA_VENDOR}" MATCHES "^Intel")
enable_language("C")
endif()
find_package("LAPACK")
endif()
if(LAPACK_FOUND)
add_compile_definitions(STDLIB_EXTERNAL_LAPACK)
endif()
endif()

# --- find preprocessor
find_program(FYPP fypp)
if(NOT FYPP)
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ Important options are
- `-DBUILD_TESTING` set to `off` in case you want to disable the stdlib tests (default: `on`).
- `-DCMAKE_VERBOSE_MAKEFILE` is by default set to `Off`, but if set to `On` will show commands used to compile the code.
- `-DCMAKE_BUILD_TYPE` is by default set to `RelWithDebInfo`, which uses compiler flags suitable for code development (but with only `-O2` optimization). Beware the compiler flags set this way will override any compiler flags specified via `FFLAGS`. To prevent this, use `-DCMAKE_BUILD_TYPE=NoConfig` in conjunction with `FFLAGS`.
- `-DFIND_BLAS` set to `off` in case you want to disable finding the external BLAS/LAPACK dependency (default: `on`).

For example, to configure a build using the Ninja backend while specifying compiler optimization via `FFLAGS`, generating procedures up to rank 7, installing to your home directory, using the `NoConfig` compiler flags, and printing the compiler commands, use

Expand Down
8 changes: 8 additions & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,14 @@ set(SRC

add_library(${PROJECT_NAME} ${SRC})

# Link to BLAS and LAPACK
if(BLAS_FOUND)
target_link_libraries(${PROJECT_NAME} "BLAS::BLAS")
endif()
if(LAPACK_FOUND)
target_link_libraries(${PROJECT_NAME} "LAPACK::LAPACK")
endif()

set_target_properties(
${PROJECT_NAME}
PROPERTIES
Expand Down
11 changes: 10 additions & 1 deletion test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,15 @@ macro(ADDTEST name)
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR})
endmacro(ADDTEST)

macro(ADDTESTPP name)
add_executable(test_${name} test_${name}.F90)
target_link_libraries(test_${name} "${PROJECT_NAME}" "test-drive::test-drive")
add_test(NAME ${name}
COMMAND $<TARGET_FILE:test_${name}> ${CMAKE_CURRENT_BINARY_DIR}
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR})
endmacro(ADDTESTPP)


add_subdirectory(array)
add_subdirectory(ascii)
add_subdirectory(bitsets)
Expand All @@ -30,4 +39,4 @@ add_subdirectory(system)
add_subdirectory(quadrature)
add_subdirectory(math)
add_subdirectory(stringlist)
add_subdirectory(terminal)
add_subdirectory(terminal)
11 changes: 9 additions & 2 deletions test/linalg/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,15 +1,22 @@
set(
fppFiles
"test_linalg.fypp"
"test_blas_lapack.fypp"
"test_linalg_eigenvalues.fypp"
"test_linalg_solve.fypp"
"test_linalg_lstsq.fypp"
"test_linalg_determinant.fypp"
"test_linalg_svd.fypp"
"test_linalg_matrix_property_checks.fypp"
)

# Preprocessed files to contain preprocessor directives -> .F90
set(
cppFiles
"test_blas_lapack.fypp"
)

fypp_f90("${fyppFlags}" "${fppFiles}" outFiles)
fypp_f90pp("${fyppFlags}" "${cppFiles}" outPreprocFiles)

ADDTEST(linalg)
ADDTEST(linalg_determinant)
Expand All @@ -18,4 +25,4 @@ ADDTEST(linalg_matrix_property_checks)
ADDTEST(linalg_solve)
ADDTEST(linalg_lstsq)
ADDTEST(linalg_svd)
ADDTEST(blas_lapack)
ADDTESTPP(blas_lapack)
73 changes: 72 additions & 1 deletion test/linalg/test_blas_lapack.fypp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ contains
new_unittest("test_gemv${t1[0]}$${k1}$", test_gemv${t1[0]}$${k1}$), &
new_unittest("test_getri${t1[0]}$${k1}$", test_getri${t1[0]}$${k1}$), &
#:endfor
new_unittest("test_idamax", test_idamax) &
new_unittest("test_idamax", test_idamax), &
new_unittest("test_external_blas",external_blas_test), &
new_unittest("test_external_lapack",external_lapack_test) &
]

end subroutine collect_blas_lapack
Expand Down Expand Up @@ -117,6 +119,75 @@ contains

end subroutine test_idamax

!> Test availability of the external BLAS interface
subroutine external_blas_test(error)
!> Error handling
type(error_type), allocatable, intent(out) :: error

#ifdef STDLIB_EXTERNAL_BLAS
Copy link
Contributor

Choose a reason for hiding this comment

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

Correct me if I got this wrong, but I think the test should be calling

use stdlib_linalg_blas, only: axpy
...
call axpy( ... )

use stdlib_linalg_lapack, only: getrf
...
call getrf( ... )

Which already interface the respective routines if STDLIB_EXTERNAL_BLAS \ STDLIB_EXTERNAL_LAPACK are set or not.

In this case, the test_blas_lapack.fypp file does not need to be moved to the C preprocessed category. Which is why the fpm build is failing as it was not included in the fypp_deployment.py file, but I'm guessing that it is not needed.

Copy link
Member

@perazz perazz Jul 4, 2024

Choose a reason for hiding this comment

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

Great comment Jose, thank you for looking into it. I've tried that. The meaning of the test is to ensure that the external function is being picked. However, calling the interface does not guarantee that we're using the external library function. One thing that I had tried to check that was to use a procedure pointer:

abstract interface
    subroutine blas_saxpy(blabla)
end interface

procedure(blas_saxpy), pointer :: axpy_ 
axpy_ => axpy
! Check that we're not using the internal implementation
call check(error, .not.associated(axpy_ , stdlib_saxpy))

But unfortunately, Fortran does not allow pointing to an interface, so this is not possible. If you have a better idea how could we check this, I'm very much in for removing preprocessing from the test module!

Copy link
Member

Choose a reason for hiding this comment

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

Edit: the way that CMake uses to check if a fortran function exists is just to declare it external and try to compile a program:

https://github.com/Kitware/CMake/blob/a63bee680ce050e41f549ad9e3a811700347a3f6/Modules/CheckFortranFunctionExists.cmake#L62-L67

Copy link
Contributor

@jalvesz jalvesz Jul 4, 2024

Choose a reason for hiding this comment

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

I see, so if I understand, the objective of this test is to check if STDLIB_EXTERNAL_BLAS \ STDLIB_EXTERNAL_LAPACK are activated, that the hypothesis of an optimized library being called is fulfilled, right?

If say one does force these macros to be TRUE but the external library is not visible in the path, then a compile-time error would occur such as "'undefined reference to `dgetrf_'" which would already alert of a link issue. If the library is visible, then

use stdlib_linalg_lapack, only: getrf
...
call getrf( ... )

would have the same effect as the local test interface. So I'm just wondering if there could be a different kind of test using directly the higher-level interfaces from the stdlib modules?

Now, if the test is kept as is, this file should be added here

"stdlib_linalg_lapack_w"
to enable the fpm job to treat it accordingly :)

Copy link
Member

Choose a reason for hiding this comment

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

In the case of the stdlib CI, yes because there are testing routines that check that the base functions work anyways.

But in general, if you don't build the tests there is no guarantee that these functions are available.
Consider this example at the Compiler Explorer: if you don't call axpy, the compiler doesn't care what's in the interface, so you're never sure.

So long story short, I think we have these two options:

  • remove these two additional tests, but run a full stdlib CI with an external BLAS/LAPACK in at least one of the system configurations;
  • keep these two tests, and then add preprocessing to the testing routines in the fpm deployment.

So in light of the additional complexity, maybe let's just remove the tests and go for option 1?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would be in favor of option 1, so given this PR started by @zoziha is meant to help using the bindings in MSYS2, what about changing one of the windows CI jobs, I would also suggest one of the linux-intel jobs to link againts MKL. That should be more than good enough I think.

interface
subroutine saxpy(n,sa,sx,incx,sy,incy)
import sp,ilp
implicit none(type,external)
real(sp), intent(in) :: sa,sx(*)
integer(ilp), intent(in) :: incx,incy,n
real(sp), intent(inout) :: sy(*)
end subroutine saxpy
end interface

integer(ilp), parameter :: n = 5, inc=1
real(sp) :: a,x(n),y(n)

x = 1.0_sp
y = 2.0_sp
a = 3.0_sp

call saxpy(n,a,x,inc,y,inc)
call check(error, all(abs(y-5.0_sp)<sqrt(epsilon(0.0_sp))), "saxpy: check result")
if (allocated(error)) return

#else
call skip_test(error, "Not using an external BLAS")
#endif

end subroutine external_blas_test

!> Test availability of the external BLAS interface
subroutine external_lapack_test(error)
!> Error handling
type(error_type), allocatable, intent(out) :: error

#ifdef STDLIB_EXTERNAL_LAPACK
interface
subroutine dgetrf( m, n, a, lda, ipiv, info )
import dp,ilp
implicit none(type,external)
integer(ilp), intent(out) :: info,ipiv(*)
integer(ilp), intent(in) :: lda,m,n
real(dp), intent(inout) :: a(lda,*)
end subroutine dgetrf
end interface

integer(ilp), parameter :: n = 3
real(dp) :: A(n,n)
integer(ilp) :: ipiv(n),info


A = eye(n)
info = 123

! Factorize matrix
call dgetrf(n,n,A,n,ipiv,info)

call check(error, info==0, "dgetrf: check result")
if (allocated(error)) return

#else
call skip_test(error, "Not using an external LAPACK")
#endif

end subroutine external_lapack_test

end module test_blas_lapack


Expand Down
Loading
Loading