`DataTypeClass` does not discriminate between signed and unsigned integers

[Jacobfaib]

Describe the bug

Writing a uint32 array to disk, then reading it back with HighFive returns a int32 array.

inline DataTypeClass DataType::getClass() const {
    return convert_type_class(detail::h5t_get_class(_hid));
}

(https://github.com/highfive-devs/highfive/blob/main/include/highfive/bits/H5DataType_misc.hpp#L36-L38)

inline DataTypeClass convert_type_class(const H5T_class_t& tclass) {
    switch (tclass) {
    case H5T_TIME:
        return DataTypeClass::Time;
    case H5T_INTEGER:
        return DataTypeClass::Integer;
        ...

(https://github.com/highfive-devs/highfive/blob/main/include/highfive/bits/H5DataType_misc.hpp#L365)

convert_type_class() should probably also call H5Tget_sign() in case of integers, then return a new DataTypeClass::UnsignedInteger. Alternatively, could add a DataType::isSigned() member to DataType that queries this information.

[1uc]

Writing a uint32 array to disk, then reading it back with HighFive returns a int32 array.

I'd like to first focus on this part (it's quite scary). It sounds like we're storing unsigned integers as signed (or vice versa). I wanted to reproduce the issue and failed, e.g.

#include <highfive/highfive.hpp>
#include <cstdint>

template<class T>
void print_array(const std::vector<T>& x) {
  std::cout << "[ ";
  for(size_t i = 0; i < x.size(); ++i) {
    auto sep = (i == x.size() - 1) ? std::string("]\n") : std::string(", ");
    std::cout << std::to_string(x[i]) << sep;
  }
}

template<class T>
void write(HighFive::File& file, const std::string& dset_name) {
  std::vector<T> x{0, T(-1)/2, T(-1)};
  auto dset = file.createDataSet(dset_name, x);

  auto y = dset.template read<std::vector<uint8_t>>();
  auto z = dset.template read<std::vector<int8_t>>();
  auto a = dset.template read<std::vector<float>>();

  std::cout << "y = ";
  print_array(y);

  std::cout << "z = ";
  print_array(z);

  std::cout << "a = ";
  print_array(a);
}

void write_uint8(HighFive::File& file) {
  write<uint8_t>(file, "uint8");
}

void write_float(HighFive::File& file) {
  write<uint16_t>(file, "uint16");
}

void write_float(HighFive::File& file) {
  write<float>(file, "float");
}

int main() {
  auto file = HighFive::File("issue-77.h5", HighFive::File::Truncate);
  write_uint8(file);
  // write_uint16(file);
  write_float(file);
  return 0;
}

(Yes, the printing code is clumsy.)

If I check with h5dump I get:

$ h5dump issue-77.h5
HDF5 "issue-77.h5" {
GROUP "/" {
   DATASET "float" {
      DATATYPE  H5T_IEEE_F32LE
      DATASPACE  SIMPLE { ( 3 ) / ( 3 ) }
      DATA {
      (0): 0, -0.5, -1
      }
   }
   DATASET "uint8" {
      DATATYPE  H5T_STD_U8LE
      DATASPACE  SIMPLE { ( 3 ) / ( 3 ) }
      DATA {
      (0): 0, 127, 255
      }
   }
}
}

Meaning it's stored the integers as unsigned, 8-bit, little endian. That makes sense, they are unsigned and 8-bit and my machine is little endian.

When decoding, the user picks a type, e.g. uint8_t or int8_t. If the user picks uint8_t then HDF5 will read integers that are stored as H5T_STD_U8LE on disk into H5T_NATIVE_UCHAR. The choice H5T_NATIVE_UCHAR happens via:

highfive/include/highfive/bits/H5DataType_misc.hpp

Lines 121 to 124 in 1a5ef37

	template <>
	inline AtomicType<unsigned char>::AtomicType() {
	_hid = detail::h5t_copy(H5T_NATIVE_UCHAR);
	}

because uint8_t and unsigned char happen to be identical types (in GCC). See,
https://godbolt.org/z/vvMo4bzj1

Things get slightly less (or more) entertaining when we read back into int8_t. Remember that the representation on disk and the representation in memory can plausibly be different, because the one could be big and the other little endian. Hence, HDF5 deals with "compatible types". If two types are "compatible" HDF5 will silently transform between them, e.g. swap bytes for big <-> little endian.

The fun fact: All integers are compatible with each other and with all floating point numbers; at least AFAICT. How? Again, very big AFAICT, if the value is representable then that's the value, if it's not pick something nearby or as close as you can get.

For example this is the output (with some HighFive warnings removed):

y = [ 0, 127, 255]
z = [ 0, 127, 127]
a = [ 0.000000, 127.000000, 255.000000]
-----------------------
y = [ 0, 0, 0]
z = [ 0, 0, -1]
a = [ 0.000000, -0.500000, -1.000000]

Sorry, that's quite a lot of text to say: I'm unable to reproduce the first sentence. As far as I can tell, HighFive honors the types provided by the user: if the user says the values are unsigned they're saved as unsigned. If the user then reads them back as signed, they get read back as "compatible" signed integers.

---

The rest of the issue I don't fully understand. HDF5 has a way of serializing types. To do so they categorize types into several "classes" one of those is integers (with identifier H5T_INTEGER). IUC these are not really involved in serializing arrays with HighFive. If one wanted to read HDF5 datatype and work on them it would be useful to know if they datatype is some (signed or not) integer.

[Jacobfaib]

Sorry, my initial post probably sounded much scarier than it actually is :)! Yes, you're right that the underlying data doesn't change (and that HDF5 will silently do The Right Thing(TM)) when doing the serdez on the other side.

Rather the bug (or maybe lack-of-feature is a better term) here is that there is no way to tell -- from HighFive -- whether the underlying type is signed or unsigned.

That is, I can create a HighFive file, get the dataset, and get it's type, but I cannot tell whether it is signed or unsigned. Taking your repro code:

int main()
{
  auto file = HighFive::File("issue-77.h5", HighFive::File::Truncate);
  write_uint8(file);

  auto file2 = HighFive::File("issue-77.h5", HighFive::File::ReadOnly);

  auto dtype_class = file2.getDataSet("uint8").getDataType().getClass();
  // dtype_class is DataTypeClass::Integer... but is it signed or unsigned?

Sorry for the confusion...

[Jacobfaib]

The point of this would be to then actually do the reading/writing-to based on this type-erased query:

auto dtype = file2.getDataSet("uint8").getDataType();
auto dtype_class = dtype.getClass();

switch (dtype_class) {
  case DataTypeClass::Integer:
    switch (dtype.getSize()) {
      case 8:
        if (dtype.isSigned()) {
          dset.read<std::vector<std::int8_t>>();
        } else {
          dset.read<std::vector<std::uint8_t>>();
        }
    // ...
}

// Alternatively

switch (dtype_class) {
  case DataTypeClass::SignedInteger:
    switch (dtype.getSize()) {
      case 8:
        dset.read<std::vector<std::int8_t>>();
        // ...
    }
  case DataTypeClass::UnsignedInteger:
    switch (dtype.getSize()) {
      case 8:
        dset.read<std::vector<std::uint8_t>>();
        // ...
    }
  // ...
}

[1uc]

Okay, that makes sense. @Jacobfaib could you please look at #78 and let me know if it solves the issue?

[1uc]

Sounds like it should be fixed now.