search

thliebig / openEMS

openEMS is a free and open-source electromagnetic field solver using the EC-FDTD method.
http://openEMS.de
GNU General Public License v3.0
459 stars 156 forks source link

modified h5readatt_octave.cc to conform to new HDF5v1.8+ API #21

Closed georgmichel closed 8 years ago

georgmichel commented 8 years ago

tested with libhdf5-10 (v1.8.15-3.9) + Octave 4.0.0 and libhdf5-100 (v1.10.0-38.1) + Octave 4.2.0-rc2 on OpenSUSE Leap 42.1

georgmichel commented 8 years ago

made an additional change on 09/27/16 at 11:30 a.m. This implies that hdf5 v1.10 is required.

thliebig commented 8 years ago

Hi, can you explain why you removed the version checks and depend on v1.10? I think we should not do this as many major Distros still only have hdf5 1.8.x. For example RHEL 6 and 7. Even Ubuntu 16.04 LTS still ships 1.8.16 and that means that I cannot build this myself. I recommend to have more #ifdef checking for 1.10 if necessary.

regards Thorsten

georgmichel commented 8 years ago

You're right. This was a particular problem on OpenSUSE 42.1 when upgrading to Octave 4.2.0-rc2 from the science repository since 4.0.0 had some nasty bugs which won't be fixed anymore. In 4.0.3 HDF5 was broken and it is not planned to be fixed. 4.2.0-rc2 is linked against HDF5 1.10. Therefore the requirement for this version. Currently I am also in contact with the octave developers because of a bug in 4.2.0-rc2 which spoils the working directory after the compilation of h5readatt. Nevertheless, restarting octave is a simple workaround for this as it must be compiled only once.

I will try to make a version of h5readatt with #ifdefs which is compatible with both versions. However, the next three weeks I am unavailable.

georgmichel commented 8 years ago

You can omit the requirement for HDF5v1.10 in the CMakeLists.txt. It is not necessary. The patched hdf5readatt_octave.cc also works with HDF5v1.8 if openEMS is compiled against the same version as Octave, either 1.8 or 1.10 (tested on OpenSUSE 42.1).

thliebig commented 8 years ago

I pulled your first two commits and omitted the in/out of hdf5 1.10 commits