search

TRIQS / h5

A high-level C++ interface to the hdf5 library
https://triqs.github.io/h5
Other
3 stars 7 forks source link

Documentation and clean up #22

Closed Thoemi09 closed 4 months ago

Thoemi09 commented 5 months ago

This PR is very similar to the one from itertools and mpi. The following things have been done:

I think we should consider either restricting or improving the h5::array_interface. Right now, it is kept quite general but only works for specific cases, e.g. contiguous data/views, hyperslabs with blocks of size (1, 1, ..., 1) and so on. It probably would be good to only support our specific use cases and otherwise throw an exception.

hmenke commented 3 months ago

I'm getting lots of compiler errors when compiling 1.3.0 and it points to this change.

This is the first error, but there are more of the same kind

/home/abuild/rpmbuild/BUILD/triqs-3.3.0/_build/deps/h5_src/c++/h5/array_interface.cpp: In function 'h5::dataspace h5::array_interface::{anonymous}::make_mem_dspace(const h5::array_interface::array_view&)':
/home/abuild/rpmbuild/BUILD/triqs-3.3.0/_build/deps/h5_src/c++/h5/array_interface.cpp:46:77: error: invalid conversion from 'const long long unsigned int*' to 'const hsize_t*' {aka 'const long unsigned int*'} [-fpermissive]
   46 |       dataspace dspace = H5Screate_simple(v.slab.rank(), v.parent_shape.data(), nullptr);
      |                                                          ~~~~~~~~~~~~~~~~~~~^~
      |                                                                             |
      |                                                                             const long long unsigned int*
In file included from /mpcdf/soft/SLE_15/packages/haswell/hdf5/gcc_13-13.1.0/1.14.1/include/H5Ppublic.h:32,
                 from /mpcdf/soft/SLE_15/packages/haswell/hdf5/gcc_13-13.1.0/1.14.1/include/hdf5.h:35,
                 from /home/abuild/rpmbuild/BUILD/triqs-3.3.0/_build/deps/h5_src/c++/h5/array_interface.cpp:26:
/mpcdf/soft/SLE_15/packages/haswell/hdf5/gcc_13-13.1.0/1.14.1/include/H5Spublic.h:303:55: note:   initializing argument 2 of 'hid_t H5Screate_simple(int, const hsize_t*, const hsize_t*)'
  303 | H5_DLL hid_t H5Screate_simple(int rank, const hsize_t dims[], const hsize_t maxdims[]);
      |                                         ~~~~~~~~~~~~~~^~~~~~

My guess is that this doesn't surface in CI, because that still uses GCC 12.

hmenke commented 3 months ago

Hm, I'm also getting other errors llike the following. It looks like h5 relies on an old and deprecated API of HDF5.

https://docs.hdfgroup.org/hdf5/v1_14/group___h5_o.html#ga96ce408ffda805210844246904da2842

Deprecated: As of HDF5-1.12 this function has been deprecated in favor of the function H5Oget_info_by_name2() or the macro H5Oget_info_by_name.

/home/abuild/rpmbuild/BUILD/triqs-3.3.0/_build/deps/h5_src/c++/h5/group.cpp: In function 'herr_t h5::get_group_elements_name_ds(hid_t, const char*, const H5L_info2_t*, void*)':
/home/abuild/rpmbuild/BUILD/triqs-3.3.0/_build/deps/h5_src/c++/h5/group.cpp:167:37: error: too few arguments to function 'herr_t H5Oget_info_by_name3(hid_t, const char*, H5O_info2_t*, unsigned int, hid_t)'
  167 |     herr_t err = H5Oget_info_by_name(loc_id, name, &object_info, H5P_DEFAULT);
      |                                     ^
In file included from /mpcdf/soft/SLE_15/packages/haswell/hdf5/gcc_13-13.1.0/1.14.1/include/H5Apublic.h:21,
                 from /mpcdf/soft/SLE_15/packages/haswell/hdf5/gcc_13-13.1.0/1.14.1/include/hdf5.h:22,
                 from /home/abuild/rpmbuild/BUILD/triqs-3.3.0/_build/deps/h5_src/c++/h5/group.cpp:24:
/mpcdf/soft/SLE_15/packages/haswell/hdf5/gcc_13-13.1.0/1.14.1/include/H5Opublic.h:544:15: note: declared here
  544 | H5_DLL herr_t H5Oget_info_by_name3(hid_t loc_id, const char *name, H5O_info2_t *oinfo, unsigned fields,
      |               ^~~~~~~~~~~~~~~~~~~~
/home/abuild/rpmbuild/BUILD/triqs-3.3.0/_build/deps/h5_src/c++/h5/group.cpp: In function 'herr_t h5::get_group_elements_name_grp(hid_t, const char*, const H5L_info2_t*, void*)':
/home/abuild/rpmbuild/BUILD/triqs-3.3.0/_build/deps/h5_src/c++/h5/group.cpp:175:37: error: too few arguments to function 'herr_t H5Oget_info_by_name3(hid_t, const char*, H5O_info2_t*, unsigned int, hid_t)'
  175 |     herr_t err = H5Oget_info_by_name(loc_id, name, &object_info, H5P_DEFAULT);
      |                                     ^
/mpcdf/soft/SLE_15/packages/haswell/hdf5/gcc_13-13.1.0/1.14.1/include/H5Opublic.h:544:15: note: declared here
  544 | H5_DLL herr_t H5Oget_info_by_name3(hid_t loc_id, const char *name, H5O_info2_t *oinfo, unsigned fields,
      |               ^~~~~~~~~~~~~~~~~~~~
/home/abuild/rpmbuild/BUILD/triqs-3.3.0/_build/deps/h5_src/c++/h5/group.cpp: In function 'herr_t h5::get_group_elements_name_ds_grp(hid_t, const char*, const H5L_info2_t*, void*)':
/home/abuild/rpmbuild/BUILD/triqs-3.3.0/_build/deps/h5_src/c++/h5/group.cpp:183:37: error: too few arguments to function 'herr_t H5Oget_info_by_name3(hid_t, const char*, H5O_info2_t*, unsigned int, hid_t)'
  183 |     herr_t err = H5Oget_info_by_name(loc_id, name, &object_info, H5P_DEFAULT);
      |                                     ^
/mpcdf/soft/SLE_15/packages/haswell/hdf5/gcc_13-13.1.0/1.14.1/include/H5Opublic.h:544:15: note: declared here
  544 | H5_DLL herr_t H5Oget_info_by_name3(hid_t loc_id, const char *name, H5O_info2_t *oinfo, unsigned fields,
      |               ^~~~~~~~~~~~~~~~~~~~
Thoemi09 commented 3 months ago

I don't get any errors with gcc 13.2 and and HDF5 1.14.3 but this looks like an HDF5 version problem.

You can try to compile with H5_USE_110_API. This might fix the second issue.

I am not sure about the first issue. We define h5::hsize_t to be uint64_t which should be the same as in the HDF5 header. Could you check in H5Public.h how they define it there?

Anyway, it would definitely be a good idea to get rid of deprecated function calls.

Thoemi09 commented 3 months ago

Oh sorry, I just saw that we already define H5_USE_110_API in the h5 CMakeLists.txt file. It should be using H5Oget_info_by_name1 and not H5Oget_info_by_name3.