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.

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

Jump to bottom

migrate HAMOCC config settings to run time namelist settings #281

Merged
merged 17 commits into from
Oct 24, 2023
Merged

migrate HAMOCC config settings to run time namelist settings #281

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
9df579a
removed all hamocc cpp variables - made them namelist settings
mvertens Oct 15, 2023
b8bb56a
removed buildnml.csh
mvertens Oct 15, 2023
c0a4824
removed incorrect write statement that was re-introduced
mvertens Oct 15, 2023
e7a9ef9
added back in loop for burial variables that was omitted in this PR
mvertens Oct 15, 2023
eb003cc
more fixes
mvertens Oct 15, 2023
36e488d
removed hamocc config variables for run time namelist settings
mvertens Oct 15, 2023
9768273
update to feature/refactor_ifdefs2
mvertens Oct 15, 2023
ce12ae5
re-introduced files that should not have been deleted
mvertens Oct 16, 2023
78e347c
updates to hamocc master
mvertens Oct 17, 2023
fa9ef0f
Merge branch 'master' into feature/refactor_ifdefs3
mvertens Oct 17, 2023
e54e67b
merged to blom master
mvertens Oct 18, 2023
133564d
fixed problems that arose in merge
mvertens Oct 18, 2023
10ab1c1
updates for mct cap
mvertens Oct 18, 2023
55a03c0
added new tests for turning on all hamocc options
mvertens Oct 19, 2023
90ca8ea
fixes for hamocc tests
mvertens Oct 19, 2023
0a1082e
updates requested in PR review
mvertens Oct 20, 2023
07b389f
added missing namelists
mvertens Oct 23, 2023
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
35 changes: 1 addition & 34 deletions cime_config/buildcpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,20 +78,11 @@ def buildcpp(case):
# Determine the CPP flags values needed to build the blom component

ocn_grid = case.get_value("OCN_GRID")
blom_vcoord = case.get_value("BLOM_VCOORD")
turbclo = case.get_value("BLOM_TURBULENT_CLOSURE")
tracers = case.get_value("BLOM_TRACER_MODULES")
co2type = case.get_value("OCN_CO2_TYPE")
hamocc_cfc = case.get_value("HAMOCC_CFC")
hamocc_debug = case.get_value("HAMOCC_DEBUG")
hamocc_nattrc = case.get_value("HAMOCC_NATTRC")
hamocc_sedbypass = case.get_value("HAMOCC_SEDBYPASS")
hamocc_ciso = case.get_value("HAMOCC_CISO")
blom_unit = case.get_value("BLOM_UNIT")
pio_typename = case.get_value("PIO_TYPENAME", subgroup="OCN")

expect(blom_vcoord != "cntiso_hybrid" or not turbclo, "BLOM_VCOORD == {} and BLOM_TURBULENT_CLOSURE == {} is not a valid combination".format(blom_vcoord, turbclo))

blom_cppdefs = ""

if pio_typename == "pnetcdf":
Expand Down Expand Up @@ -130,28 +121,7 @@ def buildcpp(case):
if module == "iage":
blom_cppdefs = blom_cppdefs + " -DIDLAGE"
elif module == "ecosys":
blom_cppdefs = blom_cppdefs + " -DHAMOCC -DWLIN"
if hamocc_cfc:
blom_cppdefs = blom_cppdefs + " -DCFC"
if hamocc_debug:
blom_cppdefs = blom_cppdefs + " -DPBGC_OCNP_TIMESTEP -DPBGC_CK_TIMESTEP"
if hamocc_nattrc:
blom_cppdefs = blom_cppdefs + " -DnatDIC"
if hamocc_sedbypass:
blom_cppdefs = blom_cppdefs + " -Dsedbypass"
if hamocc_ciso:
expect(hamocc_sedbypass, "HAMOCC C-isotopes currently not supported in the sediment module. Use HAMOCC_SEDBYPASS=TRUE")
blom_cppdefs = blom_cppdefs + " -Dcisonew"
if ocn_grid in ["tnx1v4"]:
# HAMOCC bromoform scheme currently only supported on the tnx1v4 grid
# (no swa-climatology has been created for other grid configurations)"
Comment on lines -145 to -146
Copy link
Contributor

Choose a reason for hiding this comment

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

This is perhaps an important comment to preserve, i.e. why only tnx1v4 gid is supported.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TomasTorsvik - use_BROMO is now a namelist variable - not a CPP variable -
this is why I removed it from buildcpp. I have now added the comment to namelist_definition_blom.xml.

blom_cppdefs = blom_cppdefs + " -DBROMO"
if co2type == "prognostic":
blom_cppdefs = blom_cppdefs + " -DPROGCO2"
elif co2type == "diagnostic":
blom_cppdefs = blom_cppdefs + " -DDIAGCO2"
elif co2type != "constant":
expect(False, "CO2 type {} is not recognized".format(co2type))
blom_cppdefs = blom_cppdefs + " -DHAMOCC"
else:
expect(False, "tracer module {} is not recognized".format(module))

Expand All @@ -162,9 +132,6 @@ def buildcpp(case):

blom_cppdefs = "-DMPI" + blom_cppdefs

# update the xml variable BLOM_CPPDEFS with the above definition
#case.set_value("BLOM_CPPDEFS", blom_cppdefs)

return blom_cppdefs

###############################################################################
Expand Down
13 changes: 12 additions & 1 deletion cime_config/buildnml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,11 @@ def buildnml(case, caseroot, compname):
hamocc_sedspinup_ncycle = case.get_value("HAMOCC_SEDSPINUP_NCYCLE")
is_test = case.get_value("TEST")

blom_vcoord = case.get_value("BLOM_VCOORD")
turbclo = case.get_value("BLOM_TURBULENT_CLOSURE")
expect(blom_vcoord != "cntiso_hybrid" or not turbclo,
f"BLOM_VCOORD == {blom_vcoord} and BLOM_TURBULENT_CLOSURE == {turbclo} is not a valid combination")

#--------------------------
# Construct ParamGen objects:
#--------------------------
Expand Down Expand Up @@ -198,7 +203,7 @@ def buildnml(case, caseroot, compname):
case_dict = CaseDict()
var_list = ('DIN_LOC_ROOT', 'RUN_TYPE', 'BLOM_VCOORD',
'CCSM_CO2_PPMV', 'HAMOCC_SEDSPINUP_YR_START',
'HAMOCC_SEDSPINUP_YR_END')
'HAMOCC_SEDSPINUP_YR_END', 'OCN_CO2_TYPE')

for item in var_list:
case_dict[item] = case.get_value(item)
Expand All @@ -223,6 +228,11 @@ def buildnml(case, caseroot, compname):
if pg_blom.get_value('inid14c') == 'UNSET':
pg_blom.set_value('inid14c', value='')

if pg_blom.get_value('use_cisonew') == ".true.":
if pg_blom.bet_value("use_SEDBYPASS") == ".false.":
expect(False,
"HAMOCC C-isotopes currently not supported in the sediment module. Set use_SEDBYPASS to .true.")
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be removed now, since C-isotopes with sediment is technically supported now.


#---------------------------
# Write out Fortran namelist:
#---------------------------
Expand All @@ -245,6 +255,7 @@ def buildnml(case, caseroot, compname):
groups.append("bgcnml")
groups.append("bgcoafx")
groups.append("diabgc")
groups.append("config_bgc")

# for now don't write out bgcparams - just create an empty namelist
# if "ecosys" in case.get_value("BLOM_TRACER_MODULES"):
Expand Down
59 changes: 3 additions & 56 deletions cime_config/config_component.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
<value compset="_DATM%CPLHIST.*_BLOM%ECO">diagnostic</value>
<value compset="20TR_DATM%IAF.*_BLOM%ECO">diagnostic</value>
</values>
<group>build_component_blom</group>
<file>env_build.xml</file>
<group>run_component_blom</group>
<file>env_run.xml</file>
<desc>Determines provenance of atmospheric CO2 for gas flux computation.
This option is used in the BLOM ecosystem model.
The default is constant.</desc>
Expand All @@ -36,7 +36,7 @@
<valid_values>isopyc_bulkml,cntiso_hybrid</valid_values>
<default_value>isopyc_bulkml</default_value>
<group>build_component_blom</group>
<file>env_build.xml</file>
<file>env_run.xml</file>
<desc>Vertical coordinate type of BLOM</desc>
</entry>

Expand Down Expand Up @@ -113,50 +113,6 @@
<desc>Scenario for nitrogen deposition data. Requires module ecosys</desc>
</entry>

<entry id="HAMOCC_CFC">
<type>logical</type>
<valid_values>TRUE,FALSE</valid_values>
<default_value>FALSE</default_value>
<values>
<value compset="HIST_CAM60%NORESM.*_BLOM%ECO">TRUE</value>
<value compset="20TR_DATM.*_BLOM%ECO">TRUE</value>
</values>
<group>build_component_blom</group>
<file>env_build.xml</file>
<desc>Set preprocessor option to activate CFC code. Requires module ecosys</desc>
</entry>

<entry id="HAMOCC_NATTRC">
<type>logical</type>
<valid_values>TRUE,FALSE</valid_values>
<default_value>FALSE</default_value>
<values>
<value compset="HIST_CAM60%NORESM.*_BLOM%ECO">TRUE</value>
<value compset="20TR_DATM.*_BLOM%ECO">TRUE</value>
</values>
<group>build_component_blom</group>
<file>env_build.xml</file>
<desc>Set preprocessor option to activate natural tracer code. Requires module ecosys</desc>
</entry>

<entry id="HAMOCC_SEDBYPASS">
<type>logical</type>
<valid_values>TRUE,FALSE</valid_values>
<default_value>FALSE</default_value>
<group>build_component_blom</group>
<file>env_build.xml</file>
<desc>Set preprocessor option to bypass the sediment code. Requires module ecosys</desc>
</entry>

<entry id="HAMOCC_CISO">
<type>logical</type>
<valid_values>TRUE,FALSE</valid_values>
<default_value>FALSE</default_value>
<group>build_component_blom</group>
<file>env_build.xml</file>
<desc>Set preprocessor option to activate the carbon isotope code. Requires module ecosys</desc>
</entry>

<entry id="HAMOCC_SEDSPINUP">
<type>logical</type>
<valid_values>TRUE,FALSE</valid_values>
Expand Down Expand Up @@ -197,15 +153,6 @@
Requires module ecosys</desc>
</entry>

<entry id="HAMOCC_DEBUG">
<type>logical</type>
<valid_values>TRUE,FALSE</valid_values>
<default_value>FALSE</default_value>
<group>build_component_blom</group>
<file>env_build.xml</file>
<desc>Set preprocessor option to activate the debugging mode for iHAMOCC. Requires module ecosys</desc>
</entry>

<entry id="BLOM_TURBULENT_CLOSURE">
<type>char</type>
<valid_values></valid_values>
Expand Down
97 changes: 97 additions & 0 deletions cime_config/namelist_definition_blom.xml
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

@mvertens, @TomasTorsvik, @jmaerz there are some things I came across during my own testing yesterday:

  1. for some HAMOCC entries a is_input_data="yes" flag is missing (inidic, inialk,inipo3,inioxy,inino3,ndepfile,fedepfile,rivinfile,swaclimfile).

  2. the value for ndepfile isn't generated correctly if blom_ndep_scenario=ssp* i.e. any of the SSPs (I guess $BLOM_NDEP_SCENARIO doesn't exist any longer?)

  3. do_sedspinup and sedspin_ncyc should have a modify_via_xml flag

  4. I think the C-iso option should stay as an option in env_run, and get the modify_via_xml flag, too

Maybe this could also be quickly fixed with this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@JorgSchwinger - thanks for catching these! I will fix them in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for finding these. I've put in the fixes and am regenerating tests and baselines now.

Original file line number Diff line number Diff line change
Expand Up @@ -3759,6 +3759,103 @@
</desc>
</entry>

<entry id="ocn_co2_type" modify_via_xml="OCN_CO2_TYPE">
<type>char</type>
<category>config_bgc</category>
<group>bgcnml</group>
<values>
<value>$OCN_CO2_TYPE</value>
</values>
<desc>Determines provenance of atmospheric CO2 for gas flux computation.</desc>
</entry>

<!-- hamocc configuration -->

Copy link
Collaborator

Choose a reason for hiding this comment

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

Dear @mvertens , if I understand the approach correct, the entry ids for use_Agg and use_BOXATM are missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jmaerz - thanks for catching this. I think we need to add two more tests for use_BOXATM and use_AGG.
I'll do that today.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So in looking at the original buildcpp - BOXATM and AGG were never defined - and I only introduced namelists that corresponded to those variables. That was clearly an oversight on my part. Do we want to test turning these on with hamocc given that they were not defined as part of NorESM configurations before?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi Mariana, my suspicion is that both cases are not well supported, but I would hope that particularly AGG isn't broken (it was used the last time in a study by @JorgSchwinger). AGG should work within a coupled NorESM setup, while I have limited idea about BOXATM - I would need to look into it - I would suggest to skip the testing for BOXATM now, but I feel it would be good to enable switching it on via the xml. I hope to get through your changes by the end of the day.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jmaerz - use_BOXATM and use_AGG are only turned on via user_nl_blom right now - no longer throught the xml since they are no longer CPP variables. I will test that use_AGG works and add a test today.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @mvertens , I just realize that use_FB_BGC_OCE might also be missing?

Copy link
Contributor

Choose a reason for hiding this comment

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

BOXATM, AGG, FB_BGC_OCN all are options that have not been used quite some time. So currently, these are only for expert users who can activate these options via namelist switches if they know what they are doing.

How much it makes sense to include tests for these options I don't know, I wouldn't see this as a priority.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@JorgSchwinger @jmaerz - I have added missing namelists use_boxatm, use_agg and use_fb_bgc_oce to the new namelist group config_bgc. Given the last statement by @JorgSchwinger - can we test these options after this PR and add appropriate tests if everyone agrees? Otherwise - I'm happy to test it in this PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @mvertens , from my side, I am totally fine to proceed as you suggested. The testing feature is different from the nml configuration.

<entry id="use_WLIN">
<type>logical</type>
<category>config_bgc</category>
<group>config_bgc</group>
<values>
<value>.true.</value>
</values>
<desc>add desc</desc>
</entry>

<entry id="use_CFC">
<type>logical</type>
<category>config_bgc</category>
<group>config_bgc</group>
<values>
<value>.false.</value>
<value compset="HIST_CAM60%NORESM.*_BLOM%ECO">.true.</value>
<value compset="20TR_DATM.*_BLOM%ECO">.true.</value>
</values>
<desc>activates HAMOCC CFC code</desc>
</entry>

<entry id="use_natDIC">
<type>logical</type>
<category>config_bgc</category>
<group>config_bgc</group>
<values>
<value>.false.</value>
<value compset="HIST_CAM60%NORESM.*">.true.</value>
<value compset="20TR_DATM.*">.true.</value>
</values>
<desc>activates HAMOCC natural tracer code</desc>
</entry>

<entry id="use_PBGC_OCNP_TIMESTEP">
<type>logical</type>
<category>config_bgc</category>
<group>config_bgc</group>
<values>
<value>.false.</value>
</values>
<desc>HAMOCC debug mode flag</desc>
</entry>

<entry id="use_PBGC_CK_TIMESTEP">
<type>logical</type>
<category>config_bgc</category>
<group>config_bgc</group>
<values>
<value>.false.</value>
</values>
<desc>HAMOCC debug mode flag</desc>
</entry>

<entry id="use_BROMO">
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<entry id="use_BROMO">
<!-- HAMOCC bromoform scheme currently only supported on the tnx1v4 grid -->
<!-- (no swa-climatology has been created for other grid configurations) -->
<entry id="use_BROMO">

Maybe something like this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly. Thanks!

<type>logical</type>
<category>config_bgc</category>
<group>config_bgc</group>
<values>
<value>.false.</value>
<value ocn_grid="tnx1v4"> .true.</value>
</values>
<desc>add desc</desc>
</entry>

<entry id="use_SEDBYPASS">
<type>logical</type>
<category>config_bgc</category>
<group>config_bgc</group>
<values>
<value>.false.</value>
</values>
<desc>Bypass the HAMOCC sediment code</desc>
</entry>

<entry id="use_cisonew">
<type>logical</type>
<category>config_bgc</category>
<group>config_bgc</group>
<values>
<value>.false.</value>
</values>
<desc>activate the HAMOCC carbon isotope code</desc>
</entry>

<!-- ========================= -->
<!-- namelist group: bgcoafx -->
<!-- These options can be activated by expert users via user namelist. -->
Expand Down
Loading
Loading