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

Does GEOSldas still require HDF4? #55

Open
mathomp4 opened this issue Aug 12, 2024 · 6 comments
Open

Does GEOSldas still require HDF4? #55

mathomp4 opened this issue Aug 12, 2024 · 6 comments
Assignees
Labels
question Further information is requested

Comments

@mathomp4
Copy link
Member

A query for @gmao-rreichle. Recently, I've been doing some work to try and get ifx support into GEOS, Baselibs, and the GEOS CI since by the end of this year Intel will be killing ifort as a Fortran compiler.

The main issue at the moment for LDAS is that the FORTRAN 77 interfaces to HDF4 are deprecated1 and even if you force HDF4 to build them (as I do in Baselibs) even that does not work with ifx. So for now in my testing, if you build Baselibs with ifx, HDF4 is not built.

Now, I believe eventually the HDF Group plans to add "back" Fortran support with modern Fortran (see HDFGroup/hdf4#408 by @derobins), so might just be a "temporary" issue.

But perhaps the HDF code in the LDAS (namely it seems to be an AMSR-E obs reader?) isn't needed but just kept for backwards-looking work? If so, then maybe if there is a point in time where we have to move to ifx but HDF4 4.4.0 is not out yet, we can, if nothing else, have CMake check for HDF4 support and if not found, #ifdef out the code?

Footnotes

  1. Note: They might even be removed in HDF4 4.3.0. I haven't actually tried updating and testing that yet. So this issue might be an "all-compiler" issue rather than just "ifx".

@mathomp4 mathomp4 added the question Further information is requested label Aug 12, 2024
@gmao-rreichle
Copy link
Collaborator

@mathomp4, thanks, as always, for keeping on top of these things! Much appreciated. We are not currently using AMSR-E obs, so there is no immediate need for HDF4 compatibility as far as I can tell. The most recent Version 3 of the AMSR-E obs are in *.he5 format, which I assume is version 5 of hdf. (The hdf 5 file extension used in SMAP is *.h5.) If we ever use AMSR-E obs in the future, we'll adapt the reader to use the *.he5 files, so I think we can forget about HDF4 altogether.

@mathomp4
Copy link
Member Author

@gmao-rreichle Ahh. Good to know. I might look at adding in some CMake where "if ifx, don't compile certain code".

Note: We aren't quite ready for ifx yet. I think GEOSgcm v11 is unhappy with it, but GEOSgcm v12 does like it, so we are close. We also have MAPL internal tests failing with ifx as well, so a ways off...but hopefully not too long. oneAPI 2025 will be ifort-free.

@gmao-rreichle
Copy link
Collaborator

PS: I suppose we'll need a PR that disables the (hdf4) AMSR-E reader? Since the hdf4 reader will be useful if/when we convert it to h5, we should just comment it out and add a note, rather than delete the reader altogether.

@mathomp4
Copy link
Member Author

@gmao-rreichle I talked with @tclune and I might do something simple in CMake like:

option(ENABLE_HDF4_SUPPORT "Enables build of HDF4 code" OFF)

where we default to OFF and then if that is enabled by the user, set an #ifdef and try to compile the code.

I'll try and test something like that to see. Once I get something to build, I might ask @biljanaorescanin to do a quick test to make sure all runs.

@gmao-rreichle
Copy link
Collaborator

Thanks, @mathomp4. But once we use the newer compiler, wouldn't the build necessarily fail when the user turns OFF to ON? In other words, the user option would only work for as long as we're using the current compilers, right? If that's correct, then we would have to remember to remove the switch again once we upgrade to the new compiler. It may be simpler to just disable hdf4 support for good now by commenting out the offending code with an #if 0 statement.

@mathomp4
Copy link
Member Author

Thanks, @mathomp4. But once we use the newer compiler, wouldn't the build necessarily fail when the user turns OFF to ON? In other words, the user option would only work for as long as we're using the current compilers, right? If that's correct, then we would have to remember to remove the switch again once we upgrade to the new compiler. It may be simpler to just disable hdf4 support for good now by commenting out the offending code with an #if 0 statement.

Hmm. Yeah, you are probably right. Simpler at least :) I'll give that a try with ifx ...

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

No branches or pull requests

2 participants