CAM icon indicating copy to clipboard operation
CAM copied to clipboard

SCAM-SE feature addition plus bugfixes and some refactoring.

Open jtruesdal opened this issue 1 year ago • 14 comments

This update includes some refactoring of SCAM, a few bugfixes, and adding the capability to use spectral elements dycore to do vertical transport in the column. The SE feature addition follows the E3SM implementation where a complete coarse resolution (ne3np4) of the SE dycore is initialized but only a single element is run through vertical transport with all element subcolumns being copies of the single column chosen by scmlat, scmlon.

Like the Eulerian version, SCAM-SE also has a bit for bit test to validate an exact run through the same physics as the full 3d model. Because SCAM updates the solution using a slightly different order of operations, the bfb capability is tested by making a special diagnostic run of CAM where the 3d model derives the phys/dyn tendency each time step and then recalculates the prognostic solution using the derived tendencies and SCAM's prognostic equation. This new solution (which is less precise (roundoff) due to the change in order of operations) is substituted for the full 3d solution at each time step of the model run. The substitution of the roundoff state in the 3d run allows SCAM to reproduce (BFB) each time step using the captured tendencies in the cam iop history file.

The SCAM-SE vertical advection skips the horizontal step and derives the floating level tendency based on the IOP prescribed vertical velocity. The floating levels are subsequently remapped at the end of the vertically Lagrangian dynamics step.

jtruesdal avatar Jan 10 '24 17:01 jtruesdal

Hi John - thank you for your work on this, it's clear that you've put a lot of time and effort into it. There are a lot of changes using user mods or test mods that should be done in config_compsets.xml - I know there are several ways to do this - user mods are the worst in my opinion, please consider changing. I also strongly feel that this PR should include a change to shr_scam_mod.F90 to remove the requirement for MCT.

I'd like to discuss the suggestion from @jedwards4b about putting the individual SCAM setups in config_compsets.xml. I lean towards the way it is currently done, where things which apply to all compsets of a certain "flavor" are put into config_compsets.xml and the individual details are in use_cases in order to not make config_compsets.xml bloated with individual cases. This also makes it very clear to a user what is being set in a particular SCAM setup (and provides the template for them creating their own). Perhaps @jedwards4b can expand on why config_compset.sml is the better choice over user_mods? It is possible I am interpreting his suggestion incorrectly as well, so more details would be welcome.

cacraigucar avatar Jan 11 '24 17:01 cacraigucar

@cacraigucar I have to disagree, I think that user mods really obfuscate things - but I have a meeting with John at 2 today to look at this and maybe what we should do is create another branch so that we can compare and contrast the two approaches.

jedwards4b avatar Jan 11 '24 17:01 jedwards4b

@cacraigucar I have to disagree, I think that user mods really obfuscate things - but I have a meeting with John at 2 today to look at this and maybe what we should do is create another branch so that we can compare and contrast the two approaches.

I'm getting ready to start reviewing this PR and was curious about any proposed changes that might be made due to your meeting yesterday? @jedwards4b @jtruesdal

cacraigucar avatar Jan 12 '24 16:01 cacraigucar

@cacraigucar` I have to disagree, I think that user mods really obfuscate things - but I have a meeting with John at 2 today to look at this and maybe what we should do is create another branch so that we can compare and contrast the two approaches.

I'm getting ready to start reviewing this PR and was curious about any proposed changes that might be made due to your meeting yesterday? @jedwards4b @jtruesdal

We did meet and as it turns out I already had a mostly complete set of mods for moving to a compset version of the different IOPs. I wanted to see what the effect of pushing the IOP configuration under cime_config would be. So I created a new branch with them to compare to the current way of doing things.

https://github.com/jtruesdal/CAM-1/compare/b567e51..jtruesdal:CAM-1:scam_dev_exp_w_compsets?expand=1

The mods add a cam6 configuration option for each IOP. I also added an alias for each IOP although we could keep one main IOP alias and allow users to switch between the various IOPS by specifying the correct longname. Having the IOP configure options also allow the user to run any IOP case using any of the other configuration options available in the longname. We no longer need the usermods scam directories and they have been deleted in the branch. Although if someone wants to add their own IOP then they will most likely do so through the usermods directory, so from the user perspective nothing has changed in that respect.

Whether they should be a configured via compsets is still an open question. Although they are not configuring for new physics parameterizations, one of the changes being added in this commit is the ability to modify the CLUBB parameterization to be more consistent with the type of forcing represented by the IOP. After looking at the comparison, they don't seem to overly complicate the xml files and these options make it easier to use the framework default configurations for each IOP via the compset longnames instead of having to individually specify them via usermods. I agree with Jim that this is a better way to setup the standard IOP's while still allowing usermods for user defined IOPs or user_nl_cam to tweak the setup like we do with other compsets.

jtruesdal avatar Jan 21 '24 20:01 jtruesdal

@jedwards4b wrote:

I also strongly feel that this PR should include a change to shr_scam_mod.F90 to remove the requirement for MCT.

I'm happy to include this change but after looking at the code in question I don't know how to make the proper changes for NUOPC. Would you be able to help or provide a set of mods to use? I'm happy to test along with the rest of the PR.

jtruesdal avatar Jan 21 '24 20:01 jtruesdal

@jtruesdal looking at this this morning, I don't see any tests of scam on the ne grid listed in the cam/cime_config/testdefs/testlist_cam.xml Can you please add at least one test and also tell me what compset and resolution I should try.

jedwards4b avatar Jan 31 '24 16:01 jedwards4b

I tried test SMS.ne3pg3_ne3pg3_mg37.FSCAM.derecho_intel and ran into missing input data:

Model cam missing file drydep_srf_file = '/glade/campaign/cesm/cesmdata/inputdata/atm/cam/chem/trop_mam/atmsrf_ne3np4.pg3_221214.nc'
Trying to download file: 'atm/cam/chem/trop_mam/atmsrf_ne3np4.pg3_221214.nc' to path '/glade/campaign/cesm/cesmdata/inputdata/atm/cam/chem/trop_mam/atmsrf_ne3np4.pg3_221214.nc' using SVN protocol.
svn export failed with output:  and errput svn: E170000: URL 'https://svn-ccsm-inputdata.cgd.ucar.edu/trunk/inputdata/atm/cam/chem/trop_mam/atmsrf_ne3np4.pg3_221214.nc' doesn't exist


jedwards4b avatar Jan 31 '24 16:01 jedwards4b

@jtruesdal I tried to run test SCT_D_Ln7.T42_T42_mg17.QPC6.derecho_intel.cam-scm_prep_c6 and SCT_D_P36_Ln7.T42_T42_mg17.QPC6.derecho_intel.cam-scm_prep_c6 and both fail with an mpi error. Can you point me to an SCT test that runs on derecho - or izulme.

jedwards4b avatar Feb 01 '24 13:02 jedwards4b

I also tried these tests on izumi and both fail. I'm looking into the issue but where are the baselines and what tests have you run?

jedwards4b avatar Feb 01 '24 15:02 jedwards4b

Thanks Jim. You can hold for now, I see my SCT test failed also and am looking into it.

jtruesdal avatar Feb 01 '24 18:02 jtruesdal

Jim I merged and tested your share changes on Izumi and Derecho. I also corrected the SCAM testing and added new regression tests for SE SCAM. The code is now up to cam6_3_147.

jtruesdal avatar Feb 20 '24 06:02 jtruesdal

@jtruesdal looking at this this morning, I don't see any tests of scam on the ne grid listed in the cam/cime_config/testdefs/testlist_cam.xml Can you please add at least one test and also tell me what compset and resolution I should try.

The 2 new regression tests are:
SCT_D_Ln7_Vnuopc.ne3_ne3_mg37.QPC5.derecho_intel.cam-scm_prep SCT_D_Ln7_Vnuopc.ne3_ne3_mg37.QPC6.izumi_gnu.cam-scm_prep_c6

details:

PASS SCT_D_Ln7_Vnuopc.ne3_ne3_mg37.QPC6.izumi_gnu.cam-scm_prep_c6 CREATE_NEWCASE
PASS SCT_D_Ln7_Vnuopc.ne3_ne3_mg37.QPC6.izumi_gnu.cam-scm_prep_c6 XML
PASS SCT_D_Ln7_Vnuopc.ne3_ne3_mg37.QPC6.izumi_gnu.cam-scm_prep_c6 SETUP
PASS SCT_D_Ln7_Vnuopc.ne3_ne3_mg37.QPC6.izumi_gnu.cam-scm_prep_c6 SHAREDLIB_BUILD time=308
PASS SCT_D_Ln7_Vnuopc.ne3_ne3_mg37.QPC6.izumi_gnu.cam-scm_prep_c6 MODEL_BUILD time=560
PASS SCT_D_Ln7_Vnuopc.ne3_ne3_mg37.QPC6.izumi_gnu.cam-scm_prep_c6 SUBMIT
PASS SCT_D_Ln7_Vnuopc.ne3_ne3_mg37.QPC6.izumi_gnu.cam-scm_prep_c6 RUN time=41
PASS SCT_D_Ln7_Vnuopc.ne3_ne3_mg37.QPC6.izumi_gnu.cam-scm_prep_c6 COMPARE_base_default
PASS SCT_D_Ln7_Vnuopc.ne3_ne3_mg37.QPC6.izumi_gnu.cam-scm_prep_c6 MEMLEAK insufficient data for memleak test
PASS SCT_D_Ln7_Vnuopc.ne3_ne3_mg37.QPC6.izumi_gnu.cam-scm_prep_c6 SHORT_TERM_ARCHIVER

I will remove the CAM5 test once the PR is approved.

jtruesdal avatar Feb 20 '24 07:02 jtruesdal

@cacraigucar cacraigucar last week I don't see this file in the svn repo atm/cam/chem/trop_mam/atmsrf_ne3np4_230718.nc</drydep_srf_file

I added the drydep file to the repo via rimport. Thanks for the catch

jtruesdal avatar Mar 05 '24 20:03 jtruesdal

@cacraigucar I removed the bfb testing line and updated to cam6_4_006 for diagnostic testing.

jtruesdal avatar Jul 08 '24 16:07 jtruesdal

It's turning out to be more difficult to write heat_glob as a global and will require pretty extensive changes in addition to restructuring due to a circular dependency. I would ask to leave it as is for this commit. It's currently consistent with the way global values were treated by SCAM using the EUL dycore. The global value is written at each lat/lon point and the entire surface field is written to the CAM IOP file. (This is why an ncol array is created and written out). The SCAM physgrid definition only contains 1 column and the appropriate global value is read from the user chosen column.

jtruesdal avatar Aug 06 '24 03:08 jtruesdal

@jtruesdal I tried to run test SCT_D_Ln7.T42_T42_mg17.QPC6.derecho_intel.cam-scm_prep_c6 and SCT_D_P36_Ln7.T42_T42_mg17.QPC6.derecho_intel.cam-scm_prep_c6 and both fail with an mpi error. Can you point me to an SCT test that runs on derecho - or izulme.

@jedwards4b I see that I forgot to address your question about failing SCT tests. I did correct the error and the tests are running now on both Derecho and Izumi. The is a baseline difference due to a bug fix for SCAM. I'm asking for a re-review to clear this issue so I can proceed with the PR. If you would like to look at the Derecho and Izumi tests they are: /scratch/cluster/jet/aux_cam_gnu_20240826004033 /scratch/cluster/jet/aux_cam_nag_20240826004050 /glade/derecho/scratch/jet/aux_cam_intel_20240826003035

jtruesdal avatar Aug 26 '24 13:08 jtruesdal