Opened 2 years ago

Closed 22 months ago

Last modified 22 months ago

#27870 closed enhancement (fixed)

spkg-configure.m4 for $(BLAS) (i.e. atlas and openblas)

Reported by: dimpase Owned by:
Priority: major Milestone: sage-9.1
Component: build: configure Keywords: spkg-configure
Cc: embray, vbraun, tmonteil, mmezzarobba, mjo, charpent, fbissey, isuruf, mkoeppe Merged in:
Authors: Dima Pasechnik Reviewers: Isuru Fernando
Report Upstream: N/A Work issues:
Branch: 81bf522 (Commits, GitHub, GitLab) Commit:
Dependencies: #28883 Stopgaps:

Status badges

Description

one needs to understand the existing design first.

  • (as far as I can see) currently Sage generates *.pc files for pkg-config, for blas, cblas, and lapack. This is done for Atlas (and for external blas/lapack fed in by SAGE_ATLAS_LIB ?). For openblas these *.pc files are basically all point to the same library.
  • how are they used? Are they all used?
  • is the description of SAGE_ATLAS_LIB outdated? I cannot imagine any need for f77 libraries in 2019...

Change History (81)

comment:1 Changed 2 years ago by embray

Yes, we should probably get rid of some of the ATLAS-specific stuff (or re-appropriate it). The thing about a lot of the ATLAS stuff is that it doesn't actually care that much about having ATLAS BLAS; the way much of it is written it will work if it can find some BLAS anywhere. That's why on Cygwin it works when you install the lapack package, because it comes with a built-in netlib BLAS. It has nothing to do with ATLAS.

It would make sense to have an option to look for openblas if it exists, or else fall back on a generic BLAS if one can be found via various sources (pkg-config, some autoconf tests, etc.).

comment:2 Changed 2 years ago by embray

  • Milestone sage-8.8 deleted

As the Sage-8.8 release milestone is pending, we should delete the sage-8.8 milestone for tickets that are not actively being worked on or that still require significant work to move forward. If you feel that this ticket should be included in the next Sage release at the soonest please set its milestone to the next release milestone (sage-8.9).

comment:3 Changed 2 years ago by tmonteil

  • Cc tmonteil added

comment:4 Changed 2 years ago by mmezzarobba

  • Cc mmezzarobba added

comment:5 Changed 23 months ago by dimpase

  • Branch set to u/dimpase/packages/openblas/openblasconf
  • Commit set to b446427fbb1792029a12d3a53cdcddf7e92e81d9

Debian has all the needed .pc files, so there it appears to work.


New commits:

b446427a bare minumim openblas config

comment:6 Changed 23 months ago by dimpase

  • Milestone set to sage-9.1

comment:7 Changed 23 months ago by embray

That's certainly better than nothing. There should probably also be a check in REQUIRED-CHECK (the third argument to SAGE_SPKG_CONFIGURE) to set sage_require_openblas=no if $with_blas != openblas). This way it won't check for openblas if someone configured Sage with --with-blas=atlas.

comment:8 Changed 23 months ago by dimpase

One can only check for openblas with pkg-config and make blas.pc and lapack.pc just as copies of openblas.pc.

But I don't know how to look for openblas.pc to copy, other than doing Unix find /usr/ - something that is neither robust nor fast.

comment:9 Changed 23 months ago by dimpase

One can fill in templates for blas.pc by calling pkg-config on openblas, but this looks like an ugly hack...

comment:10 Changed 23 months ago by dimpase

OK, in fact one get the location of openblas.pc file:

pkg-config --variable=pcfiledir openblas

(this works), and the corresponding autoconf macro (not tested yet):

 PKG_CHECK_VAR([OPENBLASPCDIR], [openblas], [pcfiledir],
               [ACTION-IF-FOUND], [ACTION-IF-NOT-FOUND])

comment:11 Changed 23 months ago by git

  • Commit changed from b446427fbb1792029a12d3a53cdcddf7e92e81d9 to cb725d388640d1791778cb5788cdc2ee9482581e

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

22300f6use openblas.pc for blas and lapack, don't require atlas is to be used
cb725d3move blas handling to build/pkgs/

comment:12 Changed 23 months ago by dimpase

  • Authors set to Dima Pasechnik
  • Status changed from new to needs_review

this is a minimal fix for the use of system's openblas, identifiable from pkg-config.

I have not touched Atlas installation procedure.

comment:13 follow-up: Changed 23 months ago by charpent

Well... I did git trac checkout 27870, then make, and make went directly to checking the documentation.

I guess that testing your patch involves either make clean or make distclean. Which one ?

comment:14 in reply to: ↑ 13 ; follow-up: Changed 23 months ago by dimpase

Replying to charpent:

Well... I did git trac checkout 27870, then make, and make went directly to checking the documentation.

I guess that testing your patch involves either make clean or make distclean. Which one ?

Not only this :-)

make openblas-clean # uninstall package
./bootstrap # update ./configure
./configure # here the output should show that openblas is not being installed
make build

on OSX for some reason I also needed to ./sage -f numpy before make build

make distclean before bootstrap is more robust. I'm not sure in which cases one should re-install all the packages that depend upon blas, too, this might be platform-dependent.


One naturally is interested in testing against system's openblas: e.g. on debian and derived systems one needs apt install pkg-config libopenblas-dev

On OSX's Homebrew one would need brew install openblas and then export certain environment variable (as Homebrew will tell you), as for some reason it's not installed into a default location.

Last edited 23 months ago by dimpase (previous) (diff)

comment:15 Changed 23 months ago by mjo

  • Cc mjo added

There is a larger mess to deal with here, but TBH I'm fine with solving it one piece at a time. It looks like a few packages (R, numpy,... ?) do use pkg-config to discover information about the blas/lapack being used. And our pkg-config is hacked (build/pkgs/pkgconf/patches/pkg-config.in) to look in a custom directory for .pc files if they aren't found on the system. So I think the idea was that if pkg-config couldn't find e.g. lapack.pc, that it would then look in SAGE_LOCAL to find it.

This only raises more questions. If openblas is installed on the system, can we count on the system pkg-config to know that the system blas/lapack are just the system openblas? If so, then we shouldn't need to create "fallback" symlinks. If not, would linking to some other blas/lapack cause problems? Why don't those packages check for openblas or atlas directly instead of "blas" and "lapack", in that case? And so on. Your approach looks better the deeper I go.

comment:16 Changed 23 months ago by dimpase

I more or less copy the logic of openblas/dpkg-install - which does install openblas.pc under 3 different names - openblas, blas, and lapack.

comment:17 follow-up: Changed 23 months ago by jhpalmieri

I tried this (meaning make distclean; ./bootstrap; make) with an Ubuntu virtual machine. config.log says

configure:20206: result:     atlas-3.10.2.p3 will not be installed (configure check)
...
configure:20206: result:     openblas-0.3.6.p0 will not be installed (configure check)

but scipy installation fails with

numpy.distutils.system_info.NotFoundError: No lapack/blas resources found.

There are also various warnings in the numpy installation log about not finding atlas or blas.

comment:18 follow-up: Changed 23 months ago by dimpase

scipy depends on numpy to supply blas interface, so something goes wrong with numpy installation (it is weird that it foes not fail).

it works for me on Debian 10. numpy has its own extermely convoluted configure routine, which emits a lot of noise about not finding this or that.

Could you try building numpy with SAGE_CHECK ?

Last edited 23 months ago by dimpase (previous) (diff)

comment:19 Changed 23 months ago by mjo

Replying to dimpase:

I more or less copy the logic of openblas/dpkg-install - which does install openblas.pc under 3 different names - openblas, blas, and lapack.

Yeah I agree with that. Maybe two years from now, no one is using atlas any more and this is easy to clean up by dropping support for it and by having those other packages call pkg-config openblas... instead of pkg-config lapack and pkg-config blas. No need to worry about it now.

comment:20 in reply to: ↑ 17 Changed 23 months ago by mjo

Replying to jhpalmieri:

There are also various warnings in the numpy installation log about not finding atlas or blas.

FWIW numpy is one of those packages that uses pkg-config to find these libraries; see build/pkgs/numpy/lapack_conf.py.

comment:21 Changed 23 months ago by dimpase

Apart from Atlas and openblas there are other perfectly ok implementations of blas and lapack, which may be used.

comment:22 Changed 23 months ago by dimpase

e.g. there are highly optimised by hardware vendors blas and lapacks (e.g. MKL)

comment:23 in reply to: ↑ 18 Changed 23 months ago by jhpalmieri

Replying to dimpase:

Could you try building numpy with SAGE_CHECK ?

numpy has no spkg-check file, so this acts the same as what I did before.

comment:24 follow-up: Changed 23 months ago by mjo

I'm not suggesting that we don't support other blas/lapack implementations, but trying to account for multiple system implementations AND multiple sage implementations is making life harder than it has to be. Somewhere down the road, we should be (a) checking for system blas/lapack, and using what we find; then (b) as a fallback, installing openblas and using openblas.

This is all speculation right now, regardless =)

comment:25 in reply to: ↑ 24 ; follow-up: Changed 23 months ago by charpent

Replying to mjo:

I'm not suggesting that we don't support other blas/lapack implementations, but trying to account for multiple system implementations AND multiple sage implementations is making life harder than it has to be. Somewhere down the road, we should be (a) checking for system blas/lapack, and using what we find; then (b) as a fallback, installing openblas and using openblas.

This is all speculation right now, regardless =)

Do you mean "not ready for prime time" ?

comment:26 Changed 23 months ago by charpent

  • Cc charpent added

comment:27 in reply to: ↑ 14 Changed 23 months ago by charpent

  • Status changed from needs_review to needs_work

Replying to dimpase:

Replying to charpent:

Well... I did git trac checkout 27870, then make, and make went directly to checking the documentation.

I guess that testing your patch involves either make clean or make distclean. Which one ?

Not only this :-)

make openblas-clean # uninstall package
./bootstrap # update ./configure
./configure # here the output should show that openblas is not being installed

No such bllody luck. From config.log:

configure:11316: === checking whether to install the openblas SPKG ===
configure:11334: checking BLAS library
configure:11353: result: openblas
configure:11395: checking for OPENBLAS
configure:11402: $PKG_CONFIG --exists --print-errors "openblas >= 0.3.5"
Package openblas was not found in the pkg-config search path.
Perhaps you should add the directory containing `openblas.pc'
to the PKG_CONFIG_PATH environment variable
No package 'openblas' found
configure:11405: $? = 1
configure:11419: $PKG_CONFIG --exists --print-errors "openblas >= 0.3.5"
Package openblas was not found in the pkg-config search path.
Perhaps you should add the directory containing `openblas.pc'
to the PKG_CONFIG_PATH environment variable
No package 'openblas' found
configure:11422: $? = 1
configure:11436: result: no
No package 'openblas' found

But the relevant packages are installed systemwide (Debian testing) :

charpent@zen-book-flip:~$ dpkg -l "*openblas*"
Souhait=inconnU/Installé/suppRimé/Purgé/H=à garder
| État=Non/Installé/fichier-Config/dépaqUeté/échec-conFig/H=semi-installé/W=att>
|/ Err?=(aucune)/besoin Réinstallation (État,Err: majuscule=mauvais)
||/ Nom                           Version      Architecture Description
+++-=============================-============-============-===================>
ii  libopenblas-base:amd64        0.3.7+ds-5   amd64        Optimized BLAS (lin>
ii  libopenblas-dev:amd64         0.3.7+ds-5   amd64        Optimized BLAS (lin>
un  libopenblas-openmp-dev        <aucune>     <aucune>     (aucune description>
ii  libopenblas-pthread-dev:amd64 0.3.7+ds-5   amd64        Optimized BLAS (lin>
un  libopenblas-serial-dev        <aucune>     <aucune>     (aucune description>
ii  libopenblas0:amd64            0.3.7+ds-5   amd64        Optimized BLAS (lin>
un  libopenblas0-openmp           <aucune>     <aucune>     (aucune description>
ii  libopenblas0-pthread:amd64    0.3.7+ds-5   amd64        Optimized BLAS (lin>
un  libopenblas0-serial           <aucune>     <aucune>     (aucune description

==> needs_work.

Sorry.

comment:28 follow-up: Changed 23 months ago by dimpase

do you have pkg-config installed?

comment:29 in reply to: ↑ 28 Changed 23 months ago by charpent

Replying to dimpase:

do you have pkg-config installed?

Yes :

charpent@zen-book-flip:~$ dpkg -l "*pkg-config*"
Souhait=inconnU/Installé/suppRimé/Purgé/H=à garder
| État=Non/Installé/fichier-Config/dépaqUeté/échec-conFig/H=semi-installé/W=att>
|/ Err?=(aucune)/besoin Réinstallation (État,Err: majuscule=mauvais)
||/ Nom            Version      Architecture Description
+++-==============-============-============-==================================>
ii  pkg-config     0.29-6       amd64        manage compile and link flags for >
un  pkg-config-bin <aucune>     <aucune>     (aucune description n'est disponib>

I was unaware (until 30 seconds ago) of pkg-config-bin. It seems to be a virtual package (i. e. in Debianese, a placeholder for a functionality fullfillable by more than one package).

comment:30 follow-up: Changed 23 months ago by dimpase

what does

pkg-config --list-all | grep blas

show?

comment:31 in reply to: ↑ 30 ; follow-up: Changed 23 months ago by charpent

Replying to dimpase:

what does

pkg-config --list-all | grep blas

show?

charpent@zen-book-flip:~$ pkg-config --list-all | grep blas
lapack-openblas                openblas-lapack - Optimized BLAS (linear algebra) library, LAPACK
blas                           openblas-blas - Optimized BLAS (linear algebra) library based on GotoBLAS2
blas-openblas                  openblas-blas - Optimized BLAS (linear algebra) library based on GotoBLAS2
lapack                         openblas-lapack - Optimized BLAS (linear algebra) library, LAPACK
blas-netlib                    BLAS - FORTRAN reference implementation of BLAS Basic Linear Algebra Subprograms

comment:32 follow-up: Changed 23 months ago by jhpalmieri

I think my numpy/scipy problem is from each package's spkg-install script:

 export {ATLAS,PTATLAS,OPENBLAS,MKL,MKLROOT}=None

Setting OPENBLAS to None seems to cause the package not to find the system's openblas installation. Removing OPENBLAS from the list of environment variables fixes things for me. What is the purpose of this line?

comment:33 in reply to: ↑ 25 Changed 23 months ago by mjo

Replying to charpent:

Do you mean "not ready for prime time" ?

I probably should not have said anything at all. Modulo whatever bugs are being worked out, Dima's approach looks good to me.

Later on, there may be additional improvements to make sage support the system blas/lapack, but this patch is a step in the right direction.

comment:34 in reply to: ↑ 32 Changed 23 months ago by dimpase

  • Cc fbissey added

Replying to jhpalmieri:

I think my numpy/scipy problem is from each package's spkg-install script:

 export {ATLAS,PTATLAS,OPENBLAS,MKL,MKLROOT}=None

Setting OPENBLAS to None seems to cause the package not to find the system's openblas installation. Removing OPENBLAS from the list of environment variables fixes things for me. What is the purpose of this line?

I can reproduce your problem - and it seems to go away if I just not call that python script, i.e.

  • build/pkgs/numpy/spkg-install

    a b else 
    1111    export LDFLAGS="${LDFLAGS} -shared"
    1212fi
    1313
    14 sage-python23 ../lapack_conf.py
     14# sage-python23 ../lapack_conf.py
    1515
    1616# Make sure that the fortran objects are compiled with -fPIC
    1717export FFLAGS="$FFLAGS -fPIC"

I'm not saying it can be just removed unconditionally, but it needs an adjustment.

Hopefully Francois, its author, knows what's going wrong.

comment:35 in reply to: ↑ 31 ; follow-up: Changed 23 months ago by dimpase

Replying to charpent:

Replying to dimpase:

what does

pkg-config --list-all | grep blas

show?

charpent@zen-book-flip:~$ pkg-config --list-all | grep blas
lapack-openblas                openblas-lapack - Optimized BLAS (linear algebra) library, LAPACK
blas                           openblas-blas - Optimized BLAS (linear algebra) library based on GotoBLAS2
blas-openblas                  openblas-blas - Optimized BLAS (linear algebra) library based on GotoBLAS2
lapack                         openblas-lapack - Optimized BLAS (linear algebra) library, LAPACK
blas-netlib                    BLAS - FORTRAN reference implementation of BLAS Basic Linear Algebra Subprograms

So in their infinite wisdom they removed openblas.pc all together. Not sure it's a Debian bug (note that upstream openblas does create such a file, so it's really an incompatible Debian invention here).

comment:36 in reply to: ↑ 35 ; follow-ups: Changed 23 months ago by charpent

Replying to dimpase:

Replying to charpent:

Replying to dimpase:

what does

pkg-config --list-all | grep blas

show?

charpent@zen-book-flip:~$ pkg-config --list-all | grep blas
lapack-openblas                openblas-lapack - Optimized BLAS (linear algebra) library, LAPACK
blas                           openblas-blas - Optimized BLAS (linear algebra) library based on GotoBLAS2
blas-openblas                  openblas-blas - Optimized BLAS (linear algebra) library based on GotoBLAS2
lapack                         openblas-lapack - Optimized BLAS (linear algebra) library, LAPACK
blas-netlib                    BLAS - FORTRAN reference implementation of BLAS Basic Linear Algebra Subprograms

So in their infinite wisdom they removed openblas.pc all together. Not sure it's a Debian bug (note that upstream openblas does create such a file, so it's really an incompatible Debian invention here).

Nope:

charpent@zen-book-flip:~$ ls -l /usr/lib/x86_64-linux-gnu/openblas-pthread/pkgconfig/openblas.pc 
-rw-r--r-- 1 root root 570 déc.   2 06:33 /usr/lib/x86_64-linux-gnu/openblas-pthread/pkgconfig/openblas.pc
charpent@zen-book-flip:~$ cat /usr/lib/x86_64-linux-gnu/openblas-pthread/pkgconfig/openblas.pc 
libdir=/usr/lib/x86_64-linux-gnu/openblas-pthread/
includedir=/usr/include/x86_64-linux-gnu/openblas-pthread/
openblas_config= USE_64BITINT= DYNAMIC_ARCH=1 DYNAMIC_OLDER=1 NO_CBLAS= NO_LAPACK= NO_LAPACKE=1 NO_AFFINITY=1 USE_OPENMP=0 generic MAX_THREADS=64
version=0.3.7
extralib=-lm -lpthread -lgfortran -lm -lpthread -lgfortran
Name: openblas
Description: OpenBLAS is an optimized BLAS library based on GotoBLAS2 1.13 BSD version
Version: ${version}
URL: https://github.com/xianyi/OpenBLAS
Libs: -L${libdir} -lopenblas
Libs.private: ${extralib}
Cflags: -I${includedir}

The problem is elsewhere...

comment:37 in reply to: ↑ 36 Changed 23 months ago by charpent

Note, however,that my locate finds the openblas.pc created in my various Sage trees, but not the systewide one.

I don't recall having ever fiddled with its configuration.

Replying to charpent:

Replying to dimpase:

Replying to charpent:

Replying to dimpase:

what does

pkg-config --list-all | grep blas

show?

charpent@zen-book-flip:~$ pkg-config --list-all | grep blas
lapack-openblas                openblas-lapack - Optimized BLAS (linear algebra) library, LAPACK
blas                           openblas-blas - Optimized BLAS (linear algebra) library based on GotoBLAS2
blas-openblas                  openblas-blas - Optimized BLAS (linear algebra) library based on GotoBLAS2
lapack                         openblas-lapack - Optimized BLAS (linear algebra) library, LAPACK
blas-netlib                    BLAS - FORTRAN reference implementation of BLAS Basic Linear Algebra Subprograms

So in their infinite wisdom they removed openblas.pc all together. Not sure it's a Debian bug (note that upstream openblas does create such a file, so it's really an incompatible Debian invention here).

Nope:

charpent@zen-book-flip:~$ ls -l /usr/lib/x86_64-linux-gnu/openblas-pthread/pkgconfig/openblas.pc 
-rw-r--r-- 1 root root 570 déc.   2 06:33 /usr/lib/x86_64-linux-gnu/openblas-pthread/pkgconfig/openblas.pc
charpent@zen-book-flip:~$ cat /usr/lib/x86_64-linux-gnu/openblas-pthread/pkgconfig/openblas.pc 
libdir=/usr/lib/x86_64-linux-gnu/openblas-pthread/
includedir=/usr/include/x86_64-linux-gnu/openblas-pthread/
openblas_config= USE_64BITINT= DYNAMIC_ARCH=1 DYNAMIC_OLDER=1 NO_CBLAS= NO_LAPACK= NO_LAPACKE=1 NO_AFFINITY=1 USE_OPENMP=0 generic MAX_THREADS=64
version=0.3.7
extralib=-lm -lpthread -lgfortran -lm -lpthread -lgfortran
Name: openblas
Description: OpenBLAS is an optimized BLAS library based on GotoBLAS2 1.13 BSD version
Version: ${version}
URL: https://github.com/xianyi/OpenBLAS
Libs: -L${libdir} -lopenblas
Libs.private: ${extralib}
Cflags: -I${includedir}

The problem is elsewhere...

comment:38 follow-up: Changed 23 months ago by dimpase

Well, this pretty much looks like a Debian bug. The point of installing openblas.pc into a hidden from pkg-config directory only makes sense if they plan to install "competing" versions of this file in different locations...

comment:39 in reply to: ↑ 38 ; follow-up: Changed 23 months ago by charpent

Replying to dimpase:

Well, this pretty much looks like a Debian bug. The point of installing openblas.pc into a hidden from pkg-config directory only makes sense if they plan to install "competing" versions of this file in different locations...

This is now Debian bug 946698 . For what it's worth...

comment:40 Changed 23 months ago by jhpalmieri

For me, this fixes the problem.

  • build/pkgs/numpy/lapack_conf.py

    diff --git a/build/pkgs/numpy/lapack_conf.py b/build/pkgs/numpy/lapack_conf.py
    index d127d08def..f1bc559cfa 100644
    a b conf_file.write('[ALL]\n') 
    88conf_file.write('library_dirs = '+ os.environ['SAGE_LOCAL']+ '/lib\n')
    99conf_file.write('include_dirs = '+ os.environ['SAGE_LOCAL']+ '/include\n')
    1010
    11 pc_blas   = pkgconfig.parse('cblas blas')
     11pc_blas   = pkgconfig.parse('blas')
    1212pc_lapack = pkgconfig.parse('lapack')
    1313
    1414if not (os.environ['UNAME'] == 'Darwin'):

If I run ./sage --python:

>>> import pkgconfig
>>> pkgconfig.parse('blas')
defaultdict(<class 'list'>, {'include_dirs': ['/usr/include/x86_64-linux-gnu'], 'libraries': ['openblas'], 'define_macros': []})
>>> pkgconfig.parse('cblas blas')
defaultdict(<class 'list'>, {'define_macros': []})

I don't know why the space-separated list doesn't work. It is supposed to, according to the documentation. As far as I can tell, it doesn't find a cblas package, and maybe that confuses it. (it = pkgconfig.parse)

Last edited 23 months ago by jhpalmieri (previous) (diff)

comment:41 Changed 23 months ago by fbissey

I originally followed the model of the gentoo ebuild for figuring out the needed blas/cblas bits. Of course if you standardise on openblas this is not so needed.

Debian and now gentoo have this model where you have just libblas.so, libcblas.so and liblapack.so and what is underneath can be changed at runtime. So you don't need to know the implementation. On the other hand numpy has this detection model trying to figure what implementation you may have to figure the correct libraries. It used to be that something different would be built depending on implementation but not anymore. Honestly trying to figure which blas/lapack you are using leads to madness :( which is why we steered towards having standard named .pc files. And historically we shaved some bits from numpy's detection machinery.

On my TODO list is to switch my sage-on-gentoo from openblas to mkl. The runtime switch means that it should be quick and easy.

The stuff about pkgconfig.parse should be reported as a bug. Although we should check we have currently the latest version first.

comment:42 follow-up: Changed 23 months ago by dimpase

with up-to-date pkgconfig (version 1.5.1) I get

$ ./sage --python
Python 3.7.3 (default, Dec 13 2019, 17:59:09) 
[GCC 8.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import pkgconfig
>>> pkgconfig.parse('blas')
defaultdict(<class 'list'>, {'include_dirs': ['/usr/include/x86_64-linux-gnu'], 'libraries': ['openblas']})
>>> pkgconfig.parse('cblas blas')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/dimpase/sage/local/lib/python3.7/site-packages/pkgconfig/pkgconfig.py", line 248, in parse
    _raise_if_not_exists(package)
  File "/home/dimpase/sage/local/lib/python3.7/site-packages/pkgconfig/pkgconfig.py", line 103, in _raise_if_not_exists
    raise PackageNotFoundError(package)
pkgconfig.pkgconfig.PackageNotFoundError: cblas not found
>>> 

That is, after the update the script might just error out (cblas need not be present)

comment:43 in reply to: ↑ 42 Changed 23 months ago by fbissey

Replying to dimpase:

with up-to-date pkgconfig (version 1.5.1) I get

$ ./sage --python
Python 3.7.3 (default, Dec 13 2019, 17:59:09) 
[GCC 8.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import pkgconfig
>>> pkgconfig.parse('blas')
defaultdict(<class 'list'>, {'include_dirs': ['/usr/include/x86_64-linux-gnu'], 'libraries': ['openblas']})
>>> pkgconfig.parse('cblas blas')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/dimpase/sage/local/lib/python3.7/site-packages/pkgconfig/pkgconfig.py", line 248, in parse
    _raise_if_not_exists(package)
  File "/home/dimpase/sage/local/lib/python3.7/site-packages/pkgconfig/pkgconfig.py", line 103, in _raise_if_not_exists
    raise PackageNotFoundError(package)
pkgconfig.pkgconfig.PackageNotFoundError: cblas not found
>>> 

That is, after the update the script might just error out (cblas need not be present)

Yes. So I think for that part

  1. upgrade pkgconfig
  2. put a try-expect block in the script

first we try with both, if the right exception is raised we try only blas and if that errors too, something is wrong and we should bail out.

comment:44 Changed 23 months ago by dimpase

  • Dependencies set to #28883

I've opened #28883 for pkgconfig upgrade.

I should also add cblas to the list of link the branch here is installing.

comment:45 Changed 23 months ago by git

  • Commit changed from cb725d388640d1791778cb5788cdc2ee9482581e to 634157628ac3f7292e7065eae462bb6b80db7aed

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

e5c91dbupdate pkgconfig to v1.5.1. See trac #28883
1720b38a bare minumim openblas config
925266buse openblas.pc for blas and lapack, don't require atlas is to be used
b89df80move blas handling to build/pkgs/
6341576create cblas.pc link, too, not only blas and lapack

comment:46 Changed 23 months ago by dimpase

  • Keywords spkg-configure added
  • Status changed from needs_work to needs_review

comment:47 in reply to: ↑ 36 Changed 23 months ago by charpent

Possibly dumb question: why are we looking for openblas rather than for blas and lapack ?

From my systemwide Debian pkg-confi (afrter a reboot):

charpent@zen-book-flip:/usr/local/sage-9$ pkg-config --libs --cflags openblas
Package openblas was not found in the pkg-config search path.
Perhaps you should add the directory containing `openblas.pc'
to the PKG_CONFIG_PATH environment variable
No package 'openblas' found
charpent@zen-book-flip:/usr/local/sage-9$ pkg-config --libs --cflags blas
-I/usr/include/x86_64-linux-gnu -L/usr/lib/x86_64-linux-gnu/openblas -lblas
charpent@zen-book-flip:/usr/local/sage-9$ pkg-config --libs --cflags lapack
-I/usr/include/x86_64-linux-gnu -L/usr/lib/x86_64-linux-gnu/openblas -llapack

[ Snip... ]

comment:48 Changed 23 months ago by dimpase

it seems to be Debian (testing/unstable) bug.

comment:49 Changed 23 months ago by dimpase

"generic" blas and lapack may be very different from what Sage needs, it is a tough call to test whether everything needed is there.

comment:50 Changed 23 months ago by charpent

Well... Starting from #28883, I created a local branch in which I merged #27870, then make openblas-clean, then ./bootstrap, then ./configure.

When I started make, the first thing it did was to recompile openblas...

And, sure enough, my config.log contains:

configure:11316: === checking whether to install the openblas SPKG ===
configure:11334: checking BLAS library
configure:11353: result: openblas
configure:11395: checking for OPENBLAS
configure:11402: $PKG_CONFIG --exists --print-errors "openblas >= 0.3.5"
Package openblas was not found in the pkg-config search path.
Perhaps you should add the directory containing `openblas.pc'
to the PKG_CONFIG_PATH environment variable
No package 'openblas' found
configure:11405: $? = 1
configure:11419: $PKG_CONFIG --exists --print-errors "openblas >= 0.3.5"
Package openblas was not found in the pkg-config search path.
Perhaps you should add the directory containing `openblas.pc'
to the PKG_CONFIG_PATH environment variable
No package 'openblas' found
configure:11422: $? = 1
configure:11436: result: no
No package 'openblas' found
Last edited 23 months ago by charpent (previous) (diff)

comment:51 follow-up: Changed 23 months ago by dimpase

#28883 has nothing to do with testing Debian distro bug that you reported, it is meant for MacOS problem reported by John.

If you want to try to use the openblas.pc you have, you can do

export PKG_CONFIG_PATH="/usr/lib/x86_64-linux-gnu/openblas-pthread/pkgconfig"
make openblas-clean
./configure # - and check the output - for openblas not being installed

if this worked you can try building, i.e. make or make build...

comment:52 follow-up: Changed 23 months ago by jhpalmieri

It's true I usually use MacOS, but for this ticket I've been using an Ubuntu virtual machine, so #28883 is relevant.

comment:53 in reply to: ↑ 52 Changed 23 months ago by dimpase

Replying to jhpalmieri:

It's true I usually use MacOS, but for this ticket I've been using an Ubuntu virtual machine, so #28883 is relevant.

right, indeed. Anyway, #28883 is meant to fix a problem with building numpy with already configured external openblas, rather than a configuration problem of newer Debians.

comment:54 in reply to: ↑ 51 Changed 23 months ago by charpent

Replying to dimpase:

#28883 has nothing to do with testing Debian distro bug that you reported, it is meant for MacOS problem reported by John.

If you want to try to use the openblas.pc you have, you can do

export PKG_CONFIG_PATH="/usr/lib/x86_64-linux-gnu/openblas-pthread/pkgconfig"
make openblas-clean
./configure # - and check the output - for openblas not being installed

if this worked you can try building, i.e. make or make build...

This seemsto have worked. Now makeing. Don't hold your breath...

Version 0, edited 23 months ago by charpent (next)

comment:55 follow-up: Changed 23 months ago by dimpase

How can a bug in Debian, which can be worked around by setting appropriately PKG_CONFIG_PATH, be a blocker to this? It cannot work for all the broken systems out there :-)

E.g. Homebrew (on MacOS) also installs openblas somewhere strange, with a need to set PKG_CONFIG_PATH (it tells you this, actually).

comment:56 in reply to: ↑ 55 Changed 23 months ago by charpent

  • Status changed from needs_review to positive_review

Replying to dimpase:

How can a bug in Debian, which can be worked around by setting appropriately PKG_CONFIG_PATH, be a blocker to this? It cannot work for all the broken systems out there :-)

E.g. Homebrew (on MacOS) also installs openblas somewhere strange, with a need to set PKG_CONFIG_PATH (it tells you this, actually).

Okay.

Anyway, ptestlong gets two permanent failures (and no transient):

charpent@zen-book-flip:/usr/local/sage-9$ sage -t --long --warn-long 178.2 src/sage/numerical/backends/glpk_backend.pyx  # 1 doctest failed
Running doctests with ID 2019-12-14-21-20-56-a860b5d2.
Git branch: t/27870/packages/openblas/openblasconf
Using --optional=build,dochtml,memlimit,sage
Doctesting 1 file.
sage -t --long --warn-long 178.2 src/sage/numerical/backends/glpk_backend.pyx
**********************************************************************
File "src/sage/numerical/backends/glpk_backend.pyx", line 2287, in sage.numerical.backends.glpk_backend.GLPKBackend.print_ranges
Failed example:
    p.print_ranges()
Expected:
    glp_print_ranges: optimal basic solution required
    1
Got:
    1
**********************************************************************
1 item had failures:
   1 of  13 in sage.numerical.backends.glpk_backend.GLPKBackend.print_ranges
    [554 tests, 1 failure, 3.70 s]
----------------------------------------------------------------------
sage -t --long --warn-long 178.2 src/sage/numerical/backends/glpk_backend.pyx  # 1 doctest failed
----------------------------------------------------------------------
Total time for all tests: 3.8 seconds
    cpu time: 3.6 seconds
    cumulative wall time: 3.7 seconds
charpent@zen-book-flip:/usr/local/sage-9$ sage -t --long --warn-long 178.2 src/sage/libs/glpk/error.pyx  # 1 doctest failed
Running doctests with ID 2019-12-14-21-21-18-8e3dc9e1.
Git branch: t/27870/packages/openblas/openblasconf
Using --optional=build,dochtml,memlimit,sage
Doctesting 1 file.
sage -t --long --warn-long 178.2 src/sage/libs/glpk/error.pyxThos failures have already been reported for the same machine 
**********************************************************************
File "src/sage/libs/glpk/error.pyx", line 100, in sage.libs.glpk.error.setup_glpk_error_handler
Failed example:
    res = p.solve()
Expected:
          0: obj = ...
Got:
    <BLANKLINE>
**********************************************************************
1 item had failures:
   1 of  11 in sage.libs.glpk.error.setup_glpk_error_handler
    [12 tests, 1 failure, 0.72 s]
----------------------------------------------------------------------
sage -t --long --warn-long 178.2 src/sage/libs/glpk/error.pyx  # 1 doctest failed
----------------------------------------------------------------------
Total time for all tests: 0.7 seconds
    cpu time: 0.4 seconds
    cumulative wall time: 0.7 seconds

Those failures were already reported (in sage-release) for the same machine and the same version of (Python 3-based) 9.0.beta9. Therefore, they do do not seem to be related to the current patch.

==> positive_review, subject to better-informed advice...

comment:57 Changed 23 months ago by dimpase

  • Cc isuruf mkoeppe added

comment:58 Changed 23 months ago by isuruf

  • Reviewers set to Isuru Fernando

Works for me with conda.

comment:59 Changed 23 months ago by dimpase

  • Milestone changed from sage-9.1 to sage-9.0

comment:60 in reply to: ↑ 39 Changed 23 months ago by charpent

Replying to charpent:

Replying to dimpase:

Well, this pretty much looks like a Debian bug. The point of installing openblas.pc into a hidden from pkg-config directory only makes sense if they plan to install "competing" versions of this file in different locations...

This is now Debian bug 946698 . For what it's worth...

FWIW, this Debian bug is claimed. to have been fixed. In about 45 hours... Pffew !

comment:61 Changed 22 months ago by vbraun

  • Branch changed from u/dimpase/packages/openblas/openblasconf to 634157628ac3f7292e7065eae462bb6b80db7aed
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:62 Changed 22 months ago by vbraun

  • Commit 634157628ac3f7292e7065eae462bb6b80db7aed deleted
  • Resolution fixed deleted
  • Status changed from closed to new

This ticket breaks build on OSX:

************************************************************************
Traceback (most recent call last):
  File "setup.py", line 72, in <module>
    from module_list import ext_modules, library_order
  File "/Users/buildbot-sage/slave/sage_git/build/src/module_list.py", line 42, in <module>
    zlib_pc = pkgconfig.parse('zlib')
  File "/Users/buildbot-sage/slave/sage_git/build/local/lib/python3.7/site-packages/pkgconfig/pkgconfig.py", line 248, in parse
    _raise_if_not_exists(package)
  File "/Users/buildbot-sage/slave/sage_git/build/local/lib/python3.7/site-packages/pkgconfig/pkgconfig.py", line 103, in _raise_if_not_exists
    raise PackageNotFoundError(package)
pkgconfig.pkgconfig.PackageNotFoundError: zlib not found
************************************************************************
Error building the Sage library
************************************************************************

comment:63 Changed 22 months ago by dimpase

one needs details such as whether you build zlib or use zlib from the system, whether Python gets correctly built zlib module, etc.

What does happen if you do

./sage --python
>>> import zlib

comment:64 Changed 22 months ago by vbraun

  • Status changed from new to needs_review

Should be the fault of the dependency at #28883

comment:65 Changed 22 months ago by dimpase

Well, we do need a newer pkgconfig from #28883 here, and I really have no idea what breaks for you at #28883. Did you try make distclean ?

comment:66 Changed 22 months ago by vbraun

On Catalina, there is no zlib in /usr/lib/pkgconfig and we don't build our own zlib with #27870.

buildbot-sage@osx build % ./sage -sh -c 'pkgconf --cflags zlib'

Package zlib was not found in the pkg-config search path.
Perhaps you should add the directory containing `zlib.pc'
to the PKG_CONFIG_PATH environment variable
Package 'zlib', required by 'world', not found

All builds from scratch.

comment:67 Changed 22 months ago by vbraun

  • Status changed from needs_review to positive_review

Is the fault of #28883

comment:68 Changed 22 months ago by vbraun

  • Branch changed from 634157628ac3f7292e7065eae462bb6b80db7aed to u/dimpase/packages/openblas/openblasconf
  • Commit set to 634157628ac3f7292e7065eae462bb6b80db7aed

New commits:

e5c91dbupdate pkgconfig to v1.5.1. See trac #28883
1720b38a bare minumim openblas config
925266buse openblas.pc for blas and lapack, don't require atlas is to be used
b89df80move blas handling to build/pkgs/
6341576create cblas.pc link, too, not only blas and lapack

comment:69 Changed 22 months ago by git

  • Commit changed from 634157628ac3f7292e7065eae462bb6b80db7aed to 81bf522a5a417e236f9b9ee4f05256126b5b47cb
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. New commits:

f74f554update pkgconfig to v1.5.1. See trac #28883
fb8f4e5allow absent zlib.pc
b3a8891a bare minumim openblas config
c031c9euse openblas.pc for blas and lapack, don't require atlas is to be used
b49f79emove blas handling to build/pkgs/
81bf522create cblas.pc link, too, not only blas and lapack

comment:70 Changed 22 months ago by dimpase

  • Status changed from needs_review to positive_review

trivial rebase over updated #28883

comment:71 Changed 22 months ago by dimpase

  • Milestone changed from sage-9.0 to sage-9.1

comment:72 follow-up: Changed 22 months ago by embray

Currently doesn't detect system openblas on Cygwin (which just gets installed as the generic libblas when you install it). Not sure why yet. I won't hold up this ticket over it but if I can find a quick fix I might like to do that.

comment:73 Changed 22 months ago by dimpase

what does pkg-config --modversion openblas say?

comment:74 in reply to: ↑ 72 ; follow-up: Changed 22 months ago by embray

Replying to embray:

Currently doesn't detect system openblas on Cygwin (which just gets installed as the generic libblas when you install it). Not sure why yet. I won't hold up this ticket over it but if I can find a quick fix I might like to do that.

I see; it's simply that Cygwin does not install an openblas.pc. That's annoying. I'll see if I can get that fixed upstream. In the meantime we can just leave this as-is.

I think as a broader improvement to the current situation it's worth recognizing that the spkg-install for "atlas" actually does two things:

  • If you have the old SAGE_ATLAS_LIB environment variable set it will just detect and use any old generic BLAS it can find in th given path; it doesn't care if it's actually ATLAS or not.
  • Otherwise it contains steps for specifically building and installing ATLAS from source.

These two cases seem to be quite at odds with each-other, and the naming of SAGE_ATLAS_LIB is misleading.

I think what we should do is this (in a separate ticket):

  • Deprecate use of SAGE_ATLAS_LIB; it can still work for now but display a deprecation message if it is set sage is configured with --with-blas=atlas.
  • Clean up the atlas SPKG a bit so it's only responsible for installing ATLAS if requested.
  • Add a generic package called just "blas" (we could keep openblas as the default though). The spkg-configure.m4 for blas would implement the --with-blas flag, and also extend it to take an optional argument of a path (e.g. instead of --with-blas=openblas or --with-blas=atlas one can pass something like --with-blas=/usr/lib. This latter case would have essentially functionality as the current SAGE_ATLAS_LIB (maybe a little better): use that path to search for a generic blas library; maybe also check that it contains some known symbols.
    • This is nice, because if you use the --with-blas=<path> option you don't get any SPKGs installed for blas, emphasizing that some generic blas on the system is being used.
Last edited 22 months ago by embray (previous) (diff)

comment:75 follow-up: Changed 22 months ago by dimpase

iirc, SAGE_ATLAS_LIB is used for two rather different purposes. So there is some cleaning up due.

comment:76 in reply to: ↑ 75 Changed 22 months ago by embray

Replying to dimpase:

iirc, SAGE_ATLAS_LIB is used for two rather different purposes. So there is some cleaning up due.

The only use of it I could find was in build/pkgs/atlas/spkg-install.py. If set, the directory it's set to is searched (in a fairly flimsy manner) for files that look blas-ish, although in one case it does explicitly look for an existing ATLAS install (if any functionality like this is to be kept it should be moved to atlas/spkg-configure.m4). If anything is found it creates some symlinks to the found libraries into SAGE_LOCAL (this should go away), and also generates some .pc files (also probably can go away and handled more like you did with zlib...)

comment:77 Changed 22 months ago by vbraun

  • Branch changed from u/dimpase/packages/openblas/openblasconf to 81bf522a5a417e236f9b9ee4f05256126b5b47cb
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:78 Changed 22 months ago by embray

  • Commit 81bf522a5a417e236f9b9ee4f05256126b5b47cb deleted

Apparently my rather old Ubuntu lacks a PKG_CONFIG_VAR macro, so since this ticket I get:

$ ./bootstrap
rm -rf config configure build/make/Makefile-auto.in
bootstrap:69: installing 'config/config.rpath'
configure.ac:328: installing 'config/compile'
configure.ac:113: installing 'config/config.guess'
configure.ac:113: installing 'config/config.sub'
configure.ac:68: installing 'config/install-sh'
configure.ac:68: installing 'config/missing'
configure.ac:453: error: possibly undefined macro: AC_CONFIG_COMMANDS
      If this token and others are legitimate, please use m4_pattern_allow.
      See the Autoconf documentation.
configure:17006: error: possibly undefined macro: PKG_CHECK_VAR
configure:17007: error: possibly undefined macro: AC_CONFIG_LINKS
configure:17012: error: possibly undefined macro: AC_MSG_WARN

(I don't know why it complains first about AC_CONFIG_COMMANDS since that's standard autoconf, but it seems likely the problem is stemming from PKG_CHECK_VAR being missing.)

I'm going to look into it.

comment:79 Changed 22 months ago by dimpase

you must have a past-EOL Ubuntu, I gather.

comment:80 in reply to: ↑ 74 Changed 22 months ago by embray

Replying to embray:

Replying to embray:

  • Add a generic package called just "blas" (we could keep openblas as the default though). The spkg-configure.m4 for blas would implement the --with-blas flag, and also extend it to take an optional argument of a path (e.g. instead of --with-blas=openblas or --with-blas=atlas one can pass something like --with-blas=/usr/lib. This latter case would have essentially functionality as the current SAGE_ATLAS_LIB (maybe a little better): use that path to search for a generic blas library; maybe also check that it contains some known symbols.

Perhaps the idea of a "generic package" is overcomplicated for now. Although it's something I've wanted to try to do in the past (#23465) but as a first pass detection of a "generic blas" can live in the spkg-configure.m4 for openblas (sort of as we do with gmp/mpir).

comment:81 Changed 22 months ago by embray

On the third-hand, because of the way the Sage build uses .pc files for cblas and lapack, and the fact that both the openblas and atlas SPKGs install these .pc files into SAGE_LOCAL, I understand now that even when we detect a system openblas, this ticket uses AC_CONFIG_LINKS to create those .pc files as symlinks directly into $SAGE_LOCAL.

I don't really like this, because part of my long-term goals has been to make it so that running configure doesn't create and write anything to $SAGE_LOCAL (this preference is in service to issues like #21532). I'm okay with it for now as a temporary solution, but it does seem like a step backwards.

I'm going to see if I can come up with an alternative solution to this...

Note: See TracTickets for help on using tickets.