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

Stop building esmf_wrf_timemgr for unit tests; instead use the actual ESMF library #2375

Open
billsacks opened this issue Feb 16, 2024 · 4 comments · May be fixed by #2853
Open

Stop building esmf_wrf_timemgr for unit tests; instead use the actual ESMF library #2375

billsacks opened this issue Feb 16, 2024 · 4 comments · May be fixed by #2853
Labels
testing additions or changes to tests

Comments

@billsacks
Copy link
Member

In making the changes for #2374 , I noticed that the unit test build still builds the esmf_wrf_timemgr to satisfy the ESMF dependency of clm_time_manager.F90. At some point we should probably stop doing that and instead have the unit tests link against the actual ESMF library.

See

add_subdirectory(${CLM_ROOT}/share/src/esmf_wrf_timemgr esmf_wrf_timemgr)

and some related code in that file.

I briefly started down this path but couldn't see how to get the value of ESMFMKFILE for this machine/compiler in the CMake build, so I gave up for now.

Note that we could probably also remove this block of code: I assume that was put in before we needed to include ESMF stuff in the unit test build, but now we do because of the unit tests of clm_time_manager:

CTSM/src/CMakeLists.txt

Lines 57 to 73 in c030c23

# Remove shr_cal_mod from share_sources.
#
# shr_cal_mod depends on ESMF (or the lightweight esmf wrf timemgr, at
# least). Since CTSM doesn't currently use shr_cal_mod, we're avoiding
# the extra overhead of including esmf_wrf_timemgr sources in this
# build.
#
# TODO: like above, this should be moved into a general-purpose function
# in Sourcelist_utils. Then this block of code could be replaced with a
# single call, like: remove_source_file(${share_sources}
# "shr_cal_mod.F90")
foreach (sourcefile ${share_sources})
string(REGEX MATCH "shr_cal_mod.F90" match_found ${sourcefile})
if(match_found)
list(REMOVE_ITEM share_sources ${sourcefile})
endif()
endforeach()

@billsacks billsacks added priority: low Background task that doesn't need to be done right away. testing additions or changes to tests labels Feb 16, 2024
@ekluzek ekluzek added the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Feb 16, 2024
@samsrabin
Copy link
Collaborator

Decision in SE meeting 2024-03-21: This is a good thing for post-CESM3.

@samsrabin samsrabin removed the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Mar 21, 2024
@ekluzek
Copy link
Collaborator

ekluzek commented Jun 6, 2024

@jedwards4b is recommending that we use ESMF VM constructs to replace things like shr_mpi as a more general way of doing things that allow for a move to a different distributed memory. See this comment here.

ESCOMP/MOSART#94 (comment)

The reason that relates to this is that any unit-testing that uses those ESMF VM objects doing this would be *required".

@ekluzek ekluzek removed the priority: low Background task that doesn't need to be done right away. label Jun 19, 2024
@ekluzek ekluzek added this to the ctsm6.0.0 (code freeze) milestone Jun 19, 2024
@ekluzek
Copy link
Collaborator

ekluzek commented Jun 19, 2024

In the CSEG meeting we talked about removing the esmf_wrf_timemgr in a future cesm3_0 beta tag. Doing this would break our unit-testing unless this issue were fixed. Since the unit-testing is critical this will need to be fixed as part of the cesm3_0 release. Because of that I removed the low priority label here, and added the ctsm6.0.0 code freeze milestone.

@ekluzek
Copy link
Collaborator

ekluzek commented Jun 20, 2024

It looks like @jedwards4b has something that should work here. It will at least require an update of cesm_share, and possibly cime. This will enable CESM to deprecate and remove the ESMF wrf timemgr code. So we can likely bring this in and make sure it works at our next externals update.

@adrifoster

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing additions or changes to tests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants