ctsm5.3.041: Merge b4b-dev to master 2025-04-24 #3090
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ESMF was being repeatedly initialized in unittestTimeManagerMod.F90, which is an error. This led to unit tests failing silently. This commit fixes the issue by checking if ESMF is already initialized before calling ESMF_Initialize, and not calling ESMF_Finalize at the end of each unit test. Currently this skips any calls to ESMF_Finalize, as it is an error to re-call ESMF_Initialize after calling ESMF_Finalize. A better solution would be to only call ESMF_Initialize and ESMF_Finalize once per unit test executable. I am looking into whether that's possible. I'm not sure if it will cause any problems to not do the ESMF_Finalize. See #3015 (comment) for details. Resolves #3015
Without this change, the unit tests of clm_time_manager were aborting due to trying to destroy an ESMF clock that hadn't been initialized. Other pieces of this reset routine don't appear to be causing problems, but it seems most robust to just skip the whole thing if the time manager hasn't been initialized.
There is a behavior difference between the real ESMF library and the ESMF WRF Timemgr we had been using previously, in how prevTime is defined: the ESMF WRF Timemgr defined prevTime using a -dtime offset from currTime; the real ESMF library instead defines prevTime by tracking the previous value of currTime. Therefore, now that we are using the real ESMF library, if a unit test relies on prevTime (as is the case, for example, with test_year_position_start_of_timestep in test_dynTimeInfo.pf), it is now important that any setting of the time be done in a way such that there is a call to ESMF_ClockAdvance, since that is the only way that prevTime will get set correctly. Thus, this commit updates the implementation of for_test_set_curr_date so that it involves a call to ESMF_ClockAdvance. Making this change in the implementation of for_test_set_curr_date also required some other changes in some unit test code: - I needed to remove the call to unittest_timemgr_set_curr_date at the end of unittest_timemgr_setup. This call now leads to an increment of the time step count, which was causing failures in some time manager tests that check the time step count. And it turns out that (at least with the other changes here), the call to unittest_timemgr_set_curr_date was unnecessary: it wasn't actually leading to any change in time from what was set in the initialization of the time manager. - I needed to change some set_date calls to use a year of 2 instead of 1. This is needed for calls setting the date to January 1 and the time to 0: It is no longer allowed to set the date to yr,mon,day,tod = 1,1,1,0, because the new method sets the date/time to one time step earlier and then advances the clock, and it is illegal to set the date/time to one time step before 1,1,1,0.
'stop' calls were leading to pFUnit passes instead of fails. Need to either have shr_sys_abort (which raises a pFUnit exception) or stop with a non-zero integer return value (which causes the program to stop with an error code).
…aramfile in the case directory" This reverts commit 0e6eed5.
Also improve an error message.
Now a failed pylint check should actually be marked as failing.
Avoid repeated ESMF initialization in unittestTimeManagerMod.F90
Add GitHub workflows to check black, pylint, git-fleximod
fix some st_archive issues with rpointer files
samsrabin
changed the title
github-project-automation
bot
moved this to Ready to start (or start again)
in CTSM: Upcoming tags
samsrabin
moved this from Ready to start (or start again)
to In progress - release/externals / MOSART / RTM / mizu etc. tags
in CTSM: Upcoming tags
samsrabin
moved this from In progress - release/externals / MOSART / RTM / mizu etc. tags
to In progress - master
in CTSM: Upcoming tags
github-project-automation
bot
moved this from In progress - master
to Done (non release/external)
in CTSM: Upcoming tags
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No merge branch needed because it's already up-to-date with the latest master tag (ctsm5.3.040). There will thus also be no need for a merge of master back to b4b-dev.