Skip to content

Changes to CPLHIST mode for robustness and getting the right default settings for CLM and an updated 1850 ndep file #403

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.

Sign up for GitHub

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

Open
ekluzek wants to merge 44 commits into
base: main
Choose a base branch
from
+234 −46
Open

Changes to CPLHIST mode for robustness and getting the right default settings for CLM and an updated 1850 ndep file #403

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
d9f75e9
Set defaults for DATM_CPLHIST_* settings for 1850 with DATM coupled t…
ekluzek Apr 4, 2026
67a8f18
Make the default setting for DATM_CPLHIST_DIR under the users archive…
ekluzek Apr 4, 2026
fcb28ad
Correct the filenames for cplhist mode for PRES_NDEP
ekluzek Apr 4, 2026
a72518b
Add clim_1850_cesm3_for_cmip7
ekluzek Apr 5, 2026
b6ebc5c
Add clim_1850_cesm3_for_cmip7
ekluzek Apr 5, 2026
ab6e2d7
CPLHIST settings for CLM should be just CLM60, to preserve older answers
ekluzek Apr 5, 2026
82e13db
Merge tag 'cdeps1.0.94' into 1850_aero_ndep_ozone
ekluzek Apr 22, 2026
acebf06
Merge branch 'main' into 1850_aero_ndep_ozone
ekluzek May 21, 2026
c22f68b
Merge tag 'cdeps1.0.105' into 1850_aero_ndep_ozone
ekluzek Aug 18, 2026
07b976f
Needed to get the right variables for ndep in cplhist mode
ekluzek Aug 19, 2026
2e0473f
Spin off some functions to modularize the code, to make way for addin…
ekluzek Aug 19, 2026
6fb38a9
Add headers to new functions, and do suggestions from co-pilots revie…
ekluzek Aug 19, 2026
2523ab1
Do error checking for CPLHIST mode, this resolves #399
ekluzek Aug 20, 2026
3f51f39
Let 1850 cases with CLM60 use cmip7_20tr for CO2 timeseries
ekluzek Aug 20, 2026
7842d5a
Missing for PTS_LAT/LON was actually -999.99 so set it to a variable …
ekluzek Aug 20, 2026
00dbb60
Add some output expects for _handle_model_grid especially for the NEO…
ekluzek Aug 20, 2026
03abec7
Set the default case for CPLHIST with 1850 and CLM60 to a recent case…
ekluzek Aug 20, 2026
3dd89db
No longer need to set DATM_PRESNDEP for CPLHIST case
ekluzek Aug 20, 2026
b1aaf13
Have to add logic depending on the LND model since some variables are…
ekluzek Aug 21, 2026
b6d4096
Merge commit 'b1aaf13' into 1850_aero_ndep_ozone
ekluzek Aug 21, 2026
1a200c9
Remove debug print statement that shouldn't be there
ekluzek Aug 21, 2026
9b3b5a6
Make align, start, end for the CPLHIST default consistently 101
ekluzek Aug 21, 2026
7faa9cd
Update CO2 dataset fixing #410
ekluzek Aug 21, 2026
0aef11e
Set the ndep default file for 1850 to the preliminary one
ekluzek Aug 21, 2026
9e73166
Backout setting CO2 to cmip7_20tr until we have clim_cmip7_1850 options
ekluzek Aug 22, 2026
b03bd5b
Only thing needed now is to the start the run start year, the others …
ekluzek Aug 22, 2026
ca485b5
Restrict setting of cplhist for DATM_CO2_TSERIES to only if DATM%CPLH…
ekluzek Aug 25, 2026
ec8c0a1
Remove cplhist testmod as realized in code review
ekluzek Aug 25, 2026
8106de7
Set YR_ALIGN for CPLHIST to 1, as the normal thing is to start with y…
ekluzek Aug 25, 2026
816b01f
Fix XML syntax error
ekluzek Aug 25, 2026
967ef1f
Simple changes from code review adding more to docstrings, and combin…
ekluzek Aug 26, 2026
339bf37
Update datm/cime_config/config_component.xml
ekluzek Aug 26, 2026
458df47
Update datm/cime_config/config_component.xml
ekluzek Aug 26, 2026
56a9e5e
Update datm/cime_config/config_component.xml
ekluzek Aug 26, 2026
4d0489a
Update datm/cime_config/config_component.xml
ekluzek Aug 26, 2026
1083247
Update datm/cime_config/config_component.xml
ekluzek Aug 26, 2026
7d61189
A couple things caught in the code review, fix a typo, and remove a c…
ekluzek Aug 26, 2026
2951f37
Merge branch '1850_aero_ndep_ozone' of github.com:ekluzek/CDEPS into …
ekluzek Aug 26, 2026
18c23f1
More things from review, remove checking if DATM_CPLHIST_DOMAIN_FILE …
ekluzek Aug 26, 2026
07846d8
Fix expect so that PLUMBER2 sites work
ekluzek Aug 26, 2026
baddde6
Add a note on how ndep for cplhist is in units that don't need to be …
ekluzek Aug 27, 2026
93d3b15
Add a comment about the clim_1850_cesm3_for_cmip7 option to DATM_PRES…
ekluzek Aug 27, 2026
1b961f0
Remove accidental character that screwed up the XML syntax, and adjus…
ekluzek Aug 29, 2026
4a6f49c
Add coupling_mode as an attribute for the cesm settings that could co…
ekluzek Aug 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
198 changes: 166 additions & 32 deletions datm/cime_config/buildnml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,164 @@ def _get_neon_data_availability(case, neonsite):
expect(newestdate, "No tower data found on server for NEON site {}".format(neonsite))
return None

####################################################################################
def _handle_pts_mode( case, config ):
Comment thread
ekluzek marked this conversation as resolved.
####################################################################################
""" Handle PTS mode settings for single column runs. If PTS_LON and PTS_LAT are set, then set config[single_column] to true.
"""
missing = -999.99
if case.get_value('PTS_LON'):
scol_lon = float(case.get_value('PTS_LON'))
else:
scol_lon = missing
if case.get_value('PTS_LAT'):
scol_lat = float(case.get_value('PTS_LAT'))
else:
scol_lat = missing
if case.get_value('PTS_DOMAINFILE'):
if scol_lon > missing and scol_lat > missing and case.get_value("PTS_DOMAINFILE") != "UNSET":
config['single_column'] = 'true'
else:
config['single_column'] = 'false'

# Check that if either PTS_LON/PTS_LAT is set the other is as well
# TODO: NOTE: This should really be in CMEPS
if (scol_lon != missing and scol_lat == missing) or (scol_lon == missing and scol_lat != missing):
expect(False, "If either PTS_LON or PTS_LAT is set, the other must be set as well")
Comment thread
ekluzek marked this conversation as resolved.

# Verify that the expected output setting was actually done
expect(config['single_column'] is not None, "single_column should have been set in this subroutine")

@billsacks billsacks Aug 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It might feel kind of arbitrary, but I also feel like this expect is helpful. It's hard for me to explain my thinking on this one vs. the other ones in _handle_model_grid. Maybe the difference is that this is a single, simple expect that feels like it's doing a bigger job, and also less likely to need changes if you change the production code, partly because the expect is unconditional, so you don't need to replicate logic from earlier in the routine in order to do this expect? But I do acknowledge that this all feels subjective, so I'm open to disagreements with my feelings on where there should / shouldn't be expects.

However: looking at the above logic, it looks like config['single_column'] isn't set if case.get_value('PTS_DOMAINFILE') gives None or an empty string. So maybe you need to add to the logic to make this expect always pass?

@ekluzek ekluzek Aug 26, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I think you are right. The strange thing is that this should have been caught in the NEON/PLUMBER2 tests in aux_clm. So I'll look at why those are working. Or perhaps I didn't retest after adding the expect?

@ekluzek ekluzek Aug 26, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Ahhh, yes this only fails if PTS_DOMAINFILE doesn't exist or is empty. So tests pass because this is defined in CMEPS so all configurations have it available and by default set to UNSET.

But, for future proofing I think it should abort if PTS_DOMAINFILE isn't defined and just say that it isn't. Now, single single_column is a special case (and not the typical one for CESM), that should probably be allowed unless PTS_LON/PTS_LAT are set. So I'll do something along the lines of that second option...


####################################################################################
def _handle_model_grid(case, config):
####################################################################################
""" Set config['model grid'] always
Also do some checking and set needed variables for a CLM user defined grid.
For a CLM single-point/regional case (CLM user defined grid)
Set config['model_grid'] to CLM_USRDAT to signify this
For a NEON case
set config['neonsite']
config['neon'] is True
For a PLUMBER2 case
set config['plumber2site']
config['plumber2'] is True

Will fail if settings aren't setup as expected

Returns an empty list normally; for a NEON case it returns the list of available NEON data
"""
available_neon_data = None
model_grid = case.get_value("GRID")
comp_lnd = case.get_value("COMP_LND")
clm_usrdat_name = case.get_value("CLM_USRDAT_NAME") # NOTE: Will be None if not coupled to CLM
if 'CLM_USRDAT' in model_grid:
config['model_grid'] = 'CLM_USRDAT'
# If CLM_USRDAT is being used, CLM_USRDAT_NAME must be set
expect(clm_usrdat_name is not None, "CLM_USRDAT_NAME must be set when using CLM_USRDAT grid")
if 'NEON' in clm_usrdat_name:
neonsite = case.get_value("NEONSITE")
config['neonsite'] = neonsite
if neonsite:
config['neon'] = "True"
# download and use the listing.csv file to determine data availablity
available_neon_data = _get_neon_data_availability(case, neonsite)
if 'PLUMBER2' in clm_usrdat_name:
plumber2site = case.get_value('PLUMBER2SITE')
config['plumber2site'] = plumber2site
if plumber2site:
config['plumber'] = "True"
else:
config['model_grid'] = model_grid
if comp_lnd != "clm":
expect(clm_usrdat_name is None, "CLM_USRDAT_NAME is not expected to exist when the land model is NOT CLM")
expect(available_neon_data is None, "available_neon_data should NOT be set when the land model is NOT CLM")
expect(not 'neonsite' in config, "neonsite should have NOT been set when the land model is NOT CLM")
expect(not 'neon' in config, "neon should have NOT been set when the land model is NOT CLM")
expect(not 'plumber2site' in config, "plumber2site should have NOT been set when the land model is NOT CLM")
expect(not 'plumber' in config, "plumber should have NOT been set when the land model is NOT CLM")
Comment on lines +161 to +166

@billsacks billsacks Aug 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I feel like these expects don't add much value, and just make things harder to change in the future.

The expect on clm_usrdat_name feels unnecessary. I can envision a scenario where we have a component that uses CLM_USRDAT_NAME (so that it can leverage this neon / plumber logic, for example) but where we have changed the actual component name. It seems like the code logic here handles (or should handle) CLM_USRDAT_NAME independent of the comp_lnd value, so checking comp_lnd just ties us to what's currently done rather than what's allowable.

The other expects here seem to just be confirming that you didn't somehow set these values prior to the if block above. It feels like this doesn't add much value, and makes it harder to change the logic in the future because now you have an extra place that you need to keep consistent.

I don't feel super strongly about this, but I'd like to see these expects all removed.

else:
expect(clm_usrdat_name == "UNSET", "CLM_USRDAT_NAME must NOT be set when NOT using a CLM_USRDAT grid")

@billsacks billsacks Aug 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there logic in this buildnml that relies on this being UNSET (or not UNSET)? I don't think so, but I haven't looked super carefully. If not, I would like to see this expect moved into whatever component sets the default of CLM_USRDAT_NAME to UNSET and removed from here. My concern with it being here is that, if the default value changes elsewhere (or, e.g., comes to differ between CESM and NorESM), then this will break. So I only want it here if it's really necessary to ensure that the logic in this file is working correctly.


#
# Ensure output settings are done as expected
#
expect(config['model_grid'] is not None, "model_grid should have been set in this subroutine")

#
# Check for expected settings when coupled to CLM for NEON and PLUMBER2 cases
#
if comp_lnd == "clm":
if "NEON" in clm_usrdat_name:
expect(config['neon'] == "True", "neon should have been set in this subroutine, since NEON is on")
expect(config['neonsite'] is not None, "neonsite should have been set in this subroutine, since NEON is on")
expect(config['model_grid'] == "CLM_USRDAT", "model_grid should have been set to CLM_USRDAT because NEON is on")
else:
expect(available_neon_data is None, "available_neon_data should NOT be set if NEON is off")
expect(case.get_value('NEONSITE') == "", "NEONSITE should NOT have been set in this subroutine, since NEON is off")
expect(not 'neonsite' in config, "neonsite should have NOT been set in this subroutine, since NEON is off")
expect(not 'neon' in config, "neon should have NOT been set in this subroutine, since NEON is off")

missing_yr_filename = 9999
if "PLUMBER2" in clm_usrdat_name:
expect(config['plumber2site'] is not None, "plumber2site should have been set in this subroutine")
expect(config['model_grid'] == "CLM_USRDAT", "model_grid should have been set to CLM_USRDAT because PLUMBER2 is on")
expect(config['plumber'] == "True", "plumber should have been set in this subroutine, since PLUMBER2 is on")
datm_mode = case.get_value("DATM_MODE")
if ( datm_mode == "1PT" ):
expect(case.get_value('DATM_YR_START_FILENAME') != missing_yr_filename, "DATM_YR_START_FILENAME should have been set, since PLUMBER2 on")
expect(case.get_value('DATM_YR_END_FILENAME') != missing_yr_filename, "DATM_YR_END_FILENAME should have been set, since PLUMBER2 on")
else:
expect(case.get_value('DATM_YR_START_FILENAME') == missing_yr_filename, "DATM_YR_START_FILENAME should NOT have been set, since PLUMBER2 is off")
expect(case.get_value('DATM_YR_END_FILENAME') == missing_yr_filename, "DATM_YR_END_FILENAME should NOT have been set, since PLUMBER2 is off")

else:
expect(case.get_value('PLUMBER2SITE') == "", "PLUMBER2SITE should NOT have been set in this subroutine, since PLUMBER2 is off")
expect(not 'plumber2site' in config, "plumber2site should have NOT been set in this subroutine, since PLUMBER2 is off")
expect(not 'plumber' in config, "plumber should have NOT been set in this subroutine, since PLUMBER2 is off")
Comment on lines +175 to +205

@billsacks billsacks Aug 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would like to see most or all of these expects removed. It feels like you're trying to effectively do unit testing of this function via these expects, and I understand the rationale for that, but I feel like it complicates the production code unnecessarily and makes it harder to change things in the future... and so I feel that adding all of these assertions will actually make bugs more likely in the future rather than less. At a glance, most of these expects are effectively duplicating logic earlier in the function, so now any change requires changes in two places. This is acceptable in actual unit tests, but I feel like this adds confusion and complexity in the production code, and obscures any more important things in the production code.

If there are critical checks of self-consistency between user-set options, those are appropriate here, but I'd like to see a removal of expects that are essentially duplicating the logic earlier in the function.

I realize that we may have different philosophies here, so if you disagree, I'd be happy to defer to a 3rd person.


# Return the data available for NEON
return available_neon_data

####################################################################################
def _error_check_cplhist_mode(case, config):
####################################################################################
""" Error checks for cplhist mode.
Make sure needed settings are set when used.
Make sure directories expected exist
Make sure it works for all the different cplhist options
"""

check_cplhist = False
if "cplhist" in config['datm_mode']:
check_cplhist = True
if "cplhist" in case.get_value("DATM_CO2_TSERIES"):
check_cplhist = True
if "cplhist" in case.get_value("DATM_PRESAERO"):
check_cplhist = True
if "cplhist" in case.get_value("DATM_PRESNDEP"):
check_cplhist = True
if "cplhist" in case.get_value("DATM_PRESO3"):
check_cplhist = True
if "cplhist" in case.get_value("DATM_TOPO"):
check_cplhist = True

if check_cplhist:
cplhist_dir = case.get_value("DATM_CPLHIST_DIR")
cplhist_case = case.get_value("DATM_CPLHIST_CASE")
cplhist_domain = case.get_value("DATM_CPLHIST_DOMAIN_FILE")
expect(cplhist_case is not None or cplhist_case != UNSET, "DATM_CPLHIST_CASE must be set when using cplhist mode")
expect(cplhist_dir is not None, "DATM_CPLHIST_DIR must be set when using cplhist mode")
# Check that the CPLHIST directory exists
# This is a little unusual in buildnml, as the check_input_data will make sure the files exist
# But, most often the problem is that I'm pointing to the wrong directory, so seeing that the directory
# doesn't exist is easier than showing all of the myriad of files that are missing later
unres_cplhist_dir = case.get_value("DATM_CPLHIST_DIR", resolved=False)
expect(os.path.isdir(cplhist_dir), "DATM_CPLHIST_DIR {} does not exist (directory with input variables is {})".format(cplhist_dir, unres_cplhist_dir))
else:
cplhist_domain = case.get_value("DATM_CPLHIST_DOMAIN_FILE")
expect(cplhist_domain == "null", "DATM_CPLHIST_DOMAIN_FILE can only be set when using cplhist mode")
cplhist_case = case.get_value("DATM_CPLHIST_CASE")
expect(cplhist_case == "UNSET", "DATM_CPLHIST_CASE should only be set when using cplhist mode")

# pylint: disable=too-many-arguments,too-many-locals,too-many-branches,too-many-statements
####################################################################################
Expand All @@ -115,7 +273,6 @@ def _create_namelists(case, confdir, inst_string, infile, nmlgen, data_list_path
clm_usrdat_name = case.get_value("CLM_USRDAT_NAME")
datm_co2_tseries = case.get_value("DATM_CO2_TSERIES")
atm_grid = case.get_value("ATM_GRID")
model_grid = case.get_value("GRID")
comp_lnd = case.get_value("COMP_LND")
compset = case.get_value("COMPSET")

Expand All @@ -142,21 +299,8 @@ def _create_namelists(case, confdir, inst_string, infile, nmlgen, data_list_path

# Initialize namelist defaults
config = {}
available_neon_data = None
if 'CLM_USRDAT' in model_grid:
config['model_grid'] = 'CLM_USRDAT'
if 'NEON' in clm_usrdat_name:
neonsite = case.get_value("NEONSITE")
if neonsite:
config['neon'] = "True"
# download and use the listing.csv file to determine data availablity
available_neon_data = _get_neon_data_availability(case, neonsite)
if 'PLUMBER2' in clm_usrdat_name:
plumber2site = case.get_value('PLUMBER2SITE')
if plumber2site:
config['plumber'] = "True"
else:
config['model_grid'] = model_grid

available_neon_data = _handle_model_grid(case, config)

config['datm_mode'] = datm_mode
config['datm_co2_tseries'] = datm_co2_tseries
Expand All @@ -165,19 +309,9 @@ def _create_namelists(case, confdir, inst_string, infile, nmlgen, data_list_path
config['datm_preso3'] = datm_preso3
config['clm_usrdat_name'] = clm_usrdat_name

if case.get_value('PTS_LON'):
scol_lon = float(case.get_value('PTS_LON'))
else:
scol_lon = -999.
if case.get_value('PTS_LAT'):
scol_lat = float(case.get_value('PTS_LAT'))
else:
scol_lat = -999.
if case.get_value('PTS_DOMAINFILE'):
if scol_lon > -999. and scol_lat > -999. and case.get_value("PTS_DOMAINFILE") != "UNSET":
config['single_column'] = 'true'
else:
config['single_column'] = 'false'
_handle_pts_mode( case, config )

_error_check_cplhist_mode(case, config)

nmlgen.init_defaults(infile, config)

Expand Down Expand Up @@ -211,11 +345,11 @@ def _create_namelists(case, confdir, inst_string, infile, nmlgen, data_list_path
if datm_co2_tseries != "none":
streamlist.append("co2tseries.{}".format(datm_co2_tseries))
if clm_usrdat_name == 'NEON.PRISM':
streamlist.append(clm_usrdat_name+"_PRECIP."+neonsite)
streamlist.append(clm_usrdat_name+"_PRECIP."+config['neonsite'])
if clm_usrdat_name == 'NEON':
streamlist.append(clm_usrdat_name+".NEON_PRECIP."+neonsite)
streamlist.append(clm_usrdat_name+".NEON_PRECIP."+config['neonsite'])
if clm_usrdat_name == 'PLUMBER2':
streamlist.append(clm_usrdat_name+"."+plumber2site)
streamlist.append(clm_usrdat_name+"."+config['plumber2site'])

bias_correct = nmlgen.get_value("bias_correct")
if bias_correct is not None:
Expand Down
32 changes: 28 additions & 4 deletions datm/cime_config/config_component.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,13 @@

<entry id="DATM_PRESNDEP">
<type>char</type>
<valid_values>none,clim_1850_cmip7,clim_2000_cmip7,clim_2010_cmip7,hist_cmip7,clim_1850_cmip6,clim_2000_cmip6,clim_2010_cmip6,hist_cmip6,SSP1-2.6,SSP2-4.5,SSP3-7.0,SSP5-3.4,SSP5-8.5,cplhist</valid_values>
<!-- NOTE: Here clim_1850_cesm3_for_cmip7 means the 1850 climatological dataset that CESM3.0 will use for CMIP7 that is from it's spinup simulation -->
<!-- In contrast is clim_1850_cmip7 is the 1850 data provided by CMIP7 -->
<valid_values>none,clim_1850_cesm3_for_cmip7,clim_1850_cmip7,clim_2000_cmip7,clim_2010_cmip7,hist_cmip7,clim_1850_cmip6,clim_2000_cmip6,clim_2010_cmip6,hist_cmip6,SSP1-2.6,SSP2-4.5,SSP3-7.0,SSP5-3.4,SSP5-8.5,cplhist</valid_values>

@billsacks billsacks Aug 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see that you added clim_1850_cesm3_for_cmip7 in addition to the existing clim_1850_cmip7. Can you clarify the difference - ideally in a comment in the xml? Is this intending to distinguish CESM vs. NorESM? If so, is the match on CLM60 below actually the appropriate way to do this? (Also pinging @mvertens on this.)

@ekluzek ekluzek Aug 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The new option is discussed mainly in #388.

And we discussed this in the March 31st CSEG meeting here:

https://docs.google.com/document/d/186U6-dt_wWZZGU9NzYQ5zNlMnpx9XX6oweuTXzQY-oo/edit?tab=t.0#heading=h.43m98q40divy

@ekluzek ekluzek Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Adding the option isn't a problem for NorESM it's the part below, where the default is set. But, @mvertens gave the solution for that. So I'll use that there.

@ekluzek ekluzek Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Oh, and just noticed @billsacks suggested adding a comment in the code. I'll do that as well.

@ekluzek ekluzek Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

93d3b15 adds the comment

@ekluzek ekluzek Sep 2, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I will note that @klindsay28 and @mnlevy1981 and I think I agree to shorten the name from

clim_1850_cesm3_for_cmip7

to

clim_1850_cesm3

<default_value>clim_2000</default_value>
<values match="last">
Comment thread
ekluzek marked this conversation as resolved.
<value compset="^1850_" >clim_1850_cmip6</value>
<value coupling_mode="cesm" compset="^1850_.*CLM">clim_1850_cesm3_for_cmip7</value>
<value compset="^2000_" >clim_2000_cmip6</value>
<value compset="^2010_" >clim_2010_cmip6</value>
<value compset="^HIST_" >hist_cmip6</value>
Expand Down Expand Up @@ -178,11 +181,15 @@
<value compset="^20TR" >cmip6_20tr</value>
<value compset="^OMIP_DATM%IAF.*_POP2%[^_]*ECO">omip.iaf</value>
<value compset="^OMIP_DATM%JRA.*_POP2%[^_]*ECO">omip.jra</value>
<value compset="_DATM%CPLHIST">cplhist</value>
<value coupling_mode="cesm" compset="_DATM%CPLHIST_CLM">cplhist</value>

@ekluzek ekluzek Sep 2, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@klindsay28 @mnlevy1981 and I all like dropping the _CLM and going back to the original (for CESM). So this would go back to the original.

</values>
<group>run_component_datm</group>
<file>env_run.xml</file>
<desc>DATM CO2 time series</desc>
<desc>DATM CO2 time series
NOTE: This needs to be coordinated with CCSM_BGC in CMEPS, and with either the ocean or land model
For CLM coordinate it with the CLM_CO2_TYPE variable
For MOM coordinate it with the OCN_CO2_TYPE variable
</desc>

@ekluzek ekluzek Sep 2, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@klindsay28 has a brilliant idea, that we probably don't really need CCSM_BGC anymore because NUOPC now won't pass CO2 related fields unless some subcomponent needs it. He checked CMEPS and it looks like it's ignored.

I'll will check this out, as this might really simplify things.

@billsacks billsacks Sep 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@klindsay28 - as I just mentioned to @ekluzek - Based on my quick look through the code, it looks to me like CCSM_BGC is still used to set a few CMEPS variables: CCSM_BGC sets BGC_MODE in buildnml, which in turn sets the defaults for flds_co2a, flds_co2b, flds_co2c, and flds_bgc_oi. Those aren't used in CMEPS itself, but are used in CTSM (and maybe other components?) as the mechanism to figure out what the co2 coupling is. So it looks like you're right that CMEPS doesn't need this directly, presumably because it does exactly what you're suggesting, but this is still used to coordinate the co2 fields in at least some components.

</entry>

<entry id="DATM_CPLHIST_DOMAIN_FILE">
Expand All @@ -205,15 +212,30 @@
<type>char</type>
<valid_values></valid_values>
<default_value>UNSET</default_value>
<values match="last">
<value coupling_mode="cesm" compset="1850.*_DATM%CPLHIST.*_CLM">$DIN_LOC_ROOT/cplhist/cesm3_0/$DATM_CPLHIST_CASE/atm</value>

@ekluzek ekluzek Sep 2, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@klindsay28 and @mnlevy1981 suggest that we use "cpl/hist" at the end here rather than "atm" to preserve the archive directory structure.

</values>
<group>run_component_datm</group>
<file>env_run.xml</file>
<desc>directory for coupler history data mode (only used for when DATM_MODE is CPLHIST)</desc>
<desc>
Directory for coupler history data mode (used when DATM_MODE is CPLHIST)
May also be used if and of these are set to cplhist: DATM_PRESAERO, DATM_NDEP, DATM_CO2_TSERIES, DATM_PRESO3, or DATM_TOPO
NOTE: This needs to be the full directory path where the CPLHIST files are stored, including the subdirectory for the CASENAME (that can be set using $DATM_CPLHIST_CASE)
We thus recommend that $DATM_CPLHIST_CASE be used in here to specify that case subdirectory.
This allows the DATM_CPLHIST_DIR to be set for all your CPLHIST datafiles, and you only need to modify $DATM_CPLHIST_CASE to point to a different CASE.
For example, to use the CPLHIST data generated from a case that you ran do this:
./xmlchange DATM_CPLHIST_DIR='$CIME_OUTPUT_ROOT/archive/$DATM_CPLHIST_CASE/cpl/hist'
Which will then use the CPL history files in your archive directory where the case that created the CPLHIST data you want to use is"
</desc>
</entry>

<entry id="DATM_CPLHIST_CASE">
<type>char</type>
<valid_values></valid_values>
<default_value>UNSET</default_value>
<values match="last">
<value coupling_mode="cesm" compset="1850.*_DATM%CPLHIST.*_CLM">b.e30_alpha08o.B1850C_MTso.ne30_t232_wgx3.330</value>
</values>

@mvertens mvertens Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@ekluzek @billsacks - This will break things for NorESM - since the same pattern match applies.
I am wondering if we could trigger off of the xml variable COUPLING_MODE. This could be queries in buildnml - and then a config setting here could be user -

<value coupling_mode="cesm" compset="1850.*_DATM%CPLHIST.*_CLM60">b.e30_alpha08o.B1850C_MTso.ne30_t232_wgx3.330</value>

The COUPLING_MODE variable is already being used in CMEPS - but should be available for all component combinations I believe.

@ekluzek ekluzek Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Perfect. Thanks for pointing that out @mvertens! It is available in my cases, so I'll be able to use it.

I'll add this in and make sure it works. I'll post the commit that fixes once I have it.

@ekluzek ekluzek Aug 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Adding coupling_mode to the namelist parsing would have been cake. It proved a little more tricky for adding it to config_component.xml and required a CIME change to do it. I did get something that I think is correct, but I couldn't test it with just using a testmod that sets COUPLING_MODE==noresm. So I'll figure out a unit test for it in CIME.

Here's the PR in CIME to do that...

ESMCI/cime#5044

Please look that over and give suggestions there. I think this is a reasonable thing to do for cime_mode==cesm. But, it does only do it for CESM and not all models. That's probably good for limiting the impact, but it's also good to minimize places where cime_model has to be checked. Also note that this could be done out of the box without a CIME update if CIME_MODEL were different between CESM and NorESM. So I wonder if using CIME_MODEL to distringuish between CESM and NorESM isn't the better long term solution. But, this seems like it should be OK for now.

@mvertens mvertens Aug 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Adding coupling_mode to the namelist parsing would have been cake. It proved a little more tricky for adding it to config_component.xml and required a CIME change to do it. I did get something that I think is correct, but I couldn't test it with just using a testmod that sets COUPLING_MODE==noresm. So I'll figure out a unit test for it in CIME.

Here's the PR in CIME to do that...

ESMCI/cime#5044

Please look that over and give suggestions there. I think this is a reasonable thing to do for cime_mode==cesm. But, it does only do it for CESM and not all models. That's probably good for limiting the impact, but it's also good to minimize places where cime_model has to be checked. Also note that this could be done out of the box without a CIME update if CIME_MODEL were different between CESM and NorESM. So I wonder if using CIME_MODEL to distringuish between CESM and NorESM isn't the better long term solution. But, this seems like it should be OK for now.

@ekluzek - thanks for taking this on. I did not appreciating the difficulty of doing this in config_component.xml. I totally agree that the right solution is to use CIME_MODE - but that is a much more difficult to implement - particularly given the numerous places in CIME that currently need to be touched. I am happy to test this for COUPLING_MODE=noresm with your changes in CIME. But also - given the time crunch for CESM3 - if you want to defer this and bring in your changes to CDEPS without the changes to CIME - I would understand that.

<group>run_component_datm</group>
<file>env_run.xml</file>
<desc>case name used to determine stream filenames when DATM_MODE is CPLHIST</desc>
Expand Down Expand Up @@ -278,6 +300,7 @@
<value compset="1850.*_DATM%CRUv7">1901</value>
<value compset="1850.*_DATM%GSW">1901</value>
<value compset="1850.*_DATM%NLDAS2">0</value> <!-- Unsupported -->
<value coupling_mode="cesm" compset="1850.*_DATM%CPLHIST.*_CLM">101</value>

@ekluzek ekluzek Sep 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Add a line for MOM6 with the same year, as later MOM6 will have different years.

<value compset="2000.*_DATM%WISOQIA">2000</value>
<value compset="2000.*_DATM%QIA">1972</value>
<value compset="HIST.*_DATM%QIA">1948</value>
Expand Down Expand Up @@ -349,6 +372,7 @@
<value compset="1850.*_DATM%CRUv7">1920</value>
<value compset="1850.*_DATM%GSW">1920</value>
<value compset="1850.*_DATM%NLDAS2">-1</value> <!-- Unsupported -->
<value coupling_mode="cesm" compset="1850.*_DATM%CPLHIST.*_CLM">101</value>
<value compset="2000.*_DATM%WISOQIA">2004</value>
<value compset="2000.*_DATM%QIA">2004</value>
<value compset="HIST.*_DATM%QIA">1972</value>
Expand Down
Loading
Loading