Skip to content

Fix Findcustom-lapack.cmake #46

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.

Sign up for GitHub

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

Open
RMeli wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix Findcustom-lapack.cmake #46

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
43ee73e
fix
RMeli Mar 14, 2025
7f16518
scope blas vendor
RMeli Mar 18, 2025
1a34a09
explicit interface
RMeli Mar 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

cmake_minimum_required(VERSION 3.14)
cmake_minimum_required(VERSION 3.18)
get_directory_property(is-subproject PARENT_DIRECTORY)

project(
Expand All @@ -37,8 +37,8 @@ if(WITH_ILP64)
message(STATUS "Using LAPACK/BLAS ILP64 interface")
endif()

if(NOT TARGET "LAPACK::LAPACK")
find_package("custom-lapack" REQUIRED)
if(NOT TARGET "multicharge::LAPACK")
find_package("multicharge-lapack" REQUIRED)
endif()

# Collect subprojects
Expand Down Expand Up @@ -77,8 +77,8 @@ target_link_libraries(
"${PROJECT_NAME}-lib"
PUBLIC
"${lib-deps}"
"LAPACK::LAPACK"
"$<$<VERSION_LESS:${CMAKE_VERSION},3.20>:BLAS::BLAS>"
"multicharge::LAPACK"
"$<$<VERSION_LESS:${CMAKE_VERSION},3.20>:multicharge::BLAS>"
)
if(WITH_OpenMP)
target_link_libraries(
Expand Down Expand Up @@ -139,6 +139,12 @@ install(
"LICENSE"
DESTINATION "${CMAKE_INSTALL_DATADIR}/licenses/${PROJECT_NAME}"
)
install(
DIRECTORY ${PROJECT_SOURCE_DIR}/config/cmake/
DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/${PROJECT_NAME}
FILES_MATCHING
PATTERN "Find*.cmake"
Copy link
Member

@awvwgk awvwgk Mar 18, 2025

Choose a reason for hiding this comment

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

Why are we limiting the installed files to the Find modules only? The other cmake file is included as macro in most of the Find modules

Copy link
Contributor Author

@RMeli RMeli Mar 20, 2025

Choose a reason for hiding this comment

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

Good question. I need to understand why this is working, even without the macro (I tested all this compiling CP2K with DFTD4 support).

Copy link
Contributor Author

@RMeli RMeli Mar 20, 2025

Choose a reason for hiding this comment

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

The multicharge-utils.cmake is correctly installed, probably here:
https://github.com/RMeli/dftd4/blob/83f78b546ac5785ffc996d6f2f2aa6933f74ca61/config/CMakeLists.txt#L57-L61

I think what I added might not be needed, I'll test again and eventually remove it.

Copy link
Member

@marvinfriede marvinfriede Apr 2, 2025

Choose a reason for hiding this comment

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

Any update @RMeli? We would like to make a new release of multicharge, since we moved the coordination number logic to mctc-lib. Should we wait for any fixes, or can we proceed?

Copy link
Contributor Author

@RMeli RMeli Apr 4, 2025

Choose a reason for hiding this comment

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

Thanks for the ping. This work was bumped down my priority list by other items, but I can probably have a look at this in the afternoon if it is not too late. Sorry for the delay.

Otherwise please go ahead without this PR, if it is merged just after a release it should be easy to use it as a patch, if needed.

Copy link
Member

@marvinfriede marvinfriede Apr 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

No worries, just wanted to check on the current status. We will make the release some time next week then.

Copy link
Contributor Author

@RMeli RMeli Apr 7, 2025

Choose a reason for hiding this comment

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

Unfortunately I don't have access to our ARM system this week. I'll try to find an alternative to finish this off tomorrow, but please don't wait on me for a release. Sorry about the delays!

Copy link
Member

@marvinfriede marvinfriede Apr 8, 2025

Choose a reason for hiding this comment

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

Ok, then we will make the release now. There will be another release soon anyway, containing a new charge model.

Copy link
Member

@marvinfriede marvinfriede Sep 18, 2025

Choose a reason for hiding this comment

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

@RMeli We implemented the new charge model and would make another release. Any chance, you are able to fix this issue in the near future?

Copy link
Contributor Author

@RMeli RMeli Sep 18, 2025

Choose a reason for hiding this comment

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

@marvinfriede sorry, I dropped the ball on this. I'll try to resuscitate the work I did so far.

)

# add the testsuite
if(WITH_TESTS)
Expand Down
1 change: 1 addition & 0 deletions config/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ option(WITH_OpenMP "Enable support for shared memory parallelisation with OpenMP
option(WITH_ILP64 "Enable support for ILP64 BLAS/LAPACK calls" FALSE)
option(WITH_TESTS "Enable compilation of unit tests" TRUE)

set(MULTICHARGE_BLAS ${BLA_VENDOR} CACHE STRING "BLAS vendor")

if(NOT DEFINED "${PROJECT_NAME}-dependency-method")
set(
Expand Down
22 changes: 14 additions & 8 deletions config/cmake/Findcustom-blas.cmake → config/cmake/Findmulticharge-blas.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

if ((BLA_VENDOR MATCHES ^Intel) OR (DEFINED ENV{MKLROOT}))
if ((MULTICHARGE_BLAS MATCHES ^Intel) OR (DEFINED ENV{MKLROOT}))
enable_language("C")
endif()

Expand All @@ -22,26 +22,32 @@ if(WITH_ILP64)
set(_nvpl_int "_ilp64")
else()
set(_nvpl_int "_lp64")
set(BLA_SIZEOF_INTEGER 4)
endif()

if(NOT BLAS_FOUND)
if(BLA_VENDOR STREQUAL "NVPL")
if(NOT MULTICHARGE_BLAS_FOUND)
if(MULTICHARGE_BLAS STREQUAL "NVPL")
find_package("nvpl_blas" REQUIRED)
set(MULTICHARGE_BLAS_FOUND TRUE)

if((BLA_THREAD STREQUAL "OMP") OR (BLA_THREAD STREQUAL "ANY"))
set(_nvpl_thread "_omp")
else()
set(_nvpl_thread "_seq")
endif()

add_library("BLAS::BLAS" INTERFACE IMPORTED)
target_link_libraries("BLAS::BLAS" INTERFACE "nvpl::blas${_nvpl_int}${_nvpl_thread}")
add_library("multicharge::BLAS" INTERFACE IMPORTED GLOBAL)
target_link_libraries("multicharge::BLAS" INTERFACE "nvpl::blas${_nvpl_int}${_nvpl_thread}")
else()
find_package("BLAS" REQUIRED)
set(MULTICHARGE_BLAS_FOUND ${BLAS_FOUND})

if(NOT TARGET "BLAS::BLAS")
add_library("BLAS::BLAS" INTERFACE IMPORTED)
target_link_libraries("BLAS::BLAS" INTERFACE "${BLAS_LIBRARIES}")
if(NOT TARGET "multicharge::BLAS")
add_library("multicharge::BLAS" INTERFACE IMPORTED GLOBAL)
target_link_libraries("multicharge::BLAS" INTERFACE "BLAS::BLAS")
endif()
endif()
endif()

include(FindPackageHandleStandardArgs)
find_package_handle_standard_args(multicharge-blas DEFAULT_MSG MULTICHARGE_BLAS_FOUND)
32 changes: 20 additions & 12 deletions config/cmake/Findcustom-lapack.cmake → config/cmake/Findmulticharge-lapack.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

if ((BLA_VENDOR MATCHES ^Intel) OR (DEFINED ENV{MKLROOT}))
if ((MULTICHARGE_BLAS MATCHES ^Intel) OR (DEFINED ENV{MKLROOT}))
enable_language("C")
endif()

Expand All @@ -22,30 +22,38 @@ if(WITH_ILP64)
set(_nvpl_int "_ilp64")
else()
set(_nvpl_int "_lp64")
set(BLA_SIZEOF_INTEGER 4)
endif()

if(NOT LAPACK_FOUND)
if(BLA_VENDOR STREQUAL "NVPL")
if(NOT TARGET "multicharge::BLAS")
find_package("multicharge-blas" REQUIRED)
endif()

if(NOT MULTICHARGE_LAPACK_FOUND)
if(MULTICHARGE_BLAS STREQUAL "NVPL")
find_package("nvpl_lapack" REQUIRED)
set(MULTICHARGE_LAPACK_FOUND TRUE)

if((BLA_THREAD STREQUAL "OMP") OR (BLA_THREAD STREQUAL "ANY"))
set(_nvpl_thread "_omp")
else()
set(_nvpl_thread "_seq")
endif()

add_library("LAPACK::LAPACK" INTERFACE IMPORTED)
target_link_libraries("LAPACK::LAPACK" INTERFACE "nvpl::lapack${_nvpl_int}${_nvpl_thread}")
add_library("multicharge::LAPACK" INTERFACE IMPORTED GLOBAL)
target_link_libraries("multicharge::LAPACK" INTERFACE "nvpl::lapack${_nvpl_int}${_nvpl_thread}" "multicharge::BLAS")
else()
find_package("LAPACK" REQUIRED)
set(MULTICHARGE_LAPACK_FOUND ${LAPACK_FOUND})

if(NOT TARGET "multicharge::LAPACK")
add_library("multicharge::LAPACK" INTERFACE IMPORTED GLOBAL)
target_link_libraries("multicharge::LAPACK" INTERFACE "LAPACK::LAPACK" "multicharge::BLAS")
endif()
endif()

if(NOT TARGET "BLAS::BLAS")
find_package("custom-blas" REQUIRED)
endif()

if(NOT TARGET "LAPACK::LAPACK")
add_library("LAPACK::LAPACK" INTERFACE IMPORTED)
target_link_libraries("LAPACK::LAPACK" INTERFACE "${LAPACK_LIBRARIES}" "BLAS::BLAS")
endif()
endif()

include(FindPackageHandleStandardArgs)
find_package_handle_standard_args(multicharge-lapack DEFAULT_MSG MULTICHARGE_LAPACK_FOUND)
5 changes: 3 additions & 2 deletions config/template.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ if(NOT TARGET "@PROJECT_NAME@::@PROJECT_NAME@")
find_dependency("OpenMP")
endif()

if(NOT TARGET "LAPACK::LAPACK")
find_dependency("custom-lapack")
set(MULTICHARGE_BLAS @MULTICHARGE_BLAS@)
if(NOT TARGET "multicharge::LAPACK")
find_dependency("multicharge-lapack")
endif()

if(NOT TARGET "mctc-lib::mctc-lib")
Expand Down