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

Change hard-coded default for nextsw_cday_calc - important for UFS #315

Merged
merged 1 commit into from
Nov 8, 2024

Conversation

billsacks
Copy link
Member

@billsacks billsacks commented Nov 8, 2024

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 --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).

This is needed for UFS, which doesn't add this to the namelist
@billsacks billsacks self-assigned this Nov 8, 2024
Copy link
Collaborator

@uturuncoglu uturuncoglu left a comment

Choose a reason for hiding this comment

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

Thanks for the fix and sorry about extra work. The change looks good to me. I wonder if you test this without giving nextsw_cday_calc option. It seems that you test it by removing some entry from cım interface but I could not be sure about it.

@billsacks
Copy link
Member Author

Thanks for the fix and sorry about extra work. The change looks good to me. I wonder if you test this without giving nextsw_cday_calc option. It seems that you test it by removing some entry from cım interface but I could not be sure about it.

No need for you to apologize, @uturuncoglu - I apologize for not having considered the impact this might have on UFS. Yes, the diff I tested with made it so that the new nextsw_cday_calc option didn't appear in the datm input file.

@billsacks billsacks changed the title Change hard-coded default for nextsw_cday_calc Change hard-coded default for nextsw_cday_calc - important for UFS Nov 8, 2024
@billsacks billsacks merged commit 144fd9f into ESCOMP:main Nov 8, 2024
1 check passed
@billsacks billsacks deleted the nextsw_cday_default branch November 8, 2024 23:24
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.

2 participants