packages icon indicating copy to clipboard operation
packages copied to clipboard

Missing package: boost-histogram

Open matthewfeickert opened this issue 2 years ago • 18 comments

Package name

boost-histogram

Package version

all

PyPI URL

https://pypi.org/project/boost-histogram/

piwheels URL

https://www.piwheels.org/project/boost-histogram/

Python version

  • [X] Python 3.7
  • [X] Python 3.9

I am the maintainer

  • [ ] Yes

More information

I'm not a maintainer of boost-histogram but I am a member of the Scikit-HEP admin team (boost-histogram is a Scikit-HEP project). boost-histogram powers a lot of functionality in the Scikit-HEP ecosystem, so it would be quite nice if support could be added to compliment the nice support for awkward.

matthewfeickert avatar Jan 09 '23 08:01 matthewfeickert

We had to skip this package as it was repeatedly hitting the 3 hour timeout and blocking anything else from being built. We can try again, perhaps just the latest version, if you think it should build ok.

Have you tried building it on a Pi? Is there anything special that's needed? We can install apt packages as required.

If it requires Rust, then it'll have to wait for the work on rustup to be completed https://github.com/piwheels/piwheels/pull/328

bennuttall avatar Jan 10 '23 15:01 bennuttall

Have you tried building it on a Pi? Is there anything special that's needed? We can install apt packages as required.

I haven't yet, but I can try myself later.

If it requires Rust, then it'll have to wait for the work on rustup to be completed piwheels/piwheels#328

There's no Rust to my knowledge. Just C++ and pybind11.

matthewfeickert avatar Jan 10 '23 17:01 matthewfeickert

I was able to pretty quickly build a sdist (no surprise there) and a wheel for boost-histogram v1.3.2 on my Raspberry Pi 4 Model B running Raspbian 11 bullseye

$ uname -a
Linux raspberrypi 5.15.84-v7l+ #1613 SMP Thu Jan 5 12:01:26 GMT 2023 armv7l GNU/Linux
$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/arm-linux-gnueabihf/10/lto-wrapper
Target: arm-linux-gnueabihf
Configured with: ../src/configure -v --with-pkgversion='Raspbian 10.2.1-6+rpi1' --with-bugurl=file:///usr/share/doc/gcc-10/README.Bugs --enable-languages=c,ada,c++,go,d,fortran,objc,obj-c++,m2 --prefix=/usr --with-gcc-major-version-only --program-suffix=-10 --program-prefix=arm-linux-gnueabihf- --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-libitm --disable-libquadmath --disable-libquadmath-support --enable-plugin --with-system-zlib --enable-libphobos-checking=release --with-target-system-zlib=auto --enable-objc-gc=auto --enable-multiarch --disable-sjlj-exceptions --with-arch=armv6 --with-fpu=vfp --with-float=hard --disable-werror --enable-checking=release --build=arm-linux-gnueabihf --host=arm-linux-gnueabihf --target=arm-linux-gnueabihf
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 10.2.1 20210110 (Raspbian 10.2.1-6+rpi1) 

with the following

$ python -m venv build && . build/bin/activate
$ python -m pip install --upgrade pip setuptools wheel
$ python -m pip install build
$ git clone --recursive --branch v1.3.2 https://github.com/scikit-hep/boost-histogram.git
$ cd boost-histogram/
$ python -m build .
...
Successfully built boost_histogram-1.3.2.tar.gz and boost_histogram-1.3.2-cp39-cp39-linux_armv7l.whl

I tried to test the wheel by installing and getting it to pass pytest, but it fails at import

$ python -m pip install ./dist/boost_histogram-1.3.2-cp39-cp39-linux_armv7l.whl
$ python -m pip install pytest pytest-benchmark
$ python -c 'import boost_histogram'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/pi/.local/venvs/build/lib/python3.9/site-packages/boost_histogram/__init__.py", line 1, in <module>
    from . import accumulators, axis, numpy, storage
  File "/home/pi/.local/venvs/build/lib/python3.9/site-packages/boost_histogram/accumulators.py", line 1, in <module>
    from ._core.accumulators import (  # pylint: disable=import-error
ImportError: /home/pi/.local/venvs/build/lib/python3.9/site-packages/boost_histogram/_core.cpython-39-arm-linux-gnueabihf.so: undefined symbol: __atomic_load_8

From a quick glance at the Issues it seems that this undefined symbol: __atomic_load_8 is something that people have seen before, but I'm not experienced enough to know what this means without doing some looking (which I don't have time to do right right now).

If it is of interest, here's the wheel that I built (I've appended the .zip to the end so that GitHub will recognize it as a zip file and let me upload it): boost_histogram-1.3.2-cp39-cp39-linux_armv7l.whl.zip

(I don't think he'll have time to comment, but cc @henryiii — one of the maintainers.)

matthewfeickert avatar Jan 10 '23 18:01 matthewfeickert

3 hour timeout

It shouldn't take 3 hours to build! I can build in under a minute on my MacBook on 8 cores; a more normal computer still usually can do it in 5-10 mins. Maybe you are running out of memory and using the swap - I'd try forcing the build to run on a single core via setting CMAKE_BUILD_PARALLEL_LEVEL to 1 (or maybe 2 if that works). You don't need to run all the tests, just a couple of tests (maybe as simple as importing it) should be fine, maybe that would help?

henryiii avatar Jan 10 '23 20:01 henryiii

It shouldn't take 3 hours to build! I can build in under a minute on my MacBook on 8 cores; a more normal computer still usually can do it in 5-10 mins.

On my Raspberry Pi

$ time python -m build .
...
Successfully built boost_histogram-1.3.2.tar.gz and boost_histogram-1.3.2-cp39-cp39-linux_armv7l.whl

real	6m51.681s
user	12m36.840s
sys	0m30.553s
$ export CMAKE_BUILD_PARALLEL_LEVEL=1
$ time python -m build .
...
Successfully built boost_histogram-1.3.2.tar.gz and boost_histogram-1.3.2-cp39-cp39-linux_armv7l.whl

real	10m43.196s
user	10m14.357s
sys	0m23.024s
$ unset CMAKE_BUILD_PARALLEL_LEVEL

but this is also the wheel that fails on import boost_histogram, so not sure if a working wheel takes longer.

matthewfeickert avatar Jan 10 '23 21:01 matthewfeickert

Yes, that's about what I'd expect, so I'm guessing maybe the memory on the build nodes is really low and multhreaded building is filling it, causing this to take much longer multithreaded.

I think we needs -latomic injected for ARM. Currently it's using setuptools (ignore the CMAKE in the variable above!), so we can manually add it if we detect an armv7 platform, perhaps? For now, I think setting LDFLAGS would be easiest.

henryiii avatar Jan 10 '23 21:01 henryiii

I think we needs -latomic injected for ARM. Currently it's using setuptools (ignore the CMAKE in the variable above!), so we can manually add it if we detect an armv7 platform, perhaps? For now, I think setting LDFLAGS would be easiest.

I attempted this with the following patch

$ git diff v1.3.2
diff --git a/setup.py b/setup.py
index 308dbb1..b2a2768 100644
--- a/setup.py
+++ b/setup.py
@@ -1,6 +1,7 @@
 import os
 import sys
 from pathlib import Path
+import platform
 
 from setuptools import setup
 
@@ -38,6 +39,13 @@ INCLUDE_DIRS = [
     "extern/variant2/include",
 ]
 
+if sys.platform.startswith("win32"):
+    extra_compile_args=["/d2FH4-"]
+elif platform.machine() == "armv7l":
+    extra_compile_args=["-latomic"]
+else:
+    extra_compile_args=[]
+
 ext_modules = [
     Pybind11Extension(
         "boost_histogram._core",
@@ -45,7 +53,7 @@ ext_modules = [
         include_dirs=INCLUDE_DIRS,
         cxx_std=cxx_std,
         include_pybind11=False,
-        extra_compile_args=["/d2FH4-"] if sys.platform.startswith("win32") else [],
+        extra_compile_args=extra_compile_args,
     )
 ]

but unless I'm doing something wrong (very possible) this still fails with

$ python -c 'import boost_histogram'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/pi/.local/venvs/build/lib/python3.9/site-packages/boost_histogram/__init__.py", line 1, in <module>
    from . import accumulators, axis, numpy, storage
  File "/home/pi/.local/venvs/build/lib/python3.9/site-packages/boost_histogram/accumulators.py", line 1, in <module>
    from ._core.accumulators import (  # pylint: disable=import-error
ImportError: /home/pi/.local/venvs/build/lib/python3.9/site-packages/boost_histogram/_core.cpython-39-arm-linux-gnueabihf.so: undefined symbol: __atomic_load_8

matthewfeickert avatar Jan 10 '23 22:01 matthewfeickert

That's a link arg, not a compile arg. Try:

LDFLAGS="-latomic" python -m build .

henryiii avatar Jan 10 '23 22:01 henryiii

That's a link arg, not a compile arg.

Whoops! Sorry I seem to be asleep at the keyboard today — can't get much more obvious than that.

Try:

LDFLAGS="-latomic" python -m build .

Still failing though

$ LDFLAGS="-latomic" python -m build .
...
Successfully built boost_histogram-1.3.2.tar.gz and boost_histogram-1.3.2-cp39-cp39-linux_armv7l.whl
$ python -m pip uninstall -y boost-histogram
$ python -m pip install ./dist/boost_histogram*.whl
$ python -c 'import boost_histogram'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/pi/.local/venvs/build/lib/python3.9/site-packages/boost_histogram/__init__.py", line 1, in <module>
    from . import accumulators, axis, numpy, storage
  File "/home/pi/.local/venvs/build/lib/python3.9/site-packages/boost_histogram/accumulators.py", line 1, in <module>
    from ._core.accumulators import (  # pylint: disable=import-error
ImportError: /home/pi/.local/venvs/build/lib/python3.9/site-packages/boost_histogram/_core.cpython-39-arm-linux-gnueabihf.so: undefined symbol: __atomic_load_8

matthewfeickert avatar Jan 10 '23 22:01 matthewfeickert

The following patch works though

$ git diff v1.3.2
diff --git a/setup.py b/setup.py
index 308dbb1..caf5237 100644
--- a/setup.py
+++ b/setup.py
@@ -1,6 +1,7 @@
 import os
+import platform
 import sys
 from pathlib import Path
 
 from setuptools import setup
 
@@ -46,6 +47,7 @@ ext_modules = [
         cxx_std=cxx_std,
         include_pybind11=False,
         extra_compile_args=["/d2FH4-"] if sys.platform.startswith("win32") else [],
+        extra_link_args=["-latomic"] if platform.machine() == "armv7l" else [],
     )
 ]
 
$ python -m build .
...
Successfully built boost_histogram-1.3.3.dev0+g0c8659c.d20230110.tar.gz and boost_histogram-1.3.3.dev0+g0c8659c.d20230110-cp39-cp39-linux_armv7l.whl
$ python -m pip uninstall -y boost-histogram
$ python -m pip install ./dist/boost_histogram*.whl
$ python -c 'import boost_histogram; print(boost_histogram)'
<module 'boost_histogram' from '/home/pi/.local/venvs/build/lib/python3.9/site-packages/boost_histogram/__init__.py'
$ python -m pip install --upgrade "pytest>=6.0" "pytest-benchmark" "cloudpickle" "hypothesis>=6.0"
$ pytest
...
============================================================================== 830 passed in 117.87s (0:01:57) ===============================================================================

boost_histogram-1.3.3.dev0+g0c8659c.d20230110-cp39-cp39-linux_armv7l.whl.zip

where once the zip file is unpacked/wheel installed

$ objdump --all-headers _core.cpython-39-arm-linux-gnueabihf.so | grep atomic
  NEEDED               libatomic.so.1
  required from libatomic.so.1:
00000000       F *UND*	00000000              __atomic_fetch_add_8@LIBATOMIC_1.0
00000000       F *UND*	00000000              __atomic_store_8@LIBATOMIC_1.0
00000000       F *UND*	00000000              __atomic_load_8@LIBATOMIC_1.0

where

$ ldconfig --print-cache | grep atomic
	libatomic.so.1 (libc6,hard-float) => /lib/arm-linux-gnueabihf/libatomic.so.1

matthewfeickert avatar Jan 10 '23 22:01 matthewfeickert

Is it possible to build from PyPI with just pip3 wheel? That's how our automated builds work. I'll try it when I get chance.

bennuttall avatar Jan 10 '23 23:01 bennuttall

Is it possible to build from PyPI with just pip3 wheel? That's how our automated builds work. I'll try it when I get chance.

Not without the patch that I have above. I've opened https://github.com/scikit-hep/boost-histogram/issues/822 to discuss if this is worth supporting, or if the maintainers would prefer people to just move over to using a 64-bit OS on Raspberry Pi.

matthewfeickert avatar Jan 11 '23 00:01 matthewfeickert

pip3 wheel and python -m build --wheel are basically the same, with python -m build being preferred. Pip is slowly moving toward making build the backend for the wheel command (technically, I think they'd rather drop it, but too many people are using it because it predates build). It doesn't make any difference above, it's just that armv7 requires an extra non-standard link argument to use C++ atomics.

I don't know why LDFLAGS didn't work, it seems like it should here: https://setuptools.pypa.io/en/latest/userguide/ext_modules.html#compiler-and-linker-options

henryiii avatar Jan 11 '23 20:01 henryiii

I don't know why LDFLAGS didn't work, it seems like it should here: https://setuptools.pypa.io/en/latest/userguide/ext_modules.html#compiler-and-linker-options

I agree that this seems strange for the exact reasons that you mentioned

The linker options appear in the command line in the following order:

  • first, the options provided by environment variables and sysconfig variables,
  • then, a -L option for each element of Extension.library_dirs,
  • then, a linker-specific option like -Wl,-rpath for each element of Extension.runtime_library_dirs,
  • finally, the options provided by Extension.extra_link_args.

The fact that you can set enviornmental variables that get picked up by python -m build was the whole point of https://github.com/scikit-hep/pyhf/pull/1887, so this is confusing.

matthewfeickert avatar Jan 11 '23 23:01 matthewfeickert

I rebuilt the wheels and the only difference that I can see when looking at their logs is where -latomic gets placed

  • v1.3.2
$ LDFLAGS="-latomic" python -m build . &> LDFLAGS_build_out.txt
$ grep latomic LDFLAGS_build_out.txt 
arm-linux-gnueabihf-g++ -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-z,relro -g -fwrapv -O2 -latomic build/temp.linux-armv7l-cpython-39/src/module.o build/temp.linux-armv7l-cpython-39/src/register_accumulators.o build/temp.linux-armv7l-cpython-39/src/register_algorithm.o build/temp.linux-armv7l-cpython-39/src/register_axis.o build/temp.linux-armv7l-cpython-39/src/register_histograms.o build/temp.linux-armv7l-cpython-39/src/register_storage.o build/temp.linux-armv7l-cpython-39/src/register_transforms.o -L/usr/lib -o build/lib.linux-armv7l-cpython-39/boost_histogram/_core.cpython-39-arm-linux-gnueabihf.so

vs.

  • with patch
$ python -m build . &> patch_build_out.txt
$ grep latomic patch_build_out.txt 
arm-linux-gnueabihf-g++ -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-z,relro -g -fwrapv -O2 build/temp.linux-armv7l-cpython-39/src/module.o build/temp.linux-armv7l-cpython-39/src/register_accumulators.o build/temp.linux-armv7l-cpython-39/src/register_algorithm.o build/temp.linux-armv7l-cpython-39/src/register_axis.o build/temp.linux-armv7l-cpython-39/src/register_histograms.o build/temp.linux-armv7l-cpython-39/src/register_storage.o build/temp.linux-armv7l-cpython-39/src/register_transforms.o -L/usr/lib -o build/lib.linux-armv7l-cpython-39/boost_histogram/_core.cpython-39-arm-linux-gnueabihf.so -latomic

The only thing that I can think of is this comment in this Stack Overflow question:

Recent versions of GCC require that you put the object files and libraries in the order that they depend on each other - as a consequential rule of thumb, you have to put the library flags as the last switch for the linker.

matthewfeickert avatar Jan 12 '23 00:01 matthewfeickert

@bennuttall Now that boost-histogram v1.4.0 is out could you attempt this build again?

matthewfeickert avatar Sep 13 '23 16:09 matthewfeickert

That built ok for me.

We're currently working through the backlog of Python 3.11 builds so it'll try those in the next few days and then it'll try 1.4.0 on Buster and Bullseye once we re-introduce those builders.

bennuttall avatar Sep 13 '23 22:09 bennuttall

Thanks very much @bennuttall. SGTM!

matthewfeickert avatar Sep 13 '23 23:09 matthewfeickert