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

Using find_package() in FindBaselibs to support generic build environments #29

Conversation

LiamBindle
Copy link

@LiamBindle LiamBindle commented Sep 18, 2019

Hi everyone,

This is a proposed fix for #28.

Note: This proposed change is not expected to change the default behavior of ESMA_cmake. If it does, I probably missed something!

Proposed change

I would propose to use find_package() calls in FindBaselibs.cmake, rather than hardcoded paths relative to BASEDIR. These find_package() calls can resolve ESMA's dependencies in generic build environments, and the precedence of BASEDIR can be preserved by putting the hardcoded BASEDIR paths in CMAKE_PREFIX_PATH. I have added find-modules for ESMF, GFTL, and FLAP so that they can be found with find_package(). It looks like config-modules for fArgParse, pFUnit, and GFTL_SHARED come with their installations, so they should already be findable with find_package().

Special consideration

With this proposed fix, however, something else that should be considered is that not all ESMA dependencies are always necessary. For example, in GCHP, FLAP, fArgParse, GFTL_SHARED, and MKL are not necessary. Therefore, we need a mechanism for changing the "requiredness" of the dependencies that are found when esma.cmake is loaded. To do this, I added cache variables that control whether or not the "REQUIRED" argument is passed to find_package().

This is related to #26. For example, to make GFTL_SHARED quiet you would make a cache entry for GFTL_SHARED_IS_REQUIRED_ARG and set its value to QUIET.

See geoschem/gchp_ctm/CMakeLists.txt for an example of loading esma.cmake and building ESMA libraries in an external project.

Open questions

  • Is libesmf_fullylinked.a the library GEOS-ESM needs to link on Apple machines? If so, I can add this to FindESMF.cmake.
  • I made some guesses at what I think BASEDIR must look like based on the previous FindBaselibs.cmake. Are libraries in ${BASEDIR}/lib, binaries in ${BASEDIR}/bin, and then include directories in ${BASEDIR}/include/<library name>? If so, I think I set CMAKE_PREFIX_PATH properly. Otherwise, I'll have to update CMAKE_PREFIX_PATH.

Also, I'll be at GMAO on September 25-27, and I'd be happy to talk about this PR then.

Let me know if you have any questions, comments, concerns, or if any clarification is needed.

Liam

Changed FindBaselibs.cmake to use find_package() calls rather than
hardcoded paths relative to BASEDIR. Added find-modules for ESMF,
GFTL, and FLAP. Added mechanism for disabling the requiredness of ESMA
dependencies.
@mathomp4
Copy link
Member

@LiamBindle

This might take us a while to digest, but thanks for the work! As for your questions:

  • Is libesmf_fullylinked.a the library GEOS-ESM needs to link on Apple machines? If so, I can add this to FindESMF.cmake.

No. On Linux, we get libesmf.a, libesmf.so, and libesmf_fullylinked.so. On macOS, it is libesmf.a, libesmf.dylib, and libesmf_fullylinked.dylib. (I'm ignoring libesmftrace_preload.so/dylib and libesmftrace_static.a from newer ESMFs as we don't use them...yet)

But, I think for reasons @tclune might know better than I, we use libesmf_fullylinked.so on Linux, but macOS works best with libesmf.a. Probably because of RPATH or SIP or something. macOS + dylib is a fun fun time.

  • I made some guesses at what I think BASEDIR must look like based on the previous FindBaselibs.cmake. Are libraries in ${BASEDIR}/lib, binaries in ${BASEDIR}/bin, and then include directories in ${BASEDIR}/include/<library name>? If so, I think I set CMAKE_PREFIX_PATH properly. Otherwise, I'll have to update CMAKE_PREFIX_PATH.

Yeeeeeeees...sort of. For example, this is what a Baselibs directory setup looks like:

bin/
etc/
FARGPARSE-0.8/
GFTL-1.1/
GFTL_SHARED-1.0/
include/
lib/
pFUnit/
sbin/
share/

For most of the libraries, yes, the include files are in ${BASEDIR}/include/<package_name>, the bin are in ${BASEDIR}/bin, and the libs are in ${BASEDIR}/lib. But for components from @tclune's GFE--fArgParse, gFTL, gFTL-Shared, pFUnit--they are built with CMake so they have CMake like libraries:

GFTL-1.1/
├── cmake
└── include
    ├── templates
    └── types

That's why we do find_package(GFTL REQUIRED). Now I agree that fArgParse and gFTL-shared should have QUIET for now. They aren't needed (yet), so no need to worry if not found.

@mathomp4
Copy link
Member

Oh, and I want to ping @aerorahul who I think might be doing something similar with Baselibs for JEDI.

@mathomp4
Copy link
Member

And one more thing. My guess is is that find_package(NetCDF) might not work with GEOS, though I need to try. We use nf-config --flibs because Baselibs builds NetCDF with so many libraries, that it can be hard to get the library line right for dependencies. For example, we build NetCDF with OpenDAP support, so we need to let the model know how to link to libcurl.

But @aerorahul has been struggling with Baselibs so he might have more information.

@aerorahul
Copy link

And one more thing. My guess is is that find_package(NetCDF) might not work with GEOS, though I need to try. We use nf-config --flibs because Baselibs builds NetCDF with so many libraries, that it can be hard to get the library line right for dependencies. For example, we build NetCDF with OpenDAP support, so we need to let the model know how to link to libcurl.

But @aerorahul has been struggling with Baselibs so he might have more information.

We have an updated version of FindNetCDF.cmake that relies on nc-config to return the libraries and includes. The FindNetCDF.cmake also returns the library line correctly, and we have tested it with a Baselibs installation. @kgerheiser has a copy of the updated FindNetCDF.cmake If not, you can find one here. The caveat is this has been customized for use with ecbuild. If you are not using ecbuild, you will have to replace ecbuild_debug lines with message.

@tclune
Copy link
Collaborator

tclune commented Sep 19, 2019

Just to add to the history a bit here. The existing FindBaselibs was from my very early experience with CMake and was basically a kludge to allow us to proceed. Knowing better now, it is mostly a matter of prioritizing when to implement things the right/canonical way.

As @aerorahul indicates, a proper FindNetCDF.cmake is a big step in that direction.

Regarding ecbuild_debug(), rather than replacing, perhaps we should conditionally define a local macro that implements ecbuild_debug() as message(). If we use ecbuild that logic would be ignored, but other GEOS users can still ...

@LiamBindle
Copy link
Author

@mathomp4

On Linux, we get libesmf.a, libesmf.so, and libesmf_fullylinked.so. On macOS, it is libesmf.a, libesmf.dylib, and libesmf_fullylinked.dylib.

Sounds good. I can add this to FindESMF.cmake. Does your Baselibs only build one of these libraries? Or, how do you determine which one you are going to link? For example, on Linux, how do you select between libesmf.a, libesmf.so, and libesmf_fullylinked.so?

@mathomp4, @tclune

We have an updated version of FindNetCDF.cmake that relies on nc-config to return the libraries and includes. The FindNetCDF.cmake also returns the library line correctly, and we have tested it with a Baselibs installation. @kgerheiser has a copy of the updated FindNetCDF.cmake If not, you can find one here.

This FindNetCDF.cmake is ahead of GEOS-ESM/ecbuild:feature/FindMKL-portability-improvement's by JCSDA/ecbuild#1. I see that GEOS-ESM/ecbuild:release/stable has this updated FindNetCDF.cmake too though.

Is there a reason ESMA_cmake:develop's ecbuild external is on GEOS-ESM/ecbuild's feature/FindMKL-portability-improvement branch rather than release/stable? If the external needs to stay on feature/FindMKL-portability-improvement, then JCSDA/ecbuild#1 could be merged into feature/FindMKL-portability-improvement to update FindNetCDF.cmake to the version mentioned by @aerorahul.

@tclune
Copy link
Collaborator

tclune commented Sep 20, 2019

@LiamBindle Regarding your last question: at the time we started this, there were some OS X and ifort portability issues that they had not anticipated. E.g., if you have just Intel Fortran then the MKL that is provided does not contain the C interfaces (which is odd, but true). But since one can obtain a full MKL for free, it should not be a real concern at this time.

If GEOS can build with release/stable, I have no problem making that change.

@tclune
Copy link
Collaborator

tclune commented Sep 20, 2019

(And ignore my comment on the other branch.)

@LiamBindle
Copy link
Author

Hi everyone. This PR is superseded by #57.

@LiamBindle LiamBindle closed this Feb 7, 2020
@LiamBindle LiamBindle deleted the feature/support-generic-build-environments branch February 7, 2020 21:19
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.

4 participants