Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing compile issue if hdf5 not installed #2742

Closed
wants to merge 4 commits into from

Conversation

theogdoctorg
Copy link
Contributor

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
Copy link
Collaborator

oskooi commented Dec 16, 2023

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

meep/src/h5file.cpp

Lines 623 to 626 in 295fa63

static void _write_chunk(hid_t data_id, h5file::extending_s *cur, int rank,
const size_t *chunk_start, const size_t *chunk_dims, hid_t datatype,
void *data) {
#ifdef HAVE_HDF5

meep/src/h5file.cpp

Lines 773 to 775 in 295fa63

static void _read_chunk(hid_t data_id, int rank, const size_t *chunk_start,
const size_t *chunk_dims, hid_t datatype, void *data) {
#ifdef HAVE_HDF5

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

meep/src/h5file.cpp

Lines 689 to 691 in 295fa63

#else
meep::abort("not compiled with HDF5, required for HDF5 output");
#endif

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):

$(TEST_DIR)/test_dump_load.py \

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

@theogdoctorg
Copy link
Contributor Author

theogdoctorg commented Dec 16, 2023

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
Copy link
Collaborator

I would rather fix this by editing this block

meep/src/h5file.cpp

Lines 48 to 50 in 425aa39

#else
typedef int hid_t;
#endif

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants