Skip to content
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

Jump to bottom

Accept system (Open)BLAS #29398

Closed
mkoeppe opened this issue Mar 24, 2020 · 66 comments
Closed

Accept system (Open)BLAS #29398

mkoeppe opened this issue Mar 24, 2020 · 66 comments
Labels
c: build: configure p: major / 3 t: enhancement
Milestone
sage-9.1

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 24, 2020

As noted in #29397, Cygwin has openblas but does not provide openblas.pc; instead blas.pc, cblas.pc, lapack.pc are provided (but cblas.pc is broken).

From http://cygwin.1069669.n5.nabble.com/Updated-openblas-0-3-3-1-td142613.html:

  1. No devel package is provided as liblapack-devel already provide the needed headers and import. Openblas is fully compatible with Netlib BLAS.

  2. libopenblas consist of a single file /usr/bin/cygblas-0.dll that will precede in PATH the liblapack0 /usr/lib/lapack/cygblas-0.dll and used instead. Removing libopenblas will restore the usage of Netlib BLAS

Fedora has a similar omission - it does not install openblas.pc (while its openblas has cblas and lapack capabilities) - see #29393 (Use system openblas on Fedora)

CC: @dimpase @embray @isuruf @antonio-rojas @kiwifb @vbraun

Component: build: configure

Author: Matthias Koeppe

Branch/Commit: 0cda07c

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/29398
@mkoeppe mkoeppe added this to the sage-9.1 milestone Mar 24, 2020
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 24, 2020

comment:1

Should we accept any system BLAS (blas+cblas+lapack) on cygwin, or test whether it's provided by openblas, for example by testing for openblas_get_config (https://github.com/xianyi/OpenBLAS/wiki/OpenBLAS-Extensions)?

@dimpase
Copy link
Member

dimpase commented Mar 24, 2020

comment:2

if we use openblas we would need to generate missing openblas.pc
(same problem on fedora)

build/pkgs/openblas has a python script for this purpose, by the way.

we should ask cygwin maintainers to fix the install and provide pc file.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 24, 2020

comment:3

which of our packages depends specifically on openblas.pc?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 24, 2020

comment:4

Replying to @dimpase:

build/pkgs/openblas has a python script for this purpose, by the way.

write_pc_file.py seems to do the opposite direction -- it writes blas.pc, cblas.pc, lapack.pc. Cygwin already has those

@dimpase
Copy link
Member

dimpase commented Mar 24, 2020

comment:5

Replying to @mkoeppe:

which of our packages depends specifically on openblas.pc?

I meant, openblas.pc may be used by our spkg-configure for openblas. Otherwise it is not needed.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 24, 2020

comment:6

Yes, that's the problem.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 24, 2020

Branch: u/mkoeppe/cygwin__accept_system_blas

@dimpase
Copy link
Member

dimpase commented Mar 24, 2020

Commit: ba97132

@dimpase
Copy link
Member

dimpase commented Mar 24, 2020

comment:8

is this on top of a not yet merged ticket?
Please list it in Dependencies.

Also, what are these OPENBLAS_* variables?

New commits:

bb89447build/pkgs/openblas/spkg-configure.m4: Use OPENBLAS_{LIBS,CFLAGS} while checking for cblas/lapack functions
71ac6a5build/pkgs/openblas/spkg-configure.m4: Do not use AC_FC_FUNC.
7188bddMerge branch 't/29361/openblas_spkg_configure_m4__fix_the_check_for_lapack_cblas_functions' into t/29398/cygwin__accept_system_blas
ba97132build/pkgs/openblas/spkg-configure.m4: Fall back to checking whether {blas,cblas,lapack}.pc correspond to openblas - for cygwin

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 24, 2020

Dependencies: #29361

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 24, 2020

comment:10

Not ready for review yet.

OPENBLAS_* is created by the pkgconfig macro

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 24, 2020

comment:11

And "of course" it does not work at all as advertised on cygwin.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 24, 2020

comment:12

cblas.pc refers to a non-existent library "-lcblas"...

@dimpase
Copy link
Member

dimpase commented Mar 24, 2020

comment:13

we can resort to shipping platform-dependent openblas.pc

or, indeed, generate it.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 24, 2020

Changed commit from ba97132 to 089057b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 24, 2020

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

f0bd0ecbuild/pkgs/openblas/spkg-configure.m4: Try if blas.pc alone has everything
e08192bfixup
089057bUpdate comments

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 24, 2020

comment:15

This is a version that works for cygwin (at least configure finds blas now).
But it is not really checking that the implementation is provided by openblas. It seems that functions such as openblas_get_config are not accessible.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 25, 2020

comment:16

gsl's spkg-install needs updating too -- it uses pkgconfig cblas

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 25, 2020

Changed dependencies from #29361 to #29361, #29082

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 25, 2020

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

56a4f4cbuild/pkgs/glpk/distros/cygwin.txt: New
ba09bbabuild/pkgs/ncurses/distros/cygwin.txt: Fixup
391d224build/pkgs/openblas/distros/cygwin.txt: New
e9f831abuild/pkgs/ntl/distros/cygwin.txt: New
905148bbuild/pkgs/pcre/distros/cygwin.txt: New
12540acbuild/pkgs/r/distros/cygwin.txt: New
3fd30e3build/pkgs/libatomic_ops/distros/cygwin.txt: New
f29a168build/pkgs/cygwin.txt: python -> python37
0162b7eMerge branch 't/29397/add_more_cygwin_system_packages' into t/29398/cygwin__accept_system_blas
a34e7a3build/pkgs/r/distros/cygwin.txt: Add libtirpc-devel, needed for building rpy2

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 25, 2020

Changed commit from 089057b to a34e7a3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 25, 2020

Changed commit from a34e7a3 to 5df9ca3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 25, 2020

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

705da6aLink/generate pc files in make, not configure
d86cd33fixup
f5b4f62Makefile [misc-clean]: No need to remove build/lib/pkgconfig
2a38a16build/pkgs/gsl/spkg-configure.m4: Quoting fix
329843bIgnore /src/lib/pkgconfig using top-level .gitignore
5df9ca3Merge branch 't/29082/move__pc_file_from_src__to_build___clean_generated___pc_files_at__make_distclean_' into t/29398/cygwin__accept_system_blas

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 25, 2020

Changed dependencies from #29361, #29082 to #29361, #29397, #29082

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 25, 2020

comment:21

Replying to @mkoeppe:

gsl's spkg-install needs updating too -- it uses pkgconfig cblas

Or, rather, we should be generating cblas.pc

@mkoeppe

This comment has been minimized.

Sign in to view
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 1, 2020

comment:42

That wouldn't give the correct answer when users change the PATH from the default one, so that the generic blas dll wins

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 1, 2020

comment:43

Fedora seems to work too - https://github.com/mkoeppe/sage/runs/550765309

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 2, 2020

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

2873d86build/pkgs/openblas/spkg-configure.m4: Check version also when no pc file is available

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 2, 2020

Changed commit from 950ba67 to 2873d86

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 2, 2020

Author: Matthias Koeppe

@dimpase
Copy link
Member

dimpase commented Apr 2, 2020

Reviewer: Dima Pasechnik

@dimpase
Copy link
Member

dimpase commented Apr 2, 2020

comment:47

lgtm

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 2, 2020

comment:48

Thanks!

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 2, 2020

comment:50

I guess I should remove my debugging code that disables pkgconfig detection from this!

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 2, 2020

comment:51

With the pkgconfig-based detection disabled, on debian-buster-standard (https://github.com/mkoeppe/sage/runs/553984061) one sees the following:

       self._backend.solve()
      File "sage/numerical/backends/cvxopt_sdp_backend.pyx", line 369, in sage.numerical.backends.cvxopt_sdp_backend.CVXOPTSDPBackend.solve (build/cythonized/sage/numerical/backends/cvxopt_sdp_backend.c:4838)
        from cvxopt import matrix as c_matrix, solvers
      File "/sage/local/lib/python3.7/site-packages/cvxopt/__init__.py", line 279, in <module>
        from cvxopt import solvers, blas, lapack
    ImportError: /sage/local/lib/python3.7/site-packages/cvxopt/blas.cpython-37m-x86_64-linux-gnu.so: undefined symbol: dtrsm_

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 2, 2020

Changed commit from 2873d86 to 0cda07c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 2, 2020

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

0cda07cbuild/pkgs/openblas/spkg-configure.m4: Make pkgconfig-based detection work again

@dimpase
Copy link
Member

dimpase commented Apr 2, 2020

comment:54

Replying to @sagetrac-git:

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

0cda07cbuild/pkgs/openblas/spkg-configure.m4: Make pkgconfig-based detection work again

oops, I missed this.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 2, 2020

comment:55

Fixed debian-buster-standard: https://github.com/mkoeppe/sage/runs/554994789

@dimpase
Copy link
Member

dimpase commented Apr 2, 2020

comment:56

OK, looks good.
By the way (offtopic), why do we see this

2020-04-02T16:07:07.4502918Z **********************************************************************
2020-04-02T16:07:07.4503012Z File "src/sage/tests/cmdline.py", line 600, in sage.tests.cmdline.test_executable
2020-04-02T16:07:07.4503087Z Failed example:
2020-04-02T16:07:07.4503167Z     err
2020-04-02T16:07:07.4503245Z Expected:
2020-04-02T16:07:07.4503440Z     ''
2020-04-02T16:07:07.4503502Z Got:
2020-04-02T16:07:07.4503715Z     '/sage/src/bin/sage: line 564: exec: sqlite3: not found\n'

is it by design?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 2, 2020

comment:57
$ cat build/pkgs/sqlite/distros/debian.txt 
libsqlite3-dev

We can add the package with the executable sqlite3

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 9, 2020

comment:58

Replying to @mkoeppe:

We can add the package with the executable sqlite3

This is now #29487

@vbraun
Copy link
Member

vbraun commented Apr 9, 2020

Changed branch from u/mkoeppe/cygwin__accept_system_blas to 0cda07c

@vbraun vbraun closed this as completed in 94b4e90 Apr 9, 2020
@slel slel mentioned this issue Sep 13, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c: build: configure p: major / 3 t: enhancement
Projects
None yet
Milestone
sage-9.1
Development

No branches or pull requests
3 participants
@vbraun @dimpase @mkoeppe