Implement alternating control of Fortran P3 codes for eam and scream #5496

wlin7 · 2023-03-03T03:29:01Z

The alternating switch, determined by the case configuration option (a scream case or else), serves to control which version of Fortran P3 codes being used to build the model. A test is needed to track scream configuration. The version for the EAM version is currently just a skelton, which will be replaced by the eventual P3 PR from v3atm

[BFB] Two new tests SMS_Ln9_P24x1.ne4_ne4.FDPSCREAM-ARM97
and SMS_Ld1.ne30pg2_ne30pg2.F2010-SCREAM-LR-DYAMOND2

The alternating switch, determined by the case configuration option (a scream case or else), serves to control which version of Fortran P3 codes being used to build the model. A test is needed to track scream configuration. The version for the EAM version is currently just a skelton, which will be replaced by the eventual P3 PR from v3atm [BFB] A new test added.

wlin7 · 2023-03-03T03:33:58Z

The following tests verified BFB between master and this branch, with one using mg2 (but p3 codes built in) and the other with scream p3.

SMS_Ld1.ne30pg2_EC30to60E2r2.F2010.cori-knl_intel
SMS_Ld1.ne30pg2_ne30pg2.F2010-SCREAM-LR-DYAMOND2.cori-knl_intel

When reviewing, please ignore files in eam/src/physics/p3/eam. Files there are merely skeltons extracted from the actual p3_PR, and will be replaced entirely when the p3_PR is merged.

rljacob

looks good!

crterai

Thanks for creating this, @wlin7. It looks good to me, especially given that your tests with SCREAM compset and non-P3 compsets come back BFB with master before merging.

PeterCaldwell · 2023-03-06T22:36:42Z

cime_config/tests.py

@@ -210,6 +210,7 @@
            "SMS_D_Ld1.T62_oEC60to30v3.DTESTM",
            "SMS_D_Ld3.T62_oQU120.CMPASO-IAF",
            "SMS_D_Ld1.ne30pg2_r05_EC30to60E2r2.WCYCL1850",
+            "SMS_Ln5.ne30pg2_ne30pg2.F2010-SCREAM-LR-DYAMOND2",


I'd like to see a DP test as well.

DP test added to the developer test suite.

DP = "doubly-periodic" ? Hard to tell from the name that its that kind of domain.

PeterCaldwell · 2023-03-06T22:38:56Z

components/eam/bld/configure

@@ -1752,6 +1752,12 @@ if (defined $opts{'cppdefs'}) {
 }
 $cfg_ref->set('cppdefs', $usr_cppdefs);

+# define is_scream_config flag to select which P3 version codes to use
+my $is_scream_config = 0;
+if ($usr_cppdefs =~ /\-DSCREAM/) {


Sanity check question for @jgfouca or @bartgol or someone in the know - does -DSCREAM in cppdefs mean that the simulation is either a SCREAM F90 or a SCREAM C++ run, or does it just imply one of these 2 options?

Good catch!

IIRC, that's used only by Homme. But I think we never tested it for SCREAMv0, so I am not sure what it would produce there. Grepping in Homme folder, all but a few of the matches are of the form

#if !defined(CAM) && !defined(SCREAM)

or its opposite without the !'s, which are good (we do what EAM would do). There are however a few cases where CAM and SCREAM would be treated differently:

adjust_ps is false for SCREAM, and true otherwise

for theta-l_kokkos target, if SCREAM is defined, forcing is not pushed back to f90 (I guess we assume we're running eamxx), while we do push to f90 for CAM.

I suspect the last of the two points above would cause wrong results in this case. I would leave -DSCREAM out of the compile line, and use -DCAM instead (which I think is already in the compile line anyways).

Yeah, I have vague recollection that DSCREAM does not mean what a normal person would think. Maybe we should create a git issue in SCREAM to figure out whether we still need it or not? Though maybe it will naturally go away when we remove support for the F90 code in a few months?

EAMxx does need -DSCREAM (and in fact we do have it). But I think v0 should be considered as "EAM" from the point of view of Homme.

PeterCaldwell · 2023-03-07T19:39:21Z

@wlin7 - I'd like to see a DP test case and -DSCREAM removed from the configure file, but otherwise I'm happy with this PR.

wlin7 · 2023-03-23T15:51:05Z

Sorry I had an illusion this PR already merged and not watching. DP-SCREAM test added to atm_developer (and tested working well). Note that there is another global LR test for SCREAM in the integration test suite.

@PeterCaldwell , regarding use of -DSCREAM, just needing a handle in configure to determine it is a SCREAM case. This argument set in CAM_CONFIG_OPTS via config_component.xml is a handy one. EAM's configure does not see the actual compset used. If not using this argument, we could use -spa and/or -shoc_sgs, as they are currently only used for SCREAM cases. Maybe less safe if these physics options are extended for other configurations.

PeterCaldwell · 2023-03-23T15:57:03Z

@wlin7 - Hmm. I think some people (like Po-Lun) are using SCREAM configurations with MAM rather than SPA, so -spa might cause problems. I could also easily imagine people comparing SHOC vs CLUBB, which would make -shoc_sgs in appropriate. I'm not sure of a better approach though. I guess I'd recommend connecting p3 version to -shoc_sgs and adding a warning message to the log file about which version of P3 was added. It's not worth doing something complicated since F90-SCREAM isn't officially supported and will be actively discouraged from use a few months from now.

wlin7 · 2023-03-23T16:09:10Z

Good points, @PeterCaldwell . That does suggest it is not reliable to use those physics option as an indication of scream config.

How about keeping -DSCREAM for now. It is set in EAM's config_component.xml and passed to configure, and it has an impact on calculation concerning adjust_ps as @bartgol pointed above. When we remove cppdef of SCREAM altogether, we can pass another innocuous flag to configure for the sole purpose of informing it is a SCREAM case -- and not to be used as a cppdef.

PeterCaldwell · 2023-03-23T17:08:54Z

I thought Luca said above that -DSCREAM was always false for F90 scream runs (so it won’t work to differentiate EAM versus scream). Have you confirmed that’s not the case?

wlin7 · 2023-03-23T17:14:35Z

Yeah, I agreed with @bartgol 's assessment. No effect mostly because -DCAM and -DSCREAM are used redundantly. The exception is in homme/src/share/prim_driver_base.F90 and homme/src/theta-l_kokkos/prim_driver_mod.F90, which has statements solely condition on -DSCREAM.

homme/src/share/prim_driver_base.F90 is used for SCREAMv0 and regular EAM case.

wlin7 · 2023-03-23T22:08:41Z

The 9f2d8bb now determines a scream case when p3 is used together with shoc_sgs. DPSCREAM and a regular EAM F2010 cases have been tested and verified the proper p3 version is selected for the build.

bartgol

This is fine from the point of view of homme now. I can't comment much on the rest.

The alternating switch, determined by the case configuration option (a scream case or else), serves to control which version of Fortran P3 codes being used to build the model. A test is needed to track scream configuration. The version for the EAM version is currently just a skelton, which will be replaced by the eventual P3 PR from v3atm [BFB] Two new tests SMS_Ln9_P24x1.ne4_ne4.FDPSCREAM-ARM97 and SMS_Ld1.ne30pg2_ne30pg2.F2010-SCREAM-LR-DYAMOND2

brhillman · 2023-03-27T22:30:02Z

Merged to next.

brhillman · 2023-03-28T18:15:13Z

gcp12 tests had runtime fails on next:

 1:  ERROR timeaddmonths():  MM out of range
 1:  set_time_float_from_date: error return from ESMF_TimeSet for set_time_float_from_date
 1:  ERROR: CHKRC

brhillman · 2023-03-28T21:14:29Z

Merged to master. Note that the issues on gcp12 and on ascent ibm will need to be resolved in a subsequent PR.

The alternating switch, determined by the case configuration option (a scream case or else), serves to control which version of Fortran P3 codes being used to build the model. A test is needed to track scream configuration. The version for the EAM version is currently just a skelton, which will be replaced by the eventual P3 PR from v3atm [BFB] Two new tests SMS_Ln9_P24x1.ne4_ne4.FDPSCREAM-ARM97 and SMS_Ld1.ne30pg2_ne30pg2.F2010-SCREAM-LR-DYAMOND2

wlin7 requested review from rljacob, PeterCaldwell, crterai and brhillman March 3, 2023 03:29

wlin7 mentioned this pull request Mar 3, 2023

P3 cloud microphysics scheme #5168

Merged

rljacob added the Atmosphere label Mar 3, 2023

rljacob approved these changes Mar 6, 2023

View reviewed changes

rljacob assigned brhillman Mar 6, 2023

rljacob removed the request for review from brhillman March 6, 2023 19:23

crterai approved these changes Mar 6, 2023

View reviewed changes

PeterCaldwell reviewed Mar 6, 2023

View reviewed changes

Add DP-SCREAM test to atm_developer suite

2f00db8

Change to use config of p3+shoc_sgs to flag a scream case

9f2d8bb

bartgol approved these changes Mar 27, 2023

View reviewed changes

brhillman merged commit 4a313dd into master Mar 28, 2023

brhillman deleted the wlin/atm/p3_alt_eam_scream branch March 28, 2023 21:14

wlin7 mentioned this pull request Aug 1, 2023

A non-BFB puzzle of FDPSCREAM test with v3atm code base #5853

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement alternating control of Fortran P3 codes for eam and scream #5496

Implement alternating control of Fortran P3 codes for eam and scream #5496

wlin7 commented Mar 3, 2023 •

edited

Loading

wlin7 commented Mar 3, 2023

rljacob left a comment

crterai left a comment

PeterCaldwell Mar 6, 2023

wlin7 Mar 27, 2023

rljacob Mar 27, 2023

PeterCaldwell Mar 6, 2023

bartgol Mar 6, 2023 •

edited

Loading

PeterCaldwell Mar 7, 2023

bartgol Mar 7, 2023

PeterCaldwell commented Mar 7, 2023

wlin7 commented Mar 23, 2023

PeterCaldwell commented Mar 23, 2023

wlin7 commented Mar 23, 2023

PeterCaldwell commented Mar 23, 2023

wlin7 commented Mar 23, 2023 •

edited

Loading

wlin7 commented Mar 23, 2023

bartgol left a comment

brhillman commented Mar 27, 2023

brhillman commented Mar 28, 2023

brhillman commented Mar 28, 2023

Implement alternating control of Fortran P3 codes for eam and scream #5496

Implement alternating control of Fortran P3 codes for eam and scream #5496

Conversation

wlin7 commented Mar 3, 2023 • edited Loading

wlin7 commented Mar 3, 2023

rljacob left a comment

Choose a reason for hiding this comment

crterai left a comment

Choose a reason for hiding this comment

PeterCaldwell Mar 6, 2023

Choose a reason for hiding this comment

wlin7 Mar 27, 2023

Choose a reason for hiding this comment

rljacob Mar 27, 2023

Choose a reason for hiding this comment

PeterCaldwell Mar 6, 2023

Choose a reason for hiding this comment

bartgol Mar 6, 2023 • edited Loading

Choose a reason for hiding this comment

PeterCaldwell Mar 7, 2023

Choose a reason for hiding this comment

bartgol Mar 7, 2023

Choose a reason for hiding this comment

PeterCaldwell commented Mar 7, 2023

wlin7 commented Mar 23, 2023

PeterCaldwell commented Mar 23, 2023

wlin7 commented Mar 23, 2023

PeterCaldwell commented Mar 23, 2023

wlin7 commented Mar 23, 2023 • edited Loading

wlin7 commented Mar 23, 2023

bartgol left a comment

Choose a reason for hiding this comment

brhillman commented Mar 27, 2023

brhillman commented Mar 28, 2023

brhillman commented Mar 28, 2023

wlin7 commented Mar 3, 2023 •

edited

Loading

bartgol Mar 6, 2023 •

edited

Loading

wlin7 commented Mar 23, 2023 •

edited

Loading