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
[c-blosc] linking against external of blosc version doesn't work #451
Comments
Another one:
|
Hi @keszybz, I managed to reproduce it but I haven't investigated further at the moment. |
@FrancescAlted, what's the supported way to build pytables with another version of blosc? If I pass the My guess is this is the source of the problem, looking at https://github.com/PyTables/PyTables/blob/develop/c-blosc/hdf5/blosc_filter.c#L236 we don't do
for the new version of blosc. But the same file provided by blosc works differently having
rather than
and so we end up passing a null pointer to |
@keszybz my suggestion would be to try to replace the folder This doesn't work for me because |
@FrancescAlted Why not just remove hdf5/ subdirectory from c-blosc sources and keep it in PyTables? It seems to be confusing as hell to have it there but only use it here. |
@andreabedini I think Blosc/c-blosc@d84472e959 might also work for OS X. |
@andreabedini the way to compile against another blosc library is via the '--blosc' flag, as you said. And yes, using the same 'hdf5/blosc_filter.c' is intended and should work against any c-blosc library (Blosc/c-blosc@42dadb3, also cherry-picked to c-blosc 1.4.4 integrated with PyTables 3.2.0) |
@keszybz I don't think removing 'hdf5/blosc_filter.c' from c-blosc and putting it just into PyTables would be good, because this is meant to be used by any HDF5 wrapper (even as a plug-in for HDF5 itself). Please try compiling c-blosc, installing it, and then passing the location with the '-blosc' flag. That should work. |
hdf5/blosc_filter.c (as of the version found in pytables-3.2.0, which is taken from c-blosc-1.4.4) does not work with the latest versions of c-blosc. See the first comment ^^^. It needs the updates that are upstream in c-blosc, but not in the copy bundled with pytables [1]. [1] http://pkgs.fedoraproject.org/cgit/python-tables.git/tree/hdf5-blosc-1.4.4-1.6.1.diff?id=08740b86595f16134a0544e82e9b6ab286fc38ed |
@FrancescAlted That's exactly what I did to reproduce the crash, if you read my comment above I tell you exactly why it is happening. |
Ah, I see now. My mistake. Yes, complib must be set to 'blosclz'. Hmm, that will need a new release. Enviat des del meu iPhone El 09/05/2015, a les 1:48, Andrea Bedini notifications@github.com va escriure:
|
I think the correct way to solve this is to modify setup.py to pick blosc_filter.c from the path provided by |
This would only work assuming that you have a blosc directory containing sources around. Right now --blosc can be pointed as a .so file and it is enough to compile pytables. |
Of course :-/ |
If what @FrancescAlted says is true, then the latest version of c-blosc broke its own API and this is not a PyTables bug (perhaps I'm being a bit too defensive here but there's already lots to do). So I don't think this needs a new PyTables release. |
Yes, this is a c-blosc issue. |
Well, I think we need to keep this open because PyTables comes with c-blosc sources included. So, we need a new release of c-blosc and then a new release of PyTables. c-blosc is in a position now that is getting installed in most of distributions (but for Windows things are a bit more difficult because people will need to trust on Anaconda, Canopy or similar), so in the next future we should think in avoid including them and relying on having a c-blosc library installed (which means a new dependency). |
But the version of c-blosc shipped with PyTables works.
What change would the new release of PyTables include?
Not sure I understood. Would you prefer including c-blosc or not? |
@andreabedini I frankly don't know if I prefer c-blosc being part of PyTables or not. The advantage of including it is that you should not make users to install blosc separately (or wait until distributions come with it). The disadvantage is that every bug fix in blosc will require a new release of PyTables. And the c-blosc shipped with PyTables works only if users are not making use of the -blosc flag in order to link with existing libraries that are >= 1.5.0 (the case of @keszybz). For the time being, I would say that fixing the problem with c-blosc (in hdf5/blosc_filter.c more specifically) and updating the copy in PyTables is the easiest thing to do. |
On Tue, May 12, 2015 at 09:14:02AM -0700, FrancescAlted wrote:
I don't think separate distribution is an actual problem for users. If |
With the exception that c-blosc is a C library, not a python one ;) |
@FrancescAlted let me try to make my point clear. PyTables and c-blosc are two project that communicate through an API. c-blosc promised hdf5/blosc_filter.c would work with any version of c-blosc. Later on, c-blosc made changes that broke that promise, breaking compatibility with PyTables and any other project using hdf5/blosc_filter.c. I would expect c-blosc to either 1) admit it was a bug and restoring compatibility with the old hdf5/blosc_filter.c or 2) admit the API has changed. In case 1) there's nothing to do for PyTables, in case 2) we can say we don't support the new API yet (which, thanks to #437, is a non trivial job). There are other parts of PyTables in huge need of love, I don't want to spend time fixing what c-blosc has broken. PS: emails make everything sound awful, I still love you all :) 💌 😸 |
@andreabedini I love you too :) Now, let me clarify that c-blosc did not break any API, but rather that it lost the internal threading lock, which is a different thing. Now, PyTables can still use c-blosc > 1.5 if one of these conditions is fulfilled:
For me, the ideal solution would be to go with 2), but I don't know if I will have time to implement this, so perhaps we could go with 1) for now and then try with 2) (if anybody is reading this, PR are greatly appreciated). And if I would like to keep this open is to remember that we have a problem in included c-blosc sources that needs to be fixed. |
I'm seeing this on Windows now, trying to link pytables against blosc 1.7.0. tables.print_versions crashes:
What's the best fix, short term (ideally, for now) and long term? |
Here's what I have so far to replace the included c-blosc with the updated upstream version. This is still crashing, though. Do I have the threading issue mentioned in #451 (comment) ? In pytables source checkout:
|
Yes, you are most probably getting into the threading issue that I mentioned here. As said, fixing this is not exactly trivial and help would appreciated. |
All right. Thanks for the pointers so far. I don't have time to get into learning about the locks and stuff right now, but I will put this somewhere in my agenda. I think it would be really nice to be able to use externally-built c-blosc and decouple these. |
@FrancescAlted: According to the release notes, this should be fixed in current |
Yes, I think this should be fixed after c-blosc 1.8. Closing it. |
I'm getting a segfault when compiling pytables-3.2.0 against blosc-1.6.1 or blosc-1.5.4.
I'm running the following crash:
The text was updated successfully, but these errors were encountered: