kiyo-masui / bitshuffle

Filter for improving compression of typed binary data.
Other
215 stars 76 forks source link

bitshuffle filter plugin should not be linked with a particular version of HDF5 #53

Closed vasole closed 4 years ago

vasole commented 8 years ago

Hello,

According to the point 4.3 of the documentation in:

https://www.hdfgroup.org/HDF5/doc/Advanced/DynamicallyLoadedFilters/HDF5DynamicallyLoadedFilters.pdf

it should not be necessary for a filter plugin to link to the HDF5 library. Indeed, removing the bshuf_register_h5filter function (or replacing it by its error return value to show it is not called/needed) and redefining PUSH_ERR to avoid a call to HDF5 one can use the filter for decompressing. I have not tested if the call to HDF5 into bshuf_h5_set_local is actually needed and I do not know its function.

It would be desirable that the bitshuffle HDF5 filter plugin behaves the same way other filters (ex. LZ4) that do not bind to a particular version of HDF5.

Thanks for your time,

Armando

kiyo-masui commented 6 years ago

I agree that this is a problem, especially that bitshuffle doesn't work with the wheel distributions of h5py.

I'm at a loss for how to fix this:

Any suggestions?

vasole commented 6 years ago

I see.

What about two different builds? One for using it as a dynamic filter and the current one.

Kriechi commented 6 years ago

Should v0.3.4 now install h5py without wheels? Cause I'm getting this:

$ sudo pip3 install -U bitshuffle
Collecting bitshuffle
  Downloading bitshuffle-0.3.4.tar.gz (232kB)
    100% |████████████████████████████████| 235kB 3.4MB/s
Requirement already up-to-date: setuptools>=0.7 in /usr/local/lib/python3.5/dist-packages (from bitshuffle)
Requirement already up-to-date: Cython>=0.19 in /usr/local/lib/python3.5/dist-packages (from bitshuffle)
Requirement already up-to-date: numpy>=1.6.1 in /usr/local/lib/python3.5/dist-packages (from bitshuffle)
Collecting h5py>=2.4.0 (from bitshuffle)
  Downloading h5py-2.7.1-cp35-cp35m-manylinux1_x86_64.whl (5.3MB)
    100% |████████████████████████████████| 5.3MB 290kB/s
Requirement already up-to-date: six in /usr/local/lib/python3.5/dist-packages (from h5py>=2.4.0->bitshuffle)
Installing collected packages: h5py, bitshuffle
  Running setup.py install for bitshuffle ... done
Successfully installed bitshuffle-0.3.4 h5py-2.7.1

... which seems to still download & install h5py wheels.

kiyo-masui commented 6 years ago

There is no way (that I'm aware of) to force pip to install a prerequisite without wheels. All I could do was add this caveat to the README and the requirements.txt (so you can set things up with pip -r requirements.txt). So pip installation will non work automatically.

Yeah, this is a mess.

For two builds, I'm still not sure how I'd get around the call to H5Pmodify_filter. Also for pure use as a dynamic plugin, this becomes a C library, not a python one, so it could be packaged with https://github.com/nexusformat/HDF5-External-Filter-Plugins.

vasole commented 6 years ago

Well, for reading I simply do not call H5Pmodify_filter. Indeed I am building the dynamic plugin as a shared library (using CMake).

What I would like to avoid is to have a different code base. It would be desirable to be able to choose to link against HDF5 or not via a pre-processor definition. Currently we have to patch your code as indicated above to get rid of the HDF5 dependency. That is enough for reading, but not for writing.

kiyo-masui commented 6 years ago

Can you instead link against a short extra piece of code that supplies the required functions using dlopen? I think that requires no changes to bitshuffle source, just an extra layer from you. (I'm not super familiar with dynamic loading so this may not be a viable option).

vasole commented 4 years ago

Well, finally we ended up following your suggestion and it works!

SujitC commented 3 years ago

Well, for reading I simply do not call H5Pmodify_filter. Indeed I am building the dynamic plugin as a shared library (using CMake).

What I would like to avoid is to have a different code base. It would be desirable to be able to choose to link against HDF5 or not via a pre-processor definition. Currently we have to patch your code as indicated above to get rid of the HDF5 dependency. That is enough for reading, but not for writing.

@vasole , what's the best way to tackle or resolve the issue https://github.com/kiyo-masui/bitshuffle/issues/89 in my environment. Please help on this.

vasole commented 3 years ago

@vasole , what's the best way to tackle or resolve the issue #89 in my environment. Please help on this.

I do not know if the best, but the simplest could be to install the hdf5plugin module. If you are using python, then importing the module is all what you have to do. If you are not using python, just point your HDF5_PLUGIN_PATH to the location where the libraries provided by that module are installed. I understand this is to give you a fish and not to teach you fishing, but if that works for you then you can go deeper and see what we actually do to build the library.