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

Add option for CRUJRA2024 add some compsets for Clm60 for it as well as a corresponding compset for Clm5 #2956

Draft
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

slevis-lmwg
Copy link
Contributor

@slevis-lmwg slevis-lmwg commented Feb 6, 2025

Description of changes

Change the default datm input from GSWP3v1 to CRUJRA2024 for Clm6.
Add a CRUJRA2024 compset for clm5.

See #1895 for definition of DONE.

Specific notes

Contributors other than yourself (I hope I didn't forget any):
@wwieder @adrifoster @swensosc @djk2120 @ekluzek @olyson

CTSM Issues Fixed (include github issue #):
Resolves #1895

Are answers expected to change (and if so in what way)?
No. We will make a separate answer-changing PR to address Clm6 tests that do not specify the datm input, so that they get CRUJRA2024 instead of GSWP3v1.

Any User Interface Changes (namelist or namelist defaults changes)?
Adding Crujra to compset names.
Adding 2 new tests that explicitly include Crujra in their compset names.
Clm60 tests that do not include a datm option in their compset will default to Crujra in a later PR.

Does this create a need to change or add documentation? Did you do so?
It does, and I did not, yet.

Testing performed, if any:
See relevant posts below.

@slevis-lmwg slevis-lmwg self-assigned this Feb 6, 2025
@slevis-lmwg slevis-lmwg added enhancement new capability or improved behavior of existing capability priority: high High priority to fix/merge soon, e.g., because it is a problem in important configurations usability Improve or clarify user-facing options PR status: needs testing non-bfb Changes answers (incl. adding tests) labels Feb 6, 2025
@slevis-lmwg slevis-lmwg added this to the ctsm5.4.0: CMIP7 Datasets milestone Feb 6, 2025
@slevis-lmwg
Copy link
Contributor Author

First attempt at testing:
./create_test ERP_P128x2_Ld30.f45_f45_mg37.I2000Clm60FatesSpCrujraRsGs.derecho_intel.clm-FatesColdSatPhen

@slevis-lmwg
Copy link
Contributor Author

slevis-lmwg commented Feb 7, 2025

Second set of tests on derecho:

OK make black & lint (the latter gives two warnings)
PASS python
PASS ./build-namelist_test.pl
OK ./run_sys_tests -s aux_clm -c ctsm5.3.021 --skip-generate
OK? ./run_sys_tests -s aux_cdeps -c /glade/campaign/cesm/cesmdata/cseg/cesm_baselines/cdeps1.0.55 --skip-generate

Checked the last two suites with these commands:

./cs.status.fails | grep -v PASS | grep -v NLCOM | grep -v '21: DIF'
./cs.status.fails | grep -v NLCOMP

In aux_cdeps, this test fails here but also in vanilla ctsm5.3.021:

SMS_D_Ld3.f10_f10_ais8gris4_mg37.DATAMODELTEST.derecho_gnu
ERROR: Invalid compset name, DATAMODELTEST, all stub components generated

@slevis-lmwg
Copy link
Contributor Author

3rd round to test the "clm5" changes

PASS ./build-namelist_test.pl
PASS ./create_test ERP_P128x2_Ld30.f45_f45_mg37.I1850Clm50BgcCropCrujra.derecho_intel.clm-default

@slevis-lmwg
Copy link
Contributor Author

slevis-lmwg commented Feb 8, 2025

TODOs

  • @wwieder suggests making t232 the default grid with the introduction of the CRUJRA compsets in this PR if it's "dead simple"):
    Erik suggests we use t232 just for the new CRUJRA compset tests for now, because we don't have all the grid aliases for the grids we need for CTSM testing; @slevis-lmwg checked the testlist, and we already have 10 ne30pg3_t232 tests, which may cover our needs, and I will confirm with Erik
  • Go over with @ekluzek
  • Update to latest on master here and in cdeps
  • Submit tests

@ekluzek ekluzek changed the title Make CRUJRA2024 the default datm input for Clm6 plus add a corresponding compset for Clm5 Add option for CRUJRA2024 add some compsets for Clm60 for it as well as a corresponding compset for Clm5 Feb 11, 2025
Copy link
Collaborator

@ekluzek ekluzek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slevis-lmwg and I met and we went over the things to do and there's a conversation for them.

Since, we aren't doing the answer changing part here -- this could be put onto b4b-dev. What do you think about that @slevis-lmwg?

@slevis-lmwg
Copy link
Contributor Author

slevis-lmwg commented Feb 12, 2025

on izumi IN PROGRESS with the revisions only (did not update to ctsm5.3.024, yet)
./run_sys_tests -s aux_clm -c ctsm5.3.021 --skip-generate

UPDATE
Next revert my last cdeps commit, update to ctsm5.3.024 and share1.1.8, and start aux_clm + aux_cdeps again.

@slevis-lmwg
Copy link
Contributor Author

slevis-lmwg commented Feb 12, 2025

derecho testing started before 0fad468 commit to my cdeps branch

OK ./run_sys_tests -s aux_cdeps -c /glade/campaign/cesm/cesmdata/cseg/cesm_baselines/cdeps1.0.55 --skip-generate
OK ./run_sys_tests -s aux_clm -c ctsm5.3.024 --skip-generate

derecho failures now resolved

    FAIL FUNITCTSM_P1x1.f10_f10_mg37.I2000Clm50Sp.derecho_intel RUN time=67
    FAIL SMS_D.f10_f10_mg37.I2000Clm60BgcCrop.derecho_nvhpc.clm-crop RUN time=29

The FUNIT test passes with vanilla 024; a subsequent commit fixes this failure.
The nvhpc test in 024 failed in the SHAREDLIB_BUILD phase, so I will ignore this failure.
Also cmip6 tests are DIFF because we agreed to change them to CRUJRA2024 (unless I misunderstood and should have waited for the answer-changing PR to be merged and tagged at a later time).


izumi testing started after last commit to my cdeps branch
OK ./run_sys_tests -s aux_clm -c ctsm5.3.024 --skip-generate
...other than the usual long list of "failed to init" and the two other tests that always require manual handling. I will not proceed with this busywork since I'm not generating a baseline right now.


derecho additional testing started after the changes to subset_data
PASS python, black, lint
PASS ./build-namelist_test.pl

Change choice of pressure in CLMU building energy model

In the CLMU Building Energy Model (BEM), the choice of pressure for calculating indoor air density is changed from standard pressure to bottom layer of atmosphere pressure of the corresponding grid cell.

Answers change.

PR ESCOMP#2758
Issue ESCOMP#2755
@slevis-lmwg
Copy link
Contributor Author

slevis-lmwg commented Feb 13, 2025

@jedwards4b I get this unexpected FAIL that I have attributed (after testing) to the share1.1.8 update:
FUNITCTSM_P1x1.f10_f10_mg37.I2000Clm50Sp.derecho_intel RUN

Compare the first (failing one) against the second (passing baseline) in /glade/derecho/scratch/slevis:
FUNITCTSM_P1x1.f10_f10_mg37.I2000Clm50Sp.derecho_intel.C.20250213_155030_houfdw
FUNITCTSM_P1x1.f10_f10_mg37.I2000Clm50Sp.derecho_intel.C.20250213_154606_o5ajm0

In run/funit.log this message may be the explanation:
ld: /glade/work/slevis/git/LMWG_dev8/share/src/shr_log_mod.F90:148: undefined reference to esmf_logerrmod_mp_esmf_logwrite_

Shall I let you resolve this?

@@ -571,7 +571,11 @@ def setup_files(args, defaults, cesmroot):
abort("inputdata directory does not exist")

# DATM data
datm_type = "datm_gswp3"
# TODO issue #1895: allow datm_crujra, which also affects
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change TODO to say "make this option available at the command line"

Copy link
Contributor Author

@slevis-lmwg slevis-lmwg Feb 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other TODOs

  • Update .gitmodules when cdeps tag is ready
  • Finish ChangeLog/Sum

@jedwards4b
Copy link
Contributor

@slevis-lmwg It looks like this unit test is not currently linking esmf. I suggest you check with @billsacks

@@ -202,7 +227,7 @@
</compset>
<compset>
<alias>I1850Clm60BgcCropCmip6</alias>
<lname>1850_DATM%GSWP3v1_CLM60%BGC-CROP-CMIP6DECK_SICE_SOCN_MOSART_SGLC_SWAV</lname>
<lname>1850_DATM%CRUJRA2024_CLM60%BGC-CROP-CMIP6DECK_SICE_SOCN_MOSART_SGLC_SWAV</lname>
Copy link
Contributor Author

@slevis-lmwg slevis-lmwg Feb 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ekluzek did I jump the gun here and in other cmip compsets? Should I have waited to do this in the to-be-done-later answer-changing PR?

If so, make sure I have a reminder in the corresponding issue (#2961?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement new capability or improved behavior of existing capability non-bfb Changes answers (incl. adding tests) PR status: needs testing priority: high High priority to fix/merge soon, e.g., because it is a problem in important configurations usability Improve or clarify user-facing options
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add CRU-JRA as option for datm inputs for CTSM development
3 participants