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

Sync with ESCOMP/CDEPS (2024-10-07) #66

Open
wants to merge 260 commits into
base: develop
Choose a base branch
from

Conversation

NickSzapiro-NOAA
Copy link
Collaborator

@NickSzapiro-NOAA NickSzapiro-NOAA commented Oct 7, 2024

Description of changes

Merge authoritative ESCOMP into EMC fork (+cmake includes for new files)

Specific notes

Contributors other than yourself, if any: N/A

CDEPS Issues Fixed (include github issue #): #65

Are there dependencies on other component PRs (if so list): No

Are changes expected to change answers (bfb, different to roundoff, more substantial): bfb except for character dimension increase to CX

Any User Interface Changes (namelist or namelist defaults changes): No

Testing performed (e.g. aux_cdeps, CESM prealpha, etc): UFS RT suite + preceding ESCOMP/CDEPS tests

Hashes used for testing: See UFS PR (todo)

ekluzek and others added 30 commits May 8, 2023 14:52
Update stream definitions for new coupler history file format

### Description of changes

Modify stream_definition_datm.xml to generate a streams file (datm.streams.xml) with the new coupler history file format.

### Specific notes

Changes to accommodate new coupler history file names.
Change offset for solar stream from 2700 to -900 to accommodate changes due to time stamps.
These changes work in conjunction with CDEPS PR ESCOMP#224 and CDEPS PR ESCOMP#222 .
Note that I did not change the file names for ndep, or remove that stream. See ESCOMP#230

Contributors other than yourself, if any: @billsacks 

CDEPS Issues Fixed (include github issue #):  N/A

Are there dependencies on other component PRs (if so list):  No

Are changes expected to change answers (bfb, different to roundoff, more substantial):  Yes, in coupler history mode.

Any User Interface Changes (namelist or namelist defaults changes): No

Testing performed (e.g. aux_cdeps, CESM prealpha, etc):  I have conducted a pair of cases, an F-case to generate coupler history files, and an I-case to read those files, using the new file name convention, and compared the forcing output variables from clm history files between the two cases.  @billsacks and I reviewed these differences and found them to be acceptable.

@billsacks ran SMS_D_Ld1.ne30pg3_t061.I1850Clm50BgcSpinup.cheyenne_intel.clm-cplhist in the context of ESCOMP/CTSM#1999

Hashes used for testing:  N/A
update github to make cdeps ext build an action
Update SST files for historical configurations
Enable build with ESMX and introduce export_all
@uturuncoglu
Copy link
Collaborator

@NickSzapiro-NOAA Sorry for late response. Yes, it would be nice to work on ufs-community/ufs-weather-model#1548. I'll also try to review the PR. I think this PR also sync with ESCOMP. Right?

@NickSzapiro-NOAA
Copy link
Collaborator Author

Thanks in advance for review, @uturuncoglu! Yes, this sync PR is a git merge of ESCOMP/CDEPS into EMC fork

For consistent keys, it seems straightforward to just copy the xml keys (they seem more concise...)

stream_info  <-- stream_info
taxmode <-- taxmode
tintalgo <-- tInterpAlgo
readmode <-- readMode
mapalgo <-- mapalgo
dtlimit <-- dtlimit
yearFirst <-- yearFirst
yearLast <-- yearLast
yearAlign <-- yearAlign
vectors <-- stream_vectors
meshfile <-- stream_mesh_file
lev_dimname <-- stream_lev_dimname
datafiles <-- stream_data_files
datavars <-- stream_data_variables
offset <-- stream_offset

But how deep into the component source code does this impact? These are explicitly in CICE for example in a section using CDEPS inline:
https://github.com/NOAA-EMC/CICE/blob/develop/cicecore/drivers/nuopc/cmeps/ice_prescribed_mod.F90#L98-L107
If for other components too, there is a bigger cost to cleaning this up

@uturuncoglu
Copy link
Collaborator

@NickSzapiro-NOAA Yes, making those configuration files consistent would be great but I am not sure about its downstream effect. The workflows etc. (recently I implemented CDEPs driver in UWtools side) might also need to have changes. I am not sure how it will be handled. Any idea?

@NickSzapiro-NOAA
Copy link
Collaborator Author

Given the downstream impact, it's not so friendly to break backwards compatibility without a clear need. One option is to treat both sets of attribute labels as vaild synonyms in shr_stream_init_from_esmfconfig, with the "xml keys" taking precedence if present...

@uturuncoglu
Copy link
Collaborator

@NickSzapiro-NOAA I agree. Anyway, let me know if you need any help in my end.

@NickSzapiro-NOAA
Copy link
Collaborator Author

Thanks for review and discussion, @uturuncoglu ! Then, we'll proceed with this sync and separate issue of resolving divergent keys

jedwards4b and others added 21 commits October 15, 2024 05:07
…tafiles entries are present for a stream_entry
add new JRA 1.5 streams for datm and drof
Change nextsw_cday calculation for cam_dev (CAM7) in cplhist mode

### Description of changes
The radiation re-ordering in cam_dev (CAM7) requires a change to the calculation of nextsw_cday in cplhist mode. This PR introduces a new namelist variable, nextsw_cday_calc, which controls this behavior. The default value is appropriate for cplhist files generated with CAM7. Note that, by default, this namelist flag has no effect on non-cplhist cases (because of the default value of iradsw in non-cplhist cases).

### Specific notes

Contributors other than yourself, if any:  @billsacks @adamrher

CDEPS Issues Fixed (include github issue #):

Are there dependencies on other component PRs (if so list):

Are changes expected to change answers (bfb, different to roundoff, more substantial): Greater than roundoff-level changes in cases using datm in CPLHIST mode.

Any User Interface Changes (namelist or namelist defaults changes): New namelist variable, nextsw_cday_calc.

Testing performed (e.g. aux_cdeps, CESM prealpha, etc):
I have plugged this branch into the latest version of CTSM and ran the following tests:
- SMS_D_Ld3.f10_f10_mg37.I1850Clm50BgcCrop.derecho_intel.clm-default - passes and bit-for-bit
- SMS_D_Ld1.ne30pg3_t232.I1850Clm50BgcSpinup.derecho_intel.clm-cplhist - passes but changes answers as expected
- SMS_D_Ld1.ne30pg3_t232.I1850Clm50BgcSpinup.derecho_intel.clm-cplhist, with the new namelist flag changed to "cam6" - passes and bit-for-bit

Hashes used for testing:
ctsm5.3.009
backout the timestamp on rpointer files until alpha05c,
This is needed for UFS, which doesn't add this to the namelist
Change hard-coded default for nextsw_cday_calc - important for UFS

### Description of changes

Change hard-coded default for nextsw_cday_calc to be a valid option that will give backwards compatibility. This is needed for UFS, which doesn't add this to the namelist. This should have no impact for CESM.

### Specific notes

Contributors other than yourself, if any:

CDEPS Issues Fixed (include github issue #):

Are there dependencies on other component PRs (if so list): No

Are changes expected to change answers (bfb, different to roundoff, more substantial): No

Any User Interface Changes (namelist or namelist defaults changes): No

Testing performed (e.g. aux_cdeps, CESM prealpha, etc):

Ran `SMS_D_Ld1.ne30pg3_t232.I1850Clm50BgcSpinup.derecho_intel.clm-cplhist` with comparison against baseline, where I removed the nextsw_cday_calc from the namelist with these diffs:

```diff
diff --git a/datm/cime_config/namelist_definition_datm.xml b/datm/cime_config/namelist_definition_datm.xml
index 40935cf..06f6d99 100644
--- a/datm/cime_config/namelist_definition_datm.xml
+++ b/datm/cime_config/namelist_definition_datm.xml
@@ -348,31 +348,6 @@
     </values>
   </entry>
 
-  <entry id="nextsw_cday_calc">
-    <type>char</type>
-    <category>datm</category>
-    <group>datm_nml</group>
-    <valid_values>cam6,cam7</valid_values>
-    <desc>
-      Logic to use for calculating nextsw_cday. This variable has no effect when iradsw is
-      0 or 1, so by default this only applies in CPLHIST cases.
-
-      For CPLHIST cases, this should agree with the version of CAM (or other atmosphere
-      model) used to generate the CPLHIST forcings; the valid values for this variable are
-      based on this: 'cam6' is appropriate for cases generated with the driver ordering in
-      CAM6 and earlier, and 'cam7' is appropriate for cases generated with the driver
-      ordering in CAM7 and later.
-
-      For 'cam6', the next radiation timestep is set to the present time plus 2 timesteps
-      when mod(tod+dtime,delta_radsw)==0. For 'cam7', the next radiation timestep is set
-      to the present time plus 1 timestep when mod(tod,delta_radsw)==0.
-    </desc>
-    <values>
-      <value>cam6</value>
-      <value datm_mode="CPLHIST">cam7</value>
-    </values>
-  </entry>
-
   <entry id="restfilm">
     <type>char</type>
     <category>datm</category>
```

I ensured this passed and was bit-for-bit.

Hashes used for testing:
CTSM at ctsm5.3.009. The latest CDEPS doesn't work in the context of that CTSM tag, so instead I cherry-picked this commit onto 1f317a3 (from the original nextsw_cday branch).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.