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

Add Armadillo build script #388

Merged
merged 5 commits into from
Jan 17, 2020
Merged

Conversation

rcurtin
Copy link
Contributor

@rcurtin rcurtin commented Jan 15, 2020

This is a dependency of mlpack (#368). @giordano suggested that I pull it out as a separate package. I decided to package libarmadillo.so, but it's still possible to use Armadillo as a header-only library. libarmadillo.so is basically just a wrapper around OpenBLAS.

Armadillo's quite easy to package, so I think that this should be good to go. For downstream packages that link against Armadillo, there may be problems until JuliaPackaging/BinaryBuilder.jl#604 is fixed (I think that caused issues with mlpack, which depends on Armadillo). In any case, this appeared to build fine locally for me.

@giordano
Copy link
Member

Could you please allow maintainers to push to your branch? I'd like to simplify the build script

@rcurtin
Copy link
Contributor Author

rcurtin commented Jan 15, 2020

Color me confused. The "allow edits from maintainers" option doesn't seem to appear for me in the right sidebar. (https://stackoverflow.com/questions/20928727/adding-commits-to-another-persons-pull-request-on-github) Do you have that option available?

I'm also happy to merge any patches that you suggest... not sure what your preference is for a way forward.

But, I will learn more from the output.
@giordano
Copy link
Member

giordano commented Jan 15, 2020

Color me confused. The "allow edits from maintainers" option doesn't seem to appear for me in the right sidebar. (stackoverflow.com/questions/20928727/adding-commits-to-another-persons-pull-request-on-github) Do you have that option available?

Yes, usually I do 😕

I'm also happy to merge any patches that you suggest... not sure what your preference is for a way forward.

Here is the script:

script = raw"""
cd ${WORKSPACE}/srcdir/armadillo-*/
mkdir build && cd build

FLAGS=(-DCMAKE_TOOLCHAIN_FILE=${CMAKE_TARGET_TOOLCHAIN}
       -DCMAKE_INSTALL_PREFIX=${prefix}
       -DBUILD_SHARED_LIBS=ON)

# Slightly different handling is needed on different platforms.
if [[ "${nbits}" == 64 ]]; then
    FLAGS+=(-Dopenblas_LIBRARY="${libdir}/libopenblas64_.${dlext}")
fi

cmake .. "${FLAGS[@]}"
make -j${nproc}
make install
"""

Note that this will still fail I think because CMake is currently misconfigured in BinaryBuilder. Even after that, we'll need a simple trick to adapt to the different API of Julia's openblas, I'll do that when I'll come back home.

@rcurtin
Copy link
Contributor Author

rcurtin commented Jan 15, 2020

Thanks! Pushed in a94257e. I found in 2780fce that some special handling is required for Windows. Let me know if I misunderstood or overlooked something. 👍

@rcurtin
Copy link
Contributor Author

rcurtin commented Jan 15, 2020

Also, when you say that CMake is misconfigured in BinaryBuilder, do you mean that the Windows build issue is not something that I could fix here, but instead is an upstream issue?

@giordano
Copy link
Member

giordano commented Jan 16, 2020

that some special handling is required for Windows

Not only for Windows 😉

New script, this time should work for all platforms, always correctly linking to OpenBLAS:

script = raw"""
cd ${WORKSPACE}/srcdir/armadillo-*/
mkdir build && cd build

FLAGS=(-DCMAKE_TOOLCHAIN_FILE=${CMAKE_TARGET_TOOLCHAIN}
       -DCMAKE_INSTALL_PREFIX=${prefix}
       -DCMAKE_BUILD_TYPE=Release
       -DBUILD_SHARED_LIBS=ON)

if [[ "${nbits}" == 64 ]] && [[ "${target}" != aarch64* ]]; then
    FLAGS+=(-Dopenblas_LIBRARY="${libdir}/libopenblas64_.${dlext}")

    SYMB_DEFS=()
    for sym in sasum dasum snrm2 dnrm2 sdot ddot sgemv dgemv cgemv  zgemv sgemm dgemm cgemm zgemm ssyrk dsyrk cherk zherk; do
        SYMB_DEFS+=("-D${sym}=${sym}_64")
    done
    if [[ "${target}" == *-apple-* ]]; then
        FLAGS+=(-DALLOW_OPENBLAS_MACOS=ON)
        for sym in cgbcon cgbsv cgbsvx cgbtrf cgbtrs cgecon cgees cgeev cgeevx cgehrd cgels cgelsd cgemm cgemv cgeqrf cgesdd cgesv cgesvd cgesvx cgetrf cgetri cgetrs cgges cggev cgtsv cgtsvx cheev cheevd cherk clangb clange clanhe clansy cpbtrf cpocon cposv cposvx cpotrf cpotri cpotrs ctrcon ctrsyl ctrtri ctrtrs cungqr dasum ddot dgbcon dgbsv dgbsvx dgbtrf dgbtrs dgecon dgees dgeev dgeevx dgehrd dgels dgelsd dgemm dgemv dgeqrf dgesdd dgesv dgesvd dgesvx dgetrf dgetri dgetrs dgges dggev dgtsv dgtsvx dlahqr dlangb dlange dlansy dlarnv dnrm2 dorgqr dpbtrf dpocon dposv dposvx dpotrf dpotri dpotrs dstedc dsyev dsyevd dsyrk dtrcon dtrevc dtrsyl dtrtri dtrtrs ilaenv sasum sdot sgbcon sgbsv sgbsvx sgbtrf sgbtrs sgecon sgees sgeev sgeevx sgehrd sgels sgelsd sgemm sgemv sgeqrf sgesdd sgesv sgesvd sgesvx sgetrf sgetri sgetrs sgges sggev sgtsv sgtsvx slahqr slangb slange slansy slarnv snrm2 sorgqr spbtrf spocon sposv sposvx spotrf spotri spotrs sstedc ssyev ssyevd ssyrk strcon strevc strsyl strtri strtrs zgbcon zgbsv zgbsvx zgbtrf zgbtrs zgecon zgees zgeev zgeevx zgehrd zgels zgelsd zgemm zgemv zgeqrf zgesdd zgesv zgesvd zgesvx zgetrf zgetri zgetrs zgges zggev zgtsv zgtsvx zheev zheevd zherk zlangb zlange zlanhe zlansy zpbtrf zpocon zposv zposvx zpotrf zpotri zpotrs ztrcon ztrsyl ztrtri ztrtrs zungqr; do
            SYMB_DEFS+=("-D${sym}=${sym}_64")
        done
    fi
    export CXXFLAGS="${SYMB_DEFS[@]}"
fi

cmake .. "${FLAGS[@]}"
make -j${nproc}
make install

# Armadillo links against a _very_ specific version of OpenBLAS on macOS by default:
if [[ ${target} == *apple* ]]; then
    # Figure out what version it probably latched on to:
    OPENBLAS_LINK=$(otool -L ${libdir}/libarmadillo.dylib | grep libopenblas64_ | awk '{ print $1 }')
    install_name_tool -change ${OPENBLAS_LINK} @rpath/libopenblas64_.dylib ${libdir}/libarmadillo.dylib
fi
"""

@giordano giordano mentioned this pull request Jan 16, 2020
@rcurtin
Copy link
Contributor Author

rcurtin commented Jan 16, 2020

@giordano thanks! It all seems to be working now. Once this is merged I'll update #368 and steal some of the tricks here. :)

@giordano giordano merged commit c06f88d into JuliaPackaging:master Jan 17, 2020
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.

2 participants