Updated community support PR #57
Conversation
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.
Merge develop into master for release
Merge findloc fix from develop to master
Merge develop onto master for 2.0 release
Removed gFTL from mandatory Baselibs
# Conflicts: # FindBaselibs.cmake Signed-off-by: Lizzie Lundgren <[email protected]>
Merge develop into master
Merge Develop into Master
…with new GMAO libs CMakeLists.
Merge develop into master
|
Yes - this means there is a missing dependency on gFTL that was being inappropriately obtained via transitivity of a PUBLIC in the past. OTOH, this is pFIO which is pretty much at the bottom of the food chain. I'd have thought it would have the explicit mention. |
|
@LiamBindle We haven't forgotten this. I'm going to do some tests today. Due to all the changes we've made recently there are conflicts, but I've made a first try at merging/resolving those with I'll keep you up to date. |
|
And we have our first error on CMake step: |
|
I was able to sidestep the issue Matt encountered by commenting out an INCLUDE for a completely unrelated directory. But then got lots of shell errors: Apparently due to constructs like: set (BYTERECLEN "\"SHELL:-assume byterecl\"")Reverting Intel.cmake to make progress. |
|
@LiamBindle The expedient solution might be to teach you how to build our GCM and let you trouble shoot. But I'll give it a bit more effort before punting. |
|
The next issue is that GEOS_BuildProperties is not propagating as expected. E.g. , in MAPL_Base, I have updated the link to be: target_link_libraries (${this} PUBLIC ${GEOS_BuildProperties} ${ESMF_LIBRARIES} ${MPI_Fortran_LIBRARIES})(And ... I know the other 2 elements are now obsolete, but one thing at a time.) I still find that H5_HAVE_PARALLEL is not being passed to the compiler and thus breaking the build. |
My bad. I had put "${}" around GEOS_... |
|
I've resolved the "SHELL" issue. Some of the DEBUG flags were not being guarded. |
|
@LiamBindle Unfortunately this SHELL issue propagates into lots of other CMakeLists.txt files where we do things like: set_property(SOURCE rsg3_vinterp.F APPEND_STRING PROPERTY COMPILE_FLAGS "${FREAL8} ${BYTERECLEN} ${EXTENDED_SOURCE}")Which now does not evaluate cleanly. Why was this duplicate arguments issue not plaguing us before? |
|
@tclune @mathomp4 Thanks for the updates. Sorry about this turning into a bit of a project. I'm confused why the missing "SHELL:" prefixes didn't cause problems for you guys before as well. When we omit them we get errors because a lot of first words of multi-word arguments get removed. And currently, we aren't able to build GCHPctm in the Debug build type because these "SHELL:" prefixes haven't been added to all the multi-word debug arguments yet. Perhaps the reason it didn't cause issues for you before is because the compiler flags were actually being set as one big string, rather than a list of strings (e.g. de-duplicator only runs if list length is greater than 1). For example, this set_property(SOURCE rsg3_vinterp.F APPEND_STRING PROPERTY COMPILE_FLAGS "${FREAL8} ${BYTERECLEN} ${EXTENDED_SOURCE}")Appends the string What's the problem with how set_property(SOURCE rsg3_vinterp.F APPEND_STRING PROPERTY COMPILE_FLAGS "${FREAL8} ${BYTERECLEN} ${EXTENDED_SOURCE}")is being evaluated? Can |
|
Also, note that the other way to do what you did above would be set_property(SOURCE rsg3_vinterp.F APPEND PROPERTY COMPILE_FLAGS ${FREAL8} ${BYTERECLEN} ${EXTENDED_SOURCE})which would append |
|
find_package(NetCDF ...) appears to be broken for our build of NetCDF. |
Ok - I'll try that. At least it is cleaner than the kludge I came up with which was to have 2 vars for each such flag. |
|
Spoke too soon. The flags are passed through an ecbuild macro ... |
|
OK - I was able to produce GEOSgcm.x which is a useful milestone. The changes were few but generally undesirable. Will discuss at the telecon this afternoon. And ... it looks like something is wrong with the linkage for the libraries produced with F2PY. Unfortunately the error message is short and useless. Best guess is that it is related to the other wonkiness with NetCDF. |
|
@LiamBindle @tclune The f2py business will be quite twitchy and might need help from both of you. Essentially f2py is not good at...is not good. We already have a very fragile system for it. What I do right now is like this: if (F2PY_FOUND)
if (precision STREQUAL "r4")
add_f2py_module(GFIO_ SOURCES GFIO_py.F90
DESTINATION lib/Python
ONLY gfioopen gfiocreate gfiodiminquire gfioinquire
gfiogetvar gfiogetvart gfioputvar gfiogetbegdatetime
gfiointerpxy gfiointerpnn gfiocoordnn gfioclose
LIBRARIES GMAO_gfio_r4 ${NETCDF_LIBRARIES}
INCLUDEDIRS ${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_BINARY_DIR}/lib ${BASEDIR}/lib ${include_${this}} ${INC_NETCDF}
USE_MPI
)
add_dependencies(GFIO_ ${this})
endif ()
endif (F2PY_FOUND)The main issue is I need something that looks like |
|
Okay. f2py is not due to the include bits. It's a library issue. Before, f2py was seeing the Now though, it's seeing full path static libraries: |
|
I think I have a first good shot at fixing the f2py biz. |
|
@LiamBindle More issues with this. I think we have all the library business taken care of (though maybe not committed). However, the big issue is the flags. Your changes sort of break all of the GCM. The reasons are manifold. First, in the GCM, because we had to preserve zero-diff-ness from CVS, I had to do some not-good-things to the CMake system to get that. So, what I did was construct a string of options that for exactly matched the flags used in GNU/CVS, and then crammed that into CMAKE_Fortran_FLAGS_: set (GEOS_Fortran_FLAGS_DEBUG "${GEOS_Fortran_Debug_Flags} ${common_Fortran_flags} ${GEOS_Fortran_Debug_FPE_Flags} ${ALIGNCOM}")
set (GEOS_Fortran_FLAGS_RELEASE "${GEOS_Fortran_Release_Flags} ${common_Fortran_flags} ${GEOS_Fortran_Release_FPE_Flags} ${ALIGNCOM}")
set (GEOS_Fortran_FLAGS_VECT "${GEOS_Fortran_Vect_Flags} ${common_Fortran_flags} ${GEOS_Fortran_Vect_FPE_Flags} ${ALIGNCOM}")
set (CMAKE_Fortran_FLAGS_DEBUG "${GEOS_Fortran_FLAGS_DEBUG}" CACHE STRING "Debug Fortran flags" FORCE )
set (CMAKE_Fortran_FLAGS_RELEASE "${GEOS_Fortran_FLAGS_RELEASE}" CACHE STRING "Release Fortran flags" FORCE )Moreover, in some directories, because they didn't use these "default" flags in this order, I sometimes had to create new flags: set (CMAKE_Fortran_FLAGS_RELEASE "-O2 ${common_Fortran_flags} ${GEOS_Fortran_Release_FPE_Flags} ${FIXED_SOURCE} ${ALIGNCOM}")and in FV3 and FMS, I have to go to the VECT flags: set (CMAKE_Fortran_FLAGS_RELEASE "${GEOS_Fortran_FLAGS_VECT}")The problem is, with your changes, the CMAKE_Fortran_FLAGS are no longer reset so they are back to the default for CMake: Now I, in my naivety, tried: separate_arguments(CMAKE_Fortran_FLAGS_RELEASE NATIVE_COMMAND "${GEOS_Fortran_FLAGS_RELEASE}")
separate_arguments(CMAKE_Fortran_FLAGS_DEBUG NATIVE_COMMAND "${GEOS_Fortran_FLAGS_DEBUG}")which leads to: Of course, the make goes nuts since these are semicolon separated and they see them as separate commands. I tried a bit of ugliness: set (GEOS_Fortran_FLAGS_RELEASE "${GEOS_Fortran_Release_Flags} ${common_Fortran_flags} ${GEOS_Fortran_Release_FPE_Flags} ${ALIGNCOM}")
separate_arguments(GEOS_Fortran_FLAGS_RELEASE NATIVE_COMMAND ${GEOS_Fortran_FLAGS_RELEASE})
string(REPLACE "SHELL:" "" TMP_RELEASE "${GEOS_Fortran_FLAGS_RELEASE}")
string(REPLACE ";" " " CMAKE_Fortran_FLAGS_RELEASE "${TMP_RELEASE}")which does indeed get but of course this: set (CMAKE_Fortran_FLAGS_RELEASE "-O3 ${common_Fortran_flags} ${ALIGNCOM}")becomes fun because now some of those flags in that string have SHELL:... Any ideas on how to solve some of these issues? |
|
I didn't realize this before, but it seems the deduplicator might only affect properties (i.e. not variables like Is there a reason the If you guys would prefer to continue using the |
|
I'm not sure if my explanation of how I think target properties could be used to set compiler options was clear, so here's a more detailed explanation.
target_compile_options(targetA PUBLIC|PRIVATE ${common_Fortran_flags})or transitively like (below GEOS_BuildProperties' target_link_libraries(targetA PUBLIC|PRIVATE GEOS_BuildProperties)Does this make sense? Let me know if I can clarify anything. |
|
@mathomp4 @tclune I just learned this, and I think it might be of interest to you two. There's a generator expression, |
|
Thanks. I recently went a bit deeper in this direction to enable a situation where my tests are linked with a mock library while the app itself is linked to a real library. Cant's say I find the syntax intuitive though. On a related front, I've had to start making more target_include_directories() use PUBLIC. The latest Intel compiler is now chaining some interface information through secondary .mod files. E.g., if package A produces a.mod, and Package B uses a.mod and produces b.mod, then if Package C uses b.mod it may need the include dirs for A. |
|
Random query for @LiamBindle : is it possible this is now taken care of in MAPL/ESMA_cmake? All the changes and stuff we made for UFS/NOAA seemed to be aimed at "UFS doesn't use Baselibs". |
|
@mathomp4 Yeah, I suspect so. I bet a lot of that work fulfills what we need on our end---I haven't had a chance to rebase and try out all the new work, but coincidentally I am hoping to do that this afternoon. Sorry, I didn't realize this PR was still open. I'll close it. If new things arise, I will open new issues/PR, but I think the contents of this PR are outdated/redundant now. |
|
@LiamBindle Sounds good. Though realize any "no-Baselibs" changes will now also need to work with UFS/NOAA. The "fun" of a multi-organization library! But we'll get there. Thanks for the patience |
Hi everyone,
This is an updated 0-diff structural PR for changes that make MAPL buildable in external projects like GCHPctm (replacing #29, and #31). This PR make it possible for external project to use ESMA_cmake without modification, while preserving ESMA_cmake's existing functionality. Note that currently external projects that use MAPL have to implement equivalent functionality to use MAPL's build system.
The problem is that projects that depend on MAPL have to
include(esma.cmake)to get the macros MAPL and its supporting infrastructure use in their CMakeLists, butinclude(esma.cmake)relies onFindBaselibs.cmakewhich assumes your environment was set up with ESMA-Baselibs (there are hardcoded paths to places where Baselibs installs things).This problem is solvable though because one of the things CMake is best at is resolving dependencies through
find_package()modules.The feature of this PR are:
find_package()callsBaselibsis used to control "GMAO-wide" compiler options, definitions, link libraries, include directories, etc. (you can think of theBaselibstarget as a property sheet).add_definition()andinclude_libraries()which is widely regarded as an anti-pattern in CMake.As for testing, we test GCHPctm's build in >10 environments with combinations of CentOS, Ubuntu, Intel, GCC, OpenMPI, MVAPICH, MPICH, and Intel MPI. I haven't been able to test GEOSgcm's build, but I'm happy to help with any problems that are encountered.
Related issues: #50, #30, #28, #16
Accompanying PRs
Because of that 3rd point (above) there are some minor changes to MAPL and GMAO_Shared that are necessary (mainly "linking" the
Baselibstarget to compiler definitions, options, link libraries, etc. using its transitive usage requirements ). Below are the PRs that accompany this one. Note that these are also 0-diff structural.Thanks! Let me know if there's anything I can do to make merging this PR easier!
Liam