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

next round of HDF4 separation from libsrc4 #900

Closed

Conversation

edhartnett
Copy link
Contributor

@edhartnett edhartnett commented Mar 13, 2018

In this PR the HDF4-specific fields in NC_HDF5_FILE_INFO_T and NC_VAR_INFO_T are replaced with void pointers, and the HDF4 metadata are handled entirely within the code of libhdf4. These void pointers will (after #856) also be used to hold HDF5-specific metadata.

Also in this PR, generic functions (inspired by the NCDEFAULT_ functions) are used for cases when netCDF-4 is not supported by a dispatch layer (NC_NOTNC4_), or when a dispatch layer is read-only (NC_RO_), or doesn't support the base_pe functions (NC_NOTNC3_). These generic functions will also be used in user-defined formats, so users don't have to re-implement these in every dispatch layer.

These new generic functions are used in the pnetcdf dispatch layer, and the NOTNC3 ones in libsrc4. Many of the read-only ones can be used in the DAP code, but I have not tried that.

Also added is a long section to the documentation, describing the generic dispatch functions, and explaining the HDF4 dispatch layer as the simplest-case example of implementing a dispatch layer.

The final evolution of HDF4 capability will be after this PR is merged, and #870 is complete, when the build system can be changed to allow building with HDF4, even when HDF5 is not present.

Fixes #871.
Fixes #870.
Part of #177.
Part of #856.

@edhartnett
Copy link
Contributor Author

The 3 travis failures are due to the Unidata ftp server timing out on the HDF4 file requests:

--2018-03-13 16:56:30--  ftp://ftp.unidata.ucar.edu/pub/netcdf/sample_data/hdf4/MYD29.A2009152.0000.005.2009153124331.hdf.gz
  (try:20) => 'MYD29.A2009152.0000.005.2009153124331.hdf.gz'
Connecting to ftp.unidata.ucar.edu (ftp.unidata.ucar.edu)|128.117.149.10|:21... connected.
Logging in as anonymous ... Logged in!
==> SYST ... done.    ==> PWD ... done.
==> TYPE I ... done.  ==> CWD (1) /pub/netcdf/sample_data/hdf4 ... done.
==> SIZE MYD29.A2009152.0000.005.2009153124331.hdf.gz ... 2429720
==> PASV ... done.    ==> RETR MYD29.A2009152.0000.005.2009153124331.hdf.gz ... 
Error in server response, closing control connection.
Giving up.

@WardF
Copy link
Member

WardF commented Mar 13, 2018

Thanks Ed; I'll rerun the tests manually once the IT group finishes the work they're doing.

@DennisHeimbigner
Copy link
Collaborator

You might consider modifying the shell script that does the download
to retry some number of times before failing.

@edhartnett
Copy link
Contributor Author

@DennisHeimbigner good suggestion. I will do that...

@DennisHeimbigner
Copy link
Collaborator

Ed- it appears that the hdf4 code uses code from libsrc4 (e.g. nc4_rec_grp_del).
However, configure.ac does not appear to reflect the dependency that hdf4
requires netcdf-4. Am I missing something?

@edhartnett
Copy link
Contributor Author

edhartnett commented Mar 14, 2018

The HDF4 code still depends (as it always has) on HDF5 being installed and netCDF-4 enabled. This is ensured in configure.ac:

# Does the user want to turn on HDF4 read ability?
AC_MSG_CHECKING([whether reading of HDF4 SD files is to be enabled])
AC_ARG_ENABLE([hdf4], [AS_HELP_STRING([--enable-hdf4],
              [build netcdf-4 with HDF4 read capability (HDF4, HDF5 and zlib required)])])
test "x$enable_hdf4" = xyes || enable_hdf4=no
if test "x$enable_hdf4" = xyes -a "x$enable_netcdf_4" = xno; then
      AC_MSG_ERROR([NetCDF-4 is required for HDF4 features])
fi
AC_MSG_RESULT($enable_hdf4)

I have a working prototype where the HDF5 code is split from the libsrc4 code (that is, the HDF5 code is in libhdf5, and libsrc4 contains all the internal metadata model stuff). After your changes to the libsrc4 directory get merged, I will update my prototype with your changes, and put up another PR finally sepatating HDF5 from libsrc4.

After that, I can change the configure.ac to allow HDF4 even when HDF5 is not present. But for now, HDF4 continues to require HDF5.

@edhartnett edhartnett changed the title final round of HDF4 changes next round of HDF4 separation from libsrc4 Mar 14, 2018
@edhartnett
Copy link
Contributor Author

I put in the retries for the HDF4 file fetch, as Dennis suggested. Works fine and now travis passes. Perhaps it was a case of too many tests trying to get the files at the same time. Now there is a random delay if the wget doesn't work.

@edhartnett
Copy link
Contributor Author

OK, looks like I spoke too soon. There were still some ftp failures. I have tried another solution...

@edhartnett
Copy link
Contributor Author

What ended up working was adding the --passive-ftp option to the wget command. There's a good explanation here: https://stackoverflow.com/questions/1699145/what-is-the-difference-between-active-and-passive-ftp

@edhartnett
Copy link
Contributor Author

I will take this down until @DennisHeimbigner's refactoring PR is merged. I have combined index.dmh and my HDF4 work, so I have some working code ready to go.

@edhartnett edhartnett closed this Apr 2, 2018
@edhartnett edhartnett deleted the ejh_hdf4_more branch May 14, 2018 14:14
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