search

DrTimothyAldenDavis / SuiteSparse

The official SuiteSparse library: a suite of sparse matrix algorithms authored or co-authored by Tim Davis, Texas A&M University.
https://people.engr.tamu.edu/davis/suitesparse.html
Other
1.17k stars 262 forks source link

Need ability to build and package multiple configurations of suitesparse #425

Closed opoplawski closed 10 months ago

opoplawski commented 1 year ago

Is your feature request related to a problem? Please describe. Fedora currently builds 3 versions of SuiteSparse:

I'm looking to update the Fedora package to the latest version and use the cmake build system. But I'm running into a number of difficulties:

+string(TOUPPER "${CMAKE_BUILD_TYPE}" CMAKE_BUILD_TYPE_UPPER) + message ( STATUS "Building AMD version: v" ${AMD_VERSION_MAJOR}. ${AMD_VERSION_MINOR}. @@ -218,7 +220,7 @@ if ( NOT MSVC ) NEWLINE_STYLE LF ) install ( FILES ${CMAKE_CURRENT_BINARY_DIR}/AMD.pc

Describe the solution you'd like We needed a complete solution to allow users to specify and use the version of the library that they need.

Additional context

Currently only Julia makes use of the 64 libraries. I'm exploring whether or not this is still needed or if we can drop the 64 version.

opoplawski commented 1 year ago

See also https://discourse.cmake.org/t/how-to-evaluate-config-postfix/8984 for some discussion.

opoplawski commented 1 year ago

The problem I'm running into with the different build types installing into the same cmake directory is that the later install removes the prior one:

-- Old export file "/home/orion/BUILDROOT/suitesparse-7.2.0-0.2.fc40.x86_64/usr/lib64/cmake/SuiteSparse_config/SuiteSparse_configTargets.cmake" will be replaced.  Removing files [/home/orion/BUILDROOT/suitesparse-7.2.0-0.2.fc40.x86_64/usr/lib64/cmake/SuiteSparse_config/SuiteSparse_configTargets-release.cmake].

So I guess that will not work.

mmuetzel commented 1 year ago

Thanks for the report and the description of the use-case.

Would it be ok to build (and install) SuiteSparse in differing libdirs for the different configurations? (Just like you're planning to install the headers in different includedirs.)

Would configuring with something like the following work for you?

cmake -DSUITESPARSE_INCLUDEDIR=/usr/include/suitesparse64 -DSUITESPARSE_LIBDIR=/usr/lib/suitesparse64

Users of the libraries would need to configure with the following (if they are using CMake):

cmake -DCMAKE_PREFIX_PATH=/usr/lib/suitesparse64/cmake

Or the following for pkg-config:

PKG_CONFIG_PATH=/usr/lib/suitesparse64/pkgconfig

That way you wouldn't need to "invent" new `CMAKE_BUILD_TYPE's.

(I haven't tested that though.)

DrTimothyAldenDavis commented 1 year ago

Just now getting back to the keyboard (not working full days yet though), from recovering from surgery about 2 weeks ago.

ViralBShah commented 1 year ago

@opoplawski Off topic, but in case you didn't already know, in Julia we use https://github.com/JuliaLinearAlgebra/libblastrampoline instead of flexiblas.

DrTimothyAldenDavis commented 11 months ago

I think this is now resolved in SuiteSparse 7.4.0, in the default dev branch of the SuiteSparse repo. I'll release a 7.4.0.beta1 soon.

Can you give it a try?

DrTimothyAldenDavis commented 11 months ago

SuiteSparse 7.4.0.beta1 is now released (as a pre-release), with a completely revised build system. Can you give it a try?

opoplawski commented 11 months ago

Sorry for being quiet for so long. Vacation and then lots of other things piled up. I'm working on building it now and hope to test with some dependencies tomorrow. Thank you very much for working on this.

DrTimothyAldenDavis commented 11 months ago

Great! It's good to get away from time to time.

Good to hear you can give it a try. Hopefully whatever you find we can resolve in the final 7.4.0 release.

DrTimothyAldenDavis commented 11 months ago

Regarding the 32 and 64 bit BLAS: yes that's a real challenge. You can tell SuiteSparse_config.h to override the cmake determination of the BLAS integer size with -DBLAS64 or -DBLAS32. But then if an end user #includes that file then they would need to do the same.

I've thought about other mechanisms for connecting to BLAS/LAPACK. It's unfortunate that there is no standard portable way to specify 32/64 bit integers as the BLAS parameters.

opoplawski commented 11 months ago

Thanks for reminding me about the include file issue. I'm just going to install the headers into /usr/include/SuiteSparse64 for the 64-bit version. Hopefully with the cmake and pkgconfig files available finding the headers will be straightforward.

mmuetzel commented 11 months ago

There have been some more changes to the configuration flags recently. Does something like the following work for you?

cmake -DSUITESPARSE_INCLUDEDIR_POSTFIX=suitesparse64 -DSUITESPARSE_PKGFILEDIR=lib/suitesparse64 -DCMAKE_RELEASE_POSTFIX=64 [other flags for that configuration] ..
DrTimothyAldenDavis commented 11 months ago

I just tried it on my side. In the SuiteSparse/build folder, using the new top-level SuiteSparse/CMakeLists.txt file:

hypersparse $ cd SuiteSparse/build
hypersparse $ cmake -DSUITESPARSE_INCLUDEDIR_POSTFIX=suitesparse64 -DSUITESPARSE_PKGFILEDIR=lib/suitesparse64 -DCMAKE_RELEASE_POSTFIX=64 ..

and I get this output:

build_output.txt

And the cmake and pkgconfig files all point to that location.

I can't write to /usr/local on this machine so I then did this (I can write to /raid/stuff):

cmake -DSUITESPARSE_INCLUDEDIR_POSTFIX=suitesparse64 -DSUITESPARSE_PKGFILEDIR=lib/suitesparse64 -DCMAKE_RELEASE_POSTFIX=64 -DCMAKE_INSTALL_PREFIX=/raid/stuff ..
cmake --build . --config=Release -j40

and got this report:

build_output_install_stuff.txt

and all of the /raid/stuff/suitesparse64/pkgconfig and /raid/stuff/suitesparse64/cmake files look ok to me (but I don't use pkgconfig so I'm not certain about that).

DrTimothyAldenDavis commented 11 months ago

All the libraries look like "libcholmod64.so" and such.

Do the pkgconfig files need to have the POSTFIX? I see this:

$ cat /raid/stuff/lib/suitesparse64/pkgconfig/CHOLMOD.pc 
# CHOLMOD, Copyright (c) 2005-2023, Timothy A. Davis.
# All Rights Reserved.
# SPDX-License-Identifier: LGPL-2.1-or-later AND GPL-2.0-or-later AND Apache-2.0

prefix=/raid/stuff
exec_prefix=${prefix}
libdir=${exec_prefix}/lib
includedir=${prefix}/include/suitesparse64

# FIXME: Which flags do we need to statically link CUDA if needed?

Name: CHOLMOD
URL: https://github.com/DrTimothyAldenDavis/SuiteSparse
Description: Routines for factorizing sparse symmetric positive definite matrices in SuiteSparse
Version: 5.1.0
Requires.private: SuiteSparse_config AMD COLAMD  CAMD CCOLAMD
Libs: -L${libdir} -lcholmod
Libs.private:  -lgomp -lpthread -lm -lmkl_gf_lp64 -lmkl_gnu_thread -lmkl_core -lgomp -l-lpthread -l-lm -l-ldl -l-lpthread -l-lm -l-ldl -lmkl_gf_lp64 -lmkl_gnu_thread -lmkl_core -lgomp -l-lpthread -l-lm -l-ldl
Cflags: -I${includedir}  -DSUITESPARSE_CUDA

Should CHOLMOD.pc say "Libs: -L${libdir} -lcholmod64" in the above file?

opoplawski commented 11 months ago

Should CHOLMOD.pc say "Libs: -L${libdir} -lcholmod64" in the above file?

Yes, definitely.

DrTimothyAldenDavis commented 11 months ago

That's what I thought when I look at AMD.pc and the others. I this fixes it, at least if the build type is "Release":
https://github.com/DrTimothyAldenDavis/SuiteSparse/commit/361ede5d6e8ba7e7fad1719e947e39535e7f6039

When I install with this change, the CHOLMOD.pc file has:

Libs: -L${libdir} -lcholmod64

But does this need to be revised for different build types?

DrTimothyAldenDavis commented 11 months ago

I tried my push https://github.com/DrTimothyAldenDavis/SuiteSparse/commit/361ede5d6e8ba7e7fad1719e947e39535e7f6039 with a Debug build, and the *.pc files look fine, with one exception (below). So it looks like the cmake variable ${CMAKE_RELEASE_POSTFIX} applies to all build types (Release, Debug, etc). I thought the _RELEASE_ in the variable name meant it was one of many variables, one per build type.

I think all the configured *.pc files are OK now, except that in the LAGraph.pc file, I get:

Libs.private: -lgraphblas -lgomp -lpthread -lm

and not the following, which is what I think it should be:

Libs.private: -lgraphblas64 -lgomp -lpthread -lm

@mmuetzel : should the LAGraph.pc.in file here:

https://github.com/DrTimothyAldenDavis/SuiteSparse/blob/361ede5d6e8ba7e7fad1719e947e39535e7f6039/LAGraph/config/LAGraph.pc.in#L16

be the following instead?

Libs.private: -lgraphblas@CMAKE_RELEASE_POSTFIX@ @LAGRAPH_STATIC_LIBS@

We have that there because GraphBLAS could be provided by another source, not just SuiteSparse/GraphBLAS. In practice, there is no other GraphBLAS library, and if there were, the authors of that library can just create their own LAGraph.pc.

mmuetzel commented 11 months ago

Good points! Happy this was caught before the release.

I opened #582 which I hope addresses this more generally.

mmuetzel commented 11 months ago

@opoplawski: @DrTimothyAldenDavis tagged a beta2 in which this issue should be fixed. If you come around to it, could you please check if that is good enough for your use case?

DrTimothyAldenDavis commented 11 months ago

Now a beta3 just now: https://github.com/DrTimothyAldenDavis/SuiteSparse/releases/tag/v7.4.0.beta3 with a minor update to GraphBLAS. This update to GraphBLAS should have no effect on the inclusion of SuiteSparse into a linux distro. It only affects those who edit the source code itself and rebuild.

DrTimothyAldenDavis commented 10 months ago

Fixed in SuiteSparse 7.4.0.