Use CMake variables/names in our cmake macros #6014
Conversation
These got replaced by config_machines.xml
|
|
A few thoughts:
if nothing else, it would reduce the number of vars floating around.
|
|
@bartgol , thanks for looking at this.
I agree. This would be a nice upgrade.
Unfortunately, I think we must keep this one. mct and mpi-serial have an autoconf-based config system. The recent changes make it so autoconf is executed directly from the python build wrappers (buildlib.mct for example) instead of the wrapper invoking Tools/Makefile which then invokes autoconf.
I have no idea why this feature was added. It goes back many years. Is it possible that fortran linkers struggle to link CXX object files some times? We could remove this and see if all our main machines still work.
We are already doing a CMake build of Kokkos (we used to use the generate_makefile stuff, but I changed that years ago). I agree we should move towards setting the actual Kokkos vars that Kokkos understands eventually. I don't know if I want to go there now.
Good question. Once again, this feature goes back many years so I don't know why it was introduced and if it is really needed.
We don't use Tools/Makefile for anything anymore.
I am sure there's a better way to do this. None of the sharedlibs will be using raw Makefiles by the time this build refactor is done. |
|
Ok, I think we can take steps toward the final goal. I am fine merging this PR, but I would strongly encourage to follow up on
Finally, it would be nice to contribute to MCT providing a CMake build system. Since the repo is owned by ANL (I see rob as a contact for the MCSclimate org on github), we can probably find someone that can explain what the configure needs are (if the output of configure itself is too cryptic) and craft a simple CMakeLists.txt (no need to handle installation via pkg, just a simple build is enough probably). |
|
I'm the only MCT developer and I had a plan to learn Cmake by adding a build for MCT but never had time. |
|
@rljacob , we currently interface to the MCT build system through our python wrapper buildlib.mct. It would be slightly more convenient to do this if MCT were a CMake package, but it's really not a big problem. |
| @@ -11,8 +11,8 @@ set(SCXX "clang++") | |||
| set(SFC "flang") | |||
|
|
|||
| string(APPEND CMAKE_Fortran_FLAGS " -Mflushz ") | |||
| string(APPEND FIXEDFLAGS " -Mfixed") | |||
| string(APPEND FREEFLAGS " -Mfreeform") | |||
| string(APPEND CMAKE_Fortran_FORMAT_FIXED_FLAG " -Mfixed") | |||
There was a problem hiding this comment.
Nice. This is a CMake supported/handled variable, right?
There was a problem hiding this comment.
Yes. I think CMake can probably handle this automatically but I'm not 100% on that so I kept it.
| list(APPEND E3SM_CMAKE_INTERNAL_VARS_BEFORE_BUILD_INTERNAL_IGNORE "${VAR_AFTER}") | ||
| set("E3SM_CMAKE_INTERNAL_${VAR_AFTER}" "${${VAR_AFTER}}") | ||
| elseif (NOT "${${VAR_AFTER}}" STREQUAL "${E3SM_CMAKE_INTERNAL_${VAR_AFTER}}") | ||
| message("CIME_SET_MAKEFILE_VAR ${VAR_AFTER} := ${${VAR_AFTER}}") |
There was a problem hiding this comment.
Are these messages supposed to stay, or are they here just for debugging purposes while you are doing the port?
There was a problem hiding this comment.
They need to stay. This hacky stuff is how we translate processed cmake macros into Makefile macros. Nothing we have is using the Makefile macros directly anymore, but they are still useful for querying macro vars which we do in a few places when "crossing" build systems (cmake -> autoconf for example) in some of the sharelib builds.
…w away things set up by the project command
…_in_macros * origin/master: (156 commits) Avoid calling add_default when no generic values present for gw_convect_hcf and hdepth_scaling_factor Reset hdepth_scaling_factor for MMF config Add settings to re-tune QBO, kPOM, and additional hyperviscosity modifier to allow use of rough topography Make "column_package" the default column_physics_type for BGC Add build-namelist default for chemdyg vertical indicies Add namelist definitions of UCIgaschmbudget_2D_L* remove unnecessary chemdyg settings in user_nl_eam for tests remove default chemdyg vertical indices in F90 code Add default chemdyg vertical indices in xml Update default chemdyg vertical indices in xml Update bld files to match Registry changes, using automated scripts Move some description changes from bld files to Registry Homme: Fix config.h comment style. Homme: Distinguish between F90 and C++ in HOMMEXX_ENABLE_GPU symbol use. Homme: Comment out two builds that aren't used in ctests. Hommexx: Fix issues related to qsize=0. Hommexx: Remove redundant HOMMEXX_ENABLE_GPU definition. Homme: Fix potential bad access to an array. Homme: Add USE_MPI_RUN_SCRIPT to standalone system. Homme(xx)/SL: Add SL-transport feature to doubly periodic mode. ...
…_in_macros * origin/master: Add muller machine Address few reviewer comments Second attempt at fixing bmatrix equation Fixes one more equation Fixes eqnarray for Github Changes how equations are defined Updates references Adds labels to equations and cross-references Adds mathjax.js for equation numbering add mkdocs-bibtex to pip install Initial commit of TOP parameterization documentation adds syntax highlighting Minor updates Adds notes on running e3sm land developer testsuite
|
@jgfouca is this ready? Please fix the title. |
|
@rljacob , yes, this has been ready for a while but I was waiting for the dashboard to get into better shape. This is going to be a disruptive PR. |
jgfouca
changed the title
…_in_macros * origin/master: Homme/SL: Address a warning. Homme: Fix latitude in dcmip2016_test1_forcing. Minor updates to ELM tech doc. Homme: Add gllfvremap_planar_ut unit test. Homme(xx): Set up C++-vs-F90 standalone planar pg2 test. Homme: Add planar capability to gllfvremap_mod. Homme: Modify dcmip2016_test1_pg_forcing to handle planar case. Change minimum concentration and mass of sea ice velocity solution Add single missing variable to OMP private Use small pe layout for TSC test Add size 'S' pes for chrysalis ne4 tests
Use CMake variables/names in our cmake macros This should be the last major PR in CMAKE V2 effort. Changes Summary: * Macro structure changes * Add post-process step to set final values for a few items * Make cmake conversion more robust by storing pre-macro values and comparing instead of just looking at new variables * Change all macros to set variables that cmake knows about! This is a huge change with wide impacts * We no longer have to set flag properties on all files. This simplifies things but makes it hard to support e3sm_remove_flags so that feature has been removed. You should always be able to append flags to achieve the same effect as removing a flag. * Try to remove as much of the linker flag micromanagement as possible * We now default to cxx linker. Intel and PGI still have to use fortran unfortunately since our main function is in fortran * Remove as many non-cmake settings as possible from the macros * Add script for converting macros from the old style to new style * Upgrade the compare-flags tool to support parsing the e3sm.bldlog file. This is a tougher way to compare flags since you have to build the cases with -j 1, but it's more robust than looking at cmake internal files. * Remove lots of unneeded stuff from main cmake files (build_model and common_setup) * Remove unused old makefiles for COSP and MPAS * Move gptl build to cmake Testing: * mappy_gnu: e3sm_developer, with full flag compare on a couple big cases * anlgce: e3sm_developer builds * pm-cpu_intel: e3sm_developer, with full flag compare on one big case * chrysalis_intel: e3sm_developer, with full flag compare on one big case * summit: build one big case with pgigpu, ibmgpu, pgi [BFB]
|
Merged to next. |
|
Is it going to change answers? Or just break some builds somewhere? |
|
@rljacob , it should be BFB. I tested it the best I could reasonably do so on my own, but it's a major change so I'm sure something will break somewhere. |
This should be the last major PR in CMAKE V2 effort.
Here are the list of items from the macros remaining that are not cmake style:
${CPRE}USE_CONTIGUOUS=contiguous. Added for flags for .F90 files. There must be a better way to do this?-lstdc++and-cxxlib. There is probably a better way to do this. Just add to CMAKE_EXE_LINKER_FLAGS?Other todo:
Changes Summary:
e3sm_remove_flagsso that feature has been removed. You should always be able to append flags to achieve the same effect as removing a flag.mainfunction is in fortran-j 1, but it's more robust than looking at cmake internal files.Testing: