Attempt to create NC_STRING var with fill value off causes HDF5 error

[edhartnett]

I have been adding some systematic tests for fill values in nc_test/tst_formats.c, in branch ejh_fill_values.

When trying to create an NC_STRING var with fill mode turned off I get this interesting error:

HDF5-DIAG: Error detected in HDF5 (1.10.1) thread 0:
  #000: H5D.c line 145 in H5Dcreate2(): unable to create dataset
    major: Dataset
    minor: Unable to initialize object
  #001: H5Dint.c line 490 in H5D__create_named(): unable to create and link to dataset
    major: Dataset
    minor: Unable to initialize object
  #002: H5L.c line 1695 in H5L_link_object(): unable to create new link to object
    major: Links
    minor: Unable to initialize object
  #003: H5L.c line 1939 in H5L_create_real(): can't insert link
    major: Symbol table
    minor: Unable to insert object
  #004: H5Gtraverse.c line 867 in H5G_traverse(): internal path traversal failed
    major: Symbol table
    minor: Object not found
  #005: H5Gtraverse.c line 639 in H5G_traverse_real(): traversal operator failed
    major: Symbol table
    minor: Callback failed
  #006: H5L.c line 1742 in H5L_link_cb(): unable to create object
    major: Object header
    minor: Unable to initialize object
  #007: H5O.c line 3178 in H5O_obj_create(): unable to open object
    major: Object header
    minor: Can't open object
  #008: H5Doh.c line 291 in H5O__dset_create(): unable to create dataset
    major: Dataset
    minor: Unable to initialize object
  #009: H5Dint.c line 1256 in H5D__create(): can't update the metadata cache
    major: Dataset
    minor: Unable to initialize object
  #010: H5Dint.c line 864 in H5D__update_oh_info(): Dataset doesn't support VL datatype when fill value is not defined
    major: Dataset
    minor: Feature is unsupported

Looks like HDF5 is just not cool with turning off fill mode for NC_STRING.

[edhartnett]

I propose to check for this and return an NC_EINVAL if the user tries to turn off fill mode for a string type.

[DennisHeimbigner]

1. Can we produce this in a small program to give to the HDF group?
2. Perhaps a better soln is to not refuse to turn off fill so much as to set the fill value to the null string ""

[edhartnett]

1 Yes, I can produce a HDF5 program. 2 No, I think that would be wrong. The default fill value is already the null string. If the user is trying to turn off fill mode, it is because he or she is trying to improve performance by removing the step of writing fill values to every element in the dataset chunk that is being written. Since this is apparently not allowed, the user should be told that. They can then decide to live without it, or change to a NC_CHAR array, where they have control over the fill mode.

[DennisHeimbigner]

Ok. But we need to make sure that there is anote in the documentation that this is an HDF5 bug and will revert behavior one HDF5 is fixed.

[edhartnett]

I don't believe this is a HDF5 bug.

If you think about how VLENs have to be stored, it makes sense that a fill value is always required. Unlike with NC_INT or one of the other atomic types, you cannot just calculate how much space an array of 100 NC_STRINGS will take. It's not known. So it makes sense that a placeholder (fill value) is required for any values not explicitly written by the users.

So I don't believe this is a bug in HDF5 or will ever change.

I have added a note of this to the documentation of nc_dev_var_fill(). I will refrain from writing a HDF5 version of the code to prove this, but I will post on the HDF5 mailing list to confirm I am correct about VLENs always needing fill mode.

[DennisHeimbigner]

Good point, extcept that NULL would be a perfectly acceptable "fill" value.

[edhartnett]

It's not actually NULL. The fill value is a VLEN array of 1 byte containing NULL. Not the same. Not just the NULL is stored. Since it's a VLEN, the length is also stored.

[DennisHeimbigner]

In memory or in the file? In memory I would be surprised if was not stored as a vector of type char**.

[edhartnett]

I have asked on the HDF5 mailing list. However, my understanding is that they treat strings internally as VLENS. They don't take advantage of any special knowledge of the meaning of NULL in strings. We shall see what the HDF5 developers answer.

In the meantime, I have changed the docs to explain that fill mode can't be turned off for NC_STRING vars, and changed nc_def_var_fill() so that NC_EINVAL is returned if that is attempted. I will put this branch up as a PR, probably next week. No rush since Ward seems to have plenty of PRs deal with right now. ;-)

[WardF]

Thank you; I do, plus prepping for AMS. And the holidays and non NetCDF work. Busy but no more so than everybody else I suppose!

[edhartnett]

OK, well this is fixed in my branch, along with some other bugs.

I think my current branch should probably be in 4.6.0 as well as the ones you have already selected, since it does fix a bunch of problems.

But now I am working on the rename problems that Charlie Zender has brought to our attention. These are apparently a problem for the IPCC, so should represent a priority for us.

Happy holidays to all indeed. I cannot imagine any gift I would want, other than the opportunity to continue to contribute to atmospheric and climate science, by continuing to develop and test netCDF. ;-)

[DennisHeimbigner]

What is the 3.6.0 you are referring to?

[edhartnett]

I meant 4.6.0, the next release.

Scarlet O'Hara: "When I'm wearing a new bonnet, it seems like all the figures I know leave my head."

[WardF]

Hi Ed, Dennis and I were planning to sit and review the PIO code at the January meeting so that we can wrap our heads around it and make sure it’s something we’re comfortable with before we take ownership of it in the Inidata repository. It also seems like it might be worth getting it’s own release. As it stands, the 4.6 release will probably come first and then we will take a look at this patch. We have suffered in the past for trying to cram too much into a release and we already have the compression filters as the main feature of the current release :).

[WardF]

I am technically on PTO until January 3rd but I’m going to try to get the 4.6 release out before then so that we can move forward with the review.

[edhartnett]

Howdy @WardF

I hope you take as much time off as you like, and enjoy the holidays. Getting 4.6.0 out is not something that has to happen in the next few days if that's a hardship. We can all wait until early January. ;-)

I also anticipated that you would want to get 4.6.0 out before merging the PIO code. The PIO integration is worthy of it's own release. And I am happy to see all my recent non-PIO bug-fixes, documentation improvements, and warning cleanups make it into a release.

The PIO features will give netCDF a really beautiful set of features for supercomputers. With these features we will save the team of the new FV3 model from writing their own version of this functionality (which would be completely proprietary, and not involve netCDF except at the final output stage.) And the FV3 team is just the most important and prominent modeling team in this situation - there are no doubt many others around the world.

The FV3 dynamical core is going to become the heart of America's forecasting system. So this is a glittering prize, which will put netCDF at the very center of key operational code for NOAA. NetCDF has the opportunity to provide the HPC features that modelers are desperately calling for, in order to take advantage of massively multi-core machines.

Not only will we be enabling a lot of science, through increased efficiency of HPC, we will also be cementing netCDF as the data format and library of choice for HPC for decades to come. The PIO features completely eclipse the HPC features offered by HDF5, pnetcdf, or any other library.

You will not, however, be maintaining netCDF/PIO alone. Just as with netCDF-4, I fully understand that the introduction of dramatic new features must be followed with months and years of patient efforts to support, document, and maintain those features.

So here I am. What are your other two wishes? ;-)

[edhartnett]

BTW I accidently closed this ticket. I have a fix but it's on my branch, not yet merged to master. So I will reopen the ticket.

[ward]

(You highlighted the wrong Ward)

[krisfed]

I have added a note of this to the documentation of nc_dev_var_fill().

Hi all! Sorry to raise an old topic, but I can't find where this limitation was documented... I don't see it mentioned here: https://www.unidata.ucar.edu/software/netcdf/docs/group__variables.html#gabe75b6aa066e6b10a8cf644fb1c55f83 Is it covered somewhere else?