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

Compilation without HDF5 #67

Merged
merged 11 commits into from
Nov 2, 2021
Merged

Compilation without HDF5 #67

merged 11 commits into from
Nov 2, 2021

Conversation

q-posev
Copy link
Member

@q-posev q-posev commented Nov 1, 2021

Introduced optional calls to HDF5 back end using the #ifdef HAVE_HDF5 pre-processor macro, which value is defined by configure script in config.h file.

By default, ./configure proceeds to search for the HDF5 library. To disable this, provide --without-hdf5 argument. For example,

./configure --without-hdf5

@q-posev q-posev requested a review from scemama November 1, 2021 11:29
Makefile.am Outdated
src/trexio_text.c \
src/trexio_text.h

if DO_HDF5
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you change it to HAVE_HDF5 to be consistent with config.h?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Makefile.am Outdated
tests/overwrite_all_text \
tests/io_all

if DO_HDF5
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

configure.ac Outdated
@@ -119,6 +115,8 @@ AC_SUBST([PKG_HDF5])
AC_SUBST([PKG_CFLAGS])
AC_SUBST([PKG_LIBS])

AM_CONDITIONAL([DO_HDF5],[test "$with_hdf5" = "yes"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

integer(trexio_backend), parameter :: TREXIO_HDF5 = 0
integer(trexio_backend), parameter :: TREXIO_TEXT = 1
integer(trexio_backend), parameter :: TREXIO_TEXT = 0
integer(trexio_backend), parameter :: TREXIO_HDF5 = 1
! integer(trexio_backend), parameter :: TREXIO_JSON = 2
integer(trexio_backend), parameter :: TREXIO_INVALID_BACK_END = 2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here the Fortran is inconsistent with the C for the invalid back-end. The simplest would be to keep the TREXIO_HDF5=1 even if HDF5 is not present.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Also reverted TREXIO_HDF5 to 0 and TREXIO_TEXT to 1 since no need to swap them now.

@@ -739,9 +754,11 @@ trexio_open(const char* file_name, const char mode,
result_tmp = malloc(sizeof(trexio_text_t));
break;

#ifdef HAVE_HDF5
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, you can do:

  case TREXIO_HDF5:
#ifdef HAVE_HDF5
    result_tmp = malloc(sizeof(trexio_hdf5_t));
    break;
#else
   if (rc_open != NULL) *rc_open = TREXIO_INVALID_ARG_3;
   return NULL;
#endif

and keep TREXIO_HDF5=1

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Also added new exit code TREXIO_BACK_END_MISSING for better error handling.

call system('rm -rf test_write_f.h5')

call test_read_void('test_write_f.h5', TREXIO_HDF5)
! No way to conditionally check whether compilation was done with HDF5
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can make a function:

bool trexio_has_backend(int back_end) {
  switch (back_end) {
    case TREXIO_TEXT:
      return true;
    case TREXIO_HDF5:
#ifdef HAVE_HDF5
      return true;
#else
      return false;
#endif
  }
  return false;
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this function to both C and Fortran APIs and to the corresponding tests.

@scemama scemama merged commit 20f06d6 into master Nov 2, 2021
@scemama scemama deleted the optional-hdf5 branch January 20, 2022 12:27
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.

2 participants