#30706 closed enhancement (fixed)

Make sage.env.cython_aliases more flexible regarding what pkgconfig files for BLAS are required

Reported by: gh-tobiasdiez Owned by:
Priority: major Milestone: sage-9.3
Component: build Keywords: cblas, blas
Cc: mkoeppe, dimpase, fbissey, arojas, isuruf Merged in:
Authors: Tobias Diez, Matthias Koeppe Reviewers: Matthias Koeppe, Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: 9c8641a (Commits, GitHub, GitLab) Commit: 9c8641a7f6d277c969d41506d55b7cc4cc04ef8d
Dependencies: Stopgaps:

Status badges

Description (last modified by gh-tobiasdiez)

While working on #30371, I noticed that cblas is not available for Ubuntu 18.04. In fact, libblas-dev contains a README.if-you-look-for-libcblas with the following content:

    Debian science team maintainers have merged the CBLAS ABI into
    the libblas.so shared object. Everything you need from libcblas.so
    can be found in libblas.so . Please link your program against it
    instead. See also

      https://lists.debian.org/debian-devel/2019/10/msg00273.html
      https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=943712
      https://wiki.debian.org/DebianScience/LinearAlgebraLibraries

So this ticket adds support for other BLAS libraries (openplas, blas) that the user may have installed.

Change History (35)

comment:1 Changed 21 months ago by gh-tobiasdiez

  • Status changed from new to needs_info

comment:2 Changed 21 months ago by mkoeppe

$ cat build/pkgs/openblas/distros/debian.txt 
libopenblas-dev

comment:3 Changed 21 months ago by gh-tobiasdiez

Also with openblas the issue remains, as it adds a blas.pc file in usr/lib/.../pkgconfig and thus is not found via -l cblas. Or do I miss something?

Last edited 21 months ago by gh-tobiasdiez (previous) (diff)

comment:4 Changed 21 months ago by mkoeppe

Are you saying that the .pc files installed on your Ubuntu system are broken?

comment:5 Changed 21 months ago by gh-tobiasdiez

I wouldn't say broken, but there is no cblas.pc file (but a blas.pc) which is why the check in env.py was not successful without the committed change.

comment:6 Changed 21 months ago by mkoeppe

Supporting BLAS on various platforms is really tricky. See build/pkgs/openblas/spkg-configure.m4. To interface with system BLAS, in some configurations we create .pc files in $SAGE_LOCAL. If you don't run ./configure and make, you won't have those.

comment:7 follow-up: Changed 21 months ago by gh-tobiasdiez

Well, the editable install in #30371 uses only system-wide installed libraries, so things in SAGE_LOCAL don't matter (or do you suggest to set SAGE_LOCAL to say usr before calling configure?).

Would it work to take in env.py the first of the libraries openblas cblas blas that is found using pkgconfig.parse?

comment:8 in reply to: ↑ 7 Changed 21 months ago by mkoeppe

Replying to gh-tobiasdiez:

Well, the editable install in #30371 uses only system-wide installed libraries, so things in SAGE_LOCAL don't matter

As I said in #30371, I think it's best to keep the focus narrow -- make #30371 work completely (i.e., pass doctests) when run from within ./sage -sh so that $SAGE_LOCAL is set to a working installation of the Sage distribution.

Of course I do understand that currently technical problems prevent you from completing the build.

Still, make -k can build everything until you get the error with the pip package or whatever is failing. Then for example the .pc files will be available.

Would it work to take in env.py the first of the libraries openblas cblas blas that is found using pkgconfig.parse?

Something like this might work.

Making sage.env.cython_aliases more flexible is a good approach, I would think.

Note that this will need thorough testing across the supported platforms.

comment:9 Changed 21 months ago by mkoeppe

  • Summary changed from Migrate from cblas to blas to Make sage.env.cython_aliases more flexible regarding what pkgconfig files for BLAS are required

comment:10 Changed 21 months ago by git

  • Commit changed from 23d17ad3f89536404bce4cb25c20ff0fde2b193b to ea4a44cefcbc6a82efab8a8b7809f40e28bf0773

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

3cfe1c9Merge branch 'develop' of git://github.com/sagemath/sage into public/build/cblas
ea4a44cSupport different blas libraries

comment:11 Changed 21 months ago by gh-tobiasdiez

  • Description modified (diff)
  • Status changed from needs_info to needs_review

I've now added support for openblas and blas as well. So, this ticket is now ready for review.

comment:12 Changed 21 months ago by mkoeppe

  • Authors set to Tobias Diez
  • Milestone changed from sage-9.2 to sage-9.3

comment:13 Changed 21 months ago by mkoeppe

  • Cc dimpase fbissey added

This is going in a good direction but it might conflict with the Sage distribution's support of selecting ATLAS instead of OpenBLAS in configure. We can't just go and prefer the openblas.pc without adding a configuration variable that preserves this option.

comment:14 Changed 21 months ago by fbissey

I agree. Preference order should be overridable. You may very well want to build the whole sage with a different blas/lapack stack. What goes in env.py should be able to match whatever you build all the other sage dependencies with, regardless of the presence of openblas.pc.

If anything has to go first without override, I would prefer cblas or blas. This is a generic name and you could override things by providing a .pc in an appropriate PKG_CONFIG_PATH.

comment:15 Changed 21 months ago by git

  • Commit changed from ea4a44cefcbc6a82efab8a8b7809f40e28bf0773 to 75966fc0c32ffc918ff7a0cad60b7381f9b3fef5

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

75966fcPut cblas first

comment:16 Changed 21 months ago by gh-tobiasdiez

Thanks for the feedback. I wasn't sure about the order myself. I've now changed it to have cblas first. So if this package is installed (or a corresponding pc file is present), then you get the current behavior. Only in the case it's not installed we fall back to openblas.

I would leave the config option for another ticket.

comment:17 Changed 21 months ago by fbissey

I'll note that my patch to giac's autotools is going to be in sage 9.2 and it uses lapack.pc and blas.pc. I am bringing up giac because I am currently trying the remove libdir from RUNPATH in libgiac because it messes up which blas/lapack is loaded in Gentoo.

comment:18 Changed 21 months ago by git

  • Commit changed from 75966fc0c32ffc918ff7a0cad60b7381f9b3fef5 to 273bb93bd81c1165cd7206762c91073f47186bca

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

273bb93Make cblas pc module search configurable via sage_conf

comment:19 Changed 21 months ago by mkoeppe

  • Authors changed from Tobias Diez to Tobias Diez, Matthias Koeppe

comment:20 follow-up: Changed 20 months ago by gh-tobiasdiez

Thanks for the update Matthias. The code works fine on Ubunutu with openblas installed, however fails on MacOS with openblas installed via homebrew:

Traceback (most recent call last):
      File "/Users/runner/work/sage/sage/src/.venv/lib/python3.8/site-packages/pip/_vendor/pep517/_in_process.py", line 280, in <module>
        main()
      File "/Users/runner/work/sage/sage/src/.venv/lib/python3.8/site-packages/pip/_vendor/pep517/_in_process.py", line 263, in main
        json_out['return_val'] = hook(**hook_input['kwargs'])
      File "/Users/runner/work/sage/sage/src/.venv/lib/python3.8/site-packages/pip/_vendor/pep517/_in_process.py", line 133, in prepare_metadata_for_build_wheel
        return hook(metadata_directory, config_settings)
      File "/Users/runner/work/sage/sage/src/.venv/lib/python3.8/site-packages/setuptools/build_meta.py", line 157, in prepare_metadata_for_build_wheel
        self.run_setup()
      File "/Users/runner/work/sage/sage/src/.venv/lib/python3.8/site-packages/setuptools/build_meta.py", line 248, in run_setup
        super(_BuildMetaLegacyBackend,
      File "/Users/runner/work/sage/sage/src/.venv/lib/python3.8/site-packages/setuptools/build_meta.py", line 142, in run_setup
        exec(compile(code, __file__, 'exec'), locals())
      File "setup.py", line 90, in <module>
        aliases = cython_aliases()
      File "/Users/runner/work/sage/sage/src/sage/env.py", line 399, in cython_aliases
        lib = next((blas_lib for blas_lib in cblas_pc_modules if pkgconfig.exists(blas_lib)))
    StopIteration

see https://github.com/tobiasdiez/sage/runs/1285891725?check_suite_focus=true Its interesting to note that also ./configure cannot find openblas: checking for openblas >= 0.2.20... no. So I guess it's a more general issue not really related to this ticket.

comment:21 in reply to: ↑ 20 Changed 20 months ago by mkoeppe

Replying to gh-tobiasdiez:

Thanks for the update Matthias. The code works fine on Ubunutu with openblas installed, however fails on MacOS with openblas installed via homebrew:

Did you use the homebrew setup as explained in README.md?

comment:22 Changed 20 months ago by gh-tobiasdiez

Not sure, the Readme in root doesn't really contain anything about homebrew. That's what I used: https://github.com/tobiasdiez/sage/actions/runs/319537404/workflow

comment:23 Changed 20 months ago by mkoeppe

My bad, it's actually not in the README but is displayed at the end of ./configure:

# To automatically take care of homebrew messages regarding 
# keg-only packages for the current shell session:
  $ source /Users/mkoeppe/s/sage/sage-rebasing/.homebrew-build-env

comment:24 Changed 20 months ago by mkoeppe

  • Cc arojas isuruf added

comment:25 follow-up: Changed 20 months ago by gh-tobiasdiez

I just discovered that there is also a reference to cblas in src/sage/misc/cython.py. Not sure if this also needs to be changed to use the new variable CBLAS_PC_MODULES.

Moreover, I would like suggest the following change to sage_conf.py.in to make this more reusable.

DEFAULT_CBLAS_PC_MODULE = "@SAGE_CBLAS_PC_MODULE" # Set to cblas in configure

def get_cblas_pc_module()
   cblas_pc_modules = [DEFAULT_CBLAS_PC_MODULE, 'cblas', 'openblas', 'blas']
   return next((blas_lib for blas_lib in cblas_pc_modules if pkgconfig.exists(blas_lib)))

comment:26 Changed 20 months ago by mkoeppe

Search logic should go into sage/env.py, not sage_conf.

comment:27 in reply to: ↑ 25 Changed 20 months ago by mkoeppe

Replying to gh-tobiasdiez:

I just discovered that there is also a reference to cblas in src/sage/misc/cython.py. Not sure if this also needs to be changed to use the new variable CBLAS_PC_MODULES.

Yes, this code should be replaced by something equivalent that uses sage.env

comment:28 Changed 20 months ago by git

  • Commit changed from 273bb93bd81c1165cd7206762c91073f47186bca to 9c8641a7f6d277c969d41506d55b7cc4cc04ef8d

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

9c8641aReplace other usages of cblas name

comment:29 Changed 20 months ago by gh-tobiasdiez

Ok, I've now refactored the code slightly and replaced the other usages of the hard-corded cblas in sage/misc/cython.py and in sage_setup/library_order.py.

comment:30 Changed 20 months ago by mkoeppe

  • Reviewers set to Matthias Koeppe, ...

Looks good to me, but another reviewer is needed...

comment:31 Changed 20 months ago by dimpase

does this work on platforms where cblas is present in an important way, such as arch linux?

comment:32 Changed 20 months ago by mkoeppe

The behavior as part of building Sage-the-distribution is unchanged by this ticket.

comment:33 Changed 20 months ago by dimpase

  • Reviewers changed from Matthias Koeppe, ... to Matthias Koeppe, Dima Pasechnik
  • Status changed from needs_review to positive_review

comment:34 Changed 20 months ago by gh-tobiasdiez

Thanks!

comment:35 Changed 19 months ago by vbraun

  • Branch changed from public/build/cblas to 9c8641a7f6d277c969d41506d55b7cc4cc04ef8d
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.