Fix tests incompatitle with L80 and restore tentative config options for SSP #6054

wlin7 · 2023-11-06T03:24:10Z

The PR fixes two sets of tests.

SSP tests with tentative fix from # 5965 stops working. The EAM%CMIP6 generic pattern match needs to be removed to have it work. This is due to the -vbs option that would be set if having the generic pattern match. The additive style of EAM configure option would then have no mechansim to disable vbs once set.
Update the vertical dimension for 3D variables that condidiag tests track.

[BFB] Only the above mentioned tests will diff; and the the differences for these tests
following this fix are due to #5996, and other went in on the same day.

…for SSP

github-actions · 2023-11-06T03:25:27Z

PR Preview Action v1.4.4
🚀 Deployed preview to https://E3SM-Project.github.io/E3SM/pr-preview/pr-6054/
on branch `gh-pages` at 2023-11-14 02:43 UTC

whannah1 · 2023-11-06T15:45:52Z

cime_config/testmods_dirs/allactive/wcprodssp/user_nl_eam

@@ -11,3 +11,6 @@
 fincl5 = 'PRECT','PRECC','TUQ','TVQ','QFLX','SHFLX','U90M','V90M'
 fincl6 = 'CLDTOT_ISCCP','MEANCLDALB_ISCCP','MEANTAU_ISCCP','MEANPTOP_ISCCP','MEANTB_ISCCP','CLDTOT_CAL','CLDTOT_CAL_LIQ','CLDTOT_CAL_ICE','CLDTOT_CAL_UN','CLDHGH_CAL','CLDHGH_CAL_LIQ','CLDHGH_CAL_ICE','CLDHGH_CAL_UN','CLDMED_CAL','CLDMED_CAL_LIQ','CLDMED_CAL_ICE','CLDMED_CAL_UN','CLDLOW_CAL','CLDLOW_CAL_LIQ','CLDLOW_CAL_ICE','CLDLOW_CAL_UN'
 fincl7 = 'O3', 'PS', 'TROP_P'
+
+! Specify an L80 IC to override eam.i from reference case, which is still for L72
+ ncdata = '$DIN_LOC_ROOT/atm/cam/inic/homme/eami_mam4_Linoz_ne30np4_L80_c20231010.nc'


Would it be better for me to generate a L80 version of the file from the reference case?
This is very easy to do, but would still be non-BFB.

Was it already from a reference case for v3atm? I thought you did that. If not yet, yes, it would be nice to have. Note this is the same file you created for #5996.

yea, I guess it's fine to use this file. I'm just worried we will forget that this override is in place, but that's probably harmless.

If we are moving to L80, the default ncdata should work and not need special namelist overrides.

It sounds like this test requires a reference case (which makes sense) which is not L72. That's why the override is used. The alternative is to remap the reference case data, but then we might still need an override to make sure the correct initial condition file is used (unless we hide the L72 version where CIME won't see it).

Yes, the SSP tests are using a reference case (a restart point of a historical run). Since we don't have v3 historical with L80, and the reference case currently used is from v2, an override is needed for SSP tests to proceed in current code base. Much about the SSP tests will be dated once all are settled.

Ok reference case hasn't been made yet. Go ahead then with the override.

components/eam/cime_config/config_component.xml

whannah1 · 2023-11-06T15:52:19Z

components/eam/cime_config/testdefs/testmods_dirs/eam/condidiag_dcape/user_nl_eam

@@ -3,7 +3,7 @@
 qoi_chkpt = 'RAD', 'PACEND','DYNEND','DEEPCU', 'STCLD',

 qoi_name = 'CAPE','dCAPE'
- qoi_nver =  1,     72,
+ qoi_nver =  1,     80,


To make this more robust we should change this so that the the levels default to something like -1, and then internally make sure they get set to 1 and pver. If that's an acceptable alternative I'm happy to implement it. Same goes for the two changes below.

Agreed setting to something like -1 and inside the code to resolve it to pver would be more elegant. That said, for users who do want to use this condidiag tool, it is probably more straightforward for them to specify the actual # of model levels they run with.

right, but if most of the time they just want the upper and lower limit of the vertical levels then having it set internally makes sense, and the edge case where a user wants something else can be handled by editing these.

Hi @huiwanpnnl , the condidiag related tests failed at runtime for 80-level configurations. Reseting qoi_nver to 80 from 72 will allow the tests to run. Should qoi_nver value always be the same as the vertical dimension size of the corresponding diagnostic variable?

Hi @wlin7 and @whannah1, I agree it will be an improvement to let qoi_nver and metric_nver default to -1 in the source code and then, after the namelist has been read in, reset -1 to pver. Only a few lines need to be changed in components/eam/src/physics/cam/conditional_diag.F90:

Replace 0 by -1 in this line and this line.

After this end do, add where ( metric_nver(1:ncnd) == -1 ) metric_nver(1:ncnd) = pver

Before this if-statement, add where ( qoi_nver(1:nqoi) == -1 ) qoi_nver(1:nqoi) = pver

After these updates, the 72 or 80 in the two usr_nl_eam files can be changed to -1.

Does this sound good to you? Hope it works. I'd be happy to let either of you implement this. Thanks!!

Thank you, @huiwanpnnl . Your suggested changes are straightforward. I am going to put them in. BTW, would it be better to keep the initialization/default value (0) unchanged at line 219 and 228, as the new implementation would interpret -1 as pver. That is just to avoid confusion as the where statement has strict upper bound in the array scope.

@wlin7, your suggestion sounds good to me. In that scenario, -1 will be interpreted as an intentional alias for pver, while 0 will indicate an unset value.

Exactly, @huiwanpnnl, thanks.. index -1 as used in accessing the last element of an array in some languages. This is mainly for pre-defined testing cases. Users can still specify the actual number of levels in practical use, which would be equivalent. The changes are in ee14d67 and verified working well.

wlin7 · 2023-11-07T03:42:06Z

@whannah1 , thank you for generating L80 ne120np4 IC. It is added to this PR. After this and #6053, the nightly hires test should be good then.

huiwanpnnl

The changes for CondiDiag look good to me. Thanks!

whannah1

Looks good!

Fix tests incompatitle with L80 and restore tentative config options for SSP The PR fixes two sets of tests. . SSP tests with tentative fix from # 5965 stops working. The EAM%CMIP6 generic pattern match needs to be removed to have it work. This is due to the -vbs option that would be set if having the generic pattern match. The additive style of EAM configure option would then have no mechansim to disable vbs once set. . Update the vertical dimension for 3D variables that condidiag tests track. [BFB] Only the above mentioned tests will diff; and the the differences for these tests following this fix are due to #5996, and other went in on the same day.

wlin7 · 2023-11-16T04:08:19Z

Merged to next.

Fix tests incompatitle with L80 and restore tentative config options …

336123b

…for SSP

wlin7 requested a review from whannah1 November 6, 2023 03:36

wlin7 self-assigned this Nov 6, 2023

wlin7 added Atmosphere bug fix PR BFB PR leaves answers BFB labels Nov 6, 2023

whannah1 reviewed Nov 6, 2023

View reviewed changes

components/eam/cime_config/config_component.xml Show resolved Hide resolved

whannah1 reviewed Nov 6, 2023

View reviewed changes

Add default IC (ncdata) for ne120np4 with L80

ae74855

wlin7 added 2 commits November 9, 2023 08:54

Replace ne11_oQU240 WCYCL1850NS tests with ne4

4951e07

Change to use ne4pg2_oQU480 for WCYCL1850NS tests

49e6d68

wlin7 requested a review from huiwanpnnl November 13, 2023 22:21

Implement an adaptable index as pver in condidiag nml

ee14d67

huiwanpnnl approved these changes Nov 14, 2023

View reviewed changes

whannah1 approved these changes Nov 14, 2023

View reviewed changes

wlin7 mentioned this pull request Nov 16, 2023

Move h2osno limiter to ensure positive snow depth. #6034

Merged

wlin7 merged commit d2ffe91 into master Nov 16, 2023
2 checks passed

wlin7 deleted the wlin/atm/fix_tests_with_L80 branch November 16, 2023 20:27

mkstratos mentioned this pull request Dec 19, 2023

Run failure in MVK test with F2010 compset, ne4_oQU240 grid #6123

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix tests incompatitle with L80 and restore tentative config options for SSP #6054

Fix tests incompatitle with L80 and restore tentative config options for SSP #6054

wlin7 commented Nov 6, 2023 •

edited

Loading

github-actions bot commented Nov 6, 2023 •

edited

Loading

whannah1 Nov 6, 2023

wlin7 Nov 6, 2023

whannah1 Nov 6, 2023

rljacob Nov 6, 2023

whannah1 Nov 6, 2023

wlin7 Nov 7, 2023

rljacob Nov 7, 2023

whannah1 Nov 6, 2023 •

edited

Loading

wlin7 Nov 7, 2023

whannah1 Nov 7, 2023

wlin7 Nov 13, 2023

huiwanpnnl Nov 13, 2023

wlin7 Nov 13, 2023

huiwanpnnl Nov 14, 2023

wlin7 Nov 14, 2023

wlin7 commented Nov 7, 2023

huiwanpnnl left a comment

whannah1 left a comment

wlin7 commented Nov 16, 2023

Fix tests incompatitle with L80 and restore tentative config options for SSP #6054

Fix tests incompatitle with L80 and restore tentative config options for SSP #6054

Conversation

wlin7 commented Nov 6, 2023 • edited Loading

github-actions bot commented Nov 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whannah1 Nov 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wlin7 commented Nov 7, 2023

huiwanpnnl left a comment

Choose a reason for hiding this comment

whannah1 left a comment

Choose a reason for hiding this comment

wlin7 commented Nov 16, 2023

wlin7 commented Nov 6, 2023 •

edited

Loading

github-actions bot commented Nov 6, 2023 •

edited

Loading

whannah1 Nov 6, 2023 •

edited

Loading