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.

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

Bump OpenBLAS to 0.3.3 #87

Merged
merged 3 commits into from Oct 10, 2018

Conversation

jakirkham
Copy link
Member

@jakirkham jakirkham commented Jul 3, 2018

Checklist

  • Used a fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy
  • Ensured the license file is being packaged.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@isuruf
Copy link
Member

isuruf commented Jul 3, 2018

I wonder whether we should split openblas to libblas and liblapack like Debian does. libblas has a stable API while liblapack is not. Most packages in conda-forge depends on libblas.

cc @msarahan

@jakirkham
Copy link
Member Author

Well technically the BLAS and LAPACK APIs are quite old and stable. NETLIB of course being the model. Ultimately we end up pinning exactly as OpenBLAS includes the version in the SONAME down to the patch. Doubt they actually break their API that often though. Should add whatever API they are breaking is not something we are likely using anywhere. We could discuss with them to adjust their SONAME to be a bit more lax.

@isuruf
Copy link
Member

isuruf commented Jul 5, 2018

Ultimately we end up pinning exactly as OpenBLAS includes the version in the SONAME down to the patch.

No they don't. SONAME on Linux is libopenblas.so.0, while on OSX we manually change it to libopenblas.so.0

@jakirkham
Copy link
Member Author

That file is a symlink to the actual library (e.g. libopenblasp-r0.3.1.so). As a result anything that links to ends up pointing the actual library not the symlink.

@isuruf
Copy link
Member

isuruf commented Jul 5, 2018

That file is a symlink to the actual library (e.g. libopenblasp-r0.3.1.so). As a result anything that links to ends up pointing the actual library not the symlink.

That's not how SONAME works. run ldd on numpy/core/multiarray.so and check.

$ ldd ~/miniconda3/lib/python3.6/site-packages/numpy/core/multiarray.cpython-36m-x86_64-linux-gnu.so 
        linux-vdso.so.1 =>  (0x00007ffd61faf000)
        libopenblas.so.0 => /home/isuru/miniconda3/lib/python3.6/site-packages/numpy/core/../../../../libopenblas.so.0 (0x00007ff8ea085000)
        libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007ff8e9d7c000)
        libpython3.6m.so.1.0 => /home/isuru/miniconda3/lib/python3.6/site-packages/numpy/core/../../../../libpython3.6m.so.1.0 (0x00007ff8e9841000)
        libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007ff8e9624000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007ff8e925a000)
        /lib64/ld-linux-x86-64.so.2 (0x00007ff8ecb1b000)
        libgfortran.so.3 => /usr/lib/x86_64-linux-gnu/libgfortran.so.3 (0x00007ff8e8f29000)
        libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007ff8e8d25000)
        libutil.so.1 => /lib/x86_64-linux-gnu/libutil.so.1 (0x00007ff8e8b22000)
        librt.so.1 => /lib/x86_64-linux-gnu/librt.so.1 (0x00007ff8e891a000)
        libquadmath.so.0 => /home/isuru/miniconda3/lib/python3.6/site-packages/numpy/core/../../../.././libquadmath.so.0 (0x00007ff8e86e9000)
        libgcc_s.so.1 => /home/isuru/miniconda3/lib/python3.6/site-packages/numpy/core/../../../.././libgcc_s.so.1 (0x00007ff8e84d7000)

@jakirkham
Copy link
Member Author

jakirkham commented Jul 6, 2018

Good point. This also checks out on macOS as well surprisingly.

FWIW we had decided on this pinning due to some discussion in issue ( conda-forge/numpy-feedstock#1 ) and set it more generally in PR ( conda-forge/conda-forge.github.io#198 ).

Just so I understand, is your point that we should consider relaxing the openblas pinning? I'm not totally opposed to that if it can be proven to work (maybe with something simple like numpy).

However we will need a rebuild for this release as things were broken in the move to 0.3.0. Also things have broken historically in patch releases. Though admittedly not so often.

@jakirkham
Copy link
Member Author

@conda-forge/core, thoughts?

@isuruf
Copy link
Member

isuruf commented Jul 10, 2018

Just so I understand, is your point that we should consider relaxing the openblas pinning? I'm not totally opposed to that if it can be proven to work (maybe with something simple like numpy).

Yes. (I'm not sure about lapack symbols though.)

However we will need a rebuild for this release as things were broken in the move to 0.3.0. Also things have broken historically in patch releases. Though admittedly not so often.

No, 0.3.0 is ABI compatible with 0.2.20. (Just not API compatible. double * was changed to void * in the signature, but this is not an issue ABI wise.)

@jakirkham
Copy link
Member Author

What concerns do you have with the LAPACK symbols? Are there specific issues that you are aware of?

IDK haven't read all the warnings in that report. It's quite long. If that's all that is in there, maybe it's less of an issue (especially as they went to and not from void *).

In any event, even to relax the pinning we will need to rebuild many packages to accept the relaxed pinning. Personally would rather just start with relaxing on 0.3.0 given some older versions of 0.2.* were problematic either because of how we built them (e.g. without pthreads) or due to actual issues in much older versions of openblas.

@isuruf
Copy link
Member

isuruf commented Jul 10, 2018

What concerns do you have with the LAPACK symbols? Are there specific issues that you are aware of?

I know that symbols are added in new versions and some symbols that were deprecated a decade ago were removed. Is there a way to request a lapack report in abi-laboratory?

In any event, even to relax the pinning we will need to rebuild many packages to accept the relaxed pinning. Personally would rather just start with relaxing on 0.3.0 given some older versions of 0.2.* were problematic either because of how we built them (e.g. without pthreads) or due to actual issues in much older versions of openblas.

With LAPACK adding new symbols, we definitely want to have cb3 pinning where run pin >= version built with.

@jakirkham
Copy link
Member Author

Sure, raise an issue on this tracker with what you have in mind.

My understanding is OpenBLAS basically vendors NETLIB LAPACK. Not sure that they are doing anything too special with it, but we should give it a closer look to be sure or perhaps just ask upstream about their development model with regards to LAPACK.

@jakirkham
Copy link
Member Author

IDK if we are unsure about LAPACK, my vote is to stick with this pinning even if it is tight. Thoughts from others, @conda-forge/core?

@msarahan
Copy link
Member

How many things are built against openblas? I ask because this is what should guide our decision. If loads of stuff is built against it, it is more compelling to do what @isuruf suggests, so that rebuilds are not required as often. On the other hand, if there aren't many, the cost of potential incompatibility issues may outweigh a smaller rebuild cost.

I have no strong opinion either way. I think the bot makes or will soon make rebuilds easy enough that we shouldn't be too concerned about overly narrow openblas constraints.

@jjhelmus
Copy link
Contributor

How many things are built against openblas?

My naive search finds 69 packages in conda-forge which depend on openblas. Given that OpenBLAS releases are infrequent, ~1/year, rebuilding all of these for each new release does not seem too much of a burden.

$  conda --version
conda 4.3.34
$ conda search --reverse-dependency openblas -c conda-forge | grep conda-forge | awk '{print $1}' | grep ^[a-z] | wc -l
69
$ conda search --reverse-dependency openblas -c conda-forge | grep conda-forge | awk '{print $1}' | grep ^[a-z]
adept
alps
armadillo
arpack
blas
casacore
casadi
celerite
coincbc
cyclus
cyclus-build-deps
dlib
fenics
fflas-ffpack
giac
gmt
gsl
harminv
hmat-oss
hypre
iml
ipopt
jags
julia
kaldi
r7271.1a4dbf6
r7271.1a4dbf6
kwant
lal
linbox
moab
mpb
multinest
mumps
mumps-mpi
ncl
nco
numcosmo
octave
opencv
openmeeg
openturns
petsc
pycvodes
pygslodeiv2
pymc
pymeep
pyne
pypolyagamma
python-lal
python-spams
pythran
r-copula
root5
sage
sagelib
scalapack
scikit-umfpack
scs
shogun
shogun-cpp
siesta
simbody
slepc
slycot
sundials
superlu
trilinos
xtensor-blas

@isuruf
Copy link
Member

isuruf commented Jul 20, 2018

I don't see obvious packages like numpy, scipy in that list.

@mbargull
Copy link
Member

The number seems mostly right, though. I get 70 on linux-64 and 67 on osx-64. Probably just something during the text parsing that went wrong.

$ conda search --info --platform linux-64 conda-forge::\* --json | jq -r '.[]|[.[]|select(.depends[]|match("^openblas"))|.name]|unique[]'                                                             
adept
armadillo
arpack
blas
caffe
casacore
casadi
celerite
coincbc
cvxopt
cyclus
cyclus-build-deps
dlib
dsdp
fenics
fflas-ffpack
giac
gmt
gsl
harminv
hmat-oss
hypre
iml
ipopt
jags
julia
kaldi
kwant
lal
linbox
moab
mpb
multinest
mumps
mumps-mpi
nco
numcosmo
numpy
octave
opencv
openmeeg
openturns
petsc
pycvodes
pygslodeiv2
pymc
pymeep
pyne
pypolyagamma
python-spams
r-copula
root5
sage
sagelib
scalapack
scikit-learn
scikit-umfpack
scipy
scs
shogun
shogun-cpp
siesta
simbody
slepc
slycot
suitesparse
sundials
superlu
trilinos
xtensor-blas

@jjhelmus
Copy link
Contributor

The grep conda-forge was messing things up. I'm getting 74 packages with a Python script:

import json
pkgs = json.load(open('./out.json'))

has_openblas = set()
for pkg_list in pkgs.values():
    for i in pkg_list:
        if i['channel'] == 'conda-forge':
            has_openblas.add(i['name'])

for i in sorted(has_openblas):
    print(i)
$ conda search -c conda-forge --reverse-dependency openblas --json > out.json
$ python list.py 
adept
alps
armadillo
arpack
blas
caffe
casacore
casadi
celerite
coincbc
cvxopt
cyclus
cyclus-build-deps
dlib
dsdp
fenics
fflas-ffpack
giac
gmt
gsl
harminv
hmat-oss
hypre
iml
ipopt
jags
julia
kaldi
kwant
lal
linbox
moab
mpb
multinest
mumps
mumps-mpi
ncl
nco
numcosmo
numpy
octave
opencv
openmeeg
openturns
petsc
pycvodes
pygslodeiv2
pymc
pymeep
pyne
pypolyagamma
python-lal
python-spams
pythran
r-copula
root5
sage
sagelib
scalapack
scikit-learn
scikit-umfpack
scipy
scs
shogun
shogun-cpp
siesta
simbody
slepc
slycot
suitesparse
sundials
superlu
trilinos
xtensor-blas

@isuruf
Copy link
Member

isuruf commented Aug 1, 2018

Given that OpenBLAS releases are infrequent, ~1/year, rebuilding all of these for each new release does not seem too much of a burden.

It's getting frequent. 3 releases in the last 3 months.

@jakirkham
Copy link
Member Author

That seems to be more of a product of maintenance work changing hands. Would expect the frequency goes down a bit once the process is a bit more routinized. Though I could be wrong.

@jakirkham jakirkham changed the title Bump OpenBLAS to 0.3.1 Bump OpenBLAS to 0.3.2 Aug 6, 2018
@CJ-Wright
Copy link
Member

Do you want the bot to manage the rebuild for this?

@jakirkham
Copy link
Member Author

That would be very useful. Is that something you would be interested in working on?

@CJ-Wright
Copy link
Member

CJ-Wright commented Aug 9, 2018

It shouldn't be too hard, this is what @justcalamari put together for py3.7. regro/cf-scripts#305 I imagine we can do something similar for openBLAS.

@ocefpaf
Copy link
Member

ocefpaf commented Aug 16, 2018

This one looks good to go. @isuruf should we pull the trigger or do you have any more comments?

@isuruf
Copy link
Member

isuruf commented Aug 17, 2018

I think we should figure out a better pinning strategy. 0.3.3 is set to release in a few days. Basically one release per month. Before we upgrade all our dependencies a new release will be out.

@CJ-Wright
Copy link
Member

Maybe, I guess that depends on how fast we can complete the rebuild.

@isuruf
Copy link
Member

isuruf commented Aug 17, 2018

But what's the point though? If patch releases are compatible with each other, we should pin with x.x instead of x.x.x

@jakirkham
Copy link
Member Author

Are they? I thought there was concern about LAPACK or is that no longer a concern?

@djsutherland
Copy link
Contributor

I'm not super familiar with the details but it seems like x.x should be okay....

@jakirkham
Copy link
Member Author

jakirkham commented Aug 17, 2018

The LAPACK point came up earlier in the thread. FWIW there is now LAPACK API/ABI compatibility info. While somewhat related, the real question is how often does OpenBLAS update their copy of LAPACK and how do their patches affect compatibility.

@djsutherland
Copy link
Contributor

Looking into it, OpenBLAS 0.3.2 updated their lapack to 3.8.0 from 3.7.0, which should have pulled in the 3.7.1 breaking changes in a few symbols.

OpenBLAS ships their LAPACK in libopenblas.so, but maybe the timeline report isn't including lapack?

@jakirkham jakirkham changed the title Bump OpenBLAS to 0.3.2 Bump OpenBLAS to 0.3.3 Sep 18, 2018
@jakirkham
Copy link
Member Author

Bumped to OpenBLAS 0.3.3, which landed a couple weeks back.

cc @CJ-Wright

@tadeu
Copy link
Member

tadeu commented Oct 9, 2018

Just out of curiosity, what else is needed for this merge?

@ocefpaf ocefpaf merged commit 4f7a93c into conda-forge:master Oct 10, 2018
@jakirkham jakirkham deleted the bump_openblas_0.3.1 branch October 11, 2018 15:30
@jakirkham
Copy link
Member Author

Thanks all. Glad to see this one landed 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet