add LAPACK 3.7.1 #860

NeroBurner · 2017-06-29T14:05:37Z

build dynamic libraries
needs a fortran compiler to build
- on Ubuntu: sudo apt install gfortran
- on Windows: mingw-w64 has a fortran compiler

dependency for suitesparse as stated in Issue #855

ruslo · 2017-06-29T16:33:35Z

examples/LAPACK/CMakeLists.txt

+find_package(cblas CONFIG REQUIRED)
+
+add_executable(foo foo.cpp)
+target_include_directories(foo PRIVATE ${CBLAS_INCLUDE_DIRS})


This should be a property of imported target

then I need to patch the sources. Could you make a fork of https://github.com/Reference-LAPACK/lapack so that we can add a hunter branch

Done: https://github.com/hunter-packages/lapack

ruslo · 2017-06-29T16:35:14Z

examples/LAPACK/CMakeLists.txt

+find_package(LAPACK CONFIG REQUIRED)
+
+
+find_package(cblas CONFIG REQUIRED)


This is a little bit confusing. Are we using LAPACK or cblas? According to the C++ code we depends only on cblas, meaning that find_package(LAPACK ...) not needed.

I think clbas is using lapack/blas and is just a C wrapper for the libraries. I included cblas only to get a C interface for the example. I wasn't able to directly use the lapack/blas libraries

is it possible to build cblas only for the example? Then there isn't a conflict and the user can add a buildflag for his/her project when cblas is really needed

Since this pull request is about adding lapack I expect at least one simple example with hunter_add_package(lapack) + find_package(lapack ...) + #include <lapack.h> without any other instructions. If there are other important variants of using this package they should be added separately, e.g. examples/lapack-cblas. In those examples we can use custom local config.cmake with custom CMAKE_ARGS for enabling/disabling components.

the install of lapack (without CBLAS) provides only libraries, no headers. So there is no lapack.h to include.
To have an example with c-interface I added cblas. I'll move the current example to examples/LAPACK-CBLAS but I don't know how to create an example just using liblapack.a and libblas.a

edit: example path

provides only libraries, no headers

How they are expected to be used!? 😕 Probably that's because of it's written in Fortran (?)

I think the libs are used directly and the users create the appropriate headers themselves. See https://github.com/jlblancoc/suitesparse-metis-for-windows/blob/master/SuiteSparse/CHOLMOD/Include/cholmod_blas.h for an example

I think that CBLAS and LAPACKE came later as an easier way to use the libraries

ruslo · 2017-06-29T16:36:05Z

cmake/projects/LAPACK/hunter.cmake

+hunter_cmake_args(
+    LAPACK
+    CMAKE_ARGS
+    BUILD_SHARED_LIBS=ON # build as shared libs


By default libraries should be static.

ruslo · 2017-06-29T16:38:50Z

examples/LAPACK/foo.cpp

@@ -0,0 +1,14 @@
+#include <iostream> // std::cout
+#include <cblas.h>


I think we are conflicting with OpenBLAS package here:

https://github.com/hunter-packages/OpenBLAS/blob/hunter/cblas.h

Do you know how they are related? Is it two implementation of the same standard?

I don't know. I think they are solving the same problem in slightly different ways. The creation of cblas can be turned off. I only added it to use it in the example. For suitesparse cblas is not used.

All the different blas, OpenBlas, ATLAS, LAPACK, LAPACKE are confusing me. I know suitesparse can use BLAS and LAPACK. About the others I don't know

When I compile OpenBLAS on Linux, it provides a Lapack as well if gfortran is detected.

I can't cross compile the hunter OpenBlas with mingw-w64. Does it work natively on Windows?

[ 75%] Performing build step for 'OpenBLAS' /bin/sh: 1: ./getarch: not found make[3]: *** [config.h] Error 127 OpenBLAS: Detecting fortran compiler failed. Cannot compile LAPACK. Only compile BLAS. gemv.c: In function 'sgemv_': gemv.c:223:29: error: 'GEMM_MULTITHREAD_THRESHOLD' undeclared (first use in this function) if ( 1L * m * n < 2304L * GEMM_MULTITHREAD_THRESHOLD ) ^ gemv.c:223:29: note: each undeclared identifier is reported only once for each function it appears in In file included from ../common.h:747:0, from gemv.c:40: ../common_stackalloc.h:32:46: warning: unused variable 'stack_check' [-Wunused-variable] #define STACK_ALLOC_PROTECT_SET volatile int stack_check = 0x7fc01234; ^ ../common_stackalloc.h:56:3: note: in expansion of macro 'STACK_ALLOC_PROTECT_SET' STACK_ALLOC_PROTECT_SET \ ^ gemv.c:219:3: note: in expansion of macro 'STACK_ALLOC' STACK_ALLOC(buffer_size, FLOAT, buffer); ^ Makefile:841: recipe for target 'sgemv.obj' failed make[4]: *** [sgemv.obj] Error 1 Makefile:126: recipe for target 'libs' failed make[3]: *** [libs] Error 1

Does it work natively on Windows?

There are no working toolchains on Windows in pkg.openblas:

https://ci.appveyor.com/project/ingenue/hunter/build/1.0.1047

NeroBurner · 2017-06-30T08:25:57Z

I have updated the configuration and added a local config file for the LAPACK example. But when building the example no cblas-config.cmake file can be found. I've looked in the install folder and CBLAS just wasn't built/installed. When I add the CMAKE_ARGS back to the projects/LAPACK/hunter.cmake file then CBLAS is built again

CMake Error at CMakeLists.txt:35 (find_package):
  Could not find a package configuration file provided by "cblas" with any of
  the following names:

    cblasConfig.cmake
    cblas-config.cmake

  Add the installation prefix of "cblas" to CMAKE_PREFIX_PATH or set
  "cblas_DIR" to a directory containing one of the above files.  If "cblas"
  provides a separate development package or SDK, be sure it has been
  installed.

-- Configuring incomplete, errors occurred!

NeroBurner · 2017-06-30T11:52:13Z

sent a patch upstream to add the include directories to the target cblas Reference-LAPACK/lapack#170

also added the patch on top of v3.7.1 and tagged v3.7.1-p0

the example LAPACK-CBLAS does still not work, because the cmake arguments are not passed on to the LAPACK build (the local config.cmake is not working). If I set CBLAS=ON in projects/LAPACK/hunter.cmake then the cblas target is built and installed

NeroBurner · 2017-06-30T12:03:23Z

We could add a components schema to activate CBLAS or LAPACKE (c-interfaces for BLAS and LAPACK)

If a user just wants the libraries he can use

hunter_add_package(LAPACK)

if he wants the CBLAS interface

hunter_add_package(LAPACK COMPONENTS CBLAS)

and for both optional c interfaces
if he wants the CBLAS interface

hunter_add_package(LAPACK COMPONENTS CBLAS LAPACKE)

This would be easier for the user than creating a local config.cmake file.

how much work would this be?

ruslo

because the cmake arguments are not passed on to the LAPACK build (the local config.cmake is not working)

Build with HUNTER_STATUS_DEBUG=ON. Are you sure local config.cmake used?

We could add a components schema
how much work would this be?

Pretty much. CMake itself should be patched here. I'm not aware of "CMake components" feature supported for CMake projects. Components used in Hunter for Qt and Boost libraries. They are not using CMake as a build system.

ruslo · 2017-06-30T13:05:07Z

examples/LAPACK-CBLAS/CMakeLists.txt

+cmake_minimum_required(VERSION 3.0)
+
+
+set(TESTING_CONFIG_OPT FILEPATH "${CMAKE_SOURCE_DIR}/config.cmake")


Prefer CMAKE_CURRENT_LIST_DIR:

http://cgold.readthedocs.io/en/latest/tutorials/cmake-sources/includes.html#id3

done, thx for the Info :)

NeroBurner · 2017-06-30T13:21:49Z

yes I'm sure the config.cmake file is included. I added a message(FATAL_ERROR "file included") to the config file and in the configure step of the example the message is shown and the configuration errors out (as intended)

ruslo · 2017-06-30T13:28:37Z

examples/LAPACK-CBLAS/config.cmake

@@ -0,0 +1,3 @@
+include(hunter_cmake_args)


ruslo · 2017-06-30T13:28:58Z

examples/LAPACK-CBLAS/config.cmake

@@ -0,0 +1,3 @@
+include(hunter_cmake_args)
+
+hunter_cmake_args(LAPACK CMAKE_ARGS CBLAS=ON)


It should be hunter_config, not hunter_cmake_args:

https://github.com/ruslo/hunter/wiki/example.custom.config.id#custom-cmake-options

NeroBurner · 2017-06-30T13:51:16Z

that fixed it 👍

ruslo

I think we still need examples/LAPACK. Start with simple hunter_add_package(LAPACK) without find_package and any targets. We can add more info later.

ruslo · 2017-06-30T14:10:22Z

cmake/projects/LAPACK/hunter.cmake

+hunter_cmake_args(
+    LAPACK
+    CMAKE_ARGS
+    #BUILD_SHARED_LIBS=ON # build as shared libs


Please remove unused commented options.

NeroBurner · 2017-07-03T06:28:13Z

the PR Reference-LAPACK/lapack#170 was merged, CBLAS should now work without any additional patchset.
Reference-LAPACK/lapack#171 to fix LAPACKE is still open, but I hope it will get merged soon

I'd like to recreate the tag v3.7.1-p0 on top of the current master and remove the custom branch hunter_v3.7.1. What are your thoughts on it?

ruslo · 2017-07-03T08:10:30Z

Reference-LAPACK/lapack#171 to fix LAPACKE is still open, but I hope it will get merged soon

Merged now.

I'd like to recreate the tag v3.7.1-p0 on top of the current master and remove the custom branch hunter_v3.7.1. What are your thoughts on it?

I think it make sense to keep the hunter_v3.7.1 branch so it will be clear where other patches will go if needed. Of course you can do git merge --ff-only so master and hunter_v3.7.1 will point to the same commit.

NeroBurner · 2017-07-03T09:52:48Z

nice, that was fast :)

ff-only was not possible, did a git reset --hard master and recreated the tag v3.7.1-p0

what is left to do before merge? I'd like to squash the commits before that please

ruslo · 2017-07-03T10:30:06Z

what is left to do before merge?

Please add examples/LAPACK:

I think we still need examples/LAPACK. Start with simple hunter_add_package(LAPACK) without find_package and any targets. We can add more info later.

- add LAPACK example building targets blas and lapack - add LAPACK-CBLAS example using local cmake config file

NeroBurner · 2017-07-03T10:52:54Z

added and squashed

ruslo · 2017-07-03T11:00:28Z

Testing:

NeroBurner · 2017-07-03T15:21:53Z

I've been trying to use this lapack package with ceres-solver, but when building the examples (or any other binary) I get missing fortran functions. The LAPACK config of static libraries does not properly propagate the dependency to gfortran and quadmath. When I build blas and lapack as dynamic libraries then the fortran libraries get resolved.

With the statically built blas and lapack I can make it work if I add the following to the ceres-solver project

enable_language(Fortran)

how do we add this dependency to the static blas and lapack libs?

ruslo · 2017-07-03T15:40:40Z

I don't have experience with Fortran so I'm not sure I understand all details.

With the statically built blas and lapack I can make it work if I add the following to the ceres-solver project

Sounds like you need to update ceres-solver.

NeroBurner · 2017-07-03T19:09:45Z

I think the runtime dependency should be propagated by the cmake-configure file of LAPACK, otherwise all projects using LAPACK need to be modified. But I don't know how

ruslo · 2017-07-05T10:08:12Z

I'm not sure that I understand enough all the details of the issue to give any advice here.

NeroBurner · 2017-07-05T12:02:23Z

I think for LAPACK and BLAS it is better to create a dynamic lib (so or dll). Then the consuming projects don't need to be updated.

For Windows it should be possible to let a MSVC project consume the LAPACK.dll built by mingw-gfortran or any other fortran compiler.

ruslo · 2017-07-05T13:26:58Z

We can start with that as a workaround and use BUILD_SHARED_LIBS=ON only in examples/LAPACK. But I'm afraid it will add extra mess in future. Probably you will have to remove all Linux testing because by default toolchains doesn't use -fPIC needed for mixing static/shared. Also I don't think mixing static and shared is a good idea in general.

NeroBurner · 2017-07-05T15:10:20Z

the hunter-examples for LAPACK and SuiteSparse build fine with static libs. They don't seem to use fortran provided functions (maybe because they are very basic examples)

The projects LAPACK and SuiteSparse define -fPIC. So they could be linked into another dynamic library. I think that -fPIC is only important if you are linking a static library into another dynamic library

ruslo · 2017-07-06T10:09:17Z

Testing:

NeroBurner · 2017-07-06T14:56:20Z

looks good 👍

ruslo · 2017-07-06T16:57:57Z

Uploading:

ruslo · 2017-07-07T08:20:35Z

Released:

https://github.com/ruslo/hunter/releases/tag/v0.19.24

ruslo suggested changes Jun 29, 2017

View reviewed changes

NeroBurner changed the title ~~add LAPACK 3.7.1~~ [WIP] add LAPACK 3.7.1 Jun 30, 2017

ruslo suggested changes Jun 30, 2017

View reviewed changes

NeroBurner force-pushed the lapack branch from 26ab3b9 to 93bed13 Compare June 30, 2017 13:52

ruslo suggested changes Jun 30, 2017

View reviewed changes

add LAPACK 3.7.1 (needs fortran)

760355e

- add LAPACK example building targets blas and lapack - add LAPACK-CBLAS example using local cmake config file

NeroBurner force-pushed the lapack branch from ff29c7c to 760355e Compare July 3, 2017 10:52

NeroBurner changed the title ~~[WIP] add LAPACK 3.7.1~~ add LAPACK 3.7.1 Jul 3, 2017

ruslo mentioned this pull request Jul 3, 2017

LAPACK: fix testing ingenue/hunter#86

Merged

ruslo approved these changes Jul 6, 2017

View reviewed changes

ruslo merged commit dd570d6 into ruslo:master Jul 6, 2017

NeroBurner deleted the lapack branch July 7, 2017 10:45

		find_package(LAPACK CONFIG REQUIRED)


		find_package(cblas CONFIG REQUIRED)

		@@ -0,0 +1,14 @@
		#include <iostream> // std::cout
		#include <cblas.h>

		cmake_minimum_required(VERSION 3.0)


		set(TESTING_CONFIG_OPT FILEPATH "${CMAKE_SOURCE_DIR}/config.cmake")

		@@ -0,0 +1,3 @@
		include(hunter_cmake_args)

		hunter_cmake_args(LAPACK CMAKE_ARGS CBLAS=ON)

add LAPACK 3.7.1 #860

add LAPACK 3.7.1 #860

Conversation

NeroBurner commented Jun 29, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NeroBurner Jun 30, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NeroBurner commented Jun 30, 2017

NeroBurner commented Jun 30, 2017

NeroBurner commented Jun 30, 2017

ruslo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NeroBurner commented Jun 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NeroBurner commented Jun 30, 2017

ruslo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NeroBurner commented Jul 3, 2017

ruslo commented Jul 3, 2017

NeroBurner commented Jul 3, 2017

ruslo commented Jul 3, 2017

NeroBurner commented Jul 3, 2017

ruslo commented Jul 3, 2017

NeroBurner commented Jul 3, 2017

ruslo commented Jul 3, 2017

NeroBurner commented Jul 3, 2017

ruslo commented Jul 5, 2017

NeroBurner commented Jul 5, 2017

ruslo commented Jul 5, 2017

NeroBurner commented Jul 5, 2017

ruslo commented Jul 6, 2017

NeroBurner commented Jul 6, 2017

ruslo commented Jul 6, 2017

ruslo commented Jul 7, 2017

NeroBurner Jun 30, 2017 •

edited