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

ctsm5.3.021: Standardize time metadata (we will mark this as the release tag for ctsm5.3) #2052

Merged
merged 39 commits into from
Jan 30, 2025

Conversation

samsrabin
Copy link
Collaborator

@samsrabin samsrabin commented Jul 11, 2023

Description of changes

Standardizes a dimension name of output variable time_bounds (from hist_interval to nbnd), as well as attributes for that plus mcdate, mcsec, mdcur, and mscur.

Specific notes

Contributors other than yourself: @billsacks, @ekluzek

CTSM Issues Fixed:
Resolves #1693.
Fixes #2923
Resolves ESCOMP/MOSART#66
Resolves ESCOMP/RTM#35

Are answers expected to change (and if so in what way)? No.

Any User Interface Changes (namelist or namelist defaults changes)? No.

Testing performed

New metadata in a history file from a short test:

netcdf standardize-time-metadata_rtm.clm2.h0.1850-01-01-00000 {
dimensions:
...
    nbnd = 2 ;
...
variables:
...
    int mcdate(time) ;
        ...
        mcdate:calendar = "noleap" ;
    int mcsec(time) ;
        ...
        mcsec:calendar = "noleap" ;
    int mdcur(time) ;
        ...
        mdcur:calendar = "noleap" ;
    int mscur(time) ;
        ...
        mscur:calendar = "noleap" ;
...
    double time_bounds(time, nbnd) ;
        time_bounds:long_name = "time interval endpoints" ;
        time_bounds:units = "days since 1850-01-01 00:00:00" ;
        time_bounds:calendar = "noleap" ;

All aux_clm tests pass bit-for-bit against ctsm5.1.dev131, except for expected failures.

@samsrabin samsrabin added blocked: dependency Wait to work on this until dependency is resolved next this should get some attention in the next week or two. Normally each Thursday SE meeting. labels Jul 11, 2023
@samsrabin samsrabin self-assigned this Jul 11, 2023
@samsrabin
Copy link
Collaborator Author

samsrabin commented Jul 11, 2023

Marked as blocked:dependency because, at the moment, Externals.cfg points to branches of my personal MOSART (issue ESCOMP/MOSART#53) and RTM (issue ESCOMP/RTM#31) forks that contain the same changes. I haven't yet submitted those as pull requests, because I first wanted to see if any more testing is requested.

UPDATE
PRs ESCOMP/MOSART#66 and ESCOMP/RTM#35

@samsrabin samsrabin added tag: simple bfb and removed next this should get some attention in the next week or two. Normally each Thursday SE meeting. labels Jul 13, 2023
Enable prescribed crop calendars

This branch enables CLM to read in externally-prescribed crop sowing dates and "cultivar" maturity requirements (growing degree-days, GDDs). This has so far only been tested with static values, and the results indicate that yield performance is worsened. However, this capability is required by the GGCMI phase 3 / ISIMIP3 Agriculture protocol.

Briefly, the way this works is that an offline run is first performed with prescribed sowing dates and 364-day seasons. Instantaneous GDD accumulation is saved daily. A Python script then cross-references those daily outputs with a map of mean sowing dates to determine the mean accumulated GDDs in the growing season, saving the result as a file for use as prescribed maturity requirements.
@samsrabin
Copy link
Collaborator Author

From CTSM software engineering meeting today: This doesn't need to wait on other time-related PRs. It can come in as soon as the relevant externals are updated (and Externals.cfg here is changed to point to them).

@samsrabin samsrabin added the PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete label Jan 25, 2024
@samsrabin samsrabin changed the base branch from master to b4b-dev February 15, 2024 16:44
@wwieder wwieder added this to the CESM3 Answer changing freeze milestone Jun 20, 2024
@samsrabin samsrabin added simple bfb bit-for-bit and removed simple bfb labels Aug 8, 2024
@samsrabin samsrabin added good first issue simple; good for first-time contributors and removed simple labels Oct 3, 2024
@ekluzek
Copy link
Collaborator

ekluzek commented Jan 23, 2025

@slevis-lmwg the next ccs_config tag won't help as it's unrelated and something about a change for CLUBB. Doing the bisect sounds like a useful thing to do to me.

@slevis-lmwg
Copy link
Contributor

The following PASS in aux_clm despite issue #2310. I imagine they may still fail once in a while, but I do not know...

@samsrabin @ekluzek @adrifoster do you want me to remove these from EXPECTED? I don't think we covered these in this morning's meeting, did we?

    PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm60Bgc.derecho_gnu.clm-default--clm-NEON-HARV SHAREDLIB_BUILD time=13 (UNEXPECTED: expected FAIL)
    PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm60Bgc.derecho_gnu.clm-NEON-MOAB--clm-PRISM SHAREDLIB_BUILD time=11 (UNEXPECTED: expected FAIL)
    PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm60Bgc.derecho_gnu.clm-NEON-MOAB--clm-PRISM RUN time=91 (UNEXPECTED: expected FAIL)
    PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm60Fates.derecho_gnu.clm-FatesPRISM--clm-NEON-FATES-YELL SHAREDLIB_BUILD time=9 (UNEXPECTED: expected FAIL)
    PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm60Fates.derecho_gnu.clm-FatesPRISM--clm-NEON-FATES-YELL RUN time=102 (UNEXPECTED: expected FAIL)
    PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm60Fates.derecho_intel.clm-FatesFireLightningPopDens--clm-NEON-FATES-NIWO SHAREDLIB_BUILD time=10 (UNEXPECTED: expected FAIL)

The following PASS in fates despite issue #2310. Same question.

    PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm60Fates.derecho_gnu.clm-FatesPRISM--clm-NEON-FATES-YELL SHAREDLIB_BUILD time=15 (UNEXPECTED: expected FAIL)
    PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm60Fates.derecho_gnu.clm-FatesPRISM--clm-NEON-FATES-YELL RUN time=99 (UNEXPECTED: expected FAIL)
    PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm60Fates.derecho_intel.clm-FatesFireLightningPopDens--clm-NEON-FATES-NIWO SHAREDLIB_BUILD time=102 (UNEXPECTED: expected FAIL)

@samsrabin
Copy link
Collaborator Author

The NEON ones sometimes pass—it all depends on how cooperative the data server is being. So they should stay as expected fail.

@ekluzek
Copy link
Collaborator

ekluzek commented Jan 23, 2025

@slevis-lmwg yes leave them as expected fails in case they do fail for someone because of network/server issues. It's a signal to them that they can move on. The longer term solution that @adrifoster is endorsing and I agree -- I added to the new issue #2942. Once we get that going we'd be able to remove the expected fail.

@samsrabin
Copy link
Collaborator Author

RXCROPMATURITY tests are now working!

@slevis-lmwg
Copy link
Contributor

slevis-lmwg commented Jan 25, 2025

RXCROPMATURITY tests are now working!

@samsrabin thank you! Could you tell me the vetted baselines to copy for these tests?

Reconsidering: I will rerun these tests myself due to the ccs_config update shown below. I still would like to point to your baselines for comparison.

I will assume /glade/derecho/scratch/samrabin/tests_0124-161436de unless I hear otherwise.

@ekluzek
Copy link
Collaborator

ekluzek commented Jan 29, 2025

@slevis-lmwg and went over namelist differences as outlined in the Minor version update protocol wiki page with things like:

cd bld/unit_testers
./compare_namelists -b $SCRATCH/ctsm5.3.007/bld/unit_testers/ -pa clm6_0 -pb clm6_0 | & grep -v cnfun_inparm | grep -v upplim_destruct_metamorph | grep -v maximum_leaf_wetted_fraction | grep -v interception_fraction | grep -v paramfile | grep -v clm_canopyhydrology_inparm | grep -v DIFF | grep -v "No case id data available" | grep -v "Namelist diff" | grep -v jmaxb1 | grep -v clm_inparm
./cmp_baseline_lnd_in_files ctsm5.3.007 ctsm5.3.021 | & grep -v cnfun_inparm | grep -v upplim_destruct_metamorph | grep -v maximum_leaf_wetted_fraction | grep -v interception_fraction | grep -v paramfile | grep -v clm_canopyhydrology_inparm | grep -v DIFF | grep -v "No case id data available" | grep -v "Namelist diff" | grep -v jmaxb1 | grep -v clm_inparm | grep -v "hist_"

which highlighted just the namelist differences that were unexpected. We had to do it a few times to figure out what to remove with grep, so unexpected things would pop out. Doing the above helped highlight a few things that we had forgotten about, but realized were expected. And it also helped to ensure we have confidence that the namelist settings are correct and something didn't get changed in a tag to mess them up. This gives us confidence that this tag will be a good starting point for scientists who need to do simulations with a stable version to use with their science. As such it will be marked as a development release tag.

@ekluzek
Copy link
Collaborator

ekluzek commented Jan 29, 2025

@slevis-lmwg I compared the namelists for ctsm_sci ctsm5.3.0 and ctsm5.3.021 baselines and saw the same things that popped out when we looked together. So the completes the namelist validation checking for this tag.

@slevis-lmwg and I also didn't think there was any additional code review that needed to be done by anyone for this. Tags that came in were reviewed and all reasonable ones (in terms of length). And we didn't see anything that seemed likely where a later tag might have messed something up from an earlier one.

@slevis-lmwg slevis-lmwg merged commit fcc3e81 into ESCOMP:master Jan 30, 2025
2 checks passed
@slevis-lmwg slevis-lmwg deleted the standardize-time-metadata branch January 30, 2025 00:09
glemieux added a commit to glemieux/ctsm that referenced this pull request Jan 31, 2025
Standardize time metadata (release tag for ctsm5.3)

 Last of the 3 "history" tags that ended up numbered as follows:
 ctsm5.3.018 time now middle of time_bounds
 ctsm5.3.019 eliminate 0th time step
 ctsm5.3.021 standardize time metadata

 As the release tag for ctsm5.3, this also includes the file WhatsNewInCTSM5.3.md.

 PRs that document the changes
 ESCOMP#2052
 ESCOMP/MOSART#66
 ESCOMP/RTM#35
glemieux added a commit to glemieux/ctsm that referenced this pull request Feb 4, 2025
Standardize time metadata (release tag for ctsm5.3)

 Last of the 3 "history" tags that ended up numbered as follows:
 ctsm5.3.018 time now middle of time_bounds
 ctsm5.3.019 eliminate 0th time step
 ctsm5.3.021 standardize time metadata

 As the release tag for ctsm5.3, this also includes the file WhatsNewInCTSM5.3.md.

 PRs that document the changes
 ESCOMP#2052
 ESCOMP/MOSART#66
 ESCOMP/RTM#35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bfb bit-for-bit branch tag: release Changes go on release branch as well as master documentation additions or edits to user-facing documentation external issue needs to be addressed elsewhere (submodule); issue here for the sake of project tracking PR status: needs testing test: aux_clm Pass aux_clm suite before merging test: ctsm_sci Run and check ctsm_sci suite before merging test: fates Pass fates test suite before merging test: mksurfdata Test mksurfdata_esmf before merging test: python Pass clm_pymods test suite plus Python sys/unit tests before merging test: rivers Test RTM/MOSART/mizuRoute before merging
Projects
Status: Done (non release/external)
Status: Done
4 participants