CarpetIOHDF5: fix check for old string datatype

Create issue
Issue #1849 closed
Frank Löffler created an issue

The current code contains a check for an old way the grid structure string was saved: as H5T_NATIVE_CHAR. This check test whether the type found in a (checkpoint) file is of that type. However, this fails if H5T_NATIVE_CHAR was different on the machine writing the checkpoint and the one restoring from it (e.g., big vs. little endian int8).

Instead of checking if the saved type is of type H5T_NATIVE_CHAR, this patch checks for the new datatype being of class H5T_STRING.

This change makes the two restore testsuites pass on big endian machines (the checkpoint file in the testsuite was written using the old mechanism, on a little endian machine).

https://bitbucket.org/eschnett/carpet/pull-requests/8/fixed_string_check/diff

I tested that this fixes the problems on the big endian machine, and that the testsuite also still works on a regular, little endian machine, for both an old and a new-style checkpoint.

The two lines changed both look like this:

-    HDF5_ERROR(old_data = H5Tequal(datatype, H5T_NATIVE_CHAR));
+    HDF5_ERROR(old_data = 0 == H5Tdetect_class(datatype, H5T_STRING));

I am not sure about whether we should back-port this. It would be easy, as the change is very small, and I tested it, but I would like to have a separate "yes" for that.

Keyword: hdf5

Comments (6)

  1. Erik Schnetter
    • removed comment

    I think this should use H5Tget_class instead of detect_class. The latter is meant to recursively examine structures, not find the property of a single type.

    H5Tget_class(datatype) == H5T_STRING
    
  2. Erik Schnetter
    • removed comment

    Alternatively, you can simply to to read the string into a H5T_NATIVE_CHAR. If it succeeds -- good, if it doesn't -- throw an error. There's no real need to check ahead of time whether we think that HDF5 can convert a type to a string, if we can simply ask HDF5 to convert to a string.

  3. Frank Löffler reporter
    • removed comment

    I updated the pull request. detect_class was replaced with get_class, and a new testsuite was added to test restarting from a new-style grid structure string - using the same parfile to creation as the old did.

  4. Log in to comment