Regression: KeyError in h5py with h5netcdf 0.13.0

[paugier]

The CI of fluidsim was broken by the release of h5netcdf 0.13.0. I get a lot of KeyError: 'Unable to delete attribute (record is not in B-tree)' with tracebacks looking like

    gf_params.attrs["SAVE"] = 1
  File "/builds/fluiddyn/fluidsim/.tox/py37-fft/lib/python3.7/site-packages/h5netcdf/attrs.py", line 44, in __setitem__
    self._h5attrs[key] = value
  File "h5py/_objects.pyx", line 54, in h5py._objects.with_phil.wrapper
  File "h5py/_objects.pyx", line 55, in h5py._objects.with_phil.wrapper
  File "/builds/fluiddyn/fluidsim/.tox/py37-fft/lib/python3.7/site-packages/h5py/_hl/attrs.py", line 103, in __setitem__
    self.create(name, data=value)
  File "/builds/fluiddyn/fluidsim/.tox/py37-fft/lib/python3.7/site-packages/h5py/_hl/attrs.py", line 208, in create
    h5a.delete(self._id, self._e(name))
  File "h5py/_objects.pyx", line 54, in h5py._objects.with_phil.wrapper
  File "h5py/_objects.pyx", line 55, in h5py._objects.with_phil.wrapper
  File "h5py/h5a.pyx", line 145, in h5py.h5a.delete
KeyError: 'Unable to delete attribute (record is not in B-tree)'

By pinning h5netcdf to 0.12.0, the CI is green again.

Any idea of what can cause this issue ?

[kmuehlbauer]

Either the new track_order feature or a regression of the dimension scale refactor. Can you narrow down the issue?

[kmuehlbauer]

@paugier Can you try to open the files with keyword track_order=False and look, if the error goes away?

Update: track_feature fixed to track_order

[kmuehlbauer]

@paugier Reading this h5py/h5py#1385 it's a track_order issue.

[kmuehlbauer]

@hmaarrfk Any ideas?

[hmaarrfk]

we removed the track order keyword

[kmuehlbauer]

@paugier Please add the hdf5 and h5py versions you are using in your environment. Thanks!

[kmuehlbauer]

@paugier One final request, providing a minimal example would greatly help to narrow down the issue. I couldn't reproduce this so far.

[hmaarrfk]

Hmm, ok. I just read through the log of the provided error.

My guess is that the test is using an h5 file that was created with 0.12.0 or earlier with track_order was implicitely set to False.

Then in 0.13.0 we are setting track_order to True. However, it was my intention to keep the same behavior for old files. If track order WAS True, then it should continue to be True, however, if it was False, it should continue to be false.

I think that the bug comes:

h5netcdf/h5netcdf/core.py

Line 1009 in fe17016

path, mode, track_order=track_order, **kwargs

where track order is being specified on read and append.

If you can help provide us with the dataset, and the code to recreate it will go a long way.

[paugier]

Good news, I can easily reproduce locally with the most recent versions of h5py (3.6.0) and h5netcdf (0.13.0).

There is no file created with older versions of these packages.

The error happens for example in this line https://foss.heptapod.net/fluiddyn/fluidsim/-/blob/branch/default/fluidsim/util/output.py#L121 for the (simplest) case for which mpi.nb_proc == 1 and not cfg_h5py.mpi. Basically a file a created, data is added and groups are created with attributes.

I will try to provide a minimal example reproducing the bug soon.

[paugier]

I finally manage to find time to produce this minimal example.

# error with h5netcdf 0.13.0, no error with 0.12.0
import h5netcdf as h5pack

# no error with this import
# import h5py as h5pack


def save(d, path_file):
    print(path_file)
    print(d)
    with h5pack.File(path_file, "w") as h5file:
        for key, value in d.items():
            h5file.attrs[key] = value

        try:
            h5file.attrs["a"] = 0
        except KeyError:
            print(f"ERROR for {path_file = }")

            raise


foo = {key: ord(key) for key in "abcdefghij"}
bar = {key: ord(key) for key in "bcdefghija"}

assert foo == bar

save(foo, "foo.nc")
save(foo, "foo1.nc")
save(bar, "bar.nc")

Here, it gives

foo.nc
{'a': 97, 'b': 98, 'c': 99, 'd': 100, 'e': 101, 'f': 102, 'g': 103, 'h': 104, 'i': 105, 'j': 106}
foo1.nc
{'a': 97, 'b': 98, 'c': 99, 'd': 100, 'e': 101, 'f': 102, 'g': 103, 'h': 104, 'i': 105, 'j': 106}
bar.nc
{'b': 98, 'c': 99, 'd': 100, 'e': 101, 'f': 102, 'g': 103, 'h': 104, 'i': 105, 'j': 106, 'a': 97}
ERROR for path_file = 'bar.nc'
/home/pierre/tmp/bug_h5netcdf/bug.py:9: FutureWarning: String decoding changed with h5py >= 3.0. See https://docs.h5py.org/en/latest/strings.html and https://github.com/h5netcdf/h5netcdf/issues/132 for more details. Currently backwards compatibility with h5py < 3.0 is kept by decoding vlen strings per default. This will change in future versions for consistency with h5py >= 3.0. To silence this warning set kwarg ``decode_vlen_strings=False`` which will return Python bytes from variables containing vlen strings. Setting ``decode_vlen_strings=True`` forces vlen string decoding which returns Python strings from variables containing vlen strings.
  with h5pack.File(path_file, "w") as h5file:
Traceback (most recent call last):
  File "/home/pierre/tmp/bug_h5netcdf/bug.py", line 29, in <module>
    save(bar, "bar.nc")
  File "/home/pierre/tmp/bug_h5netcdf/bug.py", line 14, in save
    h5file.attrs["a"] = 0
  File "/home/pierre/.pyenv/versions/3.9.6/lib/python3.9/site-packages/h5netcdf/attrs.py", line 44, in __setitem__
    self._h5attrs[key] = value
  File "h5py/_objects.pyx", line 54, in h5py._objects.with_phil.wrapper
  File "h5py/_objects.pyx", line 55, in h5py._objects.with_phil.wrapper
  File "/home/pierre/.pyenv/versions/3.9.6/lib/python3.9/site-packages/h5py/_hl/attrs.py", line 103, in __setitem__
    self.create(name, data=value)
  File "/home/pierre/.pyenv/versions/3.9.6/lib/python3.9/site-packages/h5py/_hl/attrs.py", line 208, in create
    h5a.delete(self._id, self._e(name))
  File "h5py/_objects.pyx", line 54, in h5py._objects.with_phil.wrapper
  File "h5py/_objects.pyx", line 55, in h5py._objects.with_phil.wrapper
  File "h5py/h5a.pyx", line 145, in h5py.h5a.delete
KeyError: 'Unable to delete attribute (record is not in B-tree)'

[kmuehlbauer]

@paugier Great, thanks!

That's the track_order issue, see here: h5py/h5py#1385. Obviously something is broken with track_order in h5py so that you only have access to the first 7 created attributes. Accessing the 8th will break. So if you select h it will work, but i, j and a will break for bar and h, i and j will break for foo.

It doesn't look like there is a solution already available in h5py, but we could work around it here.

Bottom line, we would need to set_attr_phase_change(0, 0) (if we write more than 7 attributes) on each object which can hold attributes.

[paugier]

I confirm. Simpler reproducer (error for i = 8):

import h5netcdf as h5pack

# import h5py as h5pack

with h5pack.File("foo.nc", "w") as h5file:
    for i in range(10):
        h5file.attrs[f"key{i}"] = i
        try:
            h5file.attrs[f"key{i}"] = 0
        except KeyError:
            print(f"ERROR for {i = }")

            raise

[hmaarrfk]

eek. not so sure what the best way forward is. seems rather annoying and serious. maybe we should expose the parameter again and leave the default as false.

thanks for helping us work through the bugs here

[hmaarrfk]

@kmuehlbauer I think we should definitely remove the error on track_order=False as a keyword argument.

The next question is if we should change the default value back to the 0.12.0 one due to this long standing buy (2years +)

[kmuehlbauer]

@hmaarrfk That's a pity, really. The only other way would be to fix this via h5py low level object creation.

I agree that we should remove the error. And probably switch default again for the new API, we should keep track_order=True for the legacy API.

I'm not sure, if I can get this in before the weekend. But we would need to to fix in master , to create a 0.13-release branch, cherry-pick and get 0.13.1 out with these changes.

[kmuehlbauer]

Would be best to be consistent, so switching back to track_order=False seems the best solution for the current situation.

[hmaarrfk]

If you can create the branch, I can cherry-pick. I'm interested enough, for performance reasons to move to an h5py backend, and I am making the changes as we speak.

[hmaarrfk]

Wow, lots of exciting stuff has been merged last week. Should we just make a version 0.14.0 instead?

[kmuehlbauer]

Let's fix this one as 0.13.1. There is #135 which gets merged in the next days and will be finally 0.14.0 (which will be released not shortly too).

[kmuehlbauer]

@hmaarrfk I've created https://github.com/h5netcdf/h5netcdf/tree/release-0.13

[hmaarrfk]

Ok got the PR on master to be nice. I think the one on 0.13 is ready too if there aren't major comments on the master branch.

I guess for now, the tip would be to stick to the 0.12.0 until 0.13.1 gets released.

Thanks you two!

[kmuehlbauer]

@hmaarrfk Would you mind adding the readme note to master too?

[kmuehlbauer]

@hmaarrfk @paugier h5netcdf 0.13.1 has been released on PyPI. Thanks!

[paugier]

Oh I forgot to say thank you @hmaarrfk and @kmuehlbauer for your work on this issue and on h5netcdf in general! It's really a very useful and nice package.

[kmuehlbauer]

See upstream HDFGroup/hdf5#1388 for details.