10000 asv fails to run on windows when pkg-config and OpenBLAS are set up properly · Issue #24251 · numpy/numpy · GitHub
[go: up one dir, main page]

Skip to content

asv fails to run on windows when pkg-config and OpenBLAS are set up properly #24251

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

Open
mattip opened this issue Jul 24, 2023 · 4 comments
Open

Comments

@mattip
Copy link
Member
mattip commented Jul 24, 2023

running either

python runtests.py --bench-compare <commit1> <commit2>

or

spin bench --compare <commit1> <commit2>

will end up calling

cd benchmark; asv continuous --factor 1.05 --bench main <commit1> <commit2>

Then asv creates a virtualenv for each commit, and uses a vanilla pip install to build NumPy. While this can work on macOS and linux (and use the system OpenBLAS), it will fail to correctly use OpenBLAS on windows, since the build also needs to

  • copy the OpenBLAS DLL into the correct place
  • build the _distributor_init.py file. In CI this is done via additional post-install code:

Is there a way to, if OpenBLAS is found, to do this as part of the build?

Here is what the various CI invocations to do these steps look like:

- name: post-install
run: |
$numpy_path = "${env:installed_path}\numpy"
$libs_path = "${numpy_path}\.libs"
mkdir ${libs_path}
$ob_path = "C:/opt/64/bin/"
cp $ob_path/*.dll $libs_path
# Write _distributor_init.py to load .libs DLLs.
python -c "from tools import openblas_support; openblas_support.make_init(r'${numpy_path}')"

- name: post-install
run: |
$numpy_path = "${env:installed_path}\numpy"
$libs_path = "${numpy_path}\.libs"
mkdir ${libs_path}
$ob_path = "C:/opt/64/bin/"
cp $ob_path/*.dll $libs_path
# Write _distributor_init.py to load .libs DLLs.
python -c "from tools import openblas_support; openblas_support.make_init(r'${numpy_path}')"

(on azure this also shows the complicated command line needed)

- powershell: |
python -c "from tools import openblas_support; openblas_support.make_init('numpy')"
If ( Test-Path env:NPY_USE_BLAS_ILP64 ) {
python -m pip install . -Csetup-args="--vsenv" -Csetup-args="-Duse-ilp64=true" -Csetup-args="-Dblas-symbol-suffix=64_"
} else {
python -m pip install . -Csetup-args="--vsenv"
}
# copy from c:/opt/openblas/openblas_dll to numpy/.libs to ensure it can
# get loaded when numpy is imported (no RPATH on Windows)
$target = $(python -c "import sysconfig; print(sysconfig.get_path('platlib'))")
mkdir $target/numpy/.libs

PYTHONPATH=tools python -c "import openblas_support; openblas_support.make_init('numpy')"
mkdir -p /c/opt/32/lib/pkgconfig
mkdir -p /c/opt/64/lib/pkgconfig
target=$(python -c "import tools.openblas_support as obs; plat=obs.get_plat(); ilp64=obs.get_ilp64(); target=f'openblas_{plat}.zip'; obs.download_openblas(target, plat, ilp64);print(target)")
if [[ $PLATFORM == 'win-32' ]]; then
# 32-bit openBLAS
# Download 32 bit openBLAS and put it into c/opt/32/lib
unzip -o -d /c/opt/ $target
cp /c/opt/32/bin/*.dll /c/opt/openblas/openblas_dll
else
# 64-bit openBLAS
unzip -o -d /c/opt/ $target
if [[ -f /c/opt/64/lib/pkgconfig/openblas64.pc ]]; then
# As of v0.3.23, the 64-bit interface has a openblas64.pc file,
# but this is wrong. It should be openblas.pc
cp /c/opt/64/lib/pkgconfig/openblas{64,}.pc
fi
cp /c/opt/64/bin/*.dll /c/opt/openblas/openblas_dll

@rgommers
Copy link
Member

I think this is difficult to make work in any reasonable way. Basically, the poor design of Windows and the poor design of Python packaging tooling intersect here - pip install numpy --no-binary on Windows is also fundamentally broken, for the same reason.

One hack could be to let asv run delvewheel only on Windows.

@mattip
Copy link
Member Author
mattip commented Jul 26, 2023

I assume we could get meson to build the _distributor_init.py file built as part of the build scripts, conditioned on propogating some OpenBLAS config value. Could we get the DLL copied as part of the install step if we conditionally declare it a data file?

@mattip
Copy link
Member Author
mattip commented Jul 26, 2023

Or maybe this all should wait for OpenBLAS to become a wheel, which would basically solve the whole problem.

@rgommers
Copy link
Member

Or maybe this all should wait for OpenBLAS to become a wheel, which would basically solve the whole problem.

Yes, good point. +1 for waiting. It's not like this is a new problem, I don't think this ever worked, the numpy.distutils build has the same problems.

@HaoZeke HaoZeke self-assigned this Sep 15, 2023
@HaoZeke HaoZeke removed their assignment Sep 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants
0