F2I-Consulting / fesapi

API for ENERGISTICS™ data standards (mainly RESQML™), multi-languages (C++, Java, C#, Python)
Apache License 2.0
34 stars 24 forks source link

Error when compiling with HDF5 1.10 #76

Closed hmageste closed 5 years ago

hmageste commented 5 years ago

This version of the compiler is complaining about the different signature of the method 'readArrayNdOfValues' in src/common/HdfProxy.h and the correspondent cpp file.

Anyway, the issue can be fixed using the same typedef defined in H5Ipublic.h: typedef int64_t hid_t;

Obviously the signature of all 'readArrayNdOfValues' needs to be changed accordingly: from: void readArrayNdOfValues(const std::string & datasetName, void values, const int & datatype); to: void readArrayNdOfValues(const std::string & datasetName, void values, const hid_t & datatype);

Do you want me to send a pull request for it? Or you don't plan to support this version of Visual Studio now?

philippeVerney commented 5 years ago

Oï Henrique,

Obrigado for your feeedback However, I think the proposed solution cannot be sufficient.

Indeed, H5Ipublic.h does not always declare typedef int64_t hid_t. For example, on my current HDF5 library (hdf5-1.8.18-win64-noszip-vs2013), H5Ipublic.h does declare typedef int hid_t which explains why I don't see your problem. The same declaration is in H5Ipublic.h of hdf5-1.8.19-Std-win7_32-vs2013 and of hdf5-1.8.21-Std-win7_64-vs14. Your problem may not be caused by your compiler but caused by the HDF5 library version you use. Maybe you use HDF5 1.10 which is not officialy supported https://github.com/F2I-Consulting/fesapi (see readme) because RESQML does not officialy support it.

A nice fix would be to automatically modify the signature (at cmake time) of the readArrayNdOfValues according to the associated H5Ipublic.h which can be different among the different HDF5 library versions.

I keep this issue opened untill a PR is proposed. It does not look as a high priority but it may change. Could you indicate please in this issue the HDF5 library version you use?

Thanks

hmageste commented 5 years ago

Wow, right at the spot!

You are right, I am currently using 1.10.1 (this is the version my whole company is using).

I tend to always blame visual studio because, on each new release version, it happens to cause some problem in my code (warnings treated as error being one of the worst ones :( ).

philippeVerney commented 5 years ago

OK then I close this issue but am happy to keep it archived when RESQML will officially support HDF5 1.10.

If you want to apply a workaround (if you don't want or cannot go back to HDF 1.8) then I advise you to modify the signature of the methods to use "long long" or "int64_t" instead of int. This will probably be what I am going to do when HDF5 1.10 will be officially supported by RESQML (and consequently by Fesapi too). I prefer not using hid_t in .h cause of potentially harder support with SWIG.

cgodkin commented 5 years ago

Hi Philippe & Henrique,

For what it's worth (and for our future selves), it looks like the first HDF5 1.10 version that we can use is 1.10.2 which supports specifying the minimum supported read version to H5F_LIBVER_V18 with H5Pset_libver_bounds(). They accidentally left this support out of 1.10.1.

My company will upgrade to 1.10.3 (unless there is a .4 by then) near the end of the year. I will see about updating fesapi to support 1.10.2+ at that point if no one else has done it.

By adding a H5Pset_libver_bounds() property to output file creation, it should finally be possible to write 1.8-compatible HDF5 files with newer versions of the library.

This is discussed in the release notes for 1.10.2 https://portal.hdfgroup.org/display/support/HDF5%201.10.2 if you search down for "H5F_libver_t"

philippeVerney commented 5 years ago

Thanks Carl for this update.

I changed the issue title to better identify it. I reopened it in order it is more visible if other users have the same idea (at least you are two!).

philippeVerney commented 5 years ago

Fixed in 380676b63609789c9983a5385a316ea99044c102