spkg-configure for suitesparse #29502

dimpase · 2020-04-13T03:33:31Z

Different distos use different subsets of SuiteSparse,
but this is needed to be done. Apparently all we need is CHOLMOD (with its dependencies),UMFRACK and suitesparseconfig.

CC: @mkoeppe @orlitzky @thierry-FreeBSD @kiwifb

Component: build: configure

Author: Dima Pasechnik

Branch: f2cc5b4

Reviewer: Michael Orlitzky

Issue created by migration from https://trac.sagemath.org/ticket/29502

The text was updated successfully, but these errors were encountered:

thierry-FreeBSD · 2020-04-13T09:05:50Z

Attachment: umfpack.m4.gz

thierry-FreeBSD · 2020-04-13T09:09:00Z

comment:3

Thanks! I do not know anything about macro m4 and autotools, but I already tried to work on this. For that, I borrowed a file umfpack.m4 from the Dune project, before they switched to cmake. It is released under a GPL license. Please see the attachment if it could help.

dimpase · 2020-04-13T10:10:04Z

comment:4

please try this branch (not tested much, though)

New commits:

`10ffa1f`	`spkg-configure for suitesparse (draft)`

dimpase · 2020-04-13T10:10:04Z

Branch: u/dimpase/packages/suitesparseconf

dimpase · 2020-04-13T10:10:04Z

Commit: 10ffa1f

thierry-FreeBSD · 2020-04-13T12:35:17Z

comment:5

Configure is OK for iml and suitesparse:

Checking whether SageMath should install SPKG iml...

checking whether any of gmp mpir openblas is installed or will be installed as SPKG... no

checking for iml.h... yes

checking for library containing nonsingSolvLlhsMM... -liml

configure: will use system package and not install SPKG iml

...

Checking whether SageMath should install SPKG suitesparse...

checking whether any of openblas is installed or will be installed as SPKG... no

checking suitesparse/SuiteSparse_config.h usability... yes

checking suitesparse/SuiteSparse_config.h presence... yes

checking for suitesparse/SuiteSparse_config.h... yes

checking for library containing cholmod_speye... -lcholmod

configure: will use system package and not install SPKG suitesparse

...

iml-1.0.4p1.p2: using system package; SPKG will not be installed

...

suitesparse-5.6.0: using system package; SPKG will not be installed

But now cvxopt fails:

[cvxopt-1.2.3] src/C/umfpack.c:23:10: fatal error: 'umfpack.h' file not found

[cvxopt-1.2.3] #include "umfpack.h"

Note: on FreeBSD umfpack.h is located under /usr/local/include/suitesparse/umfpack.h. Just let me know if I fix it for FreeBSD or if the same failure could occur on other systems.

dimpase · 2020-04-13T12:43:43Z

comment:6

On Debian the location of the header is also prefixed.
Probably this is a cvxopt bug.
One might try to find out how Debian works around this.

dimpase · 2020-04-13T15:06:02Z

comment:7

On Debian, the following hack makes things work:

--- a/build/pkgs/cvxopt/spkg-install.in
+++ b/build/pkgs/cvxopt/spkg-install.in
@@ -21,8 +21,8 @@ export CVXOPT_BLAS_LIB="$(pkg_libs blas)"
 export CVXOPT_BLAS_LIB_DIR="$(pkg-config --variable=libdir blas)"
 export CVXOPT_LAPACK_LIB="$(pkg_libs lapack)"
 
-export CVXOPT_SUITESPARSE_LIB_DIR="${SAGE_LOCAL}"
-export CVXOPT_SUITESPARSE_INC_DIR="${SAGE_LOCAL}/include"
+# export CVXOPT_SUITESPARSE_LIB_DIR="${SAGE_LOCAL}"
+# export CVXOPT_SUITESPARSE_INC_DIR="${SAGE_LOCAL}/include"
 
 export CVXOPT_BUILD_GLPK=1
 export CVXOPT_GLPK_LIB_DIR="${SAGE_LOCAL}"

I'll do a proper fix later, but this should get one

thierry-FreeBSD · 2020-04-13T15:55:08Z

comment:8

A naive work-around (just adding -I /usr/local/include/suitesparse) was sufficient to build everything.

I'm going to test your patch to check if it works for me.

Note about u/dimpase/packages/suitesparseconf: actually cvxopt produces 3 python libraries, site-packages/cvxopt/amd.so, site-packages/cvxopt/cholmod.so and site-packages/cvxopt/umfpack.so, and they are linked to libamd.so.2, libsuitesparseconfig.so.5 and libumfpack.so.5 from suitesparse; maybe you would check if they are available in its spkg-configure.m4?

dimpase · 2020-04-13T17:24:58Z

comment:9

Replying to @thierry-FreeBSD:

Note about u/dimpase/packages/suitesparseconf: actually cvxopt produces 3 python libraries, site-packages/cvxopt/amd.so, site-packages/cvxopt/cholmod.so and site-packages/cvxopt/umfpack.so, and they are linked to libamd.so.2, libsuitesparseconfig.so.5 and libumfpack.so.5 from suitesparse; maybe you would check if they are available in its spkg-configure.m4?

AMD, COLAMD, CCOLAMD are dependencies of CHOLMOD in SuiteSparse, so it apparently suffices to add a check for UMFPACK (and suitesparseconfig - which presumably is always there, but OK).

thierry-FreeBSD · 2020-04-13T17:59:14Z

comment:10

Same error on FreeBSD with the Debian's patch of build/pkgs/cvxopt/spkg-install.in : umfpack.h is not found.

I would set something like:

export CVXOPT_SUITESPARSE_INC_DIR="${LOCALBASE}/suitesparse/include"

but $LOCALBASE is not set!

Would it be possible to modify build/pkgs/suitesparse/spkg-configure.m4 so that it sets a variable of the include path where SuiteSparse_config.h has been found?

dimpase · 2020-04-14T02:09:20Z

comment:11

Replying to @thierry-FreeBSD:

Same error on FreeBSD with the Debian's patch of build/pkgs/cvxopt/spkg-install.in : umfpack.h is not found.

I would set something like:

export CVXOPT_SUITESPARSE_INC_DIR="${LOCALBASE}/suitesparse/include"

but $LOCALBASE is not set!

Would it be possible to modify build/pkgs/suitesparse/spkg-configure.m4 so that it sets a variable of the include path where SuiteSparse_config.h has been found?

getting a full include path is always a pain, and should not be needed as long as it is in the default compiler path for headers. Doesn't FreeBSD's clang look in /usr/local/include/ by default (which is always the case on every Linux I know, and on MacOS) ? Apparently it does not (which is insane, IMHO) - otherwise I don't understand this error on FreeBSD.

sagetrac-git · 2020-04-14T03:48:28Z

Changed commit from 10ffa1f to b7b85d7

sagetrac-git · 2020-04-14T03:48:28Z

Branch pushed to git repo; I updated commit sha1. New commits:

`b7b85d7`	`more distros info`

dimpase · 2020-04-14T03:51:49Z

Author: Dima Pasechnik

dimpase · 2020-04-14T03:55:13Z

comment:14

So the FreeBSD question is whether adding -I /usr/local/include/ to CFLAGS and comment:7 allow the thing to build.

sagetrac-git · 2020-04-14T05:32:37Z

Changed commit from b7b85d7 to 184b88a

sagetrac-git · 2020-04-14T05:32:37Z

Branch pushed to git repo; I updated commit sha1. New commits:

`0f5619c`	`set up SAGE_SUITESPARSE_PREFIX`
`9b6b1a8`	`remove an old FreeBSD leftover`
`184b88a`	`tests for other libs in sparsesuite`

thierry-FreeBSD · 2020-04-14T07:48:00Z

comment:17

Great! The missing part was SAGE_SUITESPARSE_PREFIX.

dimpase · 2020-04-14T15:53:11Z

comment:18

tests being run here: https://github.com/dimpase/sage/actions?query=workflow%3A%22Run+SAGE_ROOT%2Ftox.ini%22

looks good (modulo unrelated things, e.g. problems with brial on Fedora, etc)

dimpase · 2020-04-19T04:51:32Z

comment:19

perhaps this can make it into 9.1?

kiwifb · 2020-04-28T21:46:37Z

comment:42

Replying to @orlitzky:

Replying to @kiwifb:

It is exclusively a dependency of cvxopt. Nothing else uses it - although we could unbundle some bit of glpk to use it. That's about it.

Oh, I could have sworn that cvxopt was an optional package. That makes sense, then.

This test,
AC_CHECK_HEADERS([SuiteSparse_config.h amd.h],
                 [suispa_header_found=yes])
is intended to set suispa_header_found to "yes" if both headers are found, but the docs for AC_CHECK_HEADERS say "If action-if-found is given, it is additional shell code to execute when one of the header files is found."

Yes, you don't want to do that. That code would be executed if either of the headers were to be found, so the variable would be defined even if one is missing. You would be better to check for ac_cv_header_suitesparse_config_h and ac_cv_header_amd_h (are those name correct?) in a shell test after the call to AC_CHECK_HEADERS.

orlitzky · 2020-04-28T22:56:16Z

comment:43

Another option is to set the "headers found" variable to "yes" by default, and then use the action-if-not-found branch of AC_CHECK_HEADERS to set it to "no" if some header is not found. You'd have to reset the variable between the prefixed and unprefixed checks, but it looks like it would work anyway.

kiwifb · 2020-04-28T22:59:43Z

comment:44

Replying to @orlitzky:

Another option is to set the "headers found" variable to "yes" by default, and then use the action-if-not-found branch of AC_CHECK_HEADERS to set it to "no" if some header is not found. You'd have to reset the variable between the prefixed and unprefixed checks, but it looks like it would work anyway.

Sounds valid.

sagetrac-git · 2020-04-29T13:07:18Z

Branch pushed to git repo; I updated commit sha1. New commits:

`e0d812f`	`correct the use of AC_CHECK_HEADERS`

sagetrac-git · 2020-04-29T13:07:18Z

Changed commit from 45d6d07 to e0d812f

dimpase · 2020-04-29T13:08:31Z

comment:46

this appears to fix the use of AC_CHECK_HEADERS - thanks for catching this.

orlitzky · 2020-04-29T13:27:23Z

comment:47

Looks good now. One last nitpick; you should add quotes here for the people who have spaces in their sage path:

if test x$SAGE_SUITESPARSE_PREFIX != x; then
   export CVXOPT_SUITESPARSE_LIB_DIR="${SAGE_LOCAL}"
   export CVXOPT_SUITESPARSE_INC_DIR="${SAGE_LOCAL}/include"
fi

And while the two are equivalent, I think using SAGE_SUITESPARSE_PREFIX instead of SAGE_LOCAL...

if test "x$SAGE_SUITESPARSE_PREFIX" != "x"; then
   export CVXOPT_SUITESPARSE_LIB_DIR="${SAGE_SUITESPARSE_PREFIX}"
   export CVXOPT_SUITESPARSE_INC_DIR="${SAGE_SUITESPARSE_PREFIX}/include"
fi

avoids some duplication and uses the SAGE_SUITESPARSE_PREFIX variable the way you intended it?

dimpase · 2020-04-29T16:08:28Z

comment:48

I am not trying to compute and pass the prefix of the installation of suitesparse.
So it is probably a misnaming.

sagetrac-git · 2020-04-29T18:48:39Z

Branch pushed to git repo; I updated commit sha1. New commits:

`f2cc5b4`	`added quoting, renamed SAGE_SUITESPARSE_PREFIX to <>_LOCALINSTALL`

sagetrac-git · 2020-04-29T18:48:39Z

Changed commit from e0d812f to f2cc5b4

orlitzky · 2020-04-29T18:55:33Z

comment:50

Replying to @dimpase:

I am not trying to compute and pass the prefix of the installation of suitesparse.
So it is probably a misnaming.

Is it just supposed to be a boolean? I guess what I'm asking is, why define SAGE_SUITESPARSE_LOCALINSTALL to either $SAGE_LOCAL or the empty string if it's only used as a flag?

dimpase · 2020-04-29T20:06:07Z

comment:51

yes, it is a boolean

orlitzky · 2020-04-30T00:33:02Z

comment:52

Replying to @dimpase:

yes, it is a boolean

It think it would make more sense to set it to "yes" rather than "$SAGE_LOCAL" in that case?

Either way, it works fine on Gentoo with suitesparse-5.4.0. Should we merge it and cross our fingers, or run it though github first to make sure all of the other distros have usable versions of suitesparse?

mkoeppe · 2020-04-30T02:00:25Z

comment:53

I would suggest to defer this to 9.2

mkoeppe · 2020-04-30T02:04:41Z

comment:54

Replying to @orlitzky:

run it though github first to make sure all of the other distros have usable versions of suitesparse?

Yes please

dimpase · 2020-04-30T07:28:03Z

comment:55

Replying to @mkoeppe:

Replying to @orlitzky:

run it though github first to make sure all of the other distros have usable versions of suitesparse?

Yes please

I did this https://github.com/dimpase/sage/actions/runs/88161666
before commits starting at comment:45 and have not found any problems.
(apart from the trivial absense of useful system openblas implying no suitesparse,
either)

orlitzky · 2020-05-19T13:45:22Z

Reviewer: Michael Orlitzky

orlitzky · 2020-05-19T13:45:22Z

comment:57

The spkg-install for cvxopt is doing some weird stuff, but I can't find any immediate problems (and don't think we need to fix them here, in any case). For example,

export CVXOPT_GLPK_LIB_DIR="${SAGE_LOCAL}"
export CVXOPT_GLPK_INC_DIR="${SAGE_LOCAL}/include"

is unconditionally adding those two library/include paths to the command line as -L and -I flags, even when we're using system packages. This doesn't cause an immediate problem because --with-system-foo=yes and --with-system-foo=force get ignored if the foo SPKG is installed, and that's the only way the extra dirs on the command-line could cause a problem. So for example if I install the suitesparse SPKG manually and attempt to use the system suitesparse, there's no risk of the SPKG being accidentally linked into cvxopt via the lines above, because there's no way to avoid using the SPKG in favor of the system copy in the first place.

These calls to pkg-config share the same risk,

export CVXOPT_GSL_LIB_DIR="$(pkg-config --variable=libdir gsl)"
export CVXOPT_GSL_INC_DIR="$(pkg-config --variable=includedir gsl)"

but so long as any SPKG that is installed and provides a *.pc file must be used (ignoring --with-system flags), they won't hurt.

vbraun · 2020-05-26T21:44:06Z

Changed branch from u/dimpase/packages/suitesparseconf to f2cc5b4

mkoeppe · 2020-07-03T04:18:18Z

Changed commit from f2cc5b4 to none

mkoeppe · 2020-07-03T04:18:18Z

comment:59

Follow-up: #30052 ubuntu-eoan-i386: cvxopt build fails

dimpase added this to the sage-9.2 milestone Apr 13, 2020

dimpase added c: build: configure labels Apr 13, 2020

This comment has been minimized.

Sign in to view

dimpase added the s: needs review label Apr 14, 2020

dimpase modified the milestones: sage-9.2, sage-9.1 Apr 19, 2020

dimpase added s: needs review and removed s: needs work labels Apr 29, 2020

mkoeppe modified the milestones: sage-9.1, sage-9.2 May 4, 2020

orlitzky added s: positive review and removed s: needs review labels May 19, 2020

vbraun removed the s: positive review label May 26, 2020

vbraun closed this as completed in fbb11c3 May 26, 2020

embray mentioned this issue Jan 29, 2023

Meta-ticket: spkg-configure: Try to use as many system packages as possible #27330

Open

spkg-configure for suitesparse #29502

spkg-configure for suitesparse #29502

Comments

dimpase commented Apr 13, 2020

This comment has been minimized.

thierry-FreeBSD commented Apr 13, 2020

thierry-FreeBSD commented Apr 13, 2020

dimpase commented Apr 13, 2020

dimpase commented Apr 13, 2020

dimpase commented Apr 13, 2020

thierry-FreeBSD commented Apr 13, 2020

dimpase commented Apr 13, 2020

dimpase commented Apr 13, 2020

thierry-FreeBSD commented Apr 13, 2020

dimpase commented Apr 13, 2020

thierry-FreeBSD commented Apr 13, 2020

dimpase commented Apr 14, 2020

sagetrac-git mannequin commented Apr 14, 2020

sagetrac-git mannequin commented Apr 14, 2020

dimpase commented Apr 14, 2020

This comment has been minimized.

dimpase commented Apr 14, 2020

sagetrac-git mannequin commented Apr 14, 2020

sagetrac-git mannequin commented Apr 14, 2020

thierry-FreeBSD commented Apr 14, 2020

dimpase commented Apr 14, 2020

dimpase commented Apr 19, 2020

kiwifb commented Apr 28, 2020

orlitzky commented Apr 28, 2020

kiwifb commented Apr 28, 2020

sagetrac-git mannequin commented Apr 29, 2020

sagetrac-git mannequin commented Apr 29, 2020

dimpase commented Apr 29, 2020

orlitzky commented Apr 29, 2020

dimpase commented Apr 29, 2020

sagetrac-git mannequin commented Apr 29, 2020

sagetrac-git mannequin commented Apr 29, 2020

orlitzky commented Apr 29, 2020

dimpase commented Apr 29, 2020

orlitzky commented Apr 30, 2020

mkoeppe commented Apr 30, 2020

mkoeppe commented Apr 30, 2020

dimpase commented Apr 30, 2020

orlitzky commented May 19, 2020

orlitzky commented May 19, 2020

vbraun commented May 26, 2020

mkoeppe commented Jul 3, 2020

mkoeppe commented Jul 3, 2020