Open hernot opened 2 years ago
Hi @hernot it would be great to get this merged at some point soon, thanks for all the effort.
It kind of arrived right about the time we were merging in the zstd support, and a large number of other changes so it's ended up with a bunch of merge conflicts. Would you be able to resolve those and then I'll take a close look at the changes?
I guess this also won't have the compliant behaviour for the zstd pipeline, but we can look into adding that later on based on your changes.
Thanks!
Sure. I already expected. Just need to find some time to rebase to master/head and check the differences. I guess that zstd support is not so much different from lz4 so i can have a look and include necessary adjustments or at leas come up with some ideas how to also apply to zstd.
Ok rebase done but have some issues with running tests
test_h5plugin.py
compains about filter 32008 not defined/registered
test_h5filter.py
complains about missing zstd libraries
and test_regression.py
complains about missing lib/plugin
running python 3.8.10 and h5py 3.6.0
compiled with
python3 setup.py build_ext --inplace
and set exprort PYTHONPATH=./
In other words i need some hint what could be missing or wrong. git difftool -t vimdiff master is not really helpful for figuring.
Ok figured at least for building and text_ext.py and test_h5filter.py
libzstd latest version seems to include some assembly language files but these have postfix .S
instead of .s
expected by setuptools
build_ext_
. so to make the whole compile i had to disable assembly support by adding a define in setup.py
. Further a small code-mixup introduced during rebasing and fixing conflicts prevented tests from running . The remaining tests you have to check if proper.
Should i squash my commits into one single commit or keep them?
Question
Just see that pullrequest #78 is still open since 3 years. What is the reason it was not considered? If it is just cause sumiter had no time to resolve conflicts? In this case I could offer to reflect it in this pull-request. As a side-mark: Windows can be a bitch with its tons of versions of c runtime environment and their neither compatible nor linked memory allocators.
Makes bitshuffle with lz4 compression enabled abort when output size exceeds input size as recommended by libhdf5 documentation when H5Z_FLAG_OPTIONAL is set on compression/compressing filters as h5py high level interface does for all registered compression filters. Plausibility check whether bithsuffling in combination with lz4 can reduce size of input more than the 16 bytes required to store raw input size, overall output size and the size of the first compressed block plus a minimum of 2bytes payload resulting from lz4 compression following bitshuffling. If it input is too short for this minimum gain bithsuffling with lz4 compression is not attempted at all. In case lz4 compression is not enabled all data is shuffled disregarding whether H5Z_FLAG_OPTIONAL is set or not.
Further noticed that when compiled with omp enabled than in case of error the locks an mutices used to protect ioc_chain are not torn down properly in case of error. Just when bithsullfling with and without compression enabled completes successfully. Details see commit message.
This PR is related to and resolves issue #110
Awaiting your your comments and review.