NanoComp / meep

free finite-difference time-domain (FDTD) software for electromagnetic simulations
GNU General Public License v2.0
1.16k stars 596 forks source link

Fixing compile issue if hdf5 not installed #2742

Closed theogdoctorg closed 3 months ago

theogdoctorg commented 6 months ago

When trying to compile from source, if HDF5 is not enabled, the "HAVE_HDF5" identifier is false and certain sections within h5file.cpp are ignored. Some types (H5T_NATIVE_DOUBLE, "H5T_NATIVE_FLOAT) are used within a few h5file::***_chunk functions, and without HDF5, are undefined. This prevents the file from compiling. Added #ifdef blocks to get file to compile without HDF5.

oskooi commented 6 months ago

Note that both _write_chunk(...) and _read_chunk(...) have #if def HAVE_HDF5 macros as their first line:

https://github.com/NanoComp/meep/blob/295fa633cefb016a8ffd52d98e393ae7781e2bc7/src/h5file.cpp#L623-L626

https://github.com/NanoComp/meep/blob/295fa633cefb016a8ffd52d98e393ae7781e2bc7/src/h5file.cpp#L773-L775

This means these functions should compile without HDF5 but will fail at runtime due to:

https://github.com/NanoComp/meep/blob/295fa633cefb016a8ffd52d98e393ae7781e2bc7/src/h5file.cpp#L689-L691

This means that make will likely succeed but make check will fail when running python/tests/test_dump_load.py (which invokes dump_structure and load_structure which in turn invoke structure::dump and structure::load of structure_dump.cpp which in turn invoke _write_chunk and _read_chunk, respectively):

https://github.com/NanoComp/meep/blob/295fa633cefb016a8ffd52d98e393ae7781e2bc7/python/Makefile.am#L61

To get make check to pass even when HDF5 is not installed, we would need to exclude this test.

theogdoctorg commented 6 months ago

For me, "make" did not work. I think it's because "H5T_NATIVE_DOUBLE" has to exist since it is given as an argument for the underscore function, even if _write_chunk has the #ifdef as its first line.

(still new to github, not sure how you do the code snippet preview, but this is line 702 from src/h5file.cpp) void h5file::write_chunk(int rank, const size_t *chunk_start, const size_t *chunk_dims, double *data) { _write_chunk(HID(cur_id), get_extending(cur_dataname), rank, chunk_start, chunk_dims, H5T_NATIVE_DOUBLE, data); }

When HDF5 isn't installed, "H5T_NATIVE_DOUBLE" is undefined since it's not given as an argument to the method, and not defined anywhere else in the Meep files. The compiler still needs to be able to call _write_chunk even if it's an empty function.

stevengj commented 6 months ago

I would rather fix this by editing this block https://github.com/NanoComp/meep/blob/425aa398faf1bf28b21f92c63c94fcb587819aa0/src/h5file.cpp#L48-L50

to add dummy definitions

#  define H5T_NATIVE_FLOAT  0  // ignored
#  define H5T_NATIVE_DOUBLE 0  // ignored

That way, it will still fall through to the error exception if HDF5 is missing.